-
Notifications
You must be signed in to change notification settings - Fork 12
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
DPE-5540 Skip plugin install for not file found #524
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.
looks good. still confused about the symptoms of the issue - if the charm would enter an error state previously, i would recommend adding a test that runs upgrade without pre-upgrade-check
to ensure that things start up normally. thoughts?
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.
looks good short-term
when adding refresh v3, would recommend not modifying plugins while a refresh is in progress
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 here, ACK to Shayan: adding a test to run refresh without pre-upgrade-check will help us returning here for the following candidate revisions.
Update: the ticket has no juju status output and I do not remeber the state. Will post better ticket the next time. AFAIK, refresh was in error state, yes. |
You are right. I will include a test before merge |
Issue
config_change
hook will try to install plugins if configuration diff so indicates, even if plugins files are not available.Case:
refresh
ran withoutpre-upgrade-check
- so snap is not updated (no plugin files) but config changed is triggered.Solution
config_change
event