-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update deployment guide components and network checks #607
Conversation
I've kickstarted #611 to tackle that in an automated fashion, you can rebase if merged soon or we will take care of that later 👍🏻 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fix for this failing e2e is already in master
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slight change
roles: | ||
- role: '../roles/helper_modules' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you calling the role if you include the tasks explicitly?
To be fair, that stuff should not even be a role. If you feel like moving it to the playbook level feel free to do so.
If not, then just remove the roles
execution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the other play of this playbook do that, I simply duplicated the ADW play and changed all adw
references with acc
, since that seems to be what was required for ACC as well. Isn't that to load the library listen_port.py
that is then used inside the check_port.yml
task? If that isn't the reason, I do not know, you did it like that for other plays so I just did the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh lord yes you're most likely right. That's a really ugly hack :-/
I've done some quick tests and it seems a single role is needed for the complete set of plays in this playbook to find the module file.
That said, I have created a new ticket to completely get rid of that roles: OPSEXP-2147 (shadows #438)
For now we can move forward with current code in this PR.
ce084ac
to
f9a2d14
Compare
be19337
to
69af5f4
Compare
Adding ACC port to fix conflict with master branch updated as part of PR Alfresco#607
In relation to the previous PR #589, I split the doc & network checks related improvements/corrections in this new PR.
Included in this PR:
Not included in this PR:
docs/resources/*.png
files. I believe that you generate the png files based on the puml. I suggested in this PR an update of the puml files, but I didn't re-generate the png, as I assume you have what is needed for that while I don'tFeel free to let me know if I need to update/remove something.