diff --git a/pkg/machinepool/machinepool.go b/pkg/machinepool/machinepool.go index 5fec85e7ca..5c557e742e 100644 --- a/pkg/machinepool/machinepool.go +++ b/pkg/machinepool/machinepool.go @@ -1534,22 +1534,8 @@ func editNodePool(cmd *cobra.Command, nodePoolID string, return fmt.Errorf("Failed to get autoscaling or replicas: '%s'", err) } - if !autoscaling && replicas < 0 { - 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("The number of machine pool min-replicas needs to be a non-negative integer") - } - - if autoscaling && cmd.Flags().Changed("max-replicas") && maxReplicas < 1 { - return fmt.Errorf("The number of machine pool max-replicas needs to be a non-negative integer") - } - - if autoscaling && cmd.Flags().Changed("max-replicas") && cmd.Flags().Changed( - "min-replicas") && minReplicas > maxReplicas { - return fmt.Errorf("The number of machine pool min-replicas needs to be less than the number of " + - "machine pool max-replicas") + if validateNodePoolEdit(cmd, autoscaling, replicas, minReplicas, maxReplicas) != nil { + return err } labels := cmd.Flags().Lookup("labels").Value.String() @@ -1788,6 +1774,27 @@ func editNodePool(cmd *cobra.Command, nodePoolID string, return nil } +func validateNodePoolEdit(cmd *cobra.Command, autoscaling bool, replicas int, minReplicas int, maxReplicas int) error { + if !autoscaling && replicas < 0 { + 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("max-replicas") && maxReplicas < 1 { + return fmt.Errorf("max-replicas must be greater than zero.") + } + + if autoscaling && cmd.Flags().Changed("max-replicas") && cmd.Flags().Changed( + "min-replicas") && minReplicas > maxReplicas { + return fmt.Errorf("The number of machine pool min-replicas needs to be less than the number of " + + "machine pool max-replicas") + } + return nil +} + // promptForNodePoolNodeRecreate - prompts the user to accept that their changes will cause the nodes // in their nodepool to be recreated. This primarily applies to KubeletConfig modifications. func promptForNodePoolNodeRecreate( diff --git a/pkg/machinepool/machinepool_test.go b/pkg/machinepool/machinepool_test.go index f0963059fc..32457c43ca 100644 --- a/pkg/machinepool/machinepool_test.go +++ b/pkg/machinepool/machinepool_test.go @@ -38,8 +38,11 @@ var date time.Time var _ = Describe("Machinepool and nodepool", func() { var ( - mockClient *mock.MockClient - mockCtrl *gomock.Controller + mockClient *mock.MockClient + mockCtrl *gomock.Controller + testCommand *cobra.Command + maxReplicas int + minReplicas int ) Context("Nodepools", Ordered, func() { BeforeAll(func() { @@ -52,7 +55,24 @@ var _ = Describe("Machinepool and nodepool", func() { NextRun(date) mockCtrl = gomock.NewController(GinkgoT()) mockClient = mock.NewMockClient(mockCtrl) - + testCommand = &cobra.Command{} + flags := testCommand.Flags() + flags.IntVar( + &maxReplicas, + "max-replicas", + 0, + "Maximum number of machines for the machine pool.", + ) + flags.IntVar( + &minReplicas, + "min-replicas", + 0, + "Minimum number of machines for the machine pool.", + ) + testCommand.Flags().Set("min-replicas", "0") + testCommand.Flags().Set("max-replicas", "1") + testCommand.Flags().Lookup("min-replicas").Changed = true + testCommand.Flags().Lookup("max-replicas").Changed = true }) It("editAutoscaling should equal nil if nothing is changed", func() { nodepool, err := cmv1.NewNodePool(). @@ -62,7 +82,7 @@ var _ = Describe("Machinepool and nodepool", func() { builder := editAutoscaling(nodepool, 1, 2) Expect(builder).To(BeNil()) }) - It("editAutoscaling should equal the exepcted output", func() { + It("editAutoscaling should equal the expected output", func() { nodepool, err := cmv1.NewNodePool(). Autoscaling(cmv1.NewNodePoolAutoscaling().MaxReplica(2).MinReplica(1)). Build() @@ -71,7 +91,7 @@ var _ = Describe("Machinepool and nodepool", func() { asBuilder := cmv1.NewNodePoolAutoscaling().MaxReplica(3).MinReplica(2) Expect(builder).To(Equal(asBuilder)) }) - It("editAutoscaling should equal the exepcted output with no min replica value", func() { + It("editAutoscaling should equal the expected output with no min replica value", func() { nodepool, err := cmv1.NewNodePool(). Autoscaling(cmv1.NewNodePoolAutoscaling().MaxReplica(2).MinReplica(1)). Build() @@ -80,8 +100,7 @@ var _ = Describe("Machinepool and nodepool", func() { asBuilder := cmv1.NewNodePoolAutoscaling().MaxReplica(3).MinReplica(1) Expect(builder).To(Equal(asBuilder)) }) - - It("editAutoscaling should equal the exepcted output with no max replica value", func() { + It("editAutoscaling should equal the expected output with no max replica value", func() { nodepool, err := cmv1.NewNodePool(). Autoscaling(cmv1.NewNodePoolAutoscaling().MaxReplica(4).MinReplica(1)). Build() @@ -90,6 +109,24 @@ 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() { + err := validateNodePoolEdit(testCommand, true, 0, 0, 1) + Expect(err.Error()).To(Equal("min-replicas must be greater than zero.")) + }) + It("Test edit nodepool !autoscaling and replicas < 0 for nodepools", func() { + err := validateNodePoolEdit(testCommand, false, -1, 0, 0) + Expect(err.Error()).To(Equal("The number of machine pool replicas needs to be a non-negative integer")) + }) + It("Test edit nodepool autoscaling and minReplicas > maxReplicas", func() { + err := validateNodePoolEdit(testCommand, true, 0, 5, 1) + Expect(err.Error()).To(Equal("The number of machine pool min-replicas needs to be less " + + "than the number of machine pool max-replicas")) + }) + It("Test edit nodepool autoscaling and maxReplicas < 1", func() { + err := validateNodePoolEdit(testCommand, true, 0, 1, 0) + Expect(err.Error()).To(Equal("max-replicas must be greater than zero.")) + }) + Context("Prompt For NodePoolNodeRecreate", func() { var mockPromptFuncInvoked bool var t *test.TestingRuntime