-
Notifications
You must be signed in to change notification settings - Fork 72
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
Fixes #36877 - Add a pre-upgrade check for katello-agent #782
Conversation
82c1e89
to
739bca8
Compare
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.
Thanks @wbclark!
889efcf
to
b7fbd46
Compare
b7fbd46
to
da2d1d9
Compare
da2d1d9
to
14e4609
Compare
" before proceeding with the upgrade. Alternatively, you may skip this check and proceed by"\ | ||
" running #{maintain_command} again with the `--whitelist` option, which will automatically"\ | ||
" uninstall katello-agent." |
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.
As someone new to foreman-maintain, I'm still not sure what command it's telling me to run here. I assume it's not just foreman-maintain --whitelist
? Can it give me the exact command I would run?
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.
I need to figure out how the name to pass to --whitelist
is constructed from the label :check_katello_agent_enabled
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.
OK, so it would be --whitelist="check-katello-agent-enabled"
but it turns out the exact language shouldn't be necessary here.
The reason why is that if ANY checks fail, there is general language explaining how to whitelist the failed step.
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.
In that case, LGTM 👍
@evgeni it looks like we need a green ACK, do you have a moment to look? |
No description provided.