Skip to content

Commit

Permalink
OCM-7289 | feat: create/update of maxUnavailable and maxSurge for HCP…
Browse files Browse the repository at this point in the history
… nodepools
  • Loading branch information
philipwu08 committed Jun 5, 2024
1 parent f7897c0 commit b79df90
Show file tree
Hide file tree
Showing 11 changed files with 276 additions and 7 deletions.
16 changes: 16 additions & 0 deletions cmd/create/machinepool/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ var args struct {
securityGroupIds []string
nodeDrainGracePeriod string
tags []string
maxSurge string
maxUnavailable string
}

var Cmd = &cobra.Command{
Expand Down Expand Up @@ -249,6 +251,20 @@ func init() {
"Tags are comma separated, for example: 'key value, foo bar'",
)

flags.StringVar(&args.maxSurge,
"max-surge",
"1",
"The maximum number of nodes that can be provisioned above the desired number of nodes in the machinepool during "+
"the upgrade. It can be an absolute number i.e. 1, or a percentage i.e. '20%'.",
)

flags.StringVar(&args.maxUnavailable,
"max-unavailable",
"0",
"The maximum number of nodes in the machinepool that can be unavailable during the upgrade. It can be an "+
"absolute number i.e. 1, or a percentage i.e. '20%'.",
)

interactive.AddFlag(flags)
output.AddFlag(Cmd)
}
Expand Down
43 changes: 43 additions & 0 deletions cmd/create/machinepool/nodepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,49 @@ func addNodePool(cmd *cobra.Command, clusterKey string, cluster *cmv1.Cluster, r
npBuilder.NodeDrainGracePeriod(nodeDrainBuilder)
}

maxSurge := args.maxSurge
if interactive.Enabled() {
maxSurge, err = interactive.GetString(interactive.Input{
Question: "Max surge",
Help: cmd.Flags().Lookup("max-surge").Usage,
Default: maxSurge,
Required: false,
Validators: []interactive.Validator{
machinepools.ValidateUpgradeMaxSurgeUnavailable,
},
})
if err != nil {
r.Reporter.Errorf("Expected a valid value for max surge: %s", err)
os.Exit(1)
}
}
maxUnavailable := args.maxUnavailable
if interactive.Enabled() {
maxUnavailable, err = interactive.GetString(interactive.Input{
Question: "Max unavailable",
Help: cmd.Flags().Lookup("max-unavailable").Usage,
Default: maxUnavailable,
Required: false,
Validators: []interactive.Validator{
machinepools.ValidateUpgradeMaxSurgeUnavailable,
},
})
if err != nil {
r.Reporter.Errorf("Expected a valid value for max unavailable: %s", err)
os.Exit(1)
}
}
if maxSurge != "" || maxUnavailable != "" {
mgmtUpgradeBuilder := cmv1.NewNodePoolManagementUpgrade()
if maxSurge != "" {
mgmtUpgradeBuilder.MaxSurge(maxSurge)
}
if maxUnavailable != "" {
mgmtUpgradeBuilder.MaxUnavailable(maxUnavailable)
}
npBuilder.ManagementUpgrade(mgmtUpgradeBuilder)
}

if version != "" {
npBuilder.Version(cmv1.NewVersion().ID(version))
}
Expand Down
25 changes: 23 additions & 2 deletions cmd/describe/machinepool/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ Tuning configs:
Kubelet configs:
Additional security group IDs:
Node drain grace period: 1 minute
Management upgrade:
- Type: Replace
- Max surge: 1
- Max unavailable: 0
Message:
`
describeStringWithUpgradeOutput = `
Expand All @@ -58,6 +62,10 @@ Tuning configs:
Kubelet configs:
Additional security group IDs:
Node drain grace period: 1 minute
Management upgrade:
- Type: Replace
- Max surge: 1
- Max unavailable: 0
Message:
Scheduled upgrade: scheduled 4.12.25 on 2023-08-07 15:22 UTC
`
Expand All @@ -79,6 +87,10 @@ Tuning configs:
Kubelet configs:
Additional security group IDs:
Node drain grace period: 1 minute
Management upgrade:
- Type: Replace
- Max surge: 1
- Max unavailable: 0
Message:
Scheduled upgrade: scheduled 4.12.25 on 2023-08-07 15:22 UTC
`
Expand All @@ -89,6 +101,11 @@ aws_node_pool:
kind: AWSNodePool
id: nodepool85
kind: NodePool
management_upgrade:
kind: NodePoolManagementUpgrade
max_surge: "1"
max_unavailable: "0"
type: Replace
node_drain_grace_period:
unit: minute
value: 1
Expand Down Expand Up @@ -408,8 +425,10 @@ func formatNodePool() string {
version := cmv1.NewVersion().ID("4.12.24").RawID("openshift-4.12.24")
awsNodePool := cmv1.NewAWSNodePool().InstanceType("m5.xlarge")
nodeDrain := cmv1.NewValue().Value(1).Unit("minute")
mgmtUpgrade := cmv1.NewNodePoolManagementUpgrade().Type("Replace").MaxSurge("1").MaxUnavailable("0")
np, err := cmv1.NewNodePool().ID(nodePoolName).Version(version).
AWSNodePool(awsNodePool).AvailabilityZone("us-east-1a").NodeDrainGracePeriod(nodeDrain).Build()
AWSNodePool(awsNodePool).AvailabilityZone("us-east-1a").NodeDrainGracePeriod(nodeDrain).
ManagementUpgrade(mgmtUpgrade).Build()
Expect(err).To(BeNil())
return test.FormatResource(np)
}
Expand All @@ -419,8 +438,10 @@ func formatNodePoolWithTags() string {
version := cmv1.NewVersion().ID("4.12.24").RawID("openshift-4.12.24")
awsNodePool := cmv1.NewAWSNodePool().InstanceType("m5.xlarge").Tags(map[string]string{"foo": "bar"})
nodeDrain := cmv1.NewValue().Value(1).Unit("minute")
mgmtUpgrade := cmv1.NewNodePoolManagementUpgrade().Type("Replace").MaxSurge("1").MaxUnavailable("0")
np, err := cmv1.NewNodePool().ID(nodePoolName).Version(version).
AWSNodePool(awsNodePool).AvailabilityZone("us-east-1a").NodeDrainGracePeriod(nodeDrain).Build()
AWSNodePool(awsNodePool).AvailabilityZone("us-east-1a").NodeDrainGracePeriod(nodeDrain).
ManagementUpgrade(mgmtUpgrade).Build()
Expect(err).To(BeNil())
return test.FormatResource(np)
}
Expand Down
16 changes: 16 additions & 0 deletions cmd/edit/machinepool/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ var args struct {
tuningConfigs string
kubeletConfigs string
nodeDrainGracePeriod string
maxSurge string
maxUnavailable string
}

var Cmd = &cobra.Command{
Expand Down Expand Up @@ -146,6 +148,20 @@ func init() {
"'hour|hours'. 0 or empty value means that the NodePool can be drained without any time limitations.\n"+
"This flag is only supported for Hosted Control Planes.",
)

flags.StringVar(&args.maxSurge,
"max-surge",
"",
"The maximum number of nodes that can be provisioned above the desired number of nodes in the machinepool during "+
"the upgrade. It can be an absolute number i.e. 1, or a percentage i.e. '20%'.",
)

flags.StringVar(&args.maxUnavailable,
"max-unavailable",
"",
"The maximum number of nodes in the machinepool that can be unavailable during the upgrade. It can be an "+
"absolute number i.e. 1, or a percentage i.e. '20%'.",
)
}

func run(cmd *cobra.Command, argv []string) {
Expand Down
54 changes: 54 additions & 0 deletions cmd/edit/machinepool/nodepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ func editNodePool(cmd *cobra.Command, nodePoolID string,
isTuningsConfigSet := cmd.Flags().Changed("tuning-configs")
isKubeletConfigSet := cmd.Flags().Changed("kubelet-configs")
isNodeDrainGracePeriodSet := cmd.Flags().Changed("node-drain-grace-period")
isUpgradeMaxSurgeSet := cmd.Flags().Changed("max-surge")
isUpgradeMaxUnavailableSet := cmd.Flags().Changed("max-unavailable")

// isAnyAdditionalParameterSet is true if at least one parameter not related to replicas and autoscaling is set
isAnyAdditionalParameterSet := isLabelsSet || isTaintsSet || isAutorepairSet || isTuningsConfigSet ||
Expand Down Expand Up @@ -223,6 +225,58 @@ func editNodePool(cmd *cobra.Command, nodePoolID string,
}
}

if isUpgradeMaxSurgeSet || isUpgradeMaxUnavailableSet || interactive.Enabled() {
maxSurge := args.maxSurge
if maxSurge == "" && nodePool.ManagementUpgrade().MaxSurge() != "" {
maxSurge = nodePool.ManagementUpgrade().MaxSurge()
}

maxUnavailable := args.maxUnavailable
if maxUnavailable == "" && nodePool.ManagementUpgrade().MaxUnavailable() != "" {
maxUnavailable = nodePool.ManagementUpgrade().MaxUnavailable()
}
if interactive.Enabled() {
maxSurge, err = interactive.GetString(interactive.Input{
Question: "Max surge",
Help: cmd.Flags().Lookup("max-surge").Usage,
Default: maxSurge,
Required: false,
Validators: []interactive.Validator{
machinepools.ValidateUpgradeMaxSurgeUnavailable,
},
})
if err != nil {
r.Reporter.Errorf("Expected a valid value for max surge: %s", err)
os.Exit(1)
}

maxUnavailable, err = interactive.GetString(interactive.Input{
Question: "Max unavailable",
Help: cmd.Flags().Lookup("max-unavailable").Usage,
Default: maxUnavailable,
Required: false,
Validators: []interactive.Validator{
machinepools.ValidateUpgradeMaxSurgeUnavailable,
},
})
if err != nil {
r.Reporter.Errorf("Expected a valid value for max unavailable: %s", err)
os.Exit(1)
}
}

if maxSurge != "" || maxUnavailable != "" {
mgmtUpgradeBuilder := cmv1.NewNodePoolManagementUpgrade()
if maxSurge != "" {
mgmtUpgradeBuilder.MaxSurge(maxSurge)
}
if maxUnavailable != "" {
mgmtUpgradeBuilder.MaxUnavailable(maxUnavailable)
}
npBuilder.ManagementUpgrade(mgmtUpgradeBuilder)
}
}

update, err := npBuilder.Build()
if err != nil {
return fmt.Errorf("Failed to create machine pool for hosted cluster '%s': %v", clusterKey, err)
Expand Down
27 changes: 27 additions & 0 deletions pkg/helper/machinepools/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,3 +341,30 @@ func ValidateNodeDrainGracePeriod(val interface{}) error {
}
return nil
}

func ValidateUpgradeMaxSurgeUnavailable(val interface{}) error {
maxSurgeOrUnavail := strings.TrimSpace(val.(string))
if maxSurgeOrUnavail == "" {
return nil
}

if strings.HasSuffix(maxSurgeOrUnavail, "%") {
percent, err := strconv.Atoi(strings.TrimSuffix(maxSurgeOrUnavail, "%"))
if err != nil {
return fmt.Errorf("Percentage value '%s' must be an integer", strings.TrimSuffix(maxSurgeOrUnavail, "%"))
}
if percent < 0 || percent > 100 {
return fmt.Errorf("Percentage value %d must be between 0 and 100", percent)
}
} else {
intMaxSurgeOrUnavail, err := strconv.Atoi(maxSurgeOrUnavail)
if err != nil {
return fmt.Errorf("Value '%s' must be an integer", maxSurgeOrUnavail)
}
if intMaxSurgeOrUnavail < 0 {
return fmt.Errorf("Value %d cannot be negative", intMaxSurgeOrUnavail)
}
}

return nil
}
54 changes: 54 additions & 0 deletions pkg/helper/machinepools/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,3 +330,57 @@ var _ = Describe("Validate node drain grace period", func() {
),
)
})

var _ = Describe("Validate MaxSurge and MaxUnavailable", func() {
DescribeTable("Validate MaxSurge and MaxUnavailable",
func(value interface{}, errMsg string) {
err := ValidateUpgradeMaxSurgeUnavailable(value)
if errMsg != "" {
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring(errMsg))
} else {
Expect(err).ToNot(HaveOccurred())
}
},
Entry("Should not error with empty value",
"",
"",
),
Entry("Should not error with 0 percent",
"0%",
"",
),
Entry("Should not error with 100 percent",
"100%",
"",
),
Entry("Should error with negative percentage",
"-1%",
"Percentage value -1 must be between 0 and 100",
),
Entry("Should error with 101% percent",
"101%",
"Percentage value 101 must be between 0 and 100",
),
Entry("Should error with non-integer percent",
"1.1%",
"Percentage value '1.1' must be an integer",
),
Entry("Should not error with 0",
"0",
"",
),
Entry("Should not error with positive integer",
"1",
"",
),
Entry("Should error with negative number",
"-1",
"Value -1 cannot be negative",
),
Entry("Should error with non-integer",
"1.1",
"Value '1.1' must be an integer",
),
)
})
3 changes: 3 additions & 0 deletions pkg/machinepool/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var nodePoolOutputString string = "\n" +
"Kubelet configs: %s\n" +
"Additional security group IDs: %s\n" +
"Node drain grace period: %s\n" +
"Management upgrade: %s\n" +
"Message: %s\n"

var machinePoolOutputString = "\n" +
Expand Down Expand Up @@ -81,6 +82,8 @@ func nodePoolOutput(clusterId string, nodePool *cmv1.NodePool) string {
ocmOutput.PrintNodePoolConfigs(nodePool.KubeletConfigs()),
ocmOutput.PrintNodePoolAdditionalSecurityGroups(nodePool.AWSNodePool()),
ocmOutput.PrintNodeDrainGracePeriod(nodePool.NodeDrainGracePeriod()),
ocmOutput.PrintNodePoolManagementUpgrade(nodePool.ManagementUpgrade()),

ocmOutput.PrintNodePoolMessage(nodePool.Status()),
)
}
11 changes: 8 additions & 3 deletions pkg/machinepool/output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,19 +102,24 @@ var _ = Describe("Output", Ordered, func() {
})
It("nodepool output with autoscaling", func() {
npAutoscaling := cmv1.NewNodePoolAutoscaling().ID("test-as").MinReplica(2).MaxReplica(8)
mgmtUpgradeBuilder := cmv1.NewNodePoolManagementUpgrade().MaxSurge("1").MaxUnavailable("0")
nodePoolBuilder := *cmv1.NewNodePool().ID("test-mp").Autoscaling(npAutoscaling).Replicas(4).
AvailabilityZone("test-az").Subnet("test-subnets").Version(cmv1.NewVersion().
ID("1")).AutoRepair(false).TuningConfigs("test-tc").
KubeletConfigs("test-kc").Labels(labels).Taints(taintsBuilder)
KubeletConfigs("test-kc").Labels(labels).Taints(taintsBuilder).
ManagementUpgrade(mgmtUpgradeBuilder)
nodePool, err := nodePoolBuilder.Build()
Expect(err).ToNot(HaveOccurred())
labelsOutput := ocmOutput.PrintLabels(labels)
taintsOutput := ocmOutput.PrintTaints([]*cmv1.Taint{taint})
replicasOutput := ocmOutput.PrintNodePoolReplicas((*cmv1.NodePoolAutoscaling)(npAutoscaling), 4)
mgmtUpgrade, err := mgmtUpgradeBuilder.Build()
Expect(err).ToNot(HaveOccurred())
managementUpgradeOutput := ocmOutput.PrintNodePoolManagementUpgrade(mgmtUpgrade)

out := fmt.Sprintf(nodePoolOutputString,
"test-mp", "test-cluster", "Yes", replicasOutput, "", "", labelsOutput, "", taintsOutput, "test-az",
"test-subnets", "1", "No", "test-tc", "test-kc", "", "", "")
"test-subnets", "1", "No", "test-tc", "test-kc", "", "", managementUpgradeOutput, "")

result := nodePoolOutput("test-cluster", nodePool)
Expect(out).To(Equal(result))
Expand All @@ -131,7 +136,7 @@ var _ = Describe("Output", Ordered, func() {

out := fmt.Sprintf(nodePoolOutputString,
"test-mp", "test-cluster", "No", "4", "", "", labelsOutput, "", taintsOutput, "test-az",
"test-subnets", "1", "No", "test-tc", "test-kc", "", "", "")
"test-subnets", "1", "No", "test-tc", "test-kc", "", "", "", "")

result := nodePoolOutput("test-cluster", nodePool)
Expect(out).To(Equal(result))
Expand Down
Loading

0 comments on commit b79df90

Please sign in to comment.