cmd/jujud: implement uniter upgrade

cmd/jujud/unit.go

 package main
 
 import (
         "fmt"
         "launchpad.net/gnuflag"
         "launchpad.net/juju-core/cmd"
+        "launchpad.net/juju-core/environs"
         "launchpad.net/juju-core/log"
         "launchpad.net/juju-core/state"
         "launchpad.net/juju-core/worker/uniter"
         "launchpad.net/tomb"
         "time"
 )
 
 // UnitAgent is a cmd.Command responsible for running a unit agent.
 type UnitAgent struct {
         tomb     tomb.Tomb
@@ skipping 25 matching lines @@
 // Stop stops the unit agent.
 func (a *UnitAgent) Stop() error {
         a.tomb.Kill(nil)
         return a.tomb.Wait()
 }
 
 // Run runs a unit agent.
 func (a *UnitAgent) Run(ctx *cmd.Context) error {
         defer a.tomb.Done()
         for a.tomb.Err() == tomb.ErrStillAlive {
-                err := a.runOnce()
-                log.Printf("uniter error: %v", err)
+                unit, err := a.runOnce()

[fwereade, 2012/09/28 08:32:19]

As discussed, the unit belongs only with the UpgradedError, and shouldn't really
be allowed out to complicate things here.

[rog, 2012/09/28 12:06:32]

Done.

+                if ug, ok := err.(*UpgradedError); ok {
+                        tools, err1 := environs.ChangeAgentTools(a.Conf.DataDir, unit.PathKey(), ug.Binary)
+                        if err1 == nil {
+                                log.Printf("exiting to upgrade to %v from %q", tools.Binary, tools.URL)

[niemeyer, 2012/09/27 17:07:53]

("exiting to upgrade from %v to %v (%q)", version.Current, tools.Binary,
tools.URL)

This means we can tell what the jump was.. will be useful when we stumble upon
bugs.

[rog, 2012/09/28 12:06:32]

done. the upgraded error now includes this information, and is now returned from
Run and will be printed before exit.

+                                // Return and let upstart deal with the restart.
+                                return nil
+                        }
+                        err = err1
+                }

[niemeyer, 2012/09/27 17:07:53]

log.Printf("uniter error: %v", err)

[rog, 2012/09/28 12:06:32]

done (omitting the "error" which should be self evident from the way we phrase
error messages)

                 select {
                 case <-a.tomb.Dying():
                         a.tomb.Kill(err)
                 case <-time.After(retryDelay):
-                        log.Printf("rerunning uniter")
+                        log.Printf("rerunning uniter after error: %v", err)

[niemeyer, 2012/09/27 17:07:53]

The original logging with "rerunning uniter" still sounds correct. We want to
see the error logged above, no matter which branch of the select we get here.

[rog, 2012/09/28 12:06:32]

Done.

                 }
         }
         return a.tomb.Err()
 }
 
 // runOnce runs a uniter once.
-func (a *UnitAgent) runOnce() error {
+func (a *UnitAgent) runOnce() (*state.Unit, error) {
         st, err := state.Open(&a.Conf.StateInfo)
         if err != nil {
-                return err
+                return nil, err
         }
         defer st.Close()
-        u, err := uniter.NewUniter(st, a.UnitName, a.Conf.DataDir)
+        unit, err := st.Unit(a.UnitName)
         if err != nil {
-                return err
+                return nil, err
         }

[fwereade, 2012/09/28 08:32:19]

As discussed live, please use ErrDead (and use that as the only reason for a nil
return, and add a "normal exit 0" stanza to the upstart conf). And don't return
the unit :).

[rog, 2012/09/28 12:06:32]

Done.

-        return runTasks(a.tomb.Dying(), u)
+        return unit, runTasks(a.tomb.Dying(),
+                uniter.NewUniter(st, unit.Name(), a.Conf.DataDir),

[fwereade, 2012/09/28 08:32:19]

I'm OK with the Uniter change though :).

+                NewUpgrader(st, unit, a.Conf.DataDir),
+        )
 }