diff --git a/apis/placement/v1alpha1/disruptionbudget_types.go b/apis/placement/v1alpha1/disruptionbudget_types.go index de38608b8..982180ed3 100644 --- a/apis/placement/v1alpha1/disruptionbudget_types.go +++ b/apis/placement/v1alpha1/disruptionbudget_types.go @@ -44,8 +44,7 @@ type PlacementDisruptionBudgetSpec struct { // If a percentage is specified, Fleet will calculate the corresponding absolute values // as follows: // * if the linked Placement object is of the PickFixed placement type, - // the percentage is against the number of clusters specified in the placement (i.e., the - // length of ClusterNames field in the placement policy); + // we don't perform any calculation because eviction is not allowed for PickFixed CRP. // * if the linked Placement object is of the PickAll placement type, MaxUnavailable cannot // be specified since we cannot derive the total number of clusters selected. // * if the linked Placement object is of the PickN placement type, @@ -74,8 +73,7 @@ type PlacementDisruptionBudgetSpec struct { // If a percentage is specified, Fleet will calculate the corresponding absolute values // as follows: // * if the linked Placement object is of the PickFixed placement type, - // the percentage is against the number of clusters specified in the placement (i.e., the - // length of ClusterNames field in the placement policy); + // we don't perform any calculation because eviction is not allowed for PickFixed CRP. // * if the linked Placement object is of the PickAll placement type, MinAvailable can be // specified but only as an integer since we cannot derive the total number of clusters selected. // * if the linked Placement object is of the PickN placement type, diff --git a/apis/placement/v1alpha1/eviction_types.go b/apis/placement/v1alpha1/eviction_types.go index 1525223a4..d72327606 100644 --- a/apis/placement/v1alpha1/eviction_types.go +++ b/apis/placement/v1alpha1/eviction_types.go @@ -37,6 +37,10 @@ import ( // (e.g., the CRP object or the placement appears later). To fix the situation, re-create the // Eviction object. // +// Note: Eviction of resources from a cluster propagated by a PickFixed CRP is not allowed. +// If the user wants to remove resources from a cluster propagated by a PickFixed CRP simply +// remove the cluster name from cluster names field from the CRP spec. +// // Executed evictions might be kept around for a while for auditing purposes; the Fleet controllers might // have a TTL set up for such objects and will garbage collect them automatically. For further // information, see the Fleet documentation. diff --git a/pkg/controllers/clusterresourceplacementeviction/controller.go b/pkg/controllers/clusterresourceplacementeviction/controller.go index 1ab40ffa7..beac3bebb 100644 --- a/pkg/controllers/clusterresourceplacementeviction/controller.go +++ b/pkg/controllers/clusterresourceplacementeviction/controller.go @@ -91,6 +91,11 @@ func (r *Reconciler) validateEviction(ctx context.Context, eviction *placementv1 markEvictionInvalid(eviction, condition.EvictionInvalidDeletingCRPMessage) return validationResult, nil } + if crp.Spec.Policy.PlacementType == placementv1beta1.PickFixedPlacementType { + klog.V(2).InfoS(condition.EvictionInvalidPickFixedCRPMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName) + markEvictionInvalid(eviction, condition.EvictionInvalidPickFixedCRPMessage) + return validationResult, nil + } validationResult.crp = &crp var crbList placementv1beta1.ClusterResourceBindingList @@ -256,13 +261,9 @@ func isEvictionAllowed(bindings []placementv1beta1.ClusterResourceBinding, crp p var desiredBindings int placementType := crp.Spec.Policy.PlacementType - switch placementType { - case placementv1beta1.PickNPlacementType: + // we don't know the desired bindings for PickAll and we won't evict a binding for PickFixed CRP. + if placementType == placementv1beta1.PickNPlacementType { desiredBindings = int(*crp.Spec.Policy.NumberOfClusters) - case placementv1beta1.PickFixedPlacementType: - desiredBindings = len(crp.Spec.Policy.ClusterNames) - default: - // we don't know the desired bindings for PickAll. } var disruptionsAllowed int diff --git a/pkg/controllers/clusterresourceplacementeviction/controller_test.go b/pkg/controllers/clusterresourceplacementeviction/controller_test.go index 333e6670a..f67e53fae 100644 --- a/pkg/controllers/clusterresourceplacementeviction/controller_test.go +++ b/pkg/controllers/clusterresourceplacementeviction/controller_test.go @@ -135,6 +135,31 @@ func TestValidateEviction(t *testing.T) { }, wantErr: nil, }, + { + name: "invalid eviction - pickFixed CRP", + eviction: buildTestEviction(testEvictionName, testCRPName, testClusterName), + crp: &placementv1beta1.ClusterResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: testCRPName, + }, + Spec: placementv1beta1.ClusterResourcePlacementSpec{ + Policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickFixedPlacementType, + }, + }, + }, + wantValidationResult: &evictionValidationResult{ + isValid: false, + }, + wantEvictionInvalidCondition: &metav1.Condition{ + Type: string(placementv1alpha1.PlacementEvictionConditionTypeValid), + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + Reason: condition.ClusterResourcePlacementEvictionInvalidReason, + Message: condition.EvictionInvalidPickFixedCRPMessage, + }, + wantErr: nil, + }, { name: "invalid eviction - multiple CRBs for same cluster", eviction: buildTestEviction(testEvictionName, testCRPName, testClusterName), diff --git a/pkg/utils/condition/condition.go b/pkg/utils/condition/condition.go index f3df6584f..b5ee25bb3 100644 --- a/pkg/utils/condition/condition.go +++ b/pkg/utils/condition/condition.go @@ -165,6 +165,9 @@ const ( // EvictionInvalidDeletingCRPMessage is the message string of invalid eviction condition when CRP is deleting. EvictionInvalidDeletingCRPMessage = "Found deleting ClusterResourcePlacement targeted by eviction" + // EvictionInvalidPickFixedCRPMessage is the message string of invalid eviction condition when CRP placement type is PickFixed. + EvictionInvalidPickFixedCRPMessage = "Found ClusterResourcePlacement with PickFixed placement type targeted by eviction" + // EvictionInvalidMissingCRBMessage is the message string of invalid eviction condition when CRB is missing. EvictionInvalidMissingCRBMessage = "Failed to find scheduler decision for placement in cluster targeted by eviction"