-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
fix: crio restart handler #11547
base: master
Are you sure you want to change the base?
fix: crio restart handler #11547
Conversation
crio restart handler `when` clause is overwritten by an unrelated import_role. we set the import_role to load handlers from an empty file to avoid this
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: alimehrabikoshki The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Welcome @alimehrabikoshki! |
Hi @alimehrabikoshki. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
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.
Could you give more details in the commit message on the reason why we need this ?
I understand this prevent handlers override, but from where exactly ? A link to the underlying ansible issue explaining the mechanism would be also good.
(Basically, someone reading your commit message in two year should have all the context)
/ok-to-test
@@ -151,3 +151,4 @@ | |||
import_role: | |||
name: container-engine/cri-o | |||
tasks_from: reset | |||
handlers_from: reset |
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.
Could you make it more explicit than this a workaround to not import handlers ? Change the file name to something like 'none / not_existing /whatever'
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.
Any change you could address this ?
crio restart handler
when
clause is overwritten by an unrelated import_role. we set the import_role to load handlers from an empty file to avoid thisWhat type of PR is this?
/kind bug
What this PR does / why we need it:
cri-o does not restart because import_role overwrites the when conditional of the handler
same fix was made for containerd here: #9322
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?: