OCM-7289 | feat: revert maxUnavailable and maxSurge for NodePool

cmd/create/machinepool/cmd.go

@@ -59,8 +59,6 @@ var args struct {
 	securityGroupIds      []string
 	nodeDrainGracePeriod  string
 	tags                  []string
-	maxSurge              string
-	maxUnavailable        string
 }
 
 var Cmd = &cobra.Command{
@@ -251,20 +249,6 @@ 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)
 }

cmd/create/machinepool/nodepool.go

@@ -464,49 +464,6 @@ 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))
 	}

cmd/describe/machinepool/cmd_test.go

@@ -38,10 +38,6 @@ 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 = `
@@ -62,10 +58,6 @@ 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
 `
@@ -87,10 +79,6 @@ 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
 `
@@ -101,11 +89,6 @@ 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
@@ -425,10 +408,8 @@ 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).
-		ManagementUpgrade(mgmtUpgrade).Build()
+		AWSNodePool(awsNodePool).AvailabilityZone("us-east-1a").NodeDrainGracePeriod(nodeDrain).Build()
 	Expect(err).To(BeNil())
 	return test.FormatResource(np)
 }
@@ -438,10 +419,8 @@ 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).
-		ManagementUpgrade(mgmtUpgrade).Build()
+		AWSNodePool(awsNodePool).AvailabilityZone("us-east-1a").NodeDrainGracePeriod(nodeDrain).Build()
 	Expect(err).To(BeNil())
 	return test.FormatResource(np)
 }

cmd/edit/machinepool/cmd.go

@@ -38,8 +38,6 @@ var args struct {
 	tuningConfigs        string
 	kubeletConfigs       string
 	nodeDrainGracePeriod string
-	maxSurge             string
-	maxUnavailable       string
 }
 
 var Cmd = &cobra.Command{
@@ -148,20 +146,6 @@ 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) {

cmd/edit/machinepool/nodepool.go

@@ -31,8 +31,6 @@ 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 ||
@@ -225,58 +223,6 @@ 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)

pkg/helper/machinepools/helpers.go

@@ -341,30 +341,3 @@ 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
-}

pkg/helper/machinepools/helpers_test.go

@@ -330,57 +330,3 @@ 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",
-		),
-	)
-})

pkg/machinepool/output.go

@@ -27,7 +27,6 @@ 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" +
@@ -82,8 +81,6 @@ 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()),
 	)
 }

pkg/machinepool/output_test.go

@@ -113,13 +113,10 @@ var _ = Describe("Output", Ordered, func() {
 			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", "", "", managementUpgradeOutput, "")
+				"test-subnets", "1", "No", "test-tc", "test-kc", "", "", "")
 
 			result := nodePoolOutput("test-cluster", nodePool)
 			Expect(out).To(Equal(result))
@@ -136,7 +133,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))

pkg/ocm/output/nodepools.go

@@ -115,14 +115,3 @@ 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 ""
-}