diff --git a/cmd/create/machinepool/cmd.go b/cmd/create/machinepool/cmd.go index 312903d10f..52ef685c43 100644 --- a/cmd/create/machinepool/cmd.go +++ b/cmd/create/machinepool/cmd.go @@ -59,6 +59,8 @@ var args struct { securityGroupIds []string nodeDrainGracePeriod string tags []string + maxSurge string + maxUnavailable string } var Cmd = &cobra.Command{ @@ -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) } diff --git a/cmd/create/machinepool/nodepool.go b/cmd/create/machinepool/nodepool.go index 40e2fb950a..c08515d9dc 100644 --- a/cmd/create/machinepool/nodepool.go +++ b/cmd/create/machinepool/nodepool.go @@ -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)) } diff --git a/cmd/describe/machinepool/cmd_test.go b/cmd/describe/machinepool/cmd_test.go index c2480030b9..aa0c8473b1 100644 --- a/cmd/describe/machinepool/cmd_test.go +++ b/cmd/describe/machinepool/cmd_test.go @@ -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 = ` @@ -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 ` @@ -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 ` @@ -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 @@ -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) } @@ -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) } diff --git a/cmd/edit/machinepool/cmd.go b/cmd/edit/machinepool/cmd.go index 466187aecd..bb237c9f36 100644 --- a/cmd/edit/machinepool/cmd.go +++ b/cmd/edit/machinepool/cmd.go @@ -38,6 +38,8 @@ var args struct { tuningConfigs string kubeletConfigs string nodeDrainGracePeriod string + maxSurge string + maxUnavailable string } var Cmd = &cobra.Command{ @@ -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) { diff --git a/cmd/edit/machinepool/nodepool.go b/cmd/edit/machinepool/nodepool.go index 571d9d1a58..5ee5dc6bab 100644 --- a/cmd/edit/machinepool/nodepool.go +++ b/cmd/edit/machinepool/nodepool.go @@ -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 || @@ -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) diff --git a/pkg/helper/machinepools/helpers.go b/pkg/helper/machinepools/helpers.go index 7d666f95db..5244fcffcd 100644 --- a/pkg/helper/machinepools/helpers.go +++ b/pkg/helper/machinepools/helpers.go @@ -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 +} diff --git a/pkg/helper/machinepools/helpers_test.go b/pkg/helper/machinepools/helpers_test.go index 019b879d0a..691a4f2e65 100644 --- a/pkg/helper/machinepools/helpers_test.go +++ b/pkg/helper/machinepools/helpers_test.go @@ -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", + ), + ) +}) diff --git a/pkg/machinepool/output.go b/pkg/machinepool/output.go index 03973d245c..b03e9d4e2c 100644 --- a/pkg/machinepool/output.go +++ b/pkg/machinepool/output.go @@ -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" + @@ -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()), ) } diff --git a/pkg/machinepool/output_test.go b/pkg/machinepool/output_test.go index c477b1a609..f53c89efd0 100644 --- a/pkg/machinepool/output_test.go +++ b/pkg/machinepool/output_test.go @@ -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)) @@ -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)) diff --git a/pkg/ocm/output/nodepools.go b/pkg/ocm/output/nodepools.go index f18f824845..89fd2d59d4 100644 --- a/pkg/ocm/output/nodepools.go +++ b/pkg/ocm/output/nodepools.go @@ -115,3 +115,14 @@ func PrintNodeDrainGracePeriod(period *cmv1.Value) string { return "" } + +func PrintNodePoolManagementUpgrade(upgrade *cmv1.NodePoolManagementUpgrade) string { + if upgrade != nil { + return fmt.Sprintf("\n"+ + " - Type: %s\n"+ + " - Max surge: %s\n"+ + " - Max unavailable: %s", upgrade.Type(), upgrade.MaxSurge(), upgrade.MaxUnavailable()) + } + + return "" +} diff --git a/pkg/ocm/output/nodepools_test.go b/pkg/ocm/output/nodepools_test.go index a1c6d99015..8364e27d7c 100644 --- a/pkg/ocm/output/nodepools_test.go +++ b/pkg/ocm/output/nodepools_test.go @@ -1,17 +1,19 @@ package output import ( + "fmt" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" ) -var _ = Describe("Create node drain grace period builder validations", func() { +var _ = Describe("Validate node drain grace period print output", func() { zeroValue, _ := cmv1.NewValue().Value(0).Build() oneValue, _ := cmv1.NewValue().Value(1).Build() twoValue, _ := cmv1.NewValue().Value(2).Build() - DescribeTable("Create node drain grace period builder validations", + DescribeTable("Validate node drain grace period print output", func(period *cmv1.Value, expectedOutput string) { output := PrintNodeDrainGracePeriod(period) Expect(output).To(Equal(expectedOutput)) @@ -44,3 +46,20 @@ var _ = Describe("PrintNodePoolReplicasInline", func() { }) }) + +var _ = Describe("Validate management upgrade print output", func() { + mgmtUpgrade, _ := cmv1.NewNodePoolManagementUpgrade().MaxSurge("10").MaxUnavailable("5").Type("Replace").Build() + DescribeTable("Validate management upgrade print output", + func(upgrade *cmv1.NodePoolManagementUpgrade, expectedOutput string) { + output := PrintNodePoolManagementUpgrade(upgrade) + Expect(output).To(Equal(expectedOutput)) + }, + Entry("Should return empty string", nil, + "", + ), + Entry("Should return string with type, maxSurge and maxUnavailable", + mgmtUpgrade, + fmt.Sprintf("\n - Type:%38s\n - Max surge:%28s\n - Max unavailable:%21s", "Replace", "10", "5"), + ), + ) +})