Address review feedback

VERSION

@@ -1 +1 @@
-0.5.0
+0.6.0

pkg/kubectl-argo-rollouts/cmd/get/get.go

@@ -9,24 +9,32 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
 	"github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/info"
 	"github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/options"
 )
 
 var (
 	colorMapping = map[string]int{
+		// Colors for icons
 		info.IconWaiting:     FgYellow,
-		info.IconProgressing: FgBlue,
+		info.IconProgressing: FgHiBlue,
 		info.IconWarning:     FgRed,
 		info.IconUnknown:     FgYellow,
 		info.IconOK:          FgGreen,
 		info.IconBad:         FgRed,
-		info.IconPaused:      FgWhite,
+		//info.IconPaused:      FgWhite,
 		//info.IconNeutral:     FgWhite, // (foreground is better than white)
+
+		// Colors for canary/stable/preview tags
 		info.InfoTagCanary:  FgYellow,
 		info.InfoTagStable:  FgGreen,
 		info.InfoTagActive:  FgGreen,
-		info.InfoTagPreview: FgBlue,
+		info.InfoTagPreview: FgHiBlue,
+
+		// Colors for highlighting experiment/analysisruns
+		string(v1alpha1.AnalysisStatusPending): FgHiBlue,
+		string(v1alpha1.AnalysisStatusRunning): FgHiBlue,
 	}
 )
 
@@ -54,6 +62,7 @@ const (
 	FgCyan    = 36
 	FgWhite   = 37
 	FgDefault = 39
+	FgHiBlue  = 94
 )
 
 type GetOptions struct {
@@ -72,7 +81,7 @@ func NewCmdGet(o *options.ArgoRolloutsOptions) *cobra.Command {
   # Get a rollout
   %[1]s get rollout ROLLOUT
   # Get an experiment
-  %[1]s get experiment ROLLOUT
+  %[1]s get experiment EXPERIMENT
 `),
 		SilenceUsage: true,
 		RunE: func(c *cobra.Command, args []string) error {
@@ -107,6 +116,15 @@ func (o *GetOptions) colorize(s string) string {
 	return o.ansiFormat(s, color)
 }
 
+// colorizeStatus adds ansii color codes to the string based on supplied status string
+func (o *GetOptions) colorizeStatus(s string, status string) string {
+	if o.noColor {
+		return s
+	}
+	color := colorMapping[status]
+	return o.ansiFormat(s, color)
+}
+
 // ansiFormat wraps ANSI escape codes to a string to format the string to a desired color.
 // NOTE: we still apply formatting even if there is no color formatting desired.
 // The purpose of doing this is because when we apply ANSI color escape sequences to our
@@ -125,3 +143,17 @@ func (o *GetOptions) ansiFormat(s string, codes ...int) string {
 	sequence := strings.Join(codeStrs, ";")
 	return fmt.Sprintf("%s[%sm%s%s[%dm", escape, sequence, s, escape, noFormat)
 }
+
+// returns an appropriate tree prefix characters depending on whether or not the element is the
+// last element of a list
+func getPrefixes(isLast bool, subPrefix string) (string, string) {
+	var childPrefix, childSubpfx string
+	if !isLast {
+		childPrefix = subPrefix + "├──"
+		childSubpfx = subPrefix + "│  "
+	} else {
+		childPrefix = subPrefix + "└──"
+		childSubpfx = subPrefix + "   "
+	}
+	return childPrefix, childSubpfx
+}

pkg/kubectl-argo-rollouts/cmd/get/get_experiment.go

@@ -113,38 +113,21 @@ func (o *GetOptions) PrintExperimentTree(exInfo *info.ExperimentInfo) {
 }
 
 func (o *GetOptions) PrintExperimentInfo(w io.Writer, expInfo info.ExperimentInfo, prefix string, subpfx string) {
-	name := expInfo.Name
+	name := o.colorizeStatus(expInfo.Name, expInfo.Status)
 	infoCols := []string{}
 	total := len(expInfo.ReplicaSets) + len(expInfo.AnalysisRuns)
 	curr := 0
-	if expInfo.Status == "Running" {
-		name = o.ansiFormat(name, FgBlue)
-	}
 	fmt.Fprintf(w, "%s%s %s\t%s\t%s %s\t%s\t%v\n", prefix, IconExperiment, name, "Experiment", o.colorize(expInfo.Icon), expInfo.Status, expInfo.Age(), strings.Join(infoCols, ","))
 
 	for _, rsInfo := range expInfo.ReplicaSets {
-		isLast := curr == total-1
-		curr++
-		var childPrefix, childSubpfx string
-		if !isLast {
-			childPrefix = subpfx + "├──"
-			childSubpfx = subpfx + "│  "
-		} else {
-			childPrefix = subpfx + "└──"
-			childSubpfx = subpfx + "   "
-		}
+		childPrefix, childSubpfx := getPrefixes(curr == total-1, subpfx)
 		o.PrintReplicaSetInfo(w, rsInfo, childPrefix, childSubpfx)
+		curr++
 	}
 	for _, arInfo := range expInfo.AnalysisRuns {
 		fmt.Fprintf(w, subpfx)
-		isLast := curr == total-1
-		curr++
-		var arPrefix string
-		if !isLast {
-			arPrefix = "├──"
-		} else {
-			arPrefix = "└──"
-		}
+		arPrefix, _ := getPrefixes(curr == total-1, "")
 		o.PrintAnalysisRunInfo(w, arInfo, arPrefix, "")
+		curr++
 	}
 }

pkg/kubectl-argo-rollouts/cmd/get/get_rollout.go

@@ -10,7 +10,6 @@ import (
 	"github.com/juju/ansiterm"
 	"github.com/spf13/cobra"
 
-	"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
 	"github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/info"
 	"github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/options"
 	"github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/viewcontroller"
@@ -135,14 +134,7 @@ func (o *GetOptions) PrintRolloutTree(roInfo *info.RolloutInfo) {
 	revisions := roInfo.Revisions()
 	for i, rev := range revisions {
 		isLast := i == len(revisions)-1
-		var prefix, subpfx string
-		if !isLast {
-			prefix = "├──"
-			subpfx = "│  "
-		} else {
-			prefix = "└──"
-			subpfx = "   "
-		}
+		prefix, subpfx := getPrefixes(isLast, "")
 		o.PrintRevision(w, roInfo, rev, prefix, subpfx)
 	}
 	_ = w.Flush()
@@ -157,31 +149,20 @@ func (o *GetOptions) PrintRevision(w io.Writer, roInfo *info.RolloutInfo, revisi
 	total := len(replicaSets) + len(experiments) + len(analysisRuns)
 	curr := 0
 
-	getPrefixes := func() (string, string) {
-		isLast := curr == total-1
-		curr++
-		var childPrefix, childSubpfx string
-		if !isLast {
-			childPrefix = subpfx + "├──"
-			childSubpfx = subpfx + "│  "
-		} else {
-			childPrefix = subpfx + "└──"
-			childSubpfx = subpfx + "   "
-		}
-		return childPrefix, childSubpfx
-	}
-
 	for _, rsInfo := range replicaSets {
-		childPrefix, childSubpfx := getPrefixes()
+		childPrefix, childSubpfx := getPrefixes(curr == total-1, subpfx)
 		o.PrintReplicaSetInfo(w, rsInfo, childPrefix, childSubpfx)
+		curr++
 	}
 	for _, expInfo := range experiments {
-		childPrefix, childSubpfx := getPrefixes()
+		childPrefix, childSubpfx := getPrefixes(curr == total-1, subpfx)
 		o.PrintExperimentInfo(w, expInfo, childPrefix, childSubpfx)
+		curr++
 	}
 	for _, arInfo := range analysisRuns {
-		childPrefix, childSubpfx := getPrefixes()
+		childPrefix, childSubpfx := getPrefixes(curr == total-1, subpfx)
 		o.PrintAnalysisRunInfo(w, arInfo, childPrefix, childSubpfx)
+		curr++
 	}
 }
 
@@ -190,27 +171,22 @@ func (o *GetOptions) PrintReplicaSetInfo(w io.Writer, rsInfo info.ReplicaSetInfo
 	name := rsInfo.Name
 	if rsInfo.Stable {
 		infoCols = append(infoCols, o.colorize(info.InfoTagStable))
-		name = o.ansiFormat(name, FgGreen)
+		name = o.colorizeStatus(name, info.InfoTagStable)
 	} else if rsInfo.Canary {
 		infoCols = append(infoCols, o.colorize(info.InfoTagCanary))
-		name = o.ansiFormat(name, FgYellow)
+		name = o.colorizeStatus(name, info.InfoTagCanary)
 	} else if rsInfo.Active {
 		infoCols = append(infoCols, o.colorize(info.InfoTagActive))
-		name = o.ansiFormat(name, FgGreen)
+		name = o.colorizeStatus(name, info.InfoTagActive)
 	} else if rsInfo.Preview {
 		infoCols = append(infoCols, o.colorize(info.InfoTagPreview))
-		name = o.ansiFormat(name, FgBlue)
+		name = o.colorizeStatus(name, info.InfoTagPreview)
 	}
 	fmt.Fprintf(w, "%s%s %s\t%s\t%s %s\t%s\t%v\n", prefix, IconReplicaSet, name, "ReplicaSet", o.colorize(rsInfo.Icon), rsInfo.Status, rsInfo.Age(), strings.Join(infoCols, ","))
 	for i, podInfo := range rsInfo.Pods {
 		fmt.Fprintf(w, subpfx)
 		isLast := i == len(rsInfo.Pods)-1
-		var podPrefix string
-		if !isLast {
-			podPrefix = "├──"
-		} else {
-			podPrefix = "└──"
-		}
+		podPrefix, _ := getPrefixes(isLast, "")
 		podInfoCol := []string{fmt.Sprintf("ready:%s", podInfo.Ready)}
 		if podInfo.Restarts > 0 {
 			podInfoCol = append(podInfoCol, fmt.Sprintf("restarts:%d", podInfo.Restarts))
@@ -220,11 +196,7 @@ func (o *GetOptions) PrintReplicaSetInfo(w io.Writer, rsInfo info.ReplicaSetInfo
 }
 
 func (o *GetOptions) PrintAnalysisRunInfo(w io.Writer, arInfo info.AnalysisRunInfo, prefix string, subpfx string) {
-	name := arInfo.Name
-	switch arInfo.Status {
-	case string(v1alpha1.AnalysisStatusRunning), string(v1alpha1.AnalysisStatusPending):
-		name = o.ansiFormat(name, FgBlue)
-	}
+	name := o.colorizeStatus(arInfo.Name, arInfo.Status)
 	infoCols := []string{}
 	if arInfo.Successful > 0 {
 		infoCols = append(infoCols, fmt.Sprintf("%s %d", o.colorize(info.IconOK), arInfo.Successful))

pkg/kubectl-argo-rollouts/cmd/retry/retry.go

@@ -21,7 +21,7 @@ func NewCmdRetry(o *options.ArgoRolloutsOptions) *cobra.Command {
 		Short: "Retry a rollout or experiment",
 		Example: o.Example(`
   # Retry an aborted rollout
-  %[1]s get rollout ROLLOUT
+  %[1]s retry rollout ROLLOUT
   # Retry a failed experiment
   %[1]s retry experiment EXPERIMENT
 `),