combine charm and hook states as uniter state

worker/uniter/state.go

+package uniter
+
+import (
+        "errors"
+        "fmt"
+        "launchpad.net/juju-core/charm"
+        "launchpad.net/juju-core/trivial"
+        "launchpad.net/juju-core/worker/uniter/hook"
+        "os"
+)
+
+// Op enumerates the operations the uniter can perform.
+type Op string
+
+const (
+        // Install indicates that the uniter is installing the charm.
+        Install Op = "install"
+
+        // RunHook indicates that the uniter is running a hook.
+        RunHook Op = "run-hook"
+
+        // Upgrade indicates that the uniter is upgrading the charm.
+        Upgrade Op = "upgrade"
+
+        // Abide indicates that the uniter is not currently executing
+        // any other operation.
+        Abide Op = "abide"
+)
+
+// Status enumerates the possible operation statuses.
+type Status string

[niemeyer, 2012/09/11 14:52:04]

Can we call this and the field in State as OpStep? This would make the relation
with Op more clear, and would help us moving away from the State.Status pair
which is slightly hard to grasp (the uniter isn't "pending", the op is).

[fwereade, 2012/09/12 15:16:40]

SGTM. Done.

+
+const (
+        // Queued indicates that the uniter should undertake the operation
+        // as soon as possible.
+        Queued Status = "queued"
+
+        // Pending indicates that the uniter has started, but not completed,
+        // the operation.
+        Pending Status = "pending"
+
+        // Committing indicates that the uniter has completed the operation,

[niemeyer, 2012/09/11 14:52:04]

This status is feeling a bit awkward now. All of the intermediate states are
also "committed" in the sense of synchronizing state, and it's weird to call
committ*ing* a status that means *has* complet*ed*.

I suggest calling this as "Done", and being more succinct in saying that this
means the operation was completed.

+        // but has yet to synchronize all necessary state.
+        Committing Status = "committing"
+)
+
+// State defines the local persistent state of the uniter, excluding relation
+// state.
+type State struct {
+        // Op indicates the current operation.
+        Op Op
+
+        // Status indicates the current operation's status.
+        Status Status
+
+        // Hook holds hook information relevant to the current operation. If Op
+        // is Abide, it holds the last hook that was executed; if Op is RunHook,
+        // it holds the running hook; if Op is Upgrade, a non-nil hook indicates
+        // that the uniter should return to that hook's Pending state after the
+        // upgrade is complete (instead of running an upgrade-charm hook).
+        Hook *hook.Info `yaml:"hook,omitempty"`
+
+        // Charm describes the charm being deployed by an Install or Upgrade
+        // operation, and is otherwise blank.
+        CharmURL *charm.URL `yaml:"charm,omitempty"`
+}
+
+// validate returns an error if the state violates expectations.
+func (st State) validate() error {
+        hasHook := st.Hook != nil
+        hasCharm := st.CharmURL != nil
+        switch st.Op {
+        case Install:
+                if hasHook {
+                        return fmt.Errorf("unexpected hook info")
+                }
+                fallthrough
+        case Upgrade:
+                if !hasCharm {
+                        return fmt.Errorf("missing charm URL")
+                }
+        case Abide, RunHook:
+                if !hasHook {
+                        return fmt.Errorf("missing hook info")
+                } else if hasCharm {
+                        return fmt.Errorf("unexpected charm URL")
+                }
+        default:
+                return fmt.Errorf("unknown operation %q", st.Op)
+        }
+        switch st.Status {
+        case Queued, Pending, Committing:
+        default:
+                return fmt.Errorf("unknown operation status %q", st.Status)
+        }
+        if hasHook {
+                return st.Hook.Validate()
+        }
+        return nil
+}
+
+// StateFile reads and writes uniter operation state.

[niemeyer, 2012/09/11 14:52:04]

// A StateFile holds the disk state for a uniter.

?

[fwereade, 2012/09/12 15:16:40]

Done.

+type StateFile struct {
+        path string
+}
+
+// NewStateFile returns a new StateFile using path.
+func NewStateFile(path string) *StateFile {
+        return &StateFile{path}
+}
+
+var ErrNoStateFile = errors.New("uniter state file does not exist")
+
+// Read reads a State from the file. If the file does not exist it returns
+// ErrNoStateFile.
+func (f *StateFile) Read() (State, error) {

[niemeyer, 2012/09/11 14:52:04]

A *State here would be nice to work with and idiomatic, I think.

[fwereade, 2012/09/12 15:16:40]

Done.

+        var st State
+        if err := trivial.ReadYaml(f.path, &st); err != nil {
+                if os.IsNotExist(err) {
+                        return State{}, ErrNoStateFile
+                }
+        }
+        if err := st.validate(); err != nil {
+                return State{}, fmt.Errorf("invalid uniter state at %s: %s", f.path, err)

[niemeyer, 2012/09/11 14:52:04]

s/at %s: %s/at %q: %v/

[fwereade, 2012/09/12 15:16:40]

Done.

+        }
+        return st, nil
+}
+
+// Write stores the supplied state to the file.
+func (f *StateFile) Write(op Op, status Status, hi *hook.Info, url *charm.URL) error {
+        if hi != nil {
+                // Strip membership info: it's potentially large, and can
+                // be reconstructed from relation state when required.

[niemeyer, 2012/09/11 14:52:04]

This reflects better the intended operation, and remains working when new fields
are added:

hiCopy = *hi
hiCopy.Members = nil
hi = &hiCopy

[fwereade, 2012/09/12 15:16:40]

Nice. Done.

+                hi = &hook.Info{
+                        Kind:          hi.Kind,
+                        RelationId:    hi.RelationId,
+                        RemoteUnit:    hi.RemoteUnit,
+                        ChangeVersion: hi.ChangeVersion,
+                }
+        }
+        st := &State{
+                Op:       op,
+                Status:   status,
+                Hook:     hi,
+                CharmURL: url,
+        }
+        if err := st.validate(); err != nil {
+                panic(err)
+        }
+        return trivial.WriteYaml(f.path, st)
+}