Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
During install hook, do not overwrite workload status if it has already been set by the charm #7370
Conversation
axw
approved these changes
May 22, 2017
LGTM, but I think you should remove the extra SetExecutingStatus.
| + // If the charm has already updated the unit status in a previous hook, | ||
| + // then don't overwrite that here - just set the executing status message. | ||
| + if state.StatusSet { | ||
| + err = rh.callbacks.SetExecutingStatus(status.MessageInstallingCharm) |
axw
May 22, 2017
Member
I think this is pointless, because it's going to be replaced immediately by the SetExecutingStatus call in Execute.
|
Yep, removed |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
jujubot
merged commit 277ce13
into
juju:develop
May 22, 2017
1 check passed
github-check-merge-juju
Built PR, ran unit tests, and tested LXD deploy. Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
wallyworld commentedMay 22, 2017
Description of change
When the install hook is about to run, we first check to see whether the charm has already set the workload status. If set, we don't overwrite with the default install status.
QA steps
The bug shows the ghost charm doing the wrong thing.
Deploy the ghost charm and observe the correct show-status-log output.
$ juju show-status-log ghost/0
TIME TYPE STATUS MESSAGE
22 May 2017 13:18:53+10:00 juju-unit executing running app-storage-attached hook
22 May 2017 13:24:13+10:00 workload waiting Updating apt cache
22 May 2017 13:24:17+10:00 workload waiting Installing nginx-full,nodejs
22 May 2017 13:24:41+10:00 workload active node.js is ready
22 May 2017 13:24:41+10:00 workload active NGINX is ready
22 May 2017 13:24:41+10:00 workload maintenance installing NPM dependencies for /srv/app
22 May 2017 13:24:50+10:00 workload maintenance Configuring site default
22 May 2017 13:24:51+10:00 workload maintenance updating configuration
22 May 2017 13:25:13+10:00 workload maintenance installing NPM dependencies for /srv/app
22 May 2017 13:25:48+10:00 workload active ready
22 May 2017 13:25:48+10:00 juju-unit executing installing charm software
22 May 2017 13:25:48+10:00 juju-unit executing running install hook
22 May 2017 13:25:48+10:00 workload active ready
Bug reference
https://bugs.launchpad.net/juju/+bug/1597481