added dry run support for upgrade-juju #64
Conversation
| // TODO(fwereade): this list may be incomplete, pending envtools.Upload change. | ||
| logger.Infof("available tools: %s", context.tools) | ||
| ctx.Infof("available tools: %s", context.tools) |
There was a problem hiding this comment.
I think this particular line's output will be difficult to read when the list of tools grows. Thoughts?
There was a problem hiding this comment.
yeah, might be good to make this a bit more compact. probably a separate issue though
There was a problem hiding this comment.
but... thinking of the output, I think it would be best to have "available" before "chosen".
|
Lookin good, just a few cleanups in the test and possibly a different format for the tools list. |
|
Can we use --dry-run instead of --dry please? It's possible to support -n also as a shortcut for --dry-run, if you use a gnuflag.Var() and define a Set() method to parse both. |
|
haven't looked at the code, but a comment re --dry-run: it doesn't guarantee that a new update won't land in between the dry run and the subsequent, uh, wet one. Might be nice to print the full command line to upgrade to the version that was reported? |
|
@fwereade do you mean adding something like this? |
…particular version
| @@ -78,6 +79,7 @@ func (c *UpgradeJujuCommand) Info() *cmd.Info { | |||
| func (c *UpgradeJujuCommand) SetFlags(f *gnuflag.FlagSet) { | |||
| f.StringVar(&c.vers, "version", "", "upgrade to specific version") | |||
| f.BoolVar(&c.UploadTools, "upload-tools", false, "upload local version of tools") | |||
| f.BoolVar(&c.DryRun, "dry-run", false, "Don't change anything, just report what would change") | |||
|
few more comments, a bit rushed, please get another detailed review |
|
|
||
| for i, test := range tests { | ||
| c.Logf("\ntest %d: %s", i, test.about) | ||
| //version.Current = version.MustParseBinary(test.currentVersion) |
|
LGTM, thanks |
|
|
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
|
|
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
added dry run support for upgrade-juju Fix for https://bugs.launchpad.net/juju-core/+bug/1272544. Added a --dry flag to the upgrade-juju command. When --dry is specified the action to be taken is output to Stderr. Added two tests to check that this information gets output and that the agent version's aren't changed after running the command with --dry
metrics.yaml support
Added functions related to tags in testservice
Added TestServer functions on api of tag shown below to testservice.
* GET /api/1.0/tags/ op=list
* POST /api/1.0/tags/ op=new
* GET /api/1.0/tags/{name}/
* GET /api/1.0/tags/{name}/ op=nodes
* POST /api/1.0/tags/{name}/ op=update_nodes
* PUT /api/1.0/tags/{name}/
* DELETE /api/1.0/tags/{name}/
…mepage Render the discourse content on the homepage of docs
Fix for https://bugs.launchpad.net/juju-core/+bug/1272544.
Added a --dry flag to the upgrade-juju command. When --dry is specified the action to be taken is output to Stderr. Added two tests to check that this information gets output and that the agent version's aren't changed after running the command with --dry