Apply suggestions from code review

Co-authored-by: Yuvaraj Kakaraparthi <yuva2811@gmail.com>

cmd/clusterctl/client/alpha/rollout.go

@@ -40,6 +40,10 @@ var validRollbackResourceTypes = []string{
 	MachineDeployment,
 }
 
+var validHistoryResourceTypes = []string{
+	MachineDeployment,
+}
+
 // Rollout defines the behavior of a rollout implementation.
 type Rollout interface {
 	ObjectRestarter(context.Context, cluster.Proxy, corev1.ObjectReference) error

cmd/clusterctl/client/alpha/rollout_viewer.go

@@ -21,8 +21,9 @@ import (
 	"fmt"
 	"os"
 	"sort"
-	"text/tabwriter"
+	"strconv"
 
+	"github.com/olekukonko/tablewriter"
 	"github.com/pkg/errors"
 	"gopkg.in/yaml.v2"
 	corev1 "k8s.io/api/core/v1"
@@ -45,7 +46,7 @@ func (r *rollout) ObjectViewer(ctx context.Context, proxy cluster.Proxy, ref cor
 			return err
 		}
 	default:
-		return errors.Errorf("invalid resource type %q, valid values are %v", ref.Kind, validResourceTypes)
+		return errors.Errorf("invalid resource type %q, valid values are %v", ref.Kind, validHistoryResourceTypes)
 	}
 	return nil
 }
@@ -95,15 +96,27 @@ func viewMachineDeployment(ctx context.Context, proxy cluster.Proxy, d *clusterv
 	sort.Slice(revisions, func(i, j int) bool { return revisions[i] < revisions[j] })
 
 	// Output the revisionToChangeCause map
-	writer := new(tabwriter.Writer)
-	writer.Init(os.Stdout, 0, 8, 2, ' ', 0)
-	fmt.Fprintf(writer, "REVISION\tCHANGE-CAUSE\n")
+	table := tablewriter.NewWriter(os.Stdout)
+	table.SetHeader([]string{"REVISION", "CHANGE-CAUSE"})
+	table.SetHeaderAlignment(tablewriter.ALIGN_LEFT)
+	table.SetAlignment(tablewriter.ALIGN_LEFT)
+	table.SetCenterSeparator("")
+	table.SetColumnSeparator("")
+	table.SetRowSeparator("")
+	table.SetHeaderLine(false)
+	table.SetBorder(false)
+
 	for _, r := range revisions {
 		changeCause := historyInfo[r]
 		if changeCause == "" {
 			changeCause = "<none>"
 		}
-		fmt.Fprintf(writer, "%d\t%s\n", r, changeCause)
+		table.Append([]string{
+			strconv.FormatInt(r, 10),
+			changeCause,
+		})
 	}
-	return writer.Flush()
+	table.Render()
+
+	return nil
 }

cmd/clusterctl/client/alpha/rollout_viewer_test.go

@@ -146,10 +146,10 @@ func Test_ObjectViewer(t *testing.T) {
 					Namespace: namespace,
 				},
 			},
-			expectedOutput: `REVISION  CHANGE-CAUSE
-1         update to the latest version
-2         <none>
-3         <none>
+			expectedOutput: `  REVISION  CHANGE-CAUSE                  
+  1         update to the latest version  
+  2         <none>                        
+  3         <none>                        
 `,
 		},
 		{
@@ -164,7 +164,7 @@ func Test_ObjectViewer(t *testing.T) {
 					Namespace: namespace,
 				},
 			},
-			expectedOutput: `REVISION  CHANGE-CAUSE
+			expectedOutput: `  REVISION  CHANGE-CAUSE  
 `,
 		},
 		{

cmd/clusterctl/cmd/rollout/history.go

@@ -37,31 +37,41 @@ var (
 		clusterctl alpha rollout history machinedeployment/my-md-0 --revision=3`)
 )
 
+// historyOptions is the start of the data required to perform the operation.
+type historyOptions struct {
+	kubeconfig        string
+	kubeconfigContext string
+	resources         []string
+	namespace         string
+	revision          int64
+}
+
+var historyOpt = historyOptions{}
+
 // NewCmdRolloutHistory returns a Command instance for 'rollout history' sub command.
 func NewCmdRolloutHistory(cfgFile string) *cobra.Command {
-	historyOpt := client.RolloutHistoryOptions{}
 	cmd := &cobra.Command{
 		Use:                   "history RESOURCE",
 		DisableFlagsInUseLine: true,
-		Short:                 "History a cluster-api resource",
+		Short:                 "History of a cluster-api resource",
 		Long:                  historyLong,
 		Example:               historyExample,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runHistory(cfgFile, args, historyOpt)
+			return runHistory(cfgFile, args)
 		},
 	}
-	cmd.Flags().StringVar(&historyOpt.Kubeconfig.Path, "kubeconfig", "",
+	cmd.Flags().StringVar(&historyOpt.kubeconfig, "kubeconfig", "",
 		"Path to the kubeconfig file to use for accessing the management cluster. If unspecified, default discovery rules apply.")
-	cmd.Flags().StringVar(&historyOpt.Kubeconfig.Context, "kubeconfig-context", "",
+	cmd.Flags().StringVar(&historyOpt.kubeconfigContext, "kubeconfig-context", "",
 		"Context to be used within the kubeconfig file. If empty, current context will be used.")
-	cmd.Flags().StringVarP(&historyOpt.Namespace, "namespace", "n", "", "Namespace where the resource(s) reside. If unspecified, the defult namespace will be used.")
-	cmd.Flags().Int64Var(&historyOpt.Revision, "revision", historyOpt.Revision, "See the details, including podTemplate of the revision specified.")
+	cmd.Flags().StringVarP(&historyOpt.namespace, "namespace", "n", "", "Namespace where the resource(s) reside. If unspecified, the defult namespace will be used.")
+	cmd.Flags().Int64Var(&historyOpt.revision, "revision", historyOpt.revision, "See the details, including machineTemplate of the revision specified.")
 
 	return cmd
 }
 
-func runHistory(cfgFile string, args []string, historyOpt client.RolloutHistoryOptions) error {
-	historyOpt.Resources = args
+func runHistory(cfgFile string, args []string) error {
+	historyOpt.resources = args
 
 	ctx := context.Background()
 
@@ -70,5 +80,10 @@ func runHistory(cfgFile string, args []string, historyOpt client.RolloutHistoryO
 		return err
 	}
 
-	return c.RolloutHistory(ctx, historyOpt)
+	return c.RolloutHistory(ctx, client.RolloutHistoryOptions{
+		Kubeconfig: client.Kubeconfig{Path: historyOpt.kubeconfig, Context: historyOpt.kubeconfigContext},
+		Namespace:  historyOpt.namespace,
+		Resources:  historyOpt.resources,
+		Revision:   historyOpt.revision,
+	})
 }

test/go.mod

@@ -95,6 +95,7 @@ require (
 	github.com/magiconair/properties v1.8.7 // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/mattn/go-isatty v0.0.17 // indirect
+	github.com/mattn/go-runewidth v0.0.14 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
 	github.com/mitchellh/copystructure v1.2.0 // indirect
 	github.com/mitchellh/mapstructure v1.5.0 // indirect
@@ -104,13 +105,15 @@ require (
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/morikuni/aec v1.0.0 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
+	github.com/olekukonko/tablewriter v0.0.5 // indirect
 	github.com/opencontainers/go-digest v1.0.0 // indirect
 	github.com/opencontainers/image-spec v1.0.2 // indirect
 	github.com/pelletier/go-toml v1.9.5 // indirect
 	github.com/pelletier/go-toml/v2 v2.0.8 // indirect
 	github.com/prometheus/client_model v0.4.0 // indirect
 	github.com/prometheus/common v0.44.0 // indirect
 	github.com/prometheus/procfs v0.10.1 // indirect
+	github.com/rivo/uniseg v0.4.2 // indirect
 	github.com/shopspring/decimal v1.3.1 // indirect
 	github.com/spf13/afero v1.9.5 // indirect
 	github.com/spf13/cast v1.5.1 // indirect

test/go.sum

@@ -372,6 +372,9 @@ github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNx
 github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94=
 github.com/mattn/go-isatty v0.0.17 h1:BTarxUcIeDqL27Mc+vyvdWYSL28zpIhv3RoTdsLMPng=
 github.com/mattn/go-isatty v0.0.17/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM=
+github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
+github.com/mattn/go-runewidth v0.0.14 h1:+xnbZSEeDbOIg5/mE6JF0w6n9duR1l3/WmbinWVwUuU=
+github.com/mattn/go-runewidth v0.0.14/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w=
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
 github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo=
 github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4=
@@ -408,6 +411,8 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8m
 github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
 github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U=
+github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec=
+github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY=
 github.com/onsi/ginkgo/v2 v2.12.0 h1:UIVDowFPwpg6yMUpPjGkYvf06K3RAiJXUhCxEwQVHRI=
 github.com/onsi/ginkgo/v2 v2.12.0/go.mod h1:ZNEzXISYlqpb8S36iN71ifqLi3vVD1rVJGvWRCJOUpQ=
 github.com/onsi/gomega v1.27.10 h1:naR28SdDFlqrG6kScpT8VWpu1xWY5nJRCF3XaYyBjhI=
@@ -451,6 +456,9 @@ github.com/prometheus/procfs v0.0.0-20190507164030-5867b95ac084/go.mod h1:TjEm7z
 github.com/prometheus/procfs v0.10.1 h1:kYK1Va/YMlutzCGazswoHKo//tZVlFpKYh+PymziUAg=
 github.com/prometheus/procfs v0.10.1/go.mod h1:nwNm2aOCAYw8uTR/9bWRREkZFxAUcWzPHWJq+XBB/FM=
 github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
+github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
+github.com/rivo/uniseg v0.4.2 h1:YwD0ulJSJytLpiaWua0sBDusfsCZohxjxzVTYjwxfV8=
+github.com/rivo/uniseg v0.4.2/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
 github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
 github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ=
 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=