Merge pull request juju#10373 from manadart/2.6-into-develop

juju#10373 ## Description of change Merge from 2.6 to bring forward: - juju#10372 from manadart/2.6-app-config-branch-error - juju#10368 from anastasiamac/ref-non-deprecated-update-cloud-26 - juju#10348 from hpidcock/1832779-command-fails-no-active-controller
google38438 · Jun 24, 2019 · d13b59a · d13b59a
2 parents dbedec4 + 0a3413d
commit d13b59a
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 11 deletions.
diff --git a/apiserver/facades/client/application/application.go b/apiserver/facades/client/application/application.go
@@ -800,6 +800,13 @@ func (api *APIBase) Update(args params.ApplicationUpdate) error {
 		}
 	}
 
+	// We need a guard on the API server-side for direct API callers such as
+	// python-libjuju, and for older clients.
+	// Always default to the master branch.
+	if args.Generation == "" {
+		args.Generation = model.GenerationMaster
+	}
+
 	// Set up application's settings.
 	// If the config change is generational, add the app to the generation.
 	configChange := false

diff --git a/apiserver/facades/client/application/application_test.go b/apiserver/facades/client/application/application_test.go
@@ -1816,15 +1816,23 @@ func (s *applicationSuite) TestApplicationUpdateSetMinUnitsError(c *gc.C) {
 	c.Assert(app.MinUnits(), gc.Equals, 0)
 }
 
-func (s *applicationSuite) TestApplicationUpdateSetSettingsStrings(c *gc.C) {
+func (s *applicationSuite) TestApplicationUpdateSetSettingsStringsExplicitMaster(c *gc.C) {
+	s.testApplicationUpdateSetSettingsStrings(c, model.GenerationMaster)
+}
+
+func (s *applicationSuite) TestApplicationUpdateSetSettingsStringsEmptyBranchUsesMaster(c *gc.C) {
+	s.testApplicationUpdateSetSettingsStrings(c, "")
+}
+
+func (s *applicationSuite) testApplicationUpdateSetSettingsStrings(c *gc.C, branchName string) {
 	ch := s.AddTestingCharm(c, "dummy")
 	app := s.AddTestingApplication(c, "dummy", ch)
 
 	// Update settings for the application.
 	args := params.ApplicationUpdate{
 		ApplicationName: "dummy",
 		SettingsStrings: map[string]string{"title": "s-title", "username": "s-user"},
-		Generation:      model.GenerationMaster,
+		Generation:      branchName,
 	}
 	err := s.applicationAPI.Update(args)
 	c.Assert(err, jc.ErrorIsNil)
@@ -1864,15 +1872,23 @@ func (s *applicationSuite) TestApplicationUpdateSetSettingsStringsBranch(c *gc.C
 	c.Check(gen.AssignedUnits(), jc.DeepEquals, map[string][]string{"dummy": {}})
 }
 
-func (s *applicationSuite) TestApplicationUpdateSetSettingsYAML(c *gc.C) {
+func (s *applicationSuite) TestApplicationUpdateSetSettingsYAMLExplicitMaster(c *gc.C) {
+	s.testApplicationUpdateSetSettingsYAML(c, model.GenerationMaster)
+}
+
+func (s *applicationSuite) TestApplicationUpdateSetSettingsYAMLEmptyBranchUsesMaster(c *gc.C) {
+	s.testApplicationUpdateSetSettingsYAML(c, "")
+}
+
+func (s *applicationSuite) testApplicationUpdateSetSettingsYAML(c *gc.C, branchName string) {
 	ch := s.AddTestingCharm(c, "dummy")
 	app := s.AddTestingApplication(c, "dummy", ch)
 
 	// Update settings for the application.
 	args := params.ApplicationUpdate{
 		ApplicationName: "dummy",
 		SettingsYAML:    "dummy:\n  title: y-title\n  username: y-user",
-		Generation:      model.GenerationMaster,
+		Generation:      branchName,
 	}
 	err := s.applicationAPI.Update(args)
 	c.Assert(err, jc.ErrorIsNil)

diff --git a/cmd/juju/cloud/add.go b/cmd/juju/cloud/add.go
@@ -115,7 +115,6 @@ you can specify which to upload with the --credential option.
 Examples:
     juju add-cloud
     juju add-cloud mycloud ~/mycloud.yaml
-    juju add-cloud --replace mycloud ~/mycloud2.yaml
 
 If the "multi-cloud" feature flag is turned on in the controller:
 
@@ -125,6 +124,7 @@ If the "multi-cloud" feature flag is turned on in the controller:
 
 See also: 
     clouds
+    update-cloud
     update-credential`
 
 // AddCloudAPI - Implemented by cloudapi.Client.
@@ -689,14 +689,14 @@ func (p cloudFileReader) verifyName(name string) error {
 		return err
 	}
 	if _, ok := personal[name]; ok {
-		return errors.Errorf("%q already exists; use --replace to replace this existing cloud", name)
+		return errors.Errorf("%q already exists; use `update-cloud` to replace this existing cloud", name)
 	}
 	msg, err := nameExists(name, public)
 	if err != nil {
 		return errors.Trace(err)
 	}
 	if msg != "" {
-		return errors.Errorf(msg + "; use --replace to override this definition")
+		return errors.Errorf(msg + "; use `update-cloud` to override this definition")
 	}
 	return nil
 }

diff --git a/cmd/juju/cloud/add_test.go b/cmd/juju/cloud/add_test.go
@@ -198,7 +198,7 @@ func (s *addSuite) TestAddExisting(c *gc.C) {
 	fake.Call("PersonalCloudMetadata").Returns(mockCloud, nil)
 
 	_, err = s.runCommand(c, fake, "homestack", cloudFile.Name())
-	c.Assert(err, gc.ErrorMatches, `"homestack" already exists; use --replace to replace this existing cloud`)
+	c.Assert(err, gc.ErrorMatches, "\"homestack\" already exists; use `update-cloud` to replace this existing cloud")
 }
 
 func (s *addSuite) TestAddExistingReplace(c *gc.C) {
@@ -235,7 +235,7 @@ func (s *addSuite) TestAddExistingPublic(c *gc.C) {
 	fake.Call("PersonalCloudMetadata").Returns(map[string]jujucloud.Cloud{}, nil)
 
 	_, err = s.runCommand(c, fake, "aws", cloudFile.Name())
-	c.Assert(err, gc.ErrorMatches, `"aws" is the name of a public cloud; use --replace to override this definition`)
+	c.Assert(err, gc.ErrorMatches, "\"aws\" is the name of a public cloud; use `update-cloud` to override this definition")
 }
 
 func (s *addSuite) TestAddExistingBuiltin(c *gc.C) {
@@ -252,7 +252,7 @@ func (s *addSuite) TestAddExistingBuiltin(c *gc.C) {
 	fake.Call("PersonalCloudMetadata").Returns(map[string]jujucloud.Cloud{}, nil)
 
 	_, err = s.runCommand(c, fake, "localhost", cloudFile.Name())
-	c.Assert(err, gc.ErrorMatches, `"localhost" is the name of a built-in cloud; use --replace to override this definition`)
+	c.Assert(err, gc.ErrorMatches, "\"localhost\" is the name of a built-in cloud; use `update-cloud` to override this definition")
 }
 
 func (s *addSuite) TestAddExistingPublicReplace(c *gc.C) {

diff --git a/cmd/juju/commands/upgradecontroller.go b/cmd/juju/commands/upgradecontroller.go
@@ -268,7 +268,13 @@ func (c *upgradeControllerCommand) upgradeIAASController(ctx *cmd.Context) error
 	}}
 	jcmd.SetClientStore(c.ClientStore())
 	wrapped := modelcmd.Wrap(jcmd)
-	args := append(c.rawArgs, "-m", bootstrap.ControllerModelName)
+	controllerName, err := c.ControllerName()
+	if err != nil {
+		return errors.Trace(err)
+	}
+	fullControllerModelName := modelcmd.JoinModelName(controllerName,
+		bootstrap.ControllerModelName)
+	args := append(c.rawArgs, "-m", fullControllerModelName)
 	if c.vers != "" {
 		args = append(args, "--agent-version", c.vers)
 	}

diff --git a/cmd/juju/commands/upgradecontroller_test.go b/cmd/juju/commands/upgradecontroller_test.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/juju/cmd"
 	"github.com/juju/cmd/cmdtesting"
+	"github.com/juju/errors"
 	"github.com/juju/os/series"
 	jc "github.com/juju/testing/checkers"
 	"github.com/juju/utils/arch"
@@ -23,6 +24,7 @@ import (
 	"github.com/juju/juju/environs/tools"
 	jujutesting "github.com/juju/juju/juju/testing"
 	"github.com/juju/juju/jujuclient"
+	"github.com/juju/juju/jujuclient/jujuclienttesting"
 	coretesting "github.com/juju/juju/testing"
 	jujuversion "github.com/juju/juju/version"
 )
@@ -115,6 +117,51 @@ func (s *UpgradeIAASControllerSuite) TestUpgradeWithRealUpload(c *gc.C) {
 	s.checkToolsUploaded(c, vers, vers.Number)
 }
 
+func (s *UpgradeIAASControllerSuite) TestUpgradeCorrectController(c *gc.C) {
+	badControllerName := "not-the-right-controller"
+	badControllerSelected := errors.New("bad controller selected")
+	upgradeCommand := func(minUpgradeVers map[int]version.Number) cmd.Command {
+		backingStore := s.ControllerStore
+		store := jujuclienttesting.WrapClientStore(backingStore)
+		store.ControllerByNameFunc = func(name string) (*jujuclient.ControllerDetails, error) {
+			if name == badControllerName {
+				return nil, badControllerSelected
+			}
+			return backingStore.ControllerByName(name)
+		}
+		store.CurrentControllerFunc = func() (string, error) {
+			return badControllerName, nil
+		}
+		s.ControllerStore = store
+		cmd := &upgradeControllerCommand{
+			baseUpgradeCommand: baseUpgradeCommand{minMajorUpgradeVersion: minMajorUpgradeVersion},
+		}
+		cmd.SetClientStore(s.ControllerStore)
+		return modelcmd.WrapController(cmd)
+	}
+
+	tests := []upgradeTest{
+		{
+			about:          "latest supported stable release with specified controller",
+			available:      []string{"2.1.0-quantal-amd64", "2.1.2-quantal-i386", "2.1.3-quantal-amd64", "2.1-dev1-quantal-amd64"},
+			currentVersion: "2.0.0-quantal-amd64",
+			agentVersion:   "2.0.0",
+			expectVersion:  "2.1.3",
+			args:           []string{"--controller", "kontroll"},
+		},
+		{
+			about:          "latest supported stable release without specified controller",
+			available:      []string{"2.1.0-quantal-amd64", "2.1.2-quantal-i386", "2.1.3-quantal-amd64", "2.1-dev1-quantal-amd64"},
+			currentVersion: "2.0.0-quantal-amd64",
+			agentVersion:   "2.0.0",
+			expectVersion:  "2.1.3",
+			expectErr:      badControllerSelected.Error(),
+		},
+	}
+
+	s.assertUpgradeTests(c, tests, upgradeCommand)
+}
+
 func (s *UpgradeIAASControllerSuite) TestUpgradeDryRun(c *gc.C) {
 	s.assertUpgradeDryRun(c, "upgrade-controller", s.upgradeControllerCommand)
 }