cmd/juju: upgrade-charm --switch support

cmd/juju/upgradecharm.go

 package main
 
 import (
         "errors"
         "fmt"
         "launchpad.net/gnuflag"
         "launchpad.net/juju-core/charm"
         "launchpad.net/juju-core/cmd"
         "launchpad.net/juju-core/juju"
         "launchpad.net/juju-core/state"
         "os"
 )
 
 // UpgradeCharm is responsible for upgrading a service's charm.
 type UpgradeCharmCommand struct {
         EnvCommandBase
         ServiceName string
         Force       bool
         RepoPath    string // defaults to JUJU_REPOSITORY
+        SwitchURL   string
+        Revision    int // defaults to -1 (latest)
 }
 
 const upgradeCharmDoc = `
-When no flags are set, the service's charm will be upgraded to the latest
-revision available in the repository from which it was originally deployed.
+When no flags are set, the service's charm will be upgraded to the
+latest revision available in the repository from which it was
+originally deployed. An explicit revision can be chosen with the
+--revision flag.
 
-If the charm came from a local repository, its path will be assumed to be
-$JUJU_REPOSITORY unless overridden by --repository. If there is no newer
-revision of a local charm directory, the local directory's revision will be
-automatically incremented to create a newer charm.
+If the charm came from a local repository, its path will be assumed to
+be $JUJU_REPOSITORY unless overridden by --repository. If there is no
+newer revision of a local charm directory, the local directory's
+revision will be automatically incremented to create a newer charm.
 
-The local repository behaviour is tuned specifically to the workflow of a charm
-author working on a single client machine; use of local repositories from
-multiple clients is not supported and may lead to confusing behaviour.
+The local repository behaviour is tuned specifically to the workflow
+of a charm author working on a single client machine; use of local
+repositories from multiple clients is not supported and may lead to
+confusing behaviour.
 
-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.
+The --switch flag allows you to replace the charm with an entirely
+different one. The new charm's URL and revision are inferred as they
+would be when running a deploy command.
+
+Please note that --switch is dangerous, because juju only has limited
+information with which to determine compatibility; the operation will
+succeed, regardless of potential havoc, so long as the following
+conditions hold:
+
+- The new charm must declare all relations that the service is
+currently participating in.
+- All config settings shared by the old and new charms must have the

[rog, 2013/04/29 16:11:51]

that's not quite right, is it? i believe the new charm must declare all the
settings declared by the old, whereas this seems to imply that only the
intersection of old and new matters.

but maybe we the error message will be suitably self-explanatory if this
happens.

+same types.
+
+The new charm may add new relations and configuration settings.

[fwereade, 2013/04/29 15:45:18]

I think we can drop this line.

+
+--switch and --revision are mutually exclusive. To specify a given
+revision number with --switch, give it in the charm URL, for instance
+"cs:wordpress-5" would specify revision number 5 of the wordpress
+charm.
+
+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.
 `
 
 func (c *UpgradeCharmCommand) Info() *cmd.Info {
         return &cmd.Info{
                 Name:    "upgrade-charm",
                 Args:    "<service>",
                 Purpose: "upgrade a service's charm",
                 Doc:     upgradeCharmDoc,
         }
 }
 
 func (c *UpgradeCharmCommand) SetFlags(f *gnuflag.FlagSet) {
         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", "", "crossgrade to a different charm")
+        f.IntVar(&c.Revision, "revision", -1, "explicit revision of current charm")
 }
 
 func (c *UpgradeCharmCommand) Init(args []string) error {
         switch len(args) {
         case 1:
                 if !state.IsServiceName(args[0]) {
                         return fmt.Errorf("invalid service name %q", args[0])
                 }
                 c.ServiceName = args[0]
         case 0:
                 return errors.New("no service specified")
         default:
                 return cmd.CheckEmpty(args[1:])
         }
-        // TODO(dimitern): add the other flags --switch and --revision.
+        if c.SwitchURL != "" && c.Revision != -1 {
+                return fmt.Errorf("--switch and --revision are mutually exclusive")
+        }
         return nil
 }
 
 // Run connects to the specified environment and starts the charm
 // upgrade process.
 func (c *UpgradeCharmCommand) Run(ctx *cmd.Context) error {
         conn, err := juju.NewConnFromName(c.EnvName)
         if err != nil {
                 return err
         }
         defer conn.Close()
         service, err := conn.State.Service(c.ServiceName)
         if err != nil {
                 return err
         }
-        curl, _ := service.CharmURL()
-        repo, err := charm.InferRepository(curl, ctx.AbsPath(c.RepoPath))
+        oldURL, _ := service.CharmURL()
+        var newURL *charm.URL
+        if c.SwitchURL != "" {
+                // A new charm URL was explicitly specified.
+                conf, err := conn.State.EnvironConfig()
+                if err != nil {
+                        return err
+                }
+                newURL, err = charm.InferURL(c.SwitchURL, conf.DefaultSeries())
+                if err != nil {
+                        return err
+                }
+        } else {
+                // No new URL specified, but revision might have been.
+                newURL = oldURL.WithRevision(c.Revision)
+        }
+        repo, err := charm.InferRepository(newURL, ctx.AbsPath(c.RepoPath))
         if err != nil {
                 return err
         }
-        rev, err := repo.Latest(curl)
+        // If no explicit revision was set with either SwitchURL
+        // or Revision flags, discover the latest.
+        explicitRevision := true
+        if newURL.Revision == -1 {
+                explicitRevision = false
+                latest, err := repo.Latest(newURL)
+                if err != nil {
+                        return err
+                }
+                newURL = newURL.WithRevision(latest)
+        }
+        bumpRevision := false
+        if *newURL == *oldURL && !explicitRevision {

[fwereade, 2013/04/29 15:45:18]

If newURL matches oldURL, we have to take this branch. We don't need to worry
about explicitRevision until...

[rog, 2013/04/29 16:11:51]

as discussed online, i'm not sure this is quite right yet.

[16:53:37] <rogpeppe> fwereade: but the case i'm thinking of is something like
this:
[16:54:14] <rogpeppe> fwereade: we have a local copy of a charm, deploy it, push
it to the charm store, switch to that, then make some changes to the local copy
and switch back to that
[16:54:48] <rogpeppe> fwereade: in that case, we want the version to be bumped,
but the urls are different

+                // Only try bumping the revision when necessary
+                // (local dir charm and no explicit revision).
+                if _, isLocal := repo.(*charm.LocalRepository); !isLocal {
+                        // TODO(dimitern): If the --force flag is set to something
+                        // different to before, we might actually want to allow this
+                        // case (and the other error below). LP bug #1174287
+                        return fmt.Errorf("already running latest charm %q", newURL)
+                }
+                // This is a local repository.
+                if ch, err := repo.Get(newURL); err != nil {
+                        return err
+                } else if _, bumpRevision = ch.(*charm.Dir); !bumpRevision {
+                        // Only bump the revision when it's a directory.
+                        return fmt.Errorf("already running latest charm %q", newURL)
+                }
+        }
+        sch, err := conn.PutCharm(newURL, repo, bumpRevision)

[fwereade, 2013/04/29 15:45:18]

It's somewhat crazy that we can specify a URL with an additional param that
means "lol not really". But that's way out of scope.

         if err != nil {
                 return err
         }
-        bumpRevision := false
-        if curl.Revision == rev {
-                if _, isLocal := repo.(*charm.LocalRepository); !isLocal {
-                        return fmt.Errorf("already running latest charm %q", curl)
-                }
-                // This is a local repository.
-                if ch, err := repo.Get(curl); err != nil {
-                        return err
-                } else if _, bumpRevision = ch.(*charm.Dir); !bumpRevision {

[fwereade, 2013/04/29 15:45:18]

...here -- in which case we should only set bumpRevision if explicitRevision is
false.

-                        // Only bump the revision when it's a directory.
-                        return fmt.Errorf("already running latest charm %q", curl)
-                }
-        }
-        sch, err := conn.PutCharm(curl.WithRevision(rev), repo, bumpRevision)
-        if err != nil {
-                return err
-        }
         return service.SetCharm(sch, c.Force)
 }