Skip to content
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

worker/modelupgrade: set model status #7609

Merged
merged 1 commit into from Jul 10, 2017
Merged

Conversation

axw
Copy link
Contributor

@axw axw commented Jul 10, 2017

Description of change

Set model status before and after upgrading the
model's environ. Allow the "error" status for
models, so that we can indicate that the model
requires attention (e.g. credentials are out of
date and need to be refreshed.)

QA steps

  1. juju bootstrap localhost
  2. modify provider code, bumping provider version to 1, and introducing an environ upgrade step that fails
  3. juju upgrade-juju -m controller
  4. observe model status indicates a failure occurred
  5. fix the provider code, re-run juju upgrade-juju -m controller
  6. observe model status is back to normal

Documentation changes

None.

Bug reference

https://bugs.launchpad.net/juju/+bug/1700451

Copy link
Member

@wallyworld wallyworld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to surface the actual error in the status message so the user know what went wrong

@@ -174,8 +189,14 @@ func newUpgradeWorker(config Config, targetVersion int) (worker.Worker, error) {
targetVersion,
setVersion,
); err != nil {
if err := setStatus(status.Error, "failed to upgrade environ"); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what not include the error in the status message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it might be TMI for non-controller admins. I'll include it, we can change based on feedback.

Set model status before and after upgrading the
model's environ. Allow the "error" status for
models, so that we can indicate that the model
requires attention (e.g. credentials are out of
date and need to be refreshed.)
@axw axw force-pushed the modelupgrader-setstatus branch from b5a55f5 to 4ffe441 Compare July 10, 2017 06:44
@axw
Copy link
Contributor Author

axw commented Jul 10, 2017

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Jul 10, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 2643110 into juju:2.2 Jul 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants