diff --git a/cmd/create/autoscaler/cmd.go b/cmd/create/autoscaler/cmd.go index 5d5605a898..a706c109ab 100644 --- a/cmd/create/autoscaler/cmd.go +++ b/cmd/create/autoscaler/cmd.go @@ -67,7 +67,6 @@ func run(cmd *cobra.Command, _ []string) { defer r.Cleanup() clusterKey := r.GetClusterKey() - cluster := r.FetchCluster() if cluster.Hypershift().Enabled() { diff --git a/cmd/describe/autoscaler/cmd.go b/cmd/describe/autoscaler/cmd.go new file mode 100644 index 0000000000..926c954871 --- /dev/null +++ b/cmd/describe/autoscaler/cmd.go @@ -0,0 +1,66 @@ +package autoscaler + +import ( + "context" + "fmt" + + "github.com/spf13/cobra" + + "github.com/openshift/rosa/pkg/clusterautoscaler" + "github.com/openshift/rosa/pkg/ocm" + "github.com/openshift/rosa/pkg/output" + "github.com/openshift/rosa/pkg/rosa" +) + +const ( + use = "autoscaler" + short = "Show details of the autoscaler for a cluster" + long = short + example = ` # Describe the autoscaler for cluster 'foo' +rosa describe autoscaler --cluster foo` +) + +func NewDescribeAutoscalerCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: use, + Short: short, + Long: long, + Example: example, + Args: cobra.NoArgs, + Run: rosa.DefaultRunner(rosa.RuntimeWithOCM(), DescribeAutoscalerRunner()), + } + + output.AddFlag(cmd) + ocm.AddClusterFlag(cmd) + return cmd +} + +func DescribeAutoscalerRunner() rosa.CommandRunner { + return func(_ context.Context, runtime *rosa.Runtime, _ *cobra.Command, _ []string) error { + cluster, err := runtime.OCMClient.GetCluster(runtime.GetClusterKey(), runtime.Creator) + if err != nil { + return err + } + + err = clusterautoscaler.IsAutoscalerSupported(runtime, cluster) + if err != nil { + return err + } + + autoscaler, err := runtime.OCMClient.GetClusterAutoscaler(cluster.ID()) + if err != nil { + return err + } + + if autoscaler == nil { + return fmt.Errorf("No autoscaler exists for cluster '%s'", runtime.ClusterKey) + } + + if output.HasFlag() { + output.Print(autoscaler) + } else { + fmt.Print(clusterautoscaler.PrintAutoscaler(autoscaler)) + } + return nil + } +} diff --git a/cmd/describe/autoscaler/cmd_test.go b/cmd/describe/autoscaler/cmd_test.go new file mode 100644 index 0000000000..558e6227df --- /dev/null +++ b/cmd/describe/autoscaler/cmd_test.go @@ -0,0 +1,198 @@ +package autoscaler + +import ( + "context" + "fmt" + "net/http" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" + . "github.com/openshift-online/ocm-sdk-go/testing" + + "github.com/openshift/rosa/pkg/clusterautoscaler" + "github.com/openshift/rosa/pkg/output" + . "github.com/openshift/rosa/pkg/test" +) + +func TestDescribeAutoscaler(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "rosa describe autoscaler") +} + +var _ = Describe("rosa describe autoscaler", func() { + Context("Create Command", func() { + It("Returns Command", func() { + + cmd := NewDescribeAutoscalerCommand() + Expect(cmd).NotTo(BeNil()) + + Expect(cmd.Use).To(Equal(use)) + Expect(cmd.Example).To(Equal(example)) + Expect(cmd.Short).To(Equal(short)) + Expect(cmd.Long).To(Equal(long)) + Expect(cmd.Args).NotTo(BeNil()) + Expect(cmd.Run).NotTo(BeNil()) + + flag := cmd.Flags().Lookup("cluster") + Expect(flag).NotTo(BeNil()) + + flag = cmd.Flags().Lookup("output") + Expect(flag).NotTo(BeNil()) + }) + }) + + Context("Execute command", func() { + + var t *TestingRuntime + + BeforeEach(func() { + t = NewTestRuntime() + output.SetOutput("") + }) + + AfterEach(func() { + output.SetOutput("") + }) + + It("Returns an error if the cluster does not exist", func() { + + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, FormatClusterList(make([]*cmv1.Cluster, 0)))) + t.SetCluster("cluster", nil) + + runner := DescribeAutoscalerRunner() + err := runner(context.Background(), t.RosaRuntime, nil, nil) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To( + Equal("There is no cluster with identifier or name 'cluster'")) + }) + + It("Returns an error if the cluster is HCP", func() { + cluster := MockCluster(func(c *cmv1.ClusterBuilder) { + h := &cmv1.HypershiftBuilder{} + h.Enabled(true) + c.Hypershift(h) + }) + + t.SetCluster(cluster.Name(), cluster) + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, FormatClusterList([]*cmv1.Cluster{cluster}))) + + runner := DescribeAutoscalerRunner() + err := runner(context.Background(), t.RosaRuntime, nil, nil) + + Expect(err).NotTo(BeNil()) + Expect(err.Error()).To(ContainSubstring(clusterautoscaler.NoHCPAutoscalerSupportMessage)) + }) + + It("Returns an error if the cluster is not ready", func() { + cluster := MockCluster(func(c *cmv1.ClusterBuilder) { + c.State(cmv1.ClusterStateInstalling) + }) + + t.SetCluster(cluster.Name(), cluster) + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, FormatClusterList([]*cmv1.Cluster{cluster}))) + + runner := DescribeAutoscalerRunner() + err := runner(context.Background(), t.RosaRuntime, nil, nil) + + Expect(err).NotTo(BeNil()) + Expect(err.Error()).To(ContainSubstring(" is not yet ready")) + }) + + It("Returns an error if OCM API fails to return autoscaler", func() { + cluster := MockCluster(func(c *cmv1.ClusterBuilder) { + c.State(cmv1.ClusterStateReady) + }) + + t.SetCluster(cluster.Name(), cluster) + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, FormatClusterList([]*cmv1.Cluster{cluster}))) + t.ApiServer.RouteToHandler( + "GET", + fmt.Sprintf("/api/clusters_mgmt/v1/clusters/%s/autoscaler", cluster.ID()), + RespondWithJSON(http.StatusInternalServerError, "{}")) + + runner := DescribeAutoscalerRunner() + err := runner(context.Background(), t.RosaRuntime, nil, nil) + + Expect(err).NotTo(BeNil()) + Expect(err.Error()).To(ContainSubstring("status is 500")) + + }) + + It("Returns an error if no autoscaler exists", func() { + cluster := MockCluster(func(c *cmv1.ClusterBuilder) { + c.State(cmv1.ClusterStateReady) + }) + + t.SetCluster(cluster.Name(), cluster) + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, FormatClusterList([]*cmv1.Cluster{cluster}))) + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusNotFound, "{}")) + + runner := DescribeAutoscalerRunner() + err := runner(context.Background(), t.RosaRuntime, nil, nil) + + Expect(err).NotTo(BeNil()) + Expect(err.Error()).To(ContainSubstring("No autoscaler exists for cluster 'cluster'")) + + }) + + It("Prints the autoscaler to stdout", func() { + cluster := MockCluster(func(c *cmv1.ClusterBuilder) { + c.State(cmv1.ClusterStateReady) + }) + + t.SetCluster(cluster.Name(), cluster) + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, FormatClusterList([]*cmv1.Cluster{cluster}))) + + autoscaler := MockAutoscaler(nil) + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, FormatResource(autoscaler))) + + runner := DescribeAutoscalerRunner() + err := runner(context.Background(), t.RosaRuntime, nil, nil) + + Expect(err).NotTo(HaveOccurred()) + }) + + It("Prints the autoscaler in JSON", func() { + output.SetOutput("json") + Expect(output.HasFlag()).To(BeTrue()) + + cluster := MockCluster(func(c *cmv1.ClusterBuilder) { + c.State(cmv1.ClusterStateReady) + }) + + t.SetCluster(cluster.Name(), cluster) + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, FormatClusterList([]*cmv1.Cluster{cluster}))) + + autoscaler := MockAutoscaler(nil) + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, FormatResource(autoscaler))) + + runner := DescribeAutoscalerRunner() + err := runner(context.Background(), t.RosaRuntime, nil, nil) + + Expect(err).NotTo(HaveOccurred()) + + }) + + It("Prints the autoscaler in YAML", func() { + output.SetOutput("yaml") + Expect(output.HasFlag()).To(BeTrue()) + + cluster := MockCluster(func(c *cmv1.ClusterBuilder) { + c.State(cmv1.ClusterStateReady) + }) + + t.SetCluster(cluster.Name(), cluster) + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, FormatClusterList([]*cmv1.Cluster{cluster}))) + + autoscaler := MockAutoscaler(nil) + t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, FormatResource(autoscaler))) + + runner := DescribeAutoscalerRunner() + err := runner(context.Background(), t.RosaRuntime, nil, nil) + + Expect(err).NotTo(HaveOccurred()) + }) + }) +}) diff --git a/cmd/describe/cluster/cmd.go b/cmd/describe/cluster/cmd.go index 68579b79b2..f0be6ce07c 100644 --- a/cmd/describe/cluster/cmd.go +++ b/cmd/describe/cluster/cmd.go @@ -29,7 +29,6 @@ import ( "github.com/spf13/cobra" "github.com/openshift/rosa/pkg/ocm" - ocmOutput "github.com/openshift/rosa/pkg/ocm/output" "github.com/openshift/rosa/pkg/output" "github.com/openshift/rosa/pkg/rosa" ) @@ -185,7 +184,7 @@ func run(cmd *cobra.Command, argv []string) { subnetsStr := "" if len(cluster.AWS().SubnetIDs()) > 0 { subnetsStr = fmt.Sprintf(" - Subnets: %s\n", - ocmOutput.PrintStringSlice(cluster.AWS().SubnetIDs())) + output.PrintStringSlice(cluster.AWS().SubnetIDs())) } var machinePools []*cmv1.MachinePool @@ -646,13 +645,13 @@ Nodes: if hasSgsControlPlane { nodeConfig += fmt.Sprintf( " - Control Plane: %s\n", - ocmOutput.PrintStringSlice( + output.PrintStringSlice( cluster.AWS().AdditionalControlPlaneSecurityGroupIds())) } if hasSgsInfra { nodeConfig += fmt.Sprintf( " - Infra: %s\n", - ocmOutput.PrintStringSlice( + output.PrintStringSlice( cluster.AWS().AdditionalInfraSecurityGroupIds())) } } diff --git a/cmd/describe/cmd.go b/cmd/describe/cmd.go index 468c4c4217..70bc4e5fbe 100644 --- a/cmd/describe/cmd.go +++ b/cmd/describe/cmd.go @@ -21,6 +21,7 @@ import ( "github.com/openshift/rosa/cmd/describe/addon" "github.com/openshift/rosa/cmd/describe/admin" + "github.com/openshift/rosa/cmd/describe/autoscaler" "github.com/openshift/rosa/cmd/describe/cluster" "github.com/openshift/rosa/cmd/describe/externalauthprovider" "github.com/openshift/rosa/cmd/describe/installation" @@ -49,6 +50,7 @@ func init() { Cmd.AddCommand(tuningconfigs.Cmd) Cmd.AddCommand(machinepool.Cmd) Cmd.AddCommand(kubeletconfig.Cmd) + Cmd.AddCommand(autoscaler.NewDescribeAutoscalerCommand()) Cmd.AddCommand(externalauthprovider.Cmd) flags := Cmd.PersistentFlags() diff --git a/cmd/describe/externalauthprovider/cmd_test.go b/cmd/describe/externalauthprovider/cmd_test.go index a35492c581..3b6dd9c50c 100644 --- a/cmd/describe/externalauthprovider/cmd_test.go +++ b/cmd/describe/externalauthprovider/cmd_test.go @@ -30,14 +30,13 @@ var _ = Describe("External authentication provider", func() { Context("Describe external authentication provider command", func() { - mockClusterReady, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) c.Hypershift(cmv1.NewHypershift().Enabled(true)) c.ExternalAuthConfig(cmv1.NewExternalAuthConfig().Enabled(true)) }) - Expect(err).To(BeNil()) hypershiftClusterReady := test.FormatClusterList([]*cmv1.Cluster{mockClusterReady}) externalAuths := make([]*cmv1.ExternalAuth, 0) @@ -73,7 +72,6 @@ var _ = Describe("External authentication provider", func() { args.name = externalAuthName testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, hypershiftClusterReady)) testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, test.FormatExternalAuthList(externalAuths))) - Expect(err).To(BeNil()) output := describeExternalAuthProviders(testRuntime.RosaRuntime, mockClusterReady, test.MockClusterID, test.BuildExternalAuth()) Expect(output).To(Equal(describeStringOutput)) diff --git a/cmd/describe/machinepool/cmd_test.go b/cmd/describe/machinepool/cmd_test.go index 2a48cfe2be..4be9e27c84 100644 --- a/cmd/describe/machinepool/cmd_test.go +++ b/cmd/describe/machinepool/cmd_test.go @@ -108,22 +108,21 @@ var _ = Describe("Upgrade machine pool", func() { format.TruncatedDiff = false var testRuntime test.TestingRuntime - mockClusterReady, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) c.Hypershift(cmv1.NewHypershift().Enabled(true)) }) - Expect(err).To(BeNil()) + hypershiftClusterReady := test.FormatClusterList([]*cmv1.Cluster{mockClusterReady}) - mockClassicClusterReady, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClassicClusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) c.Hypershift(cmv1.NewHypershift().Enabled(false)) }) - Expect(err).To(BeNil()) classicClusterReady := test.FormatClusterList([]*cmv1.Cluster{mockClassicClusterReady}) nodePoolResponse := formatNodePool() diff --git a/cmd/describe/machinepool/machinepool.go b/cmd/describe/machinepool/machinepool.go index 499e7e94d9..09b85e5159 100644 --- a/cmd/describe/machinepool/machinepool.go +++ b/cmd/describe/machinepool/machinepool.go @@ -45,11 +45,11 @@ func describeMachinePool(r *rosa.Runtime, cluster *cmv1.Cluster, clusterKey stri machinePool.InstanceType(), ocmOutput.PrintLabels(machinePool.Labels()), ocmOutput.PrintTaints(machinePool.Taints()), - ocmOutput.PrintStringSlice(machinePool.AvailabilityZones()), - ocmOutput.PrintStringSlice(machinePool.Subnets()), + output.PrintStringSlice(machinePool.AvailabilityZones()), + output.PrintStringSlice(machinePool.Subnets()), ocmOutput.PrintMachinePoolSpot(machinePool), ocmOutput.PrintMachinePoolDiskSize(machinePool), - ocmOutput.PrintStringSlice(machinePool.AWS().AdditionalSecurityGroupIds()), + output.PrintStringSlice(machinePool.AWS().AdditionalSecurityGroupIds()), ) fmt.Print(machinePoolOutput) diff --git a/cmd/dlt/upgrade/cmd_test.go b/cmd/dlt/upgrade/cmd_test.go index 46b0e282ea..329118ed82 100644 --- a/cmd/dlt/upgrade/cmd_test.go +++ b/cmd/dlt/upgrade/cmd_test.go @@ -14,22 +14,21 @@ import ( var _ = Describe("Delete upgrade", func() { var testRuntime test.TestingRuntime - mockClusterError, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterError := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateError) c.Hypershift(cmv1.NewHypershift().Enabled(true)) }) - Expect(err).To(BeNil()) var hypershiftClusterNotReady = test.FormatClusterList([]*cmv1.Cluster{mockClusterError}) - mockClassicCluster, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClassicCluster := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) c.Hypershift(cmv1.NewHypershift().Enabled(false)) }) - Expect(err).To(BeNil()) + var classicCluster = test.FormatClusterList([]*cmv1.Cluster{mockClassicCluster}) BeforeEach(func() { diff --git a/cmd/edit/cluster/cmd_test.go b/cmd/edit/cluster/cmd_test.go index 9a35e3c652..7d0c7f8284 100644 --- a/cmd/edit/cluster/cmd_test.go +++ b/cmd/edit/cluster/cmd_test.go @@ -36,21 +36,19 @@ const ( var _ = Describe("Edit cluster", func() { Context("warnUserForOAuthHCPVisibility", func() { var testRuntime test.TestingRuntime - mockHypershiftClusterReady, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockHypershiftClusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) c.Hypershift(cmv1.NewHypershift().Enabled(true)) }) - Expect(err).To(BeNil()) - mockClassicCluster, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClassicCluster := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) c.Hypershift(cmv1.NewHypershift().Enabled(false)) }) - Expect(err).To(BeNil()) BeforeEach(func() { testRuntime.InitRuntime() diff --git a/cmd/list/externalauthprovider/cmd_test.go b/cmd/list/externalauthprovider/cmd_test.go index c99d8d5024..dbf6f76a92 100644 --- a/cmd/list/externalauthprovider/cmd_test.go +++ b/cmd/list/externalauthprovider/cmd_test.go @@ -17,14 +17,13 @@ var _ = Describe("list external-auth-provider", func() { Context("List external authentication provider command", func() { - mockClusterReady, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) c.Hypershift(cmv1.NewHypershift().Enabled(true)) c.ExternalAuthConfig(cmv1.NewExternalAuthConfig().Enabled(true)) }) - Expect(err).To(BeNil()) hypershiftClusterReady := test.FormatClusterList([]*cmv1.Cluster{mockClusterReady}) externalAuths := make([]*cmv1.ExternalAuth, 0) diff --git a/cmd/list/machinepool/machinepool.go b/cmd/list/machinepool/machinepool.go index f868abd5af..3dc95bd38e 100644 --- a/cmd/list/machinepool/machinepool.go +++ b/cmd/list/machinepool/machinepool.go @@ -44,11 +44,11 @@ func listMachinePools(r *rosa.Runtime, clusterKey string, cluster *cmv1.Cluster) machinePool.InstanceType(), ocmOutput.PrintLabels(machinePool.Labels()), ocmOutput.PrintTaints(machinePool.Taints()), - ocmOutput.PrintStringSlice(machinePool.AvailabilityZones()), - ocmOutput.PrintStringSlice(machinePool.Subnets()), + output.PrintStringSlice(machinePool.AvailabilityZones()), + output.PrintStringSlice(machinePool.Subnets()), ocmOutput.PrintMachinePoolSpot(machinePool), ocmOutput.PrintMachinePoolDiskSize(machinePool), - ocmOutput.PrintStringSlice(machinePool.AWS().AdditionalSecurityGroupIds()), + output.PrintStringSlice(machinePool.AWS().AdditionalSecurityGroupIds()), ) } writer.Flush() diff --git a/cmd/list/upgrade/cmd_test.go b/cmd/list/upgrade/cmd_test.go index 334e5b36de..c39b0faea0 100644 --- a/cmd/list/upgrade/cmd_test.go +++ b/cmd/list/upgrade/cmd_test.go @@ -62,16 +62,16 @@ var _ = Describe("List upgrade", func() { var testRuntime test.TestingRuntime var nodePoolName = "nodepool85" - mockClusterError, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterError := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateError) c.Hypershift(cmv1.NewHypershift().Enabled(true)) }) - Expect(err).To(BeNil()) + var hypershiftClusterNotReady = test.FormatClusterList([]*cmv1.Cluster{mockClusterError}) - mockClusterReady, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) @@ -79,16 +79,16 @@ var _ = Describe("List upgrade", func() { c.Version(cmv1.NewVersion().RawID("4.12.26").ChannelGroup("stable"). ID("4.12.26").Enabled(true).AvailableUpgrades("4.12.27")) }) - Expect(err).To(BeNil()) + var hypershiftClusterReady = test.FormatClusterList([]*cmv1.Cluster{mockClusterReady}) - mockClassicCluster, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClassicCluster := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) c.Hypershift(cmv1.NewHypershift().Enabled(false)) }) - Expect(err).To(BeNil()) + var classicCluster = test.FormatClusterList([]*cmv1.Cluster{mockClassicCluster}) versionNoUpgrades := cmv1.NewVersion().ID("openshift-v4.12.24").RawID("4.12.24").ReleaseImage("1"). diff --git a/cmd/upgrade/accountroles/cmd.go b/cmd/upgrade/accountroles/cmd.go index dd08e39e41..63dfd20efa 100644 --- a/cmd/upgrade/accountroles/cmd.go +++ b/cmd/upgrade/accountroles/cmd.go @@ -242,10 +242,8 @@ func run(cmd *cobra.Command, _ []string) { case aws.ModeAuto: if isUpgradeNeedForAccountRolePolicies { reporter.Infof("Starting to upgrade the policies") - err = r.WithSpinner(func() error { - return upgradeAccountRolePolicies(reporter, awsClient, prefix, creator.Partition, creator.AccountID, policies, - policyVersion, policyPath, isVersionChosen) - }) + err := upgradeAccountRolePolicies(reporter, awsClient, prefix, creator.Partition, creator.AccountID, policies, + policyVersion, policyPath, isVersionChosen) if err != nil { LogError(roles.RosaUpgradeAccRolesModeAuto, ocmClient, policyVersion, err, reporter) reporter.Errorf("Error upgrading the role polices: %s", err) diff --git a/cmd/upgrade/cluster/cmd_test.go b/cmd/upgrade/cluster/cmd_test.go index c884d197f4..c264b9dbf9 100644 --- a/cmd/upgrade/cluster/cmd_test.go +++ b/cmd/upgrade/cluster/cmd_test.go @@ -26,45 +26,44 @@ var _ = Describe("Upgrade", Ordered, func() { HREF("/api/clusters_mgmt/v1/versions/openshift-v4.13.0").Enabled(true).ChannelGroup("stable"). ROSAEnabled(true).HostedControlPlaneEnabled(true) - mockClusterError, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterError := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateError) c.Hypershift(cmv1.NewHypershift().Enabled(true)) }) - Expect(err).To(BeNil()) var hypershiftClusterNotReady = test.FormatClusterList([]*cmv1.Cluster{mockClusterError}) - mockClusterReady, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) c.Hypershift(cmv1.NewHypershift().Enabled(true)) c.Version(version4130) }) - Expect(err).To(BeNil()) + // hypershiftClusterReady has no available upgrades var hypershiftClusterReady = test.FormatClusterList([]*cmv1.Cluster{mockClusterReady}) version4130WithUpgrades := version4130.AvailableUpgrades("4.13.1") - mockClusterReadyWithUpgrades, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterReadyWithUpgrades := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) c.Hypershift(cmv1.NewHypershift().Enabled(true)) c.Version(version4130WithUpgrades) }) - Expect(err).To(BeNil()) + // hypershiftClusterReadyWithUpdates has one available upgrade var hypershiftClusterReadyWithUpdates = test.FormatClusterList([]*cmv1.Cluster{mockClusterReadyWithUpgrades}) - mockClassicCluster, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClassicCluster := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) c.Hypershift(cmv1.NewHypershift().Enabled(false)) }) - Expect(err).To(BeNil()) + var classicCluster = test.FormatClusterList([]*cmv1.Cluster{mockClassicCluster}) BeforeEach(func() { @@ -289,7 +288,7 @@ var _ = Describe("Upgrade", Ordered, func() { // No existing policy upgrade testRuntime.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, formatControlPlaneUpgradePolicyList([]*cmv1.ControlPlaneUpgradePolicy{}))) - err = runWithRuntime(testRuntime.RosaRuntime, Cmd) + err := runWithRuntime(testRuntime.RosaRuntime, Cmd) Expect(err).ToNot(BeNil()) Expect(err.Error()).To( ContainSubstring("node-drain-grace-period flag is not supported to hosted clusters")) diff --git a/cmd/upgrade/machinepool/cmd_test.go b/cmd/upgrade/machinepool/cmd_test.go index c8f4f39cef..2be0f7b1b8 100644 --- a/cmd/upgrade/machinepool/cmd_test.go +++ b/cmd/upgrade/machinepool/cmd_test.go @@ -25,22 +25,21 @@ var _ = Describe("Upgrade machine pool", func() { Context("Upgrade machine pool command", func() { var testRuntime test.TestingRuntime var nodePoolName = "nodepool85" - mockClusterError, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterError := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateError) c.Hypershift(cmv1.NewHypershift().Enabled(true)) }) - Expect(err).To(BeNil()) hypershiftClusterNotReady := test.FormatClusterList([]*cmv1.Cluster{mockClusterError}) - mockClusterReady, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) c.Hypershift(cmv1.NewHypershift().Enabled(true)) }) - Expect(err).To(BeNil()) + hypershiftClusterReady := test.FormatClusterList([]*cmv1.Cluster{mockClusterReady}) version41224 := cmv1.NewVersion().ID("openshift-v4.12.24").RawID("4.12.24").ReleaseImage("1"). diff --git a/cmd/verify/network/cmd_test.go b/cmd/verify/network/cmd_test.go index d4a6af3e23..07e79e7453 100644 --- a/cmd/verify/network/cmd_test.go +++ b/cmd/verify/network/cmd_test.go @@ -27,7 +27,7 @@ var _ = Describe("verify network", func() { var cmd *cobra.Command var r *rosa.Runtime - mockCluster, _ := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockCluster := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) @@ -200,12 +200,11 @@ INFO: subnet-0f87f640e56934cbc, platform: aws, tags: {"t1":"v1"}: passed cmd.Flags().Lookup(statusOnlyFlag).Changed = true cmd.Flags().Set(clusterFlag, "tomckay-vpc") - mockCluster, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockCluster := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(state) }) - Expect(err).To(BeNil()) clusterList := test.FormatClusterList([]*cmv1.Cluster{mockCluster}) // GET /api/clusters_mgmt/v1/clusters @@ -409,12 +408,11 @@ INFO: subnet-0f87f640e56934cbc, platform: aws, tags: {"t1":"v1"}: passed cmd.Flags().Lookup(statusOnlyFlag).Changed = true cmd.Flags().Set(clusterFlag, "tomckay-vpc") - mockCluster, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockCluster := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs()) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State("ready state") }) - Expect(err).To(BeNil()) clusterList := test.FormatClusterList([]*cmv1.Cluster{mockCluster}) // GET /api/clusters_mgmt/v1/clusters @@ -425,7 +423,7 @@ INFO: subnet-0f87f640e56934cbc, platform: aws, tags: {"t1":"v1"}: passed ), ) - err = runWithRuntime(r, cmd) + err := runWithRuntime(r, cmd) Expect(err).ToNot(BeNil()) Expect(err.Error()).To( ContainSubstring( diff --git a/pkg/clusterautoscaler/autoscaler_suite_test.go b/pkg/clusterautoscaler/autoscaler_suite_test.go new file mode 100644 index 0000000000..fd5993d0cb --- /dev/null +++ b/pkg/clusterautoscaler/autoscaler_suite_test.go @@ -0,0 +1,13 @@ +package clusterautoscaler + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestAutoscaler(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Autoscaler Suite") +} diff --git a/pkg/clusterautoscaler/output.go b/pkg/clusterautoscaler/output.go new file mode 100644 index 0000000000..c5126e8ec2 --- /dev/null +++ b/pkg/clusterautoscaler/output.go @@ -0,0 +1,89 @@ +package clusterautoscaler + +import ( + "fmt" + + cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" + + "github.com/openshift/rosa/pkg/output" +) + +func PrintAutoscaler(a *cmv1.ClusterAutoscaler) string { + + out := "\n" + out += fmt.Sprintf("Balance Similar Node Groups: %s\n", + output.PrintBool(a.BalanceSimilarNodeGroups())) + out += fmt.Sprintf("Skip Nodes With Local Storage %s\n", + output.PrintBool(a.SkipNodesWithLocalStorage())) + out += fmt.Sprintf("Log Verbosity: %d\n", + a.LogVerbosity()) + + if len(a.BalancingIgnoredLabels()) > 0 { + out += fmt.Sprintf("Labels Ignored For Node Balancing: %s\n", + output.PrintStringSlice(a.BalancingIgnoredLabels())) + } + + out += fmt.Sprintf("Ignore DaemonSets Utilization: %s\n", + output.PrintBool(a.IgnoreDaemonsetsUtilization())) + + if a.MaxNodeProvisionTime() != "" { + out += fmt.Sprintf("Maximum Node Provision Time: %s\n", + a.MaxNodeProvisionTime()) + } + + out += fmt.Sprintf("Maximum Pod Grace Period: %d\n", + a.MaxPodGracePeriod()) + out += fmt.Sprintf("Pod Priority Threshold: %d\n", + a.PodPriorityThreshold()) + + //Resource Limits + out += "Resource Limits:\n" + out += fmt.Sprintf(" - Maximum Nodes: %d\n", + a.ResourceLimits().MaxNodesTotal()) + out += fmt.Sprintf(" - Minimum Number of Cores: %d\n", + a.ResourceLimits().Cores().Min()) + out += fmt.Sprintf(" - Maximum Number of Cores: %d\n", + a.ResourceLimits().Cores().Max()) + out += fmt.Sprintf(" - Minimum Memory (GiB): %d\n", + a.ResourceLimits().Memory().Min()) + out += fmt.Sprintf(" - Maximum Memory (GiB): %d\n", + a.ResourceLimits().Memory().Max()) + + if len(a.ResourceLimits().GPUS()) > 0 { + out += " - GPU Limitations:\n" + for _, limitation := range a.ResourceLimits().GPUS() { + out += fmt.Sprintf(" - Type: %s\n", limitation.Type()) + out += fmt.Sprintf(" - Min: %d\n", limitation.Range().Min()) + out += fmt.Sprintf(" - Max: %d\n", limitation.Range().Max()) + } + } + + //Scale Down + out += "Scale Down:\n" + out += fmt.Sprintf(" - Enabled %s\n", + output.PrintBool(a.ScaleDown().Enabled())) + + if a.ScaleDown().UnneededTime() != "" { + out += fmt.Sprintf(" - Node Unneeded Time: %s\n", + a.ScaleDown().UnneededTime()) + } + out += fmt.Sprintf(" - Node Utilization Threshold: %s\n", + a.ScaleDown().UtilizationThreshold()) + + if a.ScaleDown().DelayAfterAdd() != "" { + out += fmt.Sprintf(" - Delay After Node Added: %s\n", + a.ScaleDown().DelayAfterAdd()) + } + + if a.ScaleDown().DelayAfterDelete() != "" { + out += fmt.Sprintf(" - Delay After Node Deleted: %s\n", + a.ScaleDown().DelayAfterDelete()) + } + + if a.ScaleDown().DelayAfterFailure() != "" { + out += fmt.Sprintf(" - Delay After Node Deletion Failure: %s\n", + a.ScaleDown().DelayAfterFailure()) + } + + return out +} diff --git a/pkg/clusterautoscaler/output_test.go b/pkg/clusterautoscaler/output_test.go new file mode 100644 index 0000000000..d8a3782a2a --- /dev/null +++ b/pkg/clusterautoscaler/output_test.go @@ -0,0 +1,140 @@ +package clusterautoscaler + +import ( + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" + + "github.com/openshift/rosa/pkg/test" +) + +var optionalFieldOutput = ` +Balance Similar Node Groups: No +Skip Nodes With Local Storage Yes +Log Verbosity: 2 +Labels Ignored For Node Balancing: foo, bar +Ignore DaemonSets Utilization: Yes +Maximum Node Provision Time: 10m +Maximum Pod Grace Period: 10 +Pod Priority Threshold: 10 +Resource Limits: + - Maximum Nodes: 10 + - Minimum Number of Cores: 20 + - Maximum Number of Cores: 30 + - Minimum Memory (GiB): 5 + - Maximum Memory (GiB): 10 + - GPU Limitations: + - Type: nvidia.com/gpu + - Min: 10 + - Max: 20 +Scale Down: + - Enabled Yes + - Node Unneeded Time: 25m + - Node Utilization Threshold: 20 + - Delay After Node Added: 5m + - Delay After Node Deleted: 20m + - Delay After Node Deletion Failure: 10m +` + +var mandatoryFieldOutput = ` +Balance Similar Node Groups: No +Skip Nodes With Local Storage Yes +Log Verbosity: 2 +Ignore DaemonSets Utilization: Yes +Maximum Pod Grace Period: 10 +Pod Priority Threshold: 10 +Resource Limits: + - Maximum Nodes: 10 + - Minimum Number of Cores: 20 + - Maximum Number of Cores: 30 + - Minimum Memory (GiB): 5 + - Maximum Memory (GiB): 10 +Scale Down: + - Enabled Yes + - Node Utilization Threshold: 20 +` + +var _ = Describe("Print Autoscaler", func() { + + It("Correctly prints with optional fields set", func() { + autoscaler := test.MockAutoscaler(func(a *cmv1.ClusterAutoscalerBuilder) { + a.MaxNodeProvisionTime("10m") + a.BalancingIgnoredLabels("foo", "bar") + a.PodPriorityThreshold(10) + a.LogVerbosity(2) + a.MaxPodGracePeriod(10) + a.IgnoreDaemonsetsUtilization(true) + a.SkipNodesWithLocalStorage(true) + a.BalanceSimilarNodeGroups(false) + + sd := &cmv1.AutoscalerScaleDownConfigBuilder{} + sd.Enabled(true) + sd.DelayAfterFailure("10m") + sd.DelayAfterAdd("5m") + sd.DelayAfterDelete("20m") + sd.UnneededTime("25m") + sd.UtilizationThreshold("20") + a.ScaleDown(sd) + + rl := &cmv1.AutoscalerResourceLimitsBuilder{} + rl.MaxNodesTotal(10) + + mem := &cmv1.ResourceRangeBuilder{} + mem.Max(10).Min(5) + rl.Memory(mem) + + cores := &cmv1.ResourceRangeBuilder{} + cores.Min(20).Max(30) + rl.Cores(cores) + + gpus := &cmv1.AutoscalerResourceLimitsGPULimitBuilder{} + gpus.Type("nvidia.com/gpu") + + gpuRR := &cmv1.ResourceRangeBuilder{} + gpuRR.Max(20).Min(10) + gpus.Range(gpuRR) + + rl.GPUS(gpus) + a.ResourceLimits(rl) + }) + + out := PrintAutoscaler(autoscaler) + Expect(out).NotTo(BeNil()) + Expect(optionalFieldOutput).To(Equal(out)) + }) + + It("Correctly sprints with mandatory fields", func() { + autoscaler := test.MockAutoscaler(func(a *cmv1.ClusterAutoscalerBuilder) { + a.PodPriorityThreshold(10) + a.LogVerbosity(2) + a.MaxPodGracePeriod(10) + a.IgnoreDaemonsetsUtilization(true) + a.SkipNodesWithLocalStorage(true) + a.BalanceSimilarNodeGroups(false) + + sd := &cmv1.AutoscalerScaleDownConfigBuilder{} + sd.Enabled(true) + sd.UtilizationThreshold("20") + a.ScaleDown(sd) + + rl := &cmv1.AutoscalerResourceLimitsBuilder{} + rl.MaxNodesTotal(10) + + mem := &cmv1.ResourceRangeBuilder{} + mem.Max(10).Min(5) + rl.Memory(mem) + + cores := &cmv1.ResourceRangeBuilder{} + cores.Min(20).Max(30) + rl.Cores(cores) + a.ResourceLimits(rl) + }) + + out := PrintAutoscaler(autoscaler) + fmt.Println(out) + Expect(out).NotTo(BeNil()) + Expect(mandatoryFieldOutput).To(Equal(out)) + }) +}) diff --git a/pkg/clusterautoscaler/validation.go b/pkg/clusterautoscaler/validation.go new file mode 100644 index 0000000000..7c70b0eadf --- /dev/null +++ b/pkg/clusterautoscaler/validation.go @@ -0,0 +1,27 @@ +package clusterautoscaler + +import ( + "fmt" + + cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" + + "github.com/openshift/rosa/pkg/rosa" +) + +const ( + NoHCPAutoscalerSupportMessage = "Hosted Control Plane clusters do not support cluster-autoscaler configuration" + ClusterNotReadyMessage = "Cluster '%s' is not yet ready. Current state is '%s'" +) + +func IsAutoscalerSupported(runtime *rosa.Runtime, cluster *cmv1.Cluster) error { + if cluster.Hypershift().Enabled() { + return fmt.Errorf(NoHCPAutoscalerSupportMessage) + + } + + if cluster.State() != cmv1.ClusterStateReady { + return fmt.Errorf(ClusterNotReadyMessage, runtime.ClusterKey, cluster.State()) + } + + return nil +} diff --git a/pkg/clusterautoscaler/validation_test.go b/pkg/clusterautoscaler/validation_test.go new file mode 100644 index 0000000000..85886d4acb --- /dev/null +++ b/pkg/clusterautoscaler/validation_test.go @@ -0,0 +1,57 @@ +package clusterautoscaler + +import ( + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" + + "github.com/openshift/rosa/pkg/test" +) + +var _ = Describe("Cluster Autoscaler validations", func() { + + var t *test.TestingRuntime + + BeforeEach(func() { + t = test.NewTestRuntime() + }) + + It("Determines Autoscalers are not valid for HCP clusters", func() { + + cluster := test.MockCluster(func(c *cmv1.ClusterBuilder) { + h := &cmv1.HypershiftBuilder{} + h.Enabled(true) + c.Hypershift(h) + }) + + err := IsAutoscalerSupported(t.RosaRuntime, cluster) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(NoHCPAutoscalerSupportMessage)) + + }) + + It("Determines Autoscalers are not valid for clusters in non-ready state", func() { + cluster := test.MockCluster(func(c *cmv1.ClusterBuilder) { + c.State(cmv1.ClusterStateInstalling) + }) + + err := IsAutoscalerSupported(t.RosaRuntime, cluster) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(fmt.Sprintf(ClusterNotReadyMessage, t.RosaRuntime.ClusterKey, cluster.State()))) + }) + + It("Determines ready, non-HCP cluster can support Autoscaler", func() { + cluster := test.MockCluster(func(c *cmv1.ClusterBuilder) { + h := &cmv1.HypershiftBuilder{} + h.Enabled(false) + c.Hypershift(h) + c.State(cmv1.ClusterStateReady) + }) + + err := IsAutoscalerSupported(t.RosaRuntime, cluster) + Expect(err).To(BeNil()) + }) + +}) diff --git a/pkg/externalauthprovider/flags_test.go b/pkg/externalauthprovider/flags_test.go index 19ac58c21c..134c8b3210 100644 --- a/pkg/externalauthprovider/flags_test.go +++ b/pkg/externalauthprovider/flags_test.go @@ -16,49 +16,46 @@ var _ = Describe("External authentication provider", func() { service := NewExternalAuthService(r.OCMClient) It("KO: cluster is not ready", func() { - mockClusterNotReady, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterNotReady := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateInstalling) c.Hypershift(cmv1.NewHypershift().Enabled(true)) }) - Expect(err).To(BeNil()) - err = service.IsExternalAuthProviderSupported(mockClusterNotReady, clusterKey) + err := service.IsExternalAuthProviderSupported(mockClusterNotReady, clusterKey) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(Equal("cluster 'test-cluster' is not yet ready")) }) It("KO: a ready cluster is not enabled with external auth provider flag", func() { - mockClusterReady, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) c.Hypershift(cmv1.NewHypershift().Enabled(true)) }) - Expect(err).To(BeNil()) - err = service.IsExternalAuthProviderSupported(mockClusterReady, clusterKey) + err := service.IsExternalAuthProviderSupported(mockClusterReady, clusterKey) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(Equal("external authentication configuration is not enabled for cluster 'test-cluster'\n" + "Create a hosted control plane with '--external-auth-providers-enabled' parameter to enabled the configuration")) }) It("KO: non hcp cluster is not supported for external auth provider", func() { - mockClusterReady, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) }) - Expect(err).To(BeNil()) - err = service.IsExternalAuthProviderSupported(mockClusterReady, clusterKey) + err := service.IsExternalAuthProviderSupported(mockClusterReady, clusterKey) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(Equal("external authentication provider is only supported for Hosted Control Planes")) }) It("OK: a ready hcp cluster is enabled with external auth provider flag", func() { - mockClusterReady, err := test.MockOCMCluster(func(c *cmv1.ClusterBuilder) { + mockClusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) { c.AWS(cmv1.NewAWS().SubnetIDs("subnet-0b761d44d3d9a4663", "subnet-0f87f640e56934cbc")) c.Region(cmv1.NewCloudRegion().ID("us-east-1")) c.State(cmv1.ClusterStateReady) @@ -66,8 +63,7 @@ var _ = Describe("External authentication provider", func() { c.ExternalAuthConfig(cmv1.NewExternalAuthConfig().Enabled(true)) }) - Expect(err).To(BeNil()) - err = service.IsExternalAuthProviderSupported(mockClusterReady, clusterKey) + err := service.IsExternalAuthProviderSupported(mockClusterReady, clusterKey) Expect(err).To(Not(HaveOccurred())) }) }) diff --git a/pkg/ocm/flag.go b/pkg/ocm/flag.go index e3ab72150d..1a2767969b 100644 --- a/pkg/ocm/flag.go +++ b/pkg/ocm/flag.go @@ -25,29 +25,28 @@ import ( "github.com/openshift/rosa/pkg/logging" ) +const ( + clusterFlagName = "cluster" + clusterFlagShortHand = "c" + clusterFlagDescription = "Name or ID of the cluster." +) + var clusterKey string func AddOptionalClusterFlag(cmd *cobra.Command) { cmd.Flags().StringVarP( &clusterKey, - "cluster", - "c", + clusterFlagName, + clusterFlagShortHand, "", - "Name or ID of the cluster.", + clusterFlagDescription, ) - cmd.RegisterFlagCompletionFunc("cluster", clusterCompletion) + cmd.RegisterFlagCompletionFunc(clusterFlagName, clusterCompletion) } func AddClusterFlag(cmd *cobra.Command) { - cmd.Flags().StringVarP( - &clusterKey, - "cluster", - "c", - "", - "Name or ID of the cluster.", - ) - cmd.MarkFlagRequired("cluster") - cmd.RegisterFlagCompletionFunc("cluster", clusterCompletion) + AddOptionalClusterFlag(cmd) + cmd.MarkFlagRequired(clusterFlagName) } func SetClusterKey(key string) { @@ -67,7 +66,7 @@ func GetClusterKey() (string, error) { return clusterKey, nil } -func clusterCompletion(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { +func clusterCompletion(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { logger := logging.NewLogger() ocmClient, err := NewClient().Logger(logger).Build() diff --git a/pkg/ocm/flag_test.go b/pkg/ocm/flag_test.go new file mode 100644 index 0000000000..1c2b6f0381 --- /dev/null +++ b/pkg/ocm/flag_test.go @@ -0,0 +1,41 @@ +package ocm + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/spf13/cobra" + flag "github.com/spf13/pflag" +) + +var _ = Describe("Cluster Flag", func() { + It("Adds cluster flag as optional to a command", func() { + cmd := &cobra.Command{} + Expect(cmd.Flag(clusterFlagName)).To(BeNil()) + + AddOptionalClusterFlag(cmd) + AssertClusterFlag(cmd.Flag(clusterFlagName), false) + }) + + It("Adds cluster flag as required to a command", func() { + cmd := &cobra.Command{} + Expect(cmd.Flag(clusterFlagName)).To(BeNil()) + + AddClusterFlag(cmd) + AssertClusterFlag(cmd.Flag(clusterFlagName), true) + }) + +}) + +func AssertClusterFlag(flag *flag.Flag, required bool) { + Expect(flag).NotTo(BeNil()) + Expect(flag.Name).To(Equal(clusterFlagName)) + Expect(flag.Shorthand).To(Equal(clusterFlagShortHand)) + Expect(flag.Usage).To(Equal(clusterFlagDescription)) + + if required { + // The cobra.BashCompOneRequiredFlag annotation is how Cobra marks flags as required + Expect(flag.Annotations).To(HaveKey(cobra.BashCompOneRequiredFlag)) + } else { + Expect(flag.Annotations).NotTo(HaveKey(cobra.BashCompOneRequiredFlag)) + } +} diff --git a/pkg/ocm/output/machinepools.go b/pkg/ocm/output/machinepools.go index 3cb9daecb6..6cce787f4a 100644 --- a/pkg/ocm/output/machinepools.go +++ b/pkg/ocm/output/machinepools.go @@ -23,22 +23,11 @@ import ( cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" "github.com/openshift/rosa/pkg/helper" -) - -const ( - Yes = "Yes" - No = "No" + "github.com/openshift/rosa/pkg/output" ) // Methods shared between node pools and machine pools -func PrintStringSlice(in []string) string { - if len(in) == 0 { - return "" - } - return strings.Join(in, ", ") -} - func PrintLabels(labels map[string]string) string { if len(labels) == 0 { return "" @@ -67,9 +56,9 @@ func PrintTaints(taints []*cmv1.Taint) string { func PrintMachinePoolAutoscaling(autoscaling *cmv1.MachinePoolAutoscaling) string { if autoscaling != nil { - return Yes + return output.Yes } - return No + return output.No } func PrintMachinePoolReplicas(autoscaling *cmv1.MachinePoolAutoscaling, replicas int) string { @@ -91,7 +80,7 @@ func PrintMachinePoolSpot(mp *cmv1.MachinePool) string { return fmt.Sprintf("Yes (%s)", price) } } - return No + return output.No } func PrintMachinePoolDiskSize(mp *cmv1.MachinePool) string { diff --git a/pkg/ocm/output/nodepools.go b/pkg/ocm/output/nodepools.go index 953a990583..675480a0b1 100644 --- a/pkg/ocm/output/nodepools.go +++ b/pkg/ocm/output/nodepools.go @@ -23,13 +23,14 @@ import ( cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" "github.com/openshift/rosa/pkg/ocm" + "github.com/openshift/rosa/pkg/output" ) func PrintNodePoolAutoscaling(autoscaling *cmv1.NodePoolAutoscaling) string { if autoscaling != nil { - return Yes + return output.Yes } - return No + return output.No } func PrintNodePoolReplicas(autoscaling *cmv1.NodePoolAutoscaling, replicas int) string { @@ -53,7 +54,7 @@ func PrintNodePoolAdditionalSecurityGroups(aws *cmv1.AWSNodePool) string { return "" } - return PrintStringSlice(aws.AdditionalSecurityGroupIds()) + return output.PrintStringSlice(aws.AdditionalSecurityGroupIds()) } func PrintNodePoolCurrentReplicas(status *cmv1.NodePoolStatus) string { @@ -80,9 +81,9 @@ func PrintNodePoolVersion(version *cmv1.Version) string { func PrintNodePoolAutorepair(autorepair bool) string { if autorepair { - return Yes + return output.Yes } - return No + return output.No } func PrintNodePoolTuningConfigs(tuningConfigs []string) string { diff --git a/pkg/output/flag.go b/pkg/output/flag.go index 7f08a4b10d..2697fe6bde 100644 --- a/pkg/output/flag.go +++ b/pkg/output/flag.go @@ -24,24 +24,31 @@ import ( "github.com/spf13/cobra" ) +const ( + JSON = "json" + YAML = "yaml" + FLAG_NAME = "output" + FLAG_SHORTHAND = "o" +) + var o string -var formats = []string{"json", "yaml"} +var formats = []string{JSON, YAML} // AddFlag adds the interactive flag to the given set of command line flags. func AddFlag(cmd *cobra.Command) { cmd.Flags().StringVarP( &o, - "output", - "o", + FLAG_NAME, + FLAG_SHORTHAND, "", fmt.Sprintf("Output format. Allowed formats are %s", formats), ) - cmd.RegisterFlagCompletionFunc("output", completion) + cmd.RegisterFlagCompletionFunc(FLAG_NAME, completion) } -func completion(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { +func completion(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { return formats, cobra.ShellCompDirectiveDefault } @@ -53,3 +60,7 @@ func HasFlag() bool { func Output() string { return o } + +func SetOutput(output string) { + o = output +} diff --git a/pkg/output/flag_test.go b/pkg/output/flag_test.go new file mode 100644 index 0000000000..8a0f6c8614 --- /dev/null +++ b/pkg/output/flag_test.go @@ -0,0 +1,53 @@ +package output + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/spf13/cobra" +) + +var _ = Describe("Output flag", func() { + + BeforeEach(func() { + SetOutput("") + }) + + AfterEach(func() { + SetOutput("") + }) + + It("Adds flag to command", func() { + + cmd := &cobra.Command{} + Expect(cmd.Flag(FLAG_NAME)).To(BeNil()) + + AddFlag(cmd) + + flag := cmd.Flag(FLAG_NAME) + Expect(flag).NotTo(BeNil()) + Expect(flag.Name).To(Equal(FLAG_NAME)) + Expect(flag.Shorthand).To(Equal(FLAG_SHORTHAND)) + Expect(flag.Value.String()).To(Equal("")) + Expect(flag.Usage).To(Equal("Output format. Allowed formats are [json yaml]")) + }) + + It("Has a completion function", func() { + args, directive := completion(nil, nil, "") + Expect(len(args)).To(Equal(2)) + Expect(args).To(ContainElements(JSON, YAML)) + + Expect(directive).To(Equal(cobra.ShellCompDirectiveDefault)) + }) + + It("Has flag", func() { + Expect(HasFlag()).To(BeFalse()) + SetOutput(JSON) + Expect(HasFlag()).To(BeTrue()) + Expect(Output()).To(Equal(JSON)) + }) + + It("Does not have flag", func() { + Expect(HasFlag()).To(BeFalse()) + }) + +}) diff --git a/pkg/output/output.go b/pkg/output/output.go index a99712b1ac..6cc5157778 100644 --- a/pkg/output/output.go +++ b/pkg/output/output.go @@ -128,6 +128,10 @@ func Print(resource interface{}) error { if kubeletConfig, ok := resource.(*cmv1.KubeletConfig); ok { cmv1.MarshalKubeletConfig(kubeletConfig, &b) } + case "*v1.ClusterAutoscaler": + if autoscaler, ok := resource.(*cmv1.ClusterAutoscaler); ok { + cmv1.MarshalClusterAutoscaler(autoscaler, &b) + } case "[]*v1.User": if users, ok := resource.([]*cmv1.User); ok { cmv1.MarshalUserList(users, &b) diff --git a/pkg/output/output_suite_test.go b/pkg/output/output_suite_test.go new file mode 100644 index 0000000000..43ffc5a848 --- /dev/null +++ b/pkg/output/output_suite_test.go @@ -0,0 +1,13 @@ +package output + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestOutput(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Output Suite") +} diff --git a/pkg/output/print.go b/pkg/output/print.go new file mode 100644 index 0000000000..93da33a2f0 --- /dev/null +++ b/pkg/output/print.go @@ -0,0 +1,24 @@ +package output + +import "strings" + +const ( + Yes = "Yes" + No = "No" + EmptySlice = "" +) + +// PrintBool returns a prettified version of a boolean. "Yes" for true, or "No" for false +func PrintBool(b bool) string { + if b { + return Yes + } + return No +} + +func PrintStringSlice(in []string) string { + if len(in) == 0 { + return EmptySlice + } + return strings.Join(in, ", ") +} diff --git a/pkg/output/print_test.go b/pkg/output/print_test.go new file mode 100644 index 0000000000..00700a19cf --- /dev/null +++ b/pkg/output/print_test.go @@ -0,0 +1,29 @@ +package output + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Print Functions test", func() { + Context("PrintBool", func() { + It("Prints true bool", func() { + Expect(PrintBool(true)).To(Equal(Yes)) + }) + + It("Prints false bool", func() { + Expect(PrintBool(false)).To(Equal(No)) + }) + }) + + Context("Print String Slice", func() { + It("Returns empty string for empty slice", func() { + Expect(PrintStringSlice(make([]string, 0))).To(Equal(EmptySlice)) + }) + + It("Returns slice elements correctly separated", func() { + values := []string{"foo", "bar"} + Expect(PrintStringSlice(values)).To(Equal("foo, bar")) + }) + }) +}) diff --git a/pkg/rosa/runner.go b/pkg/rosa/runner.go new file mode 100644 index 0000000000..00203b14e9 --- /dev/null +++ b/pkg/rosa/runner.go @@ -0,0 +1,61 @@ +package rosa + +import ( + "context" + "os" + + "github.com/spf13/cobra" +) + +// RuntimeVisitor are functions that configure the Runtime for a command. +type RuntimeVisitor func(ctx context.Context, runtime *Runtime, command *cobra.Command, args []string) + +// CommandRunner is a function supplied by Commands of the ROSA CLI that perform the actual logic of running +// the command +type CommandRunner func(ctx context.Context, runtime *Runtime, command *cobra.Command, args []string) error + +// DefaultRunner is a centralised implementation of the default Cobra Command.run function that takes care +// of instantiating several key resources on behalf of a command +func DefaultRunner(visitor RuntimeVisitor, runner CommandRunner) func(command *cobra.Command, args []string) { + return func(command *cobra.Command, args []string) { + ctx := context.Background() + r := NewRuntime() + defer r.Cleanup() + + if visitor != nil { + visitor(ctx, r, command, args) + } + + err := runner(ctx, r, command, args) + if err != nil { + r.Reporter.Errorf(err.Error()) + os.Exit(1) + } + } +} + +// DefaultRuntime returns a Runtime with the most basic of setups. None of the clients are initialised. +func DefaultRuntime() RuntimeVisitor { + return func(ctx context.Context, runtime *Runtime, command *cobra.Command, args []string) {} +} + +// RuntimeWithOCM configures the Runtime with an OCM Client +func RuntimeWithOCM() RuntimeVisitor { + return func(ctx context.Context, runtime *Runtime, command *cobra.Command, args []string) { + runtime.WithOCM() + } +} + +// RuntimeWithOCMAndAWS configures the Runtime with an OCM Client and AWS client +func RuntimeWithOCMAndAWS() RuntimeVisitor { + return func(ctx context.Context, runtime *Runtime, command *cobra.Command, args []string) { + runtime.WithOCM().WithAWS() + } +} + +// RuntimeWithAWS configures the Runtime with an AWS client +func RuntimeWithAWS() RuntimeVisitor { + return func(ctx context.Context, runtime *Runtime, command *cobra.Command, args []string) { + runtime.WithAWS() + } +} diff --git a/pkg/rosa/runner_test.go b/pkg/rosa/runner_test.go new file mode 100644 index 0000000000..78398d7434 --- /dev/null +++ b/pkg/rosa/runner_test.go @@ -0,0 +1,48 @@ +package rosa + +import ( + "context" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/spf13/cobra" +) + +func TestDefaultRunner(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "default command runner") +} + +var _ = Describe("Runner Tests", func() { + + It("Invokes RuntimeVisitor and CommandRunner", func() { + visited := false + visitor := func(ctx context.Context, runtime *Runtime, command *cobra.Command, args []string) { + visited = true + } + + run := false + runner := func(ctx context.Context, runtime *Runtime, command *cobra.Command, args []string) error { + run = true + return nil + } + + DefaultRunner(visitor, runner)(nil, nil) + + Expect(visited).To(BeTrue()) + Expect(run).To(BeTrue()) + }) + + It("Invokes Only CommandRunner if no RuntimeVisitor supplied", func() { + run := false + runner := func(ctx context.Context, runtime *Runtime, command *cobra.Command, args []string) error { + run = true + return nil + } + + DefaultRunner(nil, runner)(nil, nil) + + Expect(run).To(BeTrue()) + }) +}) diff --git a/pkg/rosa/runtime.go b/pkg/rosa/runtime.go index 4f8d61d469..186f0ec40b 100644 --- a/pkg/rosa/runtime.go +++ b/pkg/rosa/runtime.go @@ -108,13 +108,3 @@ func (r *Runtime) FetchCluster() *cmv1.Cluster { r.Cluster = cluster return cluster } - -func (r *Runtime) WithSpinner(fn func() error) error { - if r.Reporter.IsTerminal() { - r.Spinner.Start() - err := fn() // arbitrary function - r.Spinner.Stop() // stops spinner after the function completes - return err - } - return fn() -} diff --git a/pkg/test/helpers.go b/pkg/test/helpers.go index 7c6fa771c4..c0704a428a 100644 --- a/pkg/test/helpers.go +++ b/pkg/test/helpers.go @@ -9,6 +9,8 @@ import ( "reflect" "time" + "go.uber.org/mock/gomock" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/onsi/gomega/ghttp" @@ -97,7 +99,18 @@ func BuildExternalAuth() *v1.ExternalAuth { return externalAuth } -func MockOCMCluster(modifyFn func(c *v1.ClusterBuilder)) (*v1.Cluster, error) { +func MockAutoscaler(modifyFn func(a *v1.ClusterAutoscalerBuilder)) *v1.ClusterAutoscaler { + build := &v1.ClusterAutoscalerBuilder{} + if modifyFn != nil { + modifyFn(build) + } + + autoscaler, err := build.Build() + Expect(err).NotTo(HaveOccurred()) + return autoscaler +} + +func MockCluster(modifyFn func(c *v1.ClusterBuilder)) *v1.Cluster { mock := v1.NewCluster(). ID(MockClusterID). HREF(MockClusterHREF). @@ -107,7 +120,9 @@ func MockOCMCluster(modifyFn func(c *v1.ClusterBuilder)) (*v1.Cluster, error) { modifyFn(mock) } - return mock.Build() + cluster, err := mock.Build() + Expect(err).NotTo(HaveOccurred()) + return cluster } func FormatClusterList(clusters []*v1.Cluster) string { @@ -232,6 +247,10 @@ func FormatResource(resource interface{}) string { if res, ok := resource.(*v1.MachinePool); ok { err = v1.MarshalMachinePool(res, &outputJson) } + case "*v1.ClusterAutoscaler": + if res, ok := resource.(*v1.ClusterAutoscaler); ok { + err = v1.MarshalClusterAutoscaler(res, &outputJson) + } case "*v1.ControlPlaneUpgradePolicy": if res, ok := resource.(*v1.ControlPlaneUpgradePolicy); ok { err = v1.MarshalControlPlaneUpgradePolicy(res, &outputJson) @@ -248,6 +267,12 @@ func FormatResource(resource interface{}) string { return outputJson.String() } +func NewTestRuntime() *TestingRuntime { + t := &TestingRuntime{} + t.InitRuntime() + return t +} + // TestingRuntime is a wrapper for the structure used for testing type TestingRuntime struct { SsoServer *ghttp.Server @@ -291,7 +316,23 @@ func (t *TestingRuntime) InitRuntime() { AccountID: "123", IsSTS: false, } + + ctrl := gomock.NewController(GinkgoT()) + aws := aws.NewMockClient(ctrl) + t.RosaRuntime.AWSClient = aws + DeferCleanup(t.RosaRuntime.Cleanup) DeferCleanup(t.SsoServer.Close) DeferCleanup(t.ApiServer.Close) + DeferCleanup(t.Close) +} + +func (t *TestingRuntime) Close() { + ocm.SetClusterKey("") +} + +func (t *TestingRuntime) SetCluster(clusterKey string, cluster *v1.Cluster) { + ocm.SetClusterKey(clusterKey) + t.RosaRuntime.Cluster = cluster + t.RosaRuntime.ClusterKey = clusterKey }