fix: allow 0 maxUnavailable (Azure#989)

britaniar · Dec 16, 2024 · 3bb2656 · 3bb2656
1 parent dcb8c94
commit 3bb2656
Show file tree

Hide file tree

Showing 8 changed files with 21 additions and 17 deletions.
diff --git a/apis/placement/v1/clusterresourceplacement_types.go b/apis/placement/v1/clusterresourceplacement_types.go
@@ -500,7 +500,8 @@ type RollingUpdateConfig struct {
 	// Absolute number is calculated from percentage by rounding up.
 	// We consider a resource unavailable when we either remove it from a cluster or in-place
 	// upgrade the resources content on the same cluster.
-	// The minimum of MaxUnavailable is 1 to avoid rolling out stuck during in-place resource update.
+	// The minimum of MaxUnavailable is 0 to allow no downtime moving a placement from one cluster to another.
+	// Please set it to be greater than 0 to avoid rolling out stuck during in-place resource update.
 	// Defaults to 25%.
 	// +kubebuilder:default="25%"
 	// +kubebuilder:validation:XIntOrString

diff --git a/apis/placement/v1beta1/clusterresourceplacement_types.go b/apis/placement/v1beta1/clusterresourceplacement_types.go
@@ -762,7 +762,8 @@ type RollingUpdateConfig struct {
 	// Absolute number is calculated from percentage by rounding up.
 	// We consider a resource unavailable when we either remove it from a cluster or in-place
 	// upgrade the resources content on the same cluster.
-	// The minimum of MaxUnavailable is 1 to avoid rolling out stuck during in-place resource update.
+	// The minimum of MaxUnavailable is 0 to allow no downtime moving a placement from one cluster to another.
+	// Please set it to be greater than 0 to avoid rolling out stuck during in-place resource update.
 	// Defaults to 25%.
 	// +kubebuilder:default="25%"
 	// +kubebuilder:validation:XIntOrString

diff --git a/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceplacements.yaml b/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceplacements.yaml
@@ -715,7 +715,8 @@ spec:
                           Absolute number is calculated from percentage by rounding up.
                           We consider a resource unavailable when we either remove it from a cluster or in-place
                           upgrade the resources content on the same cluster.
-                          The minimum of MaxUnavailable is 1 to avoid rolling out stuck during in-place resource update.
+                          The minimum of MaxUnavailable is 0 to allow no downtime moving a placement from one cluster to another.
+                          Please set it to be greater than 0 to avoid rolling out stuck during in-place resource update.
                           Defaults to 25%.
                         pattern: ^((100|[0-9]{1,2})%|[0-9]+)$
                         x-kubernetes-int-or-string: true
@@ -2054,7 +2055,8 @@ spec:
                           Absolute number is calculated from percentage by rounding up.
                           We consider a resource unavailable when we either remove it from a cluster or in-place
                           upgrade the resources content on the same cluster.
-                          The minimum of MaxUnavailable is 1 to avoid rolling out stuck during in-place resource update.
+                          The minimum of MaxUnavailable is 0 to allow no downtime moving a placement from one cluster to another.
+                          Please set it to be greater than 0 to avoid rolling out stuck during in-place resource update.
                           Defaults to 25%.
                         pattern: ^((100|[0-9]{1,2})%|[0-9]+)$
                         x-kubernetes-int-or-string: true

diff --git a/go.mod b/go.mod
@@ -40,6 +40,7 @@ require (
 
 require (
 	dario.cat/mergo v1.0.0 // indirect
+	github.com/Azure/azure-kusto-go v0.16.1 // indirect
 	github.com/Azure/azure-sdk-for-go v68.0.0+incompatible // indirect
 	github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 // indirect
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2 v2.2.0 // indirect

diff --git a/go.sum b/go.sum
@@ -1,7 +1,7 @@
 dario.cat/mergo v1.0.0 h1:AGCNq9Evsj31mOgNPcLyXc+4PNABt905YmuqPYYpBWk=
 dario.cat/mergo v1.0.0/go.mod h1:uNxQE+84aUszobStD9th8a29P2fMDhsBdgRYvZOxGmk=
-github.com/Azure/azure-kusto-go v0.14.0 h1:5XVmjh5kVgsm2scpsWisJ6Q1ZgWHJcIOPCZC1gatD4I=
-github.com/Azure/azure-kusto-go v0.14.0/go.mod h1:wSmXIsQwBVPHDNsSQsX98nuc12VyvxoNHQa2q9t1Ce0=
+github.com/Azure/azure-kusto-go v0.16.1 h1:vCBWcQghmC1qIErUUgVNWHxGhZVStu1U/hki6iBA14k=
+github.com/Azure/azure-kusto-go v0.16.1/go.mod h1:9F2zvXH8B6eWzgI1S4k1ZXAIufnBZ1bv1cW1kB1n3D0=
 github.com/Azure/azure-sdk-for-go v68.0.0+incompatible h1:fcYLmCpyNYRnvJbPerq7U0hS+6+I79yEDJBqVNcqUzU=
 github.com/Azure/azure-sdk-for-go v68.0.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
 github.com/Azure/azure-sdk-for-go-extensions v0.1.4 h1:XNT7IWmj4u3AfSag3t2mFupHT59J58pknX+daqprjm8=

diff --git a/pkg/utils/validator/clusterresourceplacement.go b/pkg/utils/validator/clusterresourceplacement.go
@@ -388,8 +388,8 @@ func validateRolloutStrategy(rolloutStrategy placementv1beta1.RolloutStrategy) e
 			if err != nil {
 				allErr = append(allErr, fmt.Errorf("maxUnavailable `%+v` is invalid: %w", rolloutStrategy.RollingUpdate.MaxUnavailable, err))
 			}
-			if value < 1 {
-				allErr = append(allErr, fmt.Errorf("maxUnavailable must be greater than or equal to 1, got `%+v`", rolloutStrategy.RollingUpdate.MaxUnavailable))
+			if value < 0 {
+				allErr = append(allErr, fmt.Errorf("maxUnavailable must be greater than or equal to 0, got `%+v`", rolloutStrategy.RollingUpdate.MaxUnavailable))
 			}
 		}
 		if rolloutStrategy.RollingUpdate.MaxSurge != nil {

diff --git a/pkg/utils/validator/clusterresourceplacement_test.go b/pkg/utils/validator/clusterresourceplacement_test.go
@@ -407,9 +407,9 @@ func TestValidateClusterResourcePlacement_RolloutStrategy(t *testing.T) {
 				},
 			},
 			wantErr:    true,
-			wantErrMsg: "maxUnavailable must be greater than or equal to 1, got `-10`",
+			wantErrMsg: "maxUnavailable must be greater than or equal to 0, got `-10`",
 		},
-		"invalid rollout strategy - zero MaxUnavailable": {
+		"valid rollout strategy - zero MaxUnavailable": {
 			strategy: placementv1beta1.RolloutStrategy{
 				Type: placementv1beta1.RollingUpdateRolloutStrategyType,
 				RollingUpdate: &placementv1beta1.RollingUpdateConfig{
@@ -419,8 +419,7 @@ func TestValidateClusterResourcePlacement_RolloutStrategy(t *testing.T) {
 					},
 				},
 			},
-			wantErr:    true,
-			wantErrMsg: "maxUnavailable must be greater than or equal to 1, got `0`",
+			wantErr: false,
 		},
 		"invalid rollout strategy - % error MaxSurge": {
 			strategy: placementv1beta1.RolloutStrategy{

diff --git a/...hook/clusterresourceplacement/v1beta1_clusterresourceplacement_validating_webhook_test.go b/...hook/clusterresourceplacement/v1beta1_clusterresourceplacement_validating_webhook_test.go
@@ -31,7 +31,7 @@ var (
 		Kind:    "ClusterRole",
 		Name:    "test-cluster-role",
 	}
-	errString = "the rollout Strategy field  is invalid: maxUnavailable must be greater than or equal to 1, got `0`"
+	errString = "the rollout Strategy field  is invalid: maxUnavailable must be greater than or equal to 0, got `-1`"
 )
 
 func TestHandle(t *testing.T) {
@@ -48,7 +48,7 @@ func TestHandle(t *testing.T) {
 			Strategy: placementv1beta1.RolloutStrategy{
 				Type: placementv1beta1.RollingUpdateRolloutStrategyType,
 				RollingUpdate: &placementv1beta1.RollingUpdateConfig{
-					MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: 0},
+					MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: -1},
 				},
 			},
 		},
@@ -68,7 +68,7 @@ func TestHandle(t *testing.T) {
 			Strategy: placementv1beta1.RolloutStrategy{
 				Type: placementv1beta1.RollingUpdateRolloutStrategyType,
 				RollingUpdate: &placementv1beta1.RollingUpdateConfig{
-					MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: 0},
+					MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: -1},
 					MaxSurge:       &intstr.IntOrString{Type: intstr.Int, IntVal: 0},
 				},
 			},
@@ -89,7 +89,7 @@ func TestHandle(t *testing.T) {
 			Strategy: placementv1beta1.RolloutStrategy{
 				Type: placementv1beta1.RollingUpdateRolloutStrategyType,
 				RollingUpdate: &placementv1beta1.RollingUpdateConfig{
-					MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: 0},
+					MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: -1},
 				},
 			},
 		},
@@ -108,7 +108,7 @@ func TestHandle(t *testing.T) {
 			Strategy: placementv1beta1.RolloutStrategy{
 				Type: placementv1beta1.RollingUpdateRolloutStrategyType,
 				RollingUpdate: &placementv1beta1.RollingUpdateConfig{
-					MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: 0},
+					MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: -1},
 				},
 			},
 		},