Actions document - suggested changes

[Tim Penhey]

Today as on call reviewer I was looking through some pull requests
specific to actions, and it brought up many questions.

On reading the spec[1] and looking at the state documents as they are in
trunk now, I was quickly coming to the conclusion that the documents
need to change.

Right now we have two action related documents:

type actionDoc struct {
	// Id is the key for this document.  Action.Id() has a specfic form
	// to facilitate filtering the actions collection for a given unit,
	// or in the future a given service.
	// The format of the Action.Id() will be:
	//   <unit globalKey> + actionMarker + <generated state sequence>
	Id string `bson:"_id"`

	// Name identifies the action; it should match an action defined by
	// the unit's charm.
	Name string

	// Payload holds the action's parameters, if any; it should validate
	// against the schema defined by the named action in the unit's charm
	Payload map[string]interface{}
}

and

type actionResultDoc struct {
	// Id is the key for this document.  The format of the id encodes
	// the id of the Action that was used to produce this ActionResult.
	// The format is: <action id> + actionResultMarker + <generated sequence>
	Id string `bson:"_id"`

	// ActionName identifies the action that was run.
	ActionName string

	// Payload describes the parameters passed in for the action
	// when it was run.
	Payload map[string]interface{}

	// Status represents the end state of the Action; ActionFailed for an
	// action that was removed prematurely, or that failed, and
	// ActionCompleted for an action that successfully completed.
	Status ActionStatus

	// Output captures any text emitted by the action.
	Output string
}

I think these should be combined, and there are quite a few missing
fields that are clearly expected from the spec:

	invoked:  TIME
	started:  TIME
	finished: TIME
	files:
	    - this is a tricky one

Obviously we are also missing the user that asked for the action.

Consider the following structure:

type actionDoc struct {
	// InternalId is opaque and you shouldn't care
	InternalId string `bson:"_id"`

	// UUID is the unique identifier for this action
	UUID string

	// EnvUUID is the environment UUID, and is here because
	// we know we want it when we have multiple environments
	EnvUUID string

	// Action is the name of the action, eg 'backup', 'restart'
	Action string

	// UnitName is the name of the unit that the action will
	// run on
	UnitName string

	// Payload holds the action's parameters, if any; it should
	// validate against the schema defined by the named action
	// in the unit's charm
	Payload map[string]interface{}

	// User is the user that requested this action
	User string

	// Invoked is when the action was requested
	Invoked time.Time

	// Started is recorded when the execution of the
	// action is started.
	Started *time.Time

	// Finished is recorded when the execution of the
	// action has completed.
	Finished *time.Time

	// ReturnCode is the result of the action
	ReturnCode *int

	// Results are defined and set by the action themselves.
	Results map[string]interface{}

	// Files is interesting, in that they should probably live
	// in the environment file storage, what was cloud storage
	// and is now GridFS, under some particular directory:
	// perhaps  /actions/<uuid>/<filename>
	// Both stdout and stderr are recorded as files.  They are
	// only added if they are not empty.
	// Other files can be added by the action.
	Files []string

}

status becomes an emergent property:
  if Started is nil
    "pending"
  if Finished is nil
    "started"
  if ReturnCode is 0 (not nil)
    "success"
  else
    "failure"


Queues, Lists, and Results are just queries across this document set for
the environment, optionally scoped to unit names and users.


Does that seem reasonable to other people?

Tim


[1]
https://docs.google.com/a/canonical.com/document/d/14W1-QqB1pXZxyZW5QzFFoDwxxeQXBUzgj8IUkLId6cc/edit#