-
Notifications
You must be signed in to change notification settings - Fork 128
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
Skip service_resume if already enabled #882
Conversation
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.
The unit test(s) should probably be updated/added to to test the new feature.
Repeatedly calling service_enable for a service that is already enabled can lead to unintended consequences. Some charms frequently call servie_resume which will call service('enable') and this adds a check to only do so of the service is not enabled. Related-Bug: #2058505
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
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.
just tested this through the ovn-chassis charm, and works as expected
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, thanks!
I think the unit test change was added, so this should be good to merge. |
Repeatedly calling service_resume for a service that is already enabled can lead to unintended consequences. This adds a check to only resume a serice if it is not enabled.
Related-Bug: #2058505