(wip)o/h/ctlcmd, t/main/snapctl-tasks: start work on unit and spread testing

Author: Rnfudge02

overlord/hookstate/ctlcmd/helpers.go

@@ -56,14 +56,18 @@ var (
 	snapctlRemoveChangeKind  = swfeats.RegisterChangeKind("snapctl-remove")
 )
 
+var (
+	changeNotFoundError = "change %q not found"
+)
+
 func init() {
 	finalTasks = make(map[string]bool, len(snapstate.FinalTasks))
 	for _, kind := range snapstate.FinalTasks {
 		finalTasks[kind] = true
 	}
 }
 
-const snapctlDebounceWindow = 200 * time.Millisecond
+const snapctlDebounceWindow = 100 * time.Millisecond
 
 // finalSeedTask is the last task that should run during seeding. This is used
 // in the special handling of the "seed" change, which requires that we
@@ -645,17 +649,17 @@ func isReady(hctx *hookstate.Context, changeID string) (state.Status, error) {
 	chg := st.Change(changeID)
 
 	if chg == nil {
-		return state.DefaultStatus, fmt.Errorf("change %q not found", changeID)
+		return state.DefaultStatus, fmt.Errorf(changeNotFoundError, changeID)
 	}
 
 	var initiatorSnapName string
 	err := chg.Get("initiated-by-snap", &initiatorSnapName)
 	if err != nil {
-		return state.DefaultStatus, fmt.Errorf("change %q not found", changeID)
+		return state.DefaultStatus, fmt.Errorf(changeNotFoundError, changeID)
 	}
 
 	if initiatorSnapName != callerSnapName {
-		return state.DefaultStatus, fmt.Errorf("change %q not found", changeID)
+		return state.DefaultStatus, fmt.Errorf(changeNotFoundError, changeID)
 	}
 
 	wait, err := rateLimit(st, changeID, snapctlDebounceWindow)
@@ -764,8 +768,9 @@ func setChangeAccessedAt(st *state.State, accessed time.Time, changeID string) {
 	st.Cache(key, accessed.UnixNano())
 }
 
-// changeStatus checks if the change is ready, if it is, it returns the status, otherwise st.Doing.
-func changeStatus(hctx *hookstate.Context, changeID string) (state.Change, error) {
+// getAssociatedChange returns a change associated with the snapctl context and passed change ID,
+// otherwise nil with error
+func getAssociatedChange(hctx *hookstate.Context, changeID string) (*state.Change, error) {
 	callerSnapName := hctx.InstanceName()
 
 	st := hctx.State()
@@ -775,55 +780,29 @@ func changeStatus(hctx *hookstate.Context, changeID string) (state.Change, error
 	chg := st.Change(changeID)
 
 	if chg == nil {
-		return state.Change{}, fmt.Errorf("change %q not found", changeID)
+		return nil, fmt.Errorf(changeNotFoundError, changeID)
 	}
 
 	var initiatorSnapName string
 	err := chg.Get("initiated-by-snap", &initiatorSnapName)
 	if err != nil {
-		return state.Change{}, fmt.Errorf("could not find initiator attribute for change %q", changeID)
+		return nil, fmt.Errorf(changeNotFoundError, changeID)
 	}
 
 	if initiatorSnapName != callerSnapName {
-		return state.Change{}, fmt.Errorf("change %q was initiated by another snap", changeID)
+		return nil, fmt.Errorf(changeNotFoundError, changeID)
 	}
 
-	lastAccess := st.Cached(fmt.Sprintf("snapctl-%s-last-accessed", callerSnapName))
-	st.Cache(fmt.Sprintf("snapctl-%s-last-accessed", hctx.InstanceName()), time.Now().UnixNano())
-
-	// Compute how long to wait before checking the change status.
-	var toWait time.Duration
-	if lastAccess != nil {
-		lastAccessNano, ok := lastAccess.(int64)
-		if !ok {
-			return state.Change{}, fmt.Errorf("invalid last accessed time format for change %q", changeID)
-		}
-		toWait = 200*time.Millisecond - time.Since(time.Unix(0, lastAccessNano))
+	wait, err := rateLimit(st, changeID, snapctlDebounceWindow)
+	if err != nil {
+		return nil, err
 	}
 
 	st.Unlock()
+	<-timeAfter(wait)
+	st.Lock()
 
-	ready := chg.Ready()
-
-	if toWait <= 0 {
-		select {
-		case <-ready:
-			st.Lock()
-			return *chg, nil
-		default:
-			st.Lock()
-			return state.Change{}, nil
-		}
-	}
-
-	select {
-	case <-ready:
-		st.Lock()
-		return *chg, nil
-	case <-timeAfter(toWait):
-		st.Lock()
-		return state.Change{}, nil
-	}
+	return chg, nil
 }
 
 // getAttribute unmarshals into result the value of the provided key from attributes map.

overlord/hookstate/ctlcmd/tasks.go

@@ -20,35 +20,60 @@
 package ctlcmd
 
 import (
+	"encoding/json"
 	"fmt"
+	"time"
+
+	"io"
+	"text/tabwriter"
 
 	"github.com/snapcore/snapd/i18n"
 	"github.com/snapcore/snapd/overlord/state"
+	"github.com/snapcore/snapd/timeutil"
 )
 
 type tasksCommand struct {
 	baseCommand
-	json bool
+	Format string `long:"format" required:"false" description:"Output format (json)"`
 }
 
 var shortTasksHelp = i18n.G(`Return a list of information associated with all change-ids.`)
 var longTasksHelp = i18n.G(`
-The tasks command is used to query the status of all change ids associated with
+Used to query the status of all change ids associated with
 snapctl commands running in asynchronous mode.
 
-$ snapctl tasks [--json]
+$ snapctl (tasks/change) [--format FORMAT]
   0: successfully reported change information, regardless of state of change
   1: any error (invalid change ID, permissions error)
-stdout: table of tasks, mirroring "snap tasks <change-id>" output
+stdout: table of tasks, mirroring "snap changes <change-id>" output
 stderr: empty for exit code 0. Contains relevant errors for exit code 1.
 `)
 
 func init() {
+	// Not using standard alias declaration as that's in *cmdInfo, used by client, we have *commandInfo
+	addCommand("change", shortTasksHelp, longTasksHelp, func() command {
+		return &tasksCommand{}
+	})
 	addCommand("tasks", shortTasksHelp, longTasksHelp, func() command {
 		return &tasksCommand{}
 	})
 }
 
+func (c *tasksCommand) newTabWriter(output io.Writer) *tabwriter.Writer {
+	minWidth := 2
+	tabWidth := 2
+	padding := 2
+	padchar := byte(' ')
+	return tabwriter.NewWriter(output, minWidth, tabWidth, padding, padchar, 0)
+}
+
+func fmtTime(t time.Time, abs bool) string {
+	if abs {
+		return t.Format(time.RFC3339)
+	}
+	return timeutil.Human(t)
+}
+
 func (c *tasksCommand) Execute(args []string) error {
 	ctx, err := c.ensureContext()
 	if err != nil {
@@ -59,21 +84,48 @@ func (c *tasksCommand) Execute(args []string) error {
 		return fmt.Errorf("invalid number of arguments: expected 1, got %d", len(args))
 	}
 
-	c.json = c.flagSet.Lookup("json").Value.String() == "true"
+	changeID := args[0]
 
-	ready, err := isReady(ctx, c.changeID)
+	change, err := getAssociatedChange(ctx, changeID)
 
 	if err != nil {
-		fmt.Fprintf(c.stderr, err.Error())
-		return &UnsuccessfulError{ExitCode: otherErrorExitCode}
-	}
-
-	if !ready.Ready() {
-		return &UnsuccessfulError{ExitCode: changeNotReadyExitCode}
+		return err
 	}
 
-	if ready != state.DoneStatus {
-		return &UnsuccessfulError{ExitCode: changeUnsuccessfulExitCode}
+	st := ctx.State()
+
+	if c.Format == "json" {
+		st.Lock()
+		data, err := json.Marshal(change)
+		st.Unlock()
+		if err != nil {
+			return err
+		}
+		fmt.Fprint(c.stdout, string(data))
+	} else {
+		w := c.newTabWriter(c.stdout)
+
+		fmt.Fprint(w, i18n.G("ID\tStatus\tSpawn\tReady\tSummary\n"))
+
+		st.Lock()
+		for _, t := range change.Tasks() {
+			spawnTime := fmtTime(t.SpawnTime(), false)
+			readyTime := fmtTime(t.ReadyTime(), false)
+			if t.ReadyTime().IsZero() {
+				readyTime = "-"
+			}
+			summary := t.Summary()
+			status := t.Status()
+			_, done, total := t.Progress()
+			if status == state.DoingStatus && total > 1 {
+				summary = fmt.Sprintf("%s (%.2f%%)", summary, float64(done)/float64(total)*100.0)
+			}
+			fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", status, spawnTime, readyTime, summary)
+		}
+		st.Unlock()
+
+		w.Flush()
+		fmt.Fprintln(c.stdout)
 	}
 
 	return nil