diff --git a/pkg/machinepool/helper.go b/pkg/machinepool/helper.go index 43b5165a17..d372fe2994 100644 --- a/pkg/machinepool/helper.go +++ b/pkg/machinepool/helper.go @@ -263,8 +263,9 @@ func (r *ReplicaSizeValidation) MinReplicaValidator() interactive.Validator { if err != nil { return err } - if r.Autoscaling && minReplicas < 1 && r.IsHostedCp { - return fmt.Errorf("min-replicas must be greater than zero") + if r.Autoscaling && minReplicas < 0 && r.IsHostedCp { + return fmt.Errorf("min-replicas must be a number that is 0 or greater when autoscaling is" + + " enabled") } if r.Autoscaling && minReplicas < 0 && !r.IsHostedCp { return fmt.Errorf("min-replicas must be a number that is 0 or greater when autoscaling is" + diff --git a/pkg/machinepool/helper_test.go b/pkg/machinepool/helper_test.go index 68ca408ff6..efedb0fe4b 100644 --- a/pkg/machinepool/helper_test.go +++ b/pkg/machinepool/helper_test.go @@ -516,6 +516,59 @@ var _ = Describe("Machine pool min/max replicas validation", func() { false, ), ) + + DescribeTable("NodePool (HCP) min replicas validation", + func(minReplicas int, autoscaling bool, multiAZ bool, hasError bool) { + replicaSizeValidation := &ReplicaSizeValidation{ + ClusterVersion: "openshift-v4.14.14", + MultiAz: multiAZ, + Autoscaling: autoscaling, + IsHostedCp: true, + } + err := replicaSizeValidation.MinReplicaValidator()(minReplicas) + if hasError { + Expect(err).To(HaveOccurred()) + } else { + Expect(err).ToNot(HaveOccurred()) + } + }, + Entry("Zero replicas - no autoscaling - should succeed", + 0, + false, + false, + false, + ), + Entry("Zero replicas - autoscaling - should succeed", + 0, + true, + false, + false, + ), + Entry("Negative replicas - autoscaling - should fail", + -1, + true, + false, + true, + ), + Entry("One replica - autoscaling - should succeed", + 1, + true, + false, + false, + ), + Entry("Multi-AZ - 3 replicas - should succeed", + 3, + true, + true, + false, + ), + Entry("Multi-AZ - 2 replicas - should fail (not multiple of 3)", + 2, + true, + true, + true, + ), + ) }) var _ = Describe("CreateAwsNodePoolBuilder", func() { diff --git a/pkg/machinepool/machinepool.go b/pkg/machinepool/machinepool.go index 1157eb6491..aac412f78e 100644 --- a/pkg/machinepool/machinepool.go +++ b/pkg/machinepool/machinepool.go @@ -2109,8 +2109,8 @@ func validateNodePoolEdit(cmd *cobra.Command, autoscaling bool, replicas int, mi return fmt.Errorf("the number of machine pool replicas needs to be a non-negative integer") } - if autoscaling && cmd.Flags().Changed("min-replicas") && minReplicas < 1 { - return fmt.Errorf("min-replicas must be greater than zero") + if autoscaling && cmd.Flags().Changed("min-replicas") && minReplicas < 0 { + return fmt.Errorf("min-replicas must be a non-negative number when autoscaling is set") } if autoscaling && cmd.Flags().Changed("max-replicas") && maxReplicas < 1 { diff --git a/pkg/machinepool/machinepool_test.go b/pkg/machinepool/machinepool_test.go index 0cd88a5a9e..773cdaf933 100644 --- a/pkg/machinepool/machinepool_test.go +++ b/pkg/machinepool/machinepool_test.go @@ -108,9 +108,13 @@ var _ = Describe("Machinepool and nodepool", func() { asBuilder := cmv1.NewNodePoolAutoscaling().MaxReplica(4).MinReplica(2) Expect(builder).To(Equal(asBuilder)) }) - It("Test edit nodepool min-replicas < 1 when autoscaling is set", func() { + It("Test edit nodepool min-replicas = 0 when autoscaling is set - should succeed", func() { err := validateNodePoolEdit(testCommand, true, 0, 0, 1) - Expect(err.Error()).To(Equal("min-replicas must be greater than zero")) + Expect(err).ToNot(HaveOccurred()) + }) + It("Test edit nodepool min-replicas < 0 when autoscaling is set - should fail", func() { + err := validateNodePoolEdit(testCommand, true, 0, -1, 1) + Expect(err.Error()).To(Equal("min-replicas must be a non-negative number when autoscaling is set")) }) It("Test edit nodepool !autoscaling and replicas < 0 for nodepools", func() { err := validateNodePoolEdit(testCommand, false, -1, 0, 0) diff --git a/tests/e2e/hcp_machine_pool_test.go b/tests/e2e/hcp_machine_pool_test.go index 8629eb5c26..fb88a78fbf 100644 --- a/tests/e2e/hcp_machine_pool_test.go +++ b/tests/e2e/hcp_machine_pool_test.go @@ -424,7 +424,15 @@ var _ = Describe("HCP Machine Pool", labels.Feature.Machinepool, func() { "--min-replicas", fmt.Sprintf("%v", zeroMinReplica), "-y", ) - expectErrMsg := "ERR: min-replicas must be greater than zero" + Expect(err).ToNot(HaveOccurred()) + + By("Try to set machine pool min replica to negative value - should fail") + negativeMinReplica := -1 + _, err = rosaClient.MachinePool.EditMachinePool(clusterID, mpName, + "--min-replicas", fmt.Sprintf("%v", negativeMinReplica), + "-y", + ) + expectErrMsg := "ERR: min-replicas must be a non-negative number when autoscaling is set" Expect(err).To(HaveOccurred()) Expect(err.Error()).Should(ContainSubstring(expectErrMsg))