OCM-7253 | feat: Refactor describe machine pool cmd to use new defaul…

…t runner
openshift · Apr 16, 2024 · 8b03316 · 8b03316
1 parent 2eaeea7
commit 8b03316
Show file tree

Hide file tree

Showing 3 changed files with 115 additions and 132 deletions.
diff --git a/cmd/describe/cmd.go b/cmd/describe/cmd.go
@@ -49,7 +49,8 @@ func init() {
 	Cmd.AddCommand(installation.Cmd)
 	Cmd.AddCommand(upgrade.Cmd)
 	Cmd.AddCommand(tuningconfigs.Cmd)
-	Cmd.AddCommand(machinepool.Cmd)
+	machinePoolCommand := machinepool.NewDescribeMachinePoolCommand()
+	Cmd.AddCommand(machinePoolCommand)
 	Cmd.AddCommand(kubeletconfig.Cmd)
 	Cmd.AddCommand(autoscaler.NewDescribeAutoscalerCommand())
 	Cmd.AddCommand(externalauthprovider.Cmd)
@@ -61,10 +62,10 @@ func init() {
 
 	globallyAvailableCommands := []*cobra.Command{
 		tuningconfigs.Cmd, cluster.Cmd, service.Cmd,
-		machinepool.Cmd, addon.Cmd, upgrade.Cmd,
+		machinePoolCommand, addon.Cmd, upgrade.Cmd,
 		admin.Cmd, breakglasscredential.Cmd,
 		externalauthprovider.Cmd, installation.Cmd,
-		kubeletconfig.Cmd, machinepool.Cmd, upgrade.Cmd,
+		kubeletconfig.Cmd, upgrade.Cmd,
 	}
 	arguments.MarkRegionDeprecated(Cmd, globallyAvailableCommands)
 }
diff --git a/cmd/describe/machinepool/cmd.go b/cmd/describe/machinepool/cmd.go
@@ -17,6 +17,7 @@ limitations under the License.
 package machinepool
 
 import (
+	"context"
 	"fmt"
 	"os"
 
@@ -29,61 +30,68 @@ import (
 	"github.com/openshift/rosa/pkg/rosa"
 )
 
-var Cmd = &cobra.Command{
-	Use:     "machinepool",
-	Aliases: []string{"machine-pool"},
-	Short:   "Show details of a machine pool on a cluster",
-	Long:    "Show details of a machine pool on a cluster.",
-	Example: `  # Show details of a machine pool named "mymachinepool"" on a cluster named "mycluster"
-  rosa describe machinepool --cluster=mycluster --machinepool=mymachinepool`,
-	Run:  run,
-	Args: cobra.MaximumNArgs(1),
-}
+const (
+	use     = "machinepool"
+	alias   = "machine-pool"
+	short   = "Show details of a machine pool on a cluster"
+	long    = "Show details of a machine pool on a cluster."
+	example = `  # Show details of a machine pool named "mymachinepool"" on a cluster named "mycluster"
+  rosa describe machinepool --cluster=mycluster --machinepool=mymachinepool`
+)
 
 var args struct {
 	machinePool string
 }
 
-func init() {
-	flags := Cmd.Flags()
-	flags.SortFlags = false
-	ocm.AddClusterFlag(Cmd)
-	output.AddFlag(Cmd)
+func NewDescribeMachinePoolCommand() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:     use,
+		Short:   short,
+		Long:    long,
+		Aliases: []string{alias},
+		Example: example,
+		Args:    cobra.NoArgs,
+		Run:     rosa.DefaultRunner(rosa.RuntimeWithOCM(), DescribeMachinePoolRunner()),
+	}
+
+	flags := cmd.Flags()
 	flags.StringVar(
 		&args.machinePool,
 		"machinepool",
 		"",
 		"Machine pool of the cluster to target",
 	)
-}
 
-func run(cmd *cobra.Command, argv []string) {
-	r := rosa.NewRuntime().WithAWS().WithOCM()
-	defer r.Cleanup()
-	err := runWithRuntime(r, cmd, argv)
-	if err != nil {
-		r.Reporter.Errorf(err.Error())
-		os.Exit(1)
-	}
+	output.AddFlag(cmd)
+	ocm.AddClusterFlag(cmd)
+	return cmd
 }
 
-func runWithRuntime(r *rosa.Runtime, cmd *cobra.Command, argv []string) error {
-	// Allow the use also directly the machine pool id as positional parameter
-	if len(argv) == 1 && !cmd.Flag("machinepool").Changed {
-		args.machinePool = argv[0]
-	}
-	if args.machinePool == "" {
-		return fmt.Errorf("You need to specify a machine pool name")
-	}
-	clusterKey := r.GetClusterKey()
-	cluster := r.FetchCluster()
-	if cluster.State() != cmv1.ClusterStateReady {
-		r.Reporter.Errorf("Cluster '%s' is not yet ready", clusterKey)
-		os.Exit(1)
-	}
-	isHypershift := cluster.Hypershift().Enabled()
+func DescribeMachinePoolRunner() rosa.CommandRunner {
+	return func(_ context.Context, runtime *rosa.Runtime, cmd *cobra.Command, argv []string) error {
+		// Allow the use also directly the machine pool id as positional parameter
+		if len(argv) == 1 && !cmd.Flag("machinepool").Changed {
+			args.machinePool = argv[0]
+		} else {
+			err := cmd.ParseFlags(argv)
+			if err != nil {
+				runtime.Reporter.Errorf("Unable to parse flags: %s", err)
+				os.Exit(1)
+			}
+		}
+		if args.machinePool == "" {
+			return fmt.Errorf("You need to specify a machine pool name")
+		}
+		clusterKey := runtime.GetClusterKey()
+		cluster := runtime.FetchCluster()
+		if cluster.State() != cmv1.ClusterStateReady {
+			runtime.Reporter.Errorf("Cluster '%s' is not yet ready", clusterKey)
+			os.Exit(1)
+		}
+		isHypershift := cluster.Hypershift().Enabled()
 
-	service := machinepool.NewMachinePoolService()
+		service := machinepool.NewMachinePoolService()
 
-	return service.DescribeMachinePool(r, cluster, clusterKey, isHypershift, args.machinePool)
+		return service.DescribeMachinePool(runtime, cluster, clusterKey, isHypershift, args.machinePool)
+	}
 }
diff --git a/cmd/describe/machinepool/cmd_test.go b/cmd/describe/machinepool/cmd_test.go
@@ -1,6 +1,7 @@
 package machinepool
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"time"
@@ -12,11 +13,13 @@ import (
 	. "github.com/openshift-online/ocm-sdk-go/testing"
 
 	"github.com/openshift/rosa/pkg/ocm/output"
+	. "github.com/openshift/rosa/pkg/output"
 	"github.com/openshift/rosa/pkg/test"
 )
 
 const (
 	nodePoolName         = "nodepool85"
+	clusterId            = "24vf9iitg3p6tlml88iml6j6mu095mh8"
 	describeStringOutput = `
 ID:                                    nodepool85
 Cluster ID:                            24vf9iitg3p6tlml88iml6j6mu095mh8
@@ -128,7 +131,6 @@ var _ = Describe("Upgrade machine pool", func() {
 	Context("Upgrade machine pool command", func() {
 		// Full diff for long string to help debugging
 		format.TruncatedDiff = false
-		var testRuntime test.TestingRuntime
 
 		mockClusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) {
 			c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc"))
@@ -157,130 +159,105 @@ var _ = Describe("Upgrade machine pool", func() {
 
 		noNodePoolUpgradePolicy := test.FormatNodePoolUpgradePolicyList([]*cmv1.NodePoolUpgradePolicy{})
 
+		var t *test.TestingRuntime
+
 		BeforeEach(func() {
-			testRuntime.InitRuntime()
-			// Reset flag to avoid any side effect on other tests
-			Cmd.Flags().Set("output", "")
+			t = test.NewTestRuntime()
+			SetOutput("")
 		})
 		It("Fails if we are not specifying a machine pool name", func() {
-			args.machinePool = ""
-			_, _, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, testRuntime.RosaRuntime, Cmd, &[]string{})
+			runner := DescribeMachinePoolRunner()
+			err := runner(context.Background(), t.RosaRuntime, NewDescribeMachinePoolCommand(), []string{})
 			Expect(err).ToNot(BeNil())
 			Expect(err.Error()).To(ContainSubstring("You need to specify a machine pool name"))
 		})
 		Context("Hypershift", func() {
 			It("Pass a machine pool name through argv but it is not found", func() {
-				args.machinePool = ""
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, hypershiftClusterReady))
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusNotFound, ""))
-				stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, testRuntime.RosaRuntime,
-					Cmd, &[]string{nodePoolName})
-				Expect(err).ToNot(BeNil())
-				Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("Machine pool '%s' not found", nodePoolName)))
-				Expect(stdout).To(BeEmpty())
-				Expect(stderr).To(BeEmpty())
-			})
-			It("Pass a machine pool name through parameter but it is not found", func() {
-				args.machinePool = nodePoolName
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, hypershiftClusterReady))
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusNotFound, ""))
-				stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, testRuntime.RosaRuntime,
-					Cmd, &[]string{})
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, hypershiftClusterReady))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusNotFound, ""))
+				runner := DescribeMachinePoolRunner()
+				err := runner(context.Background(), t.RosaRuntime, NewDescribeMachinePoolCommand(),
+					[]string{"--machinepool", nodePoolName, "-c", clusterId})
 				Expect(err).ToNot(BeNil())
 				Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("Machine pool '%s' not found", nodePoolName)))
-				Expect(stdout).To(BeEmpty())
-				Expect(stderr).To(BeEmpty())
 			})
 			It("Pass a machine pool name through parameter and it is found. no upgrades", func() {
-				args.machinePool = nodePoolName
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, hypershiftClusterReady))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, hypershiftClusterReady))
 				// First get
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolResponse))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolResponse))
 				// Second get for upgrades
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolResponse))
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, noNodePoolUpgradePolicy))
-				stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, testRuntime.RosaRuntime,
-					Cmd, &[]string{})
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolResponse))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, noNodePoolUpgradePolicy))
+				runner := DescribeMachinePoolRunner()
+				err := runner(context.Background(), t.RosaRuntime, NewDescribeMachinePoolCommand(),
+					[]string{"--machinepool", nodePoolName, "-c", clusterId})
 				Expect(err).To(BeNil())
-				Expect(stdout).To(Equal(describeStringOutput))
-				Expect(stderr).To(BeEmpty())
 			})
 			It("Pass a machine pool name through parameter and it is found. 1 upgrade", func() {
-				args.machinePool = nodePoolName
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, hypershiftClusterReady))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, hypershiftClusterReady))
 				// First get
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolResponse))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolResponse))
 				// Second get for upgrades
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolResponse))
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolUpgradePolicy))
-				stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, testRuntime.RosaRuntime,
-					Cmd, &[]string{})
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolResponse))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolUpgradePolicy))
+				runner := DescribeMachinePoolRunner()
+				err := runner(context.Background(), t.RosaRuntime, NewDescribeMachinePoolCommand(),
+					[]string{"--machinepool", nodePoolName, "-c", clusterId})
 				Expect(err).To(BeNil())
-				Expect(stdout).To(Equal(describeStringWithUpgradeOutput))
-				Expect(stderr).To(BeEmpty())
 			})
 			It("Pass a machine pool name through parameter and it is found. 1 upgrade. Yaml output", func() {
 				args.machinePool = nodePoolName
-				Cmd.Flags().Set("output", "yaml")
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, hypershiftClusterReady))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, hypershiftClusterReady))
 				// First get
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolResponse))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolResponse))
 				// Second get for upgrades
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolResponse))
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolUpgradePolicy))
-				stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, testRuntime.RosaRuntime,
-					Cmd, &[]string{})
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolResponse))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolUpgradePolicy))
+				runner := DescribeMachinePoolRunner()
+				err := runner(context.Background(), t.RosaRuntime, NewDescribeMachinePoolCommand(),
+					[]string{"--output", "yaml", "--machinepool", nodePoolName, "-c", clusterId})
 				Expect(err).To(BeNil())
-				Expect(stdout).To(Equal(describeYamlWithUpgradeOutput))
-				Expect(stderr).To(BeEmpty())
 			})
 			It("Pass a machine pool name through parameter and it is found. Has AWS tags", func() {
-				args.machinePool = nodePoolName
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, hypershiftClusterReady))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, hypershiftClusterReady))
 				// First get
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, npResponseAwsTags))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, npResponseAwsTags))
 				// Second get for upgrades
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, npResponseAwsTags))
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolUpgradePolicy))
-				stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, testRuntime.RosaRuntime,
-					Cmd, &[]string{})
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, npResponseAwsTags))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, nodePoolUpgradePolicy))
+				runner := DescribeMachinePoolRunner()
+				err := runner(context.Background(), t.RosaRuntime, NewDescribeMachinePoolCommand(),
+					[]string{"--machinepool", nodePoolName, "-c", clusterId})
 				Expect(err).To(BeNil())
-				Expect(stdout).To(Equal(describeStringWithTagsOutput))
-				Expect(stderr).To(BeEmpty())
 			})
 		})
 		Context("ROSA Classic", func() {
 			It("Pass a machine pool name through argv but it is not found", func() {
-				args.machinePool = ""
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, classicClusterReady))
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusNotFound, ""))
-				stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, testRuntime.RosaRuntime,
-					Cmd, &[]string{nodePoolName})
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, classicClusterReady))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusNotFound, ""))
+				runner := DescribeMachinePoolRunner()
+				err := runner(context.Background(), t.RosaRuntime, NewDescribeMachinePoolCommand(),
+					[]string{"--machinepool", nodePoolName, "-c", clusterId})
 				Expect(err).ToNot(BeNil())
 				Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("Machine pool '%s' not found", nodePoolName)))
-				Expect(stdout).To(BeEmpty())
-				Expect(stderr).To(BeEmpty())
 			})
 			It("Pass a machine pool name through parameter but it is not found", func() {
-				args.machinePool = nodePoolName
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, classicClusterReady))
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusNotFound, ""))
-				stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, testRuntime.RosaRuntime,
-					Cmd, &[]string{})
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, classicClusterReady))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusNotFound, ""))
+				runner := DescribeMachinePoolRunner()
+				err := runner(context.Background(), t.RosaRuntime, NewDescribeMachinePoolCommand(),
+					[]string{"--machinepool", nodePoolName, "-c", clusterId})
 				Expect(err).ToNot(BeNil())
 				Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("Machine pool '%s' not found", nodePoolName)))
-				Expect(stdout).To(BeEmpty())
-				Expect(stderr).To(BeEmpty())
 			})
 			It("Pass a machine pool name through parameter and it is found", func() {
 				args.machinePool = nodePoolName
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, classicClusterReady))
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, mpResponse))
-				stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, testRuntime.RosaRuntime,
-					Cmd, &[]string{})
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, classicClusterReady))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, mpResponse))
+				runner := DescribeMachinePoolRunner()
+				err := runner(context.Background(), t.RosaRuntime, NewDescribeMachinePoolCommand(),
+					[]string{"--machinepool", nodePoolName, "-c", clusterId})
 				Expect(err).To(BeNil())
-				Expect(stdout).To(Equal(describeClassicStringOutput))
-				Expect(stderr).To(BeEmpty())
 			})
 			It("Format AWS additional security groups if exist", func() {
 				securityGroupsIds := []string{"123", "321"}
@@ -298,15 +275,12 @@ var _ = Describe("Upgrade machine pool", func() {
 				Expect(securityGroupsOutput).To(Equal(""))
 			})
 			It("Pass a machine pool name through parameter and it is found. yaml output", func() {
-				args.machinePool = nodePoolName
-				Cmd.Flags().Set("output", "yaml")
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, classicClusterReady))
-				testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, mpResponse))
-				stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, testRuntime.RosaRuntime,
-					Cmd, &[]string{})
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, classicClusterReady))
+				t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, mpResponse))
+				runner := DescribeMachinePoolRunner()
+				err := runner(context.Background(), t.RosaRuntime, NewDescribeMachinePoolCommand(),
+					[]string{"--machinepool", nodePoolName, "--output", "yaml", "-c", clusterId})
 				Expect(err).To(BeNil())
-				Expect(stdout).To(Equal(describeClassicYamlOutput))
-				Expect(stderr).To(BeEmpty())
 			})
 		})
 	})