Skip to content

Commit

Permalink
OCM-8337 | fix: [HCP] always set replicas when updating machinepools
Browse files Browse the repository at this point in the history
We always pass the replicas when editing a machinepool, even if
the value has not changed, as this will then be a no-op. Right now
we are not setting replicas if they are the same as the current value
and this result in an error returned to the user.

Related: OCM-8337
  • Loading branch information
andreadecorte committed May 31, 2024
1 parent f8cd646 commit 929bfbb
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 7 deletions.
18 changes: 11 additions & 7 deletions cmd/edit/machinepool/nodepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,7 @@ func editNodePool(cmd *cobra.Command, nodePoolID string,
npBuilder = npBuilder.Taints(taintBuilders...)
}

if autoscaling {
npBuilder.Autoscaling(editAutoscaling(nodePool, minReplicas, maxReplicas))
} else {
if nodePool.Replicas() != replicas {
npBuilder.Replicas(replicas)
}
}
fillAutoScalingAndReplicas(npBuilder, autoscaling, nodePool, minReplicas, maxReplicas, replicas)

if isAutorepairSet || interactive.Enabled() {
autorepair := args.autorepair
Expand Down Expand Up @@ -248,6 +242,16 @@ func editNodePool(cmd *cobra.Command, nodePoolID string,
return nil
}

// fillAutoScalingAndReplicas is filling either autoscaling or replicas value in the builder
func fillAutoScalingAndReplicas(npBuilder *cmv1.NodePoolBuilder, autoscaling bool, existingNodepool *cmv1.NodePool,
minReplicas int, maxReplicas int, replicas int) {
if autoscaling {
npBuilder.Autoscaling(editAutoscaling(existingNodepool, minReplicas, maxReplicas))
} else {
npBuilder.Replicas(replicas)
}
}

// 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(
Expand Down
26 changes: 26 additions & 0 deletions cmd/edit/machinepool/nodepool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,30 @@ var _ = Describe("Nodepool", func() {
})

})

Context("fillAutoScalingAndReplicas", func() {
var npBuilder *cmv1.NodePoolBuilder
existingNodepool, err := cmv1.NewNodePool().
Autoscaling(cmv1.NewNodePoolAutoscaling().MaxReplica(4).MinReplica(1)).
Build()
Expect(err).To(BeNil())
It("Autoscaling set", func() {
npBuilder = cmv1.NewNodePool()
fillAutoScalingAndReplicas(npBuilder, true, existingNodepool, 1, 3, 2)
npPatch, err := npBuilder.Build()
Expect(err).To(BeNil())
Expect(npPatch.Autoscaling()).ToNot(BeNil())
// Default (zero) value
Expect(npPatch.Replicas()).To(Equal(0))
})
It("Replicas set", func() {
npBuilder = cmv1.NewNodePool()
fillAutoScalingAndReplicas(npBuilder, false, existingNodepool, 0, 0, 2)
npPatch, err := npBuilder.Build()
Expect(err).To(BeNil())
Expect(npPatch.Autoscaling()).To(BeNil())
Expect(npPatch.Replicas()).To(Equal(2))
})

})
})

0 comments on commit 929bfbb

Please sign in to comment.