Skip to content

Commit

Permalink
Merge pull request juju#8102 from wallyworld/upgrade-juju-fixes
Browse files Browse the repository at this point in the history 
Fix upgrade-juju --dry-run  and upgrades from custom binaries

## Description of change

There were several small issues with upgrade-juju. The main one occurred when running a custom build (with a build number > 0). --dry-run would report n upgrade available, but then running the upgrade would not use the available version and instead upload a new custom build.
Other fixes include some message tweaks and not actually building a local jujud when using dry run.

## QA steps

Run up a custom build, eg 2.3-beta2.1
Use juju upgrade-juju with no args and ensure it upgrades to 2.3-beta3
(it would fail before)

## Documentation changes

None, just fixing bad behaviour.

## Bug reference

https://bugs.launchpad.net/juju/+bug/1732879
  • Loading branch information
@jujubot
jujubot authored Nov 17, 2017
2 parents 71a3d8e + dba98ab commit 6f19643
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 25 deletions.
72 changes: 52 additions & 20 deletions cmd/juju/commands/upgradejuju.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,33 +326,48 @@ func (c *upgradeJujuCommand) Run(ctx *cmd.Context) (err error) {
if err != nil {
return err
}
// If we're running a custom build or the user has asked for a new agent
// to be built, upload a local jujud binary if possible.
uploadLocalBinary := isControllerModel && c.Version == version.Zero && tryImplicit
if !warnCompat && (uploadLocalBinary || c.BuildAgent) && !c.DryRun {
if err := context.uploadTools(c.BuildAgent); err != nil {

// Look for any packaged binaries but only if we haven't been asked to build an agent.
var packagedAgentErr error
if !c.BuildAgent {
if packagedAgentErr = context.maybeChoosePackagedAgent(); packagedAgentErr != nil {
ctx.Verbosef("%v", packagedAgentErr)
}
}

// If there's no packaged binaries, or we're running a custom build
// or the user has asked for a new agent to be built, upload a local
// jujud binary if possible.
uploadLocalBinary := isControllerModel && packagedAgentErr != nil && tryImplicit
if !warnCompat && (uploadLocalBinary || c.BuildAgent) {
if err := context.uploadTools(c.BuildAgent, agentVersion, c.DryRun); err != nil {
return block.ProcessBlockedError(err, block.BlockChange)
}
builtMsg := ""
if c.BuildAgent {
builtMsg = " (built from source)"
}
fmt.Fprintf(ctx.Stdout, "no prepackaged agent binaries available, using local agent binary %v%s\n", context.chosen, builtMsg)
packagedAgentErr = nil
}
if packagedAgentErr != nil {
return packagedAgentErr
}

// If there was an error implicitly uploading a binary, we'll still look for any packaged binaries
// since there may still be a valid upgrade and the user didn't ask for any local binary.
if err := context.validate(); err != nil {
return err
}
// TODO(fwereade): this list may be incomplete, pending envtools.Upload change.
ctx.Verbosef("available agent binaries:\n%s", formatTools(context.tools))
ctx.Verbosef("best version:\n %s", context.chosen)
if warnCompat {
fmt.Fprintf(ctx.Stderr, "version %s incompatible with this client (%s)\n", context.chosen, jujuversion.Current)
}
if c.DryRun {
fmt.Fprintf(ctx.Stderr, "upgrade to this version by running\n juju upgrade-juju --agent-version=\"%s\"\n", context.chosen)
if c.BuildAgent {
fmt.Fprint(ctx.Stderr, "upgrade to this version by running\n juju upgrade-juju --build-agent\n")
} else {
fmt.Fprintf(ctx.Stderr, "upgrade to this version by running\n juju upgrade-juju --agent-version=\"%s\"\n", context.chosen)
}
} else {
if c.ResetPrevious {
if ok, err := c.confirmResetPreviousUpgrade(ctx); !ok || err != nil {
Expand Down Expand Up @@ -491,7 +506,7 @@ type upgradeContext struct {
// than that of any otherwise-matching available envtools.
// uploadTools resets the chosen version and replaces the available tools
// with the ones just uploaded.
func (context *upgradeContext) uploadTools(buildAgent bool) (err error) {
func (context *upgradeContext) uploadTools(buildAgent bool, agentVersion version.Number, dryRun bool) (err error) {
// TODO(fwereade): this is kinda crack: we should not assume that
// jujuversion.Current matches whatever source happens to be built. The
// ideal would be:
Expand All @@ -508,10 +523,24 @@ func (context *upgradeContext) uploadTools(buildAgent bool) (err error) {
// TODO(cherylj) If the determination of version changes, we will
// need to also change the upgrade version checks in Run() that check
// if a major upgrade is allowed.
if context.chosen == version.Zero {
context.chosen = context.client
uploadBaseVersion := context.chosen
if uploadBaseVersion == version.Zero {
uploadBaseVersion = context.client
}
// If the Juju client matches the current running agent (excluding build number),
// make sure the build number gets incremented.
agentVersionCopy := agentVersion
agentVersionCopy.Build = 0
uploadBaseVersionCopy := uploadBaseVersion
uploadBaseVersion.Build = 0
if agentVersionCopy.Compare(uploadBaseVersionCopy) == 0 {
uploadBaseVersion = agentVersion
}
context.chosen = makeUploadVersion(uploadBaseVersion, context.tools)

if dryRun {
return nil
}
context.chosen = uploadVersion(context.chosen, context.tools)

builtTools, err := sync.BuildAgentTarball(buildAgent, &context.chosen, "upgrade")
if err != nil {
Expand Down Expand Up @@ -545,11 +574,7 @@ func (context *upgradeContext) uploadTools(buildAgent bool) (err error) {
return nil
}

// validate chooses an upgrade version, if one has not already been chosen,
// and ensures the tools list contains no entries that do not have that version.
// If validate returns no error, the environment agent-version can be set to
// the value of the chosen field.
func (context *upgradeContext) validate() (err error) {
func (context *upgradeContext) maybeChoosePackagedAgent() (err error) {
if context.chosen == version.Zero {
// No explicitly specified version, so find the version to which we
// need to upgrade. We find next available stable release to upgrade
Expand Down Expand Up @@ -587,6 +612,13 @@ func (context *upgradeContext) validate() (err error) {
}
context.chosen, context.tools = context.tools.Newest()
}
return nil
}

// validate ensures an upgrade can be done using the chosen agent version.
// If validate returns no error, the environment agent-version can be set to
// the value of the chosen agent field.
func (context *upgradeContext) validate() (err error) {
if context.chosen == context.agent {
return errUpToDate
}
Expand All @@ -606,9 +638,9 @@ func (context *upgradeContext) validate() (err error) {
return nil
}

// uploadVersion returns a copy of the supplied version with a build number
// makeUploadVersion returns a copy of the supplied version with a build number
// higher than any of the supplied tools that share its major, minor and patch.
func uploadVersion(vers version.Number, existing coretools.List) version.Number {
func makeUploadVersion(vers version.Number, existing coretools.List) version.Number {
vers.Build++
for _, t := range existing {
if t.Version.Major != vers.Major || t.Version.Minor != vers.Minor || t.Version.Patch != vers.Patch {
Expand Down
17 changes: 12 additions & 5 deletions cmd/juju/commands/upgradejuju_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,11 @@ var upgradeJujuTests = []struct {
agentVersion: "2.0.0",
expectErr: "no more recent supported versions available",
}, {
about: "latest supported stable, when client is dev",
about: "latest supported stable, when client is dev, explicit upload",
tools: []string{"2.1-dev1-quantal-amd64", "2.1.0-quantal-amd64", "2.3-dev0-quantal-amd64", "3.0.1-quantal-amd64"},
currentVersion: "2.1-dev0-quantal-amd64",
agentVersion: "2.0.0",
args: []string{"--build-agent"},
expectVersion: "2.1-dev0.1",
}, {
about: "latest current, when agent is dev",
Expand Down Expand Up @@ -298,11 +299,17 @@ var upgradeJujuTests = []struct {
expectVersion: "2.7.3.2",
expectUploaded: []string{"2.7.3.2-quantal-amd64", "2.7.3.2-%LTS%-amd64", "2.7.3.2-raring-amd64"},
}, {
about: "latest supported stable release",
about: "latest supported stable release increments by one minor version number",
tools: []string{"1.21.3-quantal-amd64", "1.22.1-quantal-amd64"},
currentVersion: "1.22.1-quantal-amd64",
agentVersion: "1.20.14",
expectVersion: "1.22.1.1",
expectVersion: "1.21.3",
}, {
about: "latest supported stable release from custom version",
tools: []string{"1.21.3-quantal-amd64", "1.22.1-quantal-amd64"},
currentVersion: "1.22.1-quantal-amd64",
agentVersion: "1.20.14.1",
expectVersion: "1.21.3",
}}

func (s *UpgradeJujuSuite) TestUpgradeJuju(c *gc.C) {
Expand Down Expand Up @@ -474,7 +481,7 @@ func (s *UpgradeJujuSuite) TestUpgradeJujuWithImplicitUploadDevAgent(c *gc.C) {
_, err := cmdtesting.RunCommand(c, cmd)
c.Assert(err, jc.ErrorIsNil)
c.Assert(fakeAPI.tools, gc.Not(gc.HasLen), 0)
c.Assert(fakeAPI.tools[0].Version.Number, gc.Equals, version.MustParse("1.99.99.1"))
c.Assert(fakeAPI.tools[0].Version.Number, gc.Equals, version.MustParse("1.99.99.2"))
}

func (s *UpgradeJujuSuite) TestUpgradeJujuWithImplicitUploadNewerClient(c *gc.C) {
Expand Down Expand Up @@ -592,7 +599,7 @@ func (s *UpgradeJujuSuite) TestUpgradeDryRun(c *gc.C) {
currentVersion: "2.1.3-quantal-amd64",
agentVersion: "2.0.0",
expectedCmdOutput: `upgrade to this version by running
juju upgrade-juju --agent-version="2.1.3"
juju upgrade-juju --build-agent
`,
},
{
Expand Down

0 comments on commit 6f19643

Please sign in to comment.