Reviewers: mp+160910_code.launchpad.net,

Message:
Please take a look.

Description:
cmd/juju: upgrade-charm --switch support

This implements a new argument to upgrade-charm:
--switch <charm-url>. The passed URL is inferred
to get the complete URL and used instead of the
service's newest charm url revision.

There are a few related sanity checks that need
to be implemented as well (in follow-ups), so we
can better check charm compatibility before upgrading.
Fixes bug #1040203.

https://code.launchpad.net/~dimitern/juju-core/038-upgrade-charm-switch/+merge/160910

(do not edit description out of merge proposal)


Please review this at https://codereview.appspot.com/8540050/

Affected files:
   A [revision details]
   M cmd/juju/upgradecharm.go
   M cmd/juju/upgradecharm_test.go
   M testing/charm.go


Index: [revision details]
=== added file '[revision details]'
--- [revision details]	2012-01-01 00:00:00 +0000
+++ [revision details]	2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:  
<email address hidden>
+New revision:  
<email address hidden>

Index: testing/charm.go
=== modified file 'testing/charm.go'
--- testing/charm.go	2013-02-11 05:54:45 +0000
+++ testing/charm.go	2013-04-25 14:27:20 +0000
@@ -54,6 +54,15 @@
  	return clone(dst, r.DirPath(name))
  }

+// RenamedClonedDirPath returns the path to a new copy of the default
+// charm directory named name, but renames it to newName.
+func (r *Repo) RenamedClonedDirPath(dst, name, newName string) string {
+	newDst := clone(dst, r.DirPath(name))
+	renamedDst := filepath.Join(filepath.Dir(newDst), newName)
+	check(exec.Command("mv", newDst, renamedDst).Run())
+	return renamedDst
+}
+
  // ClonedDir returns an actual charm.Dir based on a new copy of the charm  
directory
  // named name, in the directory dst.
  func (r *Repo) ClonedDir(dst, name string) *charm.Dir {


Index: cmd/juju/upgradecharm.go
=== modified file 'cmd/juju/upgradecharm.go'
--- cmd/juju/upgradecharm.go	2013-04-25 10:46:51 +0000
+++ cmd/juju/upgradecharm.go	2013-04-25 14:27:20 +0000
@@ -17,6 +17,7 @@
  	ServiceName string
  	Force       bool
  	RepoPath    string // defaults to JUJU_REPOSITORY
+	SwitchURL   string
  }

  const upgradeCharmDoc = `
@@ -32,6 +33,10 @@
  author working on a single client machine; use of local repositories from
  multiple clients is not supported and may lead to confusing behaviour.

+To manually specify the charm URL to upgrade to, use the --switch argument.
+It will be used instead of the service's current charm newest revision.
+Note that the given charm must be compatible with the current one.
+
  Use of the --force flag is not generally recommended; units upgraded while  
in
  an error state will not have upgrade-charm hooks executed, and may cause
  unexpected behavior.
@@ -50,6 +55,7 @@
  	c.EnvCommandBase.SetFlags(f)
  	f.BoolVar(&c.Force, "force", false, "upgrade all units immediately, even  
if in error state")
  	f.StringVar(&c.RepoPath, "repository",  
os.Getenv("JUJU_REPOSITORY"), "local charm repository path")
+	f.StringVar(&c.SwitchURL, "switch", "", "charm URL to upgrade to")
  }

  func (c *UpgradeCharmCommand) Init(args []string) error {
@@ -64,7 +70,6 @@
  	default:
  		return cmd.CheckEmpty(args[1:])
  	}
-	// TODO(dimitern): add the other flags --switch and --revision.
  	return nil
  }

@@ -80,7 +85,20 @@
  	if err != nil {
  		return err
  	}
-	curl, _ := service.CharmURL()
+	var curl *charm.URL
+	if c.SwitchURL != "" {
+		var err error
+		conf, err := conn.State.EnvironConfig()
+		if err != nil {
+			return err
+		}
+		curl, err = charm.InferURL(c.SwitchURL, conf.DefaultSeries())
+		if err != nil {
+			return err
+		}
+	} else {
+		curl, _ = service.CharmURL()
+	}
  	repo, err := charm.InferRepository(curl, ctx.AbsPath(c.RepoPath))
  	if err != nil {
  		return err


Index: cmd/juju/upgradecharm_test.go
=== modified file 'cmd/juju/upgradecharm_test.go'
--- cmd/juju/upgradecharm_test.go	2013-03-27 15:42:27 +0000
+++ cmd/juju/upgradecharm_test.go	2013-04-25 14:27:20 +0000
@@ -58,6 +58,18 @@
  	c.Assert(err, ErrorMatches, `already running latest  
charm "local:precise/riak-7"`)
  }

+func (s *UpgradeCharmErrorsSuite) TestInvalidSwitchURL(c *C) {
+	testing.Charms.ClonedDirPath(s.seriesPath, "riak")
+	err := runDeploy(c, "local:riak", "riak")
+	c.Assert(err, IsNil)
+
+	err = runUpgradeCharm(c, "riak", "--switch=blah")
+	c.Assert(err, ErrorMatches, "charm not found: cs:precise/blah")
+	err = runUpgradeCharm(c, "riak", "--switch=cs:missing/one-1")
+	c.Assert(err, ErrorMatches, "charm not found: cs:missing/one")
+	// TODO(dimitern): add tests with incompatible charms
+}
+
  type UpgradeCharmSuccessSuite struct {
  	repoSuite
  	path string
@@ -79,7 +91,7 @@
  	c.Assert(forced, Equals, false)
  }

-func (s *UpgradeCharmSuccessSuite) assertUpgraded(c *C, revision int,  
forced bool) {
+func (s *UpgradeCharmSuccessSuite) assertUpgraded(c *C, revision int,  
forced bool, curl *charm.URL) {
  	err := s.riak.Refresh()
  	c.Assert(err, IsNil)
  	ch, force, err := s.riak.Charm()
@@ -87,10 +99,13 @@
  	c.Assert(ch.Revision(), Equals, revision)
  	c.Assert(force, Equals, forced)
  	s.assertCharmUploaded(c, ch.URL())
+	if curl != nil {
+		c.Assert(ch.URL().String(), Equals, curl.String())
+	}
  }

-func (s *UpgradeCharmSuccessSuite) assertLocalRevision(c *C, revision int)  
{
-	dir, err := charm.ReadDir(s.path)
+func (s *UpgradeCharmSuccessSuite) assertLocalRevision(c *C, revision int,  
path string) {
+	dir, err := charm.ReadDir(path)
  	c.Assert(err, IsNil)
  	c.Assert(dir.Revision(), Equals, revision)
  }
@@ -98,8 +113,8 @@
  func (s *UpgradeCharmSuccessSuite) TestBumpsRevisionWhenNecessary(c *C) {
  	err := runUpgradeCharm(c, "riak")
  	c.Assert(err, IsNil)
-	s.assertUpgraded(c, 8, false)
-	s.assertLocalRevision(c, 8)
+	s.assertUpgraded(c, 8, false, nil)
+	s.assertLocalRevision(c, 8, s.path)
  }

  func (s *UpgradeCharmSuccessSuite)  
TestDoesntBumpRevisionWhenNotNecessary(c *C) {
@@ -110,8 +125,8 @@

  	err = runUpgradeCharm(c, "riak")
  	c.Assert(err, IsNil)
-	s.assertUpgraded(c, 42, false)
-	s.assertLocalRevision(c, 42)
+	s.assertUpgraded(c, 42, false, nil)
+	s.assertLocalRevision(c, 42, s.path)
  }

  func (s *UpgradeCharmSuccessSuite) TestUpgradesWithBundle(c *C) {
@@ -128,13 +143,38 @@

  	err = runUpgradeCharm(c, "riak")
  	c.Assert(err, IsNil)
-	s.assertUpgraded(c, 42, false)
-	s.assertLocalRevision(c, 7)
+	s.assertUpgraded(c, 42, false, nil)
+	s.assertLocalRevision(c, 7, s.path)
  }

  func (s *UpgradeCharmSuccessSuite) TestForcedUpgrade(c *C) {
  	err := runUpgradeCharm(c, "riak", "--force")
  	c.Assert(err, IsNil)
-	s.assertUpgraded(c, 8, true)
-	s.assertLocalRevision(c, 8)
+	s.assertUpgraded(c, 8, true, nil)
+	s.assertLocalRevision(c, 8, s.path)
+}
+
+var myriakMeta = []byte(`
+name: myriak
+summary: "K/V storage engine"
+description: "Scalable K/V Store in Erlang with Clocks :-)"
+provides:
+  endpoint:
+    interface: http
+  admin:
+    interface: http
+peers:
+  ring:
+    interface: riak
+`)
+
+func (s *UpgradeCharmSuccessSuite) TestSwitch(c *C) {
+	myriakPath :=  
testing.Charms.RenamedClonedDirPath(s.seriesPath, "riak", "myriak")
+	err := ioutil.WriteFile(path.Join(myriakPath, "metadata.yaml"),  
myriakMeta, 0644)
+	c.Assert(err, IsNil)
+
+	err = runUpgradeCharm(c, "riak", "--switch=local:myriak")
+	c.Assert(err, IsNil)
+	s.assertUpgraded(c, 7, false,  
charm.MustParseURL("local:precise/myriak-7"))
+	s.assertLocalRevision(c, 7, myriakPath)
  }