Fix a couple of small issues:

1. deploy a bundle with local charms should honour --force 2. print a message when removing a non existent storage pool
bnhassin · Apr 7, 2020 · ee7f459 · ee7f459
1 parent 4760c27
commit ee7f459
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 11 deletions.
diff --git a/cmd/juju/application/bundle.go b/cmd/juju/application/bundle.go
@@ -519,7 +519,7 @@ func (h *bundleHandler) addCharm(change *bundlechanges.AddCharmChange) error {
 		if series == "" {
 			series = h.data.Series
 		}
-		ch, curl, err := charmrepo.NewCharmAtPath(charmPath, series)
+		ch, curl, err := charmrepo.NewCharmAtPathForceSeries(charmPath, series, h.force)
 		if err != nil && !os.IsNotExist(err) {
 			return errors.Annotatef(err, "cannot deploy local charm at %q", charmPath)
 		}

diff --git a/cmd/juju/application/bundle_test.go b/cmd/juju/application/bundle_test.go
@@ -475,6 +475,45 @@ func (s *BundleDeployCharmStoreSuite) TestDeployBundleLocalPath(c *gc.C) {
 	})
 }
 
+func (s *BundleDeployCharmStoreSuite) TestDeployBundleLocalPathInvalidSeriesWithForce(c *gc.C) {
+	s.assertDeployBundleLocalPathInvalidSeriesWithForce(c, true)
+}
+
+func (s *BundleDeployCharmStoreSuite) TestDeployBundleLocalPathInvalidSeriesWithoutForce(c *gc.C) {
+	s.assertDeployBundleLocalPathInvalidSeriesWithForce(c, false)
+}
+
+func (s *BundleDeployCharmStoreSuite) assertDeployBundleLocalPathInvalidSeriesWithForce(c *gc.C, force bool) {
+	dir := c.MkDir()
+	testcharms.RepoWithSeries("bionic").ClonedDir(dir, "dummy")
+	path := filepath.Join(dir, "mybundle")
+	data := `
+        series: focal
+        applications:
+            dummy:
+                charm: ./dummy
+                num_units: 1
+    `
+	err := ioutil.WriteFile(path, []byte(data), 0644)
+	c.Assert(err, jc.ErrorIsNil)
+	args := []string{path}
+	if force {
+		args = append(args, "--force")
+	}
+	err = s.runDeploy(c, args...)
+	if !force {
+		c.Assert(err, gc.ErrorMatches, "cannot deploy bundle: dummy is not available on the following series: focal not supported")
+		return
+	}
+	c.Assert(err, jc.ErrorIsNil)
+	s.assertCharmsUploaded(c, "local:focal/dummy-1")
+	ch, err := s.State.Charm(charm.MustParseURL("local:focal/dummy-1"))
+	c.Assert(err, jc.ErrorIsNil)
+	s.assertApplicationsDeployed(c, map[string]applicationInfo{
+		"dummy": {charm: "local:focal/dummy-1", config: ch.Config().DefaultSettings()},
+	})
+}
+
 func (s *BundleDeployCharmStoreSuite) TestDeployBundleLocalResources(c *gc.C) {
 	data := `
         series: bionic

diff --git a/cmd/juju/storage/pooldelete.go b/cmd/juju/storage/pooldelete.go
@@ -7,6 +7,7 @@ import (
 	"github.com/juju/cmd"
 	"github.com/juju/errors"
 
+	"github.com/juju/juju/apiserver/params"
 	jujucmd "github.com/juju/juju/cmd"
 	"github.com/juju/juju/cmd/modelcmd"
 )
@@ -77,5 +78,10 @@ func (c *poolRemoveCommand) Run(ctx *cmd.Context) (err error) {
 	if api.BestAPIVersion() < 5 {
 		return errors.New("removing storage pools is not supported by this version of Juju")
 	}
-	return api.RemovePool(c.poolName)
+	err = api.RemovePool(c.poolName)
+	if params.IsCodeNotFound(err) {
+		ctx.Infof("removing storage pool %s failed: %s", c.poolName, err)
+		return cmd.ErrSilent
+	}
+	return err
 }
diff --git a/cmd/juju/storage/pooldelete_test.go b/cmd/juju/storage/pooldelete_test.go
@@ -6,9 +6,11 @@ package storage_test
 import (
 	"github.com/juju/cmd"
 	"github.com/juju/cmd/cmdtesting"
+	"github.com/juju/errors"
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
 
+	"github.com/juju/juju/apiserver/params"
 	"github.com/juju/juju/cmd/juju/storage"
 	_ "github.com/juju/juju/provider/dummy"
 )
@@ -56,14 +58,23 @@ func (s *PoolRemoveSuite) TestPoolRemoveUnsupportedAPIVersion(c *gc.C) {
 	c.Assert(len(s.mockAPI.RemovedPools), gc.Equals, 0)
 }
 
+func (s *PoolRemoveSuite) TestPoolRemoveNotFound(c *gc.C) {
+	s.mockAPI.err = params.Error{
+		Code: params.CodeNotFound,
+	}
+	_, err := s.runPoolRemove(c, []string{"sunshine"})
+	c.Assert(errors.Cause(err), gc.Equals, cmd.ErrSilent)
+}
+
 type mockPoolRemoveAPI struct {
 	APIVersion   int
 	RemovedPools []string
+	err          error
 }
 
 func (s *mockPoolRemoveAPI) RemovePool(pname string) error {
 	s.RemovedPools = append(s.RemovedPools, pname)
-	return nil
+	return s.err
 }
 
 func (s mockPoolRemoveAPI) Close() error {

diff --git a/state/storage_test.go b/state/storage_test.go
@@ -697,13 +697,10 @@ func (s *StorageStateSuite) TestRemoveStoragePoolInUse(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(pool.Name(), gc.Equals, poolName)
 
-	// pool does not exist at all, poolmanager swallows NotFound errors.
 	_, err = s.pm.Get("nope-pool")
 	c.Assert(err, gc.ErrorMatches, `pool "nope-pool" not found`)
 	err = s.storageBackend.RemoveStoragePool("nope-pool")
-	c.Assert(err, jc.ErrorIsNil)
-	_, err = s.pm.Get("nope-pool")
-	c.Assert(err, gc.ErrorMatches, `pool "nope-pool" not found`)
+	c.Assert(err, gc.ErrorMatches, `storage pool "nope-pool" not found`)
 }
 
 func (s *StorageStateSuite) TestStorageAttachments(c *gc.C) {

diff --git a/storage/poolmanager/poolmanager.go b/storage/poolmanager/poolmanager.go
@@ -81,8 +81,8 @@ func (pm *poolManager) validatedConfig(name string, providerType storage.Provide
 // Delete is defined on PoolManager interface.
 func (pm *poolManager) Delete(name string) error {
 	err := pm.settings.RemoveSettings(globalKey(name))
-	if err == nil || errors.IsNotFound(err) {
-		return nil
+	if errors.IsNotFound(err) {
+		return errors.NotFoundf("storage pool %q", name)
 	}
 	return errors.Annotatef(err, "deleting pool %q", name)
 }

diff --git a/storage/poolmanager/poolmanager_test.go b/storage/poolmanager/poolmanager_test.go
@@ -180,7 +180,6 @@ func (s *poolSuite) TestDelete(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 	_, err = s.poolManager.Get("testpool")
 	c.Assert(err, jc.Satisfies, errors.IsNotFound)
-	// Delete again, no error.
 	err = s.poolManager.Delete("testpool")
-	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(err, jc.Satisfies, errors.IsNotFound)
 }