From 5dee9f4b8f7e2725dea86534e1b2fb8449c9f512 Mon Sep 17 00:00:00 2001 From: Jesse Jaggars Date: Tue, 4 Nov 2025 11:45:43 -0500 Subject: [PATCH 1/2] OCM-21663 | feat: Allow min-replicas=0 for HCP nodepool autoscaling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updated CLI validation to allow minimum of 0 replicas for HCP nodepool autoscaling, matching the relaxed API constraints for customers with that capability enabled. Changes: - Updated validation in pkg/machinepool/machinepool.go to allow min >= 0 - Updated interactive validator in pkg/machinepool/helper.go for HCP - Updated unit tests to verify min=0 succeeds and negative values fail - Added comprehensive HCP-specific test coverage in helper_test.go - Updated E2E tests to expect min=0 to succeed The changes align HCP nodepool validation with Classic machinepool behavior while maintaining all other safety constraints (negative values still rejected, multi-AZ multiples of 3 still enforced). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/machinepool/helper.go | 5 +-- pkg/machinepool/helper_test.go | 53 +++++++++++++++++++++++++++++ pkg/machinepool/machinepool.go | 4 +-- pkg/machinepool/machinepool_test.go | 8 +++-- tests/e2e/hcp_machine_pool_test.go | 10 +++++- 5 files changed, 73 insertions(+), 7 deletions(-) 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..f9ebf6bf9b 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)) From 231e025c5391d41757a8afd9e33fa1f8d2a226ce Mon Sep 17 00:00:00 2001 From: Jesse Jaggars Date: Thu, 15 Jan 2026 10:43:49 -0500 Subject: [PATCH 2/2] OCM-21663 | fix: Declare expectErrMsg variable in test Fix undefined variable error in HCP machine pool test by properly declaring expectErrMsg variable. Co-Authored-By: Claude Sonnet 4.5 (1M context) --- tests/e2e/hcp_machine_pool_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/hcp_machine_pool_test.go b/tests/e2e/hcp_machine_pool_test.go index f9ebf6bf9b..fb88a78fbf 100644 --- a/tests/e2e/hcp_machine_pool_test.go +++ b/tests/e2e/hcp_machine_pool_test.go @@ -432,7 +432,7 @@ var _ = Describe("HCP Machine Pool", labels.Feature.Machinepool, func() { "--min-replicas", fmt.Sprintf("%v", negativeMinReplica), "-y", ) - expectErrMsg = "ERR: min-replicas must be a non-negative number when autoscaling is set" + expectErrMsg := "ERR: min-replicas must be a non-negative number when autoscaling is set" Expect(err).To(HaveOccurred()) Expect(err.Error()).Should(ContainSubstring(expectErrMsg))