From 552c24e2fd047f3dc223e3c03f3e3f99eced7b2c Mon Sep 17 00:00:00 2001 From: hkepley Date: Tue, 16 Apr 2024 11:44:41 -0400 Subject: [PATCH] OCM-7253 | feat: Refactor describe machine pool cmd to use new default runner --- cmd/describe/cmd.go | 7 +- cmd/describe/machinepool/cmd.go | 90 ++++++++------- cmd/describe/machinepool/cmd_test.go | 163 ++++++++++----------------- 3 files changed, 115 insertions(+), 145 deletions(-) diff --git a/cmd/describe/cmd.go b/cmd/describe/cmd.go index a1cf7b03e3..80afeea9ed 100644 --- 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 index a99d0fc784..9178f9193b 100644 --- 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 index 55bce3daed..4860a54188 100644 --- 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 @@ -108,19 +111,6 @@ Subnets: Spot instances: Yes (max $5) Disk size: default Additional Security Group IDs: -` - describeClassicYamlOutput = `availability_zones: -- us-east-1a -- us-east-1b -- us-east-1c -aws: - kind: AWSMachinePool - spot_market_options: - kind: AWSSpotMarketOptions - max_price: 5 -id: nodepool85 -instance_type: m5.xlarge -kind: MachinePool ` ) @@ -128,7 +118,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 +146,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}) + 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 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{}) - 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 +262,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()) }) }) })