diff --git a/apis/placement/v1/clusterresourceplacement_types.go b/apis/placement/v1/clusterresourceplacement_types.go index ee719cbf3..f5737f86d 100644 --- a/apis/placement/v1/clusterresourceplacement_types.go +++ b/apis/placement/v1/clusterresourceplacement_types.go @@ -500,7 +500,7 @@ 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. - // This can not be 0 if MaxSurge is 0. + // The minimum of MaxUnavailable is 1 to avoid rolling out stuck during in-place resource update. // Defaults to 25%. // +kubebuilder:default="25%" // +kubebuilder:validation:XIntOrString @@ -513,7 +513,7 @@ type RollingUpdateConfig struct { // The desired number equals to the number of clusters scheduler selected when the placement type is `PickAll`. // Value can be an absolute number (ex: 5) or a percentage of desire (ex: 10%). // Absolute number is calculated from percentage by rounding up. - // This does not apply to the case that we do in-place upgrade of resources on the same cluster. + // This does not apply to the case that we do in-place update of resources on the same cluster. // This can not be 0 if MaxUnavailable is 0. // Defaults to 25%. // +kubebuilder:default="25%" diff --git a/apis/placement/v1beta1/clusterresourceplacement_types.go b/apis/placement/v1beta1/clusterresourceplacement_types.go index 28e5fa9f6..28dbf61f4 100644 --- a/apis/placement/v1beta1/clusterresourceplacement_types.go +++ b/apis/placement/v1beta1/clusterresourceplacement_types.go @@ -500,7 +500,7 @@ 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. - // This can not be 0 if MaxSurge is 0. + // The minimum of MaxUnavailable is 1 to avoid rolling out stuck during in-place resource update. // Defaults to 25%. // +kubebuilder:default="25%" // +kubebuilder:validation:XIntOrString @@ -513,7 +513,7 @@ type RollingUpdateConfig struct { // The desired number equals to the number of clusters scheduler selected when the placement type is `PickAll`. // Value can be an absolute number (ex: 5) or a percentage of desire (ex: 10%). // Absolute number is calculated from percentage by rounding up. - // This does not apply to the case that we do in-place upgrade of resources on the same cluster. + // This does not apply to the case that we do in-place update of resources on the same cluster. // This can not be 0 if MaxUnavailable is 0. // Defaults to 25%. // +kubebuilder:default="25%" diff --git a/pkg/utils/validator/clusterresourceplacement.go b/pkg/utils/validator/clusterresourceplacement.go index ea868efdd..6f57c985c 100644 --- a/pkg/utils/validator/clusterresourceplacement.go +++ b/pkg/utils/validator/clusterresourceplacement.go @@ -365,8 +365,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 < 0 { - allErr = append(allErr, fmt.Errorf("maxUnavailable must be greater than or equal to 0, got `%+v`", rolloutStrategy.RollingUpdate.MaxUnavailable)) + if value < 1 { + allErr = append(allErr, fmt.Errorf("maxUnavailable must be greater than or equal to 1, 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 index 2924c7ccb..2fab6c8e2 100644 --- a/pkg/utils/validator/clusterresourceplacement_test.go +++ b/pkg/utils/validator/clusterresourceplacement_test.go @@ -426,7 +426,20 @@ func TestValidateClusterResourcePlacement_RolloutStrategy(t *testing.T) { }, }, wantErr: true, - wantErrMsg: "maxUnavailable must be greater than or equal to 0, got `-10`", + wantErrMsg: "maxUnavailable must be greater than or equal to 1, got `-10`", + }, + "invalid rollout strategy - zero MaxUnavailable": { + strategy: placementv1beta1.RolloutStrategy{ + Type: placementv1beta1.RollingUpdateRolloutStrategyType, + RollingUpdate: &placementv1beta1.RollingUpdateConfig{ + MaxUnavailable: &intstr.IntOrString{ + Type: 0, + IntVal: 0, + }, + }, + }, + wantErr: true, + wantErrMsg: "maxUnavailable must be greater than or equal to 1, got `0`", }, "invalid rollout strategy - % error MaxSurge": { strategy: placementv1beta1.RolloutStrategy{