fix: CEL validation for CRPDB and flag to disable eviction controller…

… by default (Azure#1009)

apis/placement/v1alpha1/common.go

@@ -17,4 +17,10 @@ const (
 
 	// OverrideClusterNameVariable is the reserved variable in the override value that will be replaced by the actual cluster name.
 	OverrideClusterNameVariable = "${MEMBER-CLUSTER-NAME}"
+
+	// ClusterResourcePlacementEvictionKind is the kind of the ClusterResourcePlacementEviction.
+	ClusterResourcePlacementEvictionKind = "ClusterResourcePlacementEviction"
+
+	// ClusterResourcePlacementDisruptionBudgetKind is the kind of the ClusterResourcePlacementDisruptionBudget.
+	ClusterResourcePlacementDisruptionBudgetKind = "ClusterResourcePlacementDisruptionBudget"
 )

apis/placement/v1alpha1/disruptionbudget_types.go

@@ -59,7 +59,7 @@ type PlacementDisruptionBudgetSpec struct {
 	// of them can be set at a time.
 	//
 	// +kubebuilder:validation:XIntOrString
-	// +kubebuilder:validation:XValidation:rule="type(self) == string ? self.matches('^(100|[0-9]{1,2}%)$') : self >= 0",message="If supplied value is String should match regex '^(100|[0-9]{1,2}%)$' or If supplied value is Integer must be greater than or equal to 0"
+	// +kubebuilder:validation:XValidation:rule="type(self) == string ? self.matches('^(100|[0-9]{1,2})%$') : self >= 0",message="If supplied value is String should match regex '^(100|[0-9]{1,2})%$' or If supplied value is Integer must be greater than or equal to 0"
 	// +optional
 	MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"`
 
@@ -88,7 +88,7 @@ type PlacementDisruptionBudgetSpec struct {
 	// of them can be set at a time.
 	//
 	// +kubebuilder:validation:XIntOrString
-	// +kubebuilder:validation:XValidation:rule="type(self) == string ? self.matches('^(100|[0-9]{1,2}%)$') : self >= 0",message="If supplied value is String should match regex '^(100|[0-9]{1,2}%)$' or If supplied value is Integer must be greater than or equal to 0"
+	// +kubebuilder:validation:XValidation:rule="type(self) == string ? self.matches('^(100|[0-9]{1,2})%$') : self >= 0",message="If supplied value is String should match regex '^(100|[0-9]{1,2})%$' or If supplied value is Integer must be greater than or equal to 0"
 	// +optional
 	MinAvailable *intstr.IntOrString `json:"minAvailable,omitempty"`
 }

charts/hub-agent/templates/deployment.yaml

@@ -32,6 +32,7 @@ spec:
             - --enable-v1beta1-apis={{ .Values.enableV1Beta1APIs }}
             - --enable-cluster-inventory-apis={{ .Values.enableClusterInventoryAPI }}
             - --enable-staged-update-run-apis={{ .Values.enableStagedUpdateRunAPIs }}
+            - --enable-eviction-apis={{ .Values.enableEvictionAPIs}}
             - --max-concurrent-cluster-placement={{ .Values.MaxConcurrentClusterPlacement }}
             - --concurrent-resource-change-syncs={{ .Values.ConcurrentResourceChangeSyncs }}
             - --log_file_max_size={{ .Values.logFileMaxSize }}

charts/hub-agent/values.yaml

@@ -37,6 +37,7 @@ enableV1Alpha1APIs: false
 enableV1Beta1APIs: true
 enableClusterInventoryAPI: true
 enableStagedUpdateRunAPIs: true
+enableEvictionAPIs: true
 
 hubAPIQPS: 250
 hubAPIBurst: 1000

cmd/hubagent/options/options.go

@@ -85,6 +85,8 @@ type Options struct {
 	ForceDeleteWaitTime metav1.Duration
 	// EnableStagedUpdateRunAPIs enables the agents to watch the clusterStagedUpdateRun CRs.
 	EnableStagedUpdateRunAPIs bool
+	// EnableEvictionAPIs enables to agents to watch the eviction and placement disruption budget CRs.
+	EnableEvictionAPIs bool
 }
 
 // NewOptions builds an empty options.
@@ -144,6 +146,7 @@ func (o *Options) AddFlags(flags *flag.FlagSet) {
 	flags.BoolVar(&o.EnableClusterInventoryAPIs, "enable-cluster-inventory-apis", false, "If set, the agents will watch for the ClusterInventory APIs.")
 	flags.DurationVar(&o.ForceDeleteWaitTime.Duration, "force-delete-wait-time", 15*time.Minute, "The duration the hub agent waits before force deleting a member cluster.")
 	flags.BoolVar(&o.EnableStagedUpdateRunAPIs, "enable-staged-update-run-apis", false, "If set, the agents will watch for the ClusterStagedUpdateRun APIs.")
+	flags.BoolVar(&o.EnableEvictionAPIs, "enable-eviction-apis", false, "If set, the agents will watch for the Eviction and PlacementDisruptionBudget APIs.")
 
 	o.RateLimiterOpts.AddFlags(flags)
 }

cmd/hubagent/workload/setup.go

@@ -95,6 +95,11 @@ var (
 	clusterInventoryGVKs = []schema.GroupVersionKind{
 		clusterinventory.GroupVersion.WithKind("ClusterProfile"),
 	}
+
+	evictionGVKs = []schema.GroupVersionKind{
+		placementv1alpha1.GroupVersion.WithKind(placementv1alpha1.ClusterResourcePlacementEvictionKind),
+		placementv1alpha1.GroupVersion.WithKind(placementv1alpha1.ClusterResourcePlacementDisruptionBudgetKind),
+	}
 )
 
 // SetupControllers set up the customized controllers we developed
@@ -215,19 +220,27 @@ func SetupControllers(ctx context.Context, wg *sync.WaitGroup, mgr ctrl.Manager,
 			return err
 		}
 
-		klog.Info("Setting up cluster resource placement eviction controller")
-		if err := (&clusterresourceplacementeviction.Reconciler{
-			Client: mgr.GetClient(),
-		}).SetupWithManager(mgr); err != nil {
-			klog.ErrorS(err, "Unable to set up cluster resource placement eviction controller")
-			return err
+		if opts.EnableEvictionAPIs {
+			for _, gvk := range evictionGVKs {
+				if err = utils.CheckCRDInstalled(discoverClient, gvk); err != nil {
+					klog.ErrorS(err, "Unable to find the required CRD", "GVK", gvk)
+					return err
+				}
+			}
+			klog.Info("Setting up cluster resource placement eviction controller")
+			if err := (&clusterresourceplacementeviction.Reconciler{
+				Client: mgr.GetClient(),
+			}).SetupWithManager(mgr); err != nil {
+				klog.ErrorS(err, "Unable to set up cluster resource placement eviction controller")
+				return err
+			}
 		}
 
 		// Set up a controller to do staged update run, rolling out resources to clusters in a stage by stage manner.
 		if opts.EnableStagedUpdateRunAPIs {
 			for _, gvk := range clusterStagedUpdateRunGVKs {
 				if err = utils.CheckCRDInstalled(discoverClient, gvk); err != nil {
-					klog.ErrorS(err, "unable to find the required CRD", "GVK", gvk)
+					klog.ErrorS(err, "Unable to find the required CRD", "GVK", gvk)
 					return err
 				}
 			}
@@ -236,7 +249,7 @@ func SetupControllers(ctx context.Context, wg *sync.WaitGroup, mgr ctrl.Manager,
 				Client:          mgr.GetClient(),
 				InformerManager: dynamicInformerManager,
 			}).SetupWithManager(mgr); err != nil {
-				klog.ErrorS(err, "unable to set up clusterStagedUpdateRun controller")
+				klog.ErrorS(err, "Unable to set up clusterStagedUpdateRun controller")
 				return err
 			}
 		}

config/crd/bases/placement.kubernetes-fleet.io_clusterresourceplacementdisruptionbudgets.yaml

@@ -70,8 +70,7 @@ spec:
                   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,
@@ -88,10 +87,10 @@ spec:
                   of them can be set at a time.
                 x-kubernetes-int-or-string: true
                 x-kubernetes-validations:
-                - message: If supplied value is String should match regex '^(100|[0-9]{1,2}%)$'
+                - message: If supplied value is String should match regex '^(100|[0-9]{1,2})%$'
                     or If supplied value is Integer must be greater than or equal
                     to 0
-                  rule: 'type(self) == string ? self.matches(''^(100|[0-9]{1,2}%)$'')
+                  rule: 'type(self) == string ? self.matches(''^(100|[0-9]{1,2})%$'')
                     : self >= 0'
               minAvailable:
                 anyOf:
@@ -110,8 +109,7 @@ spec:
                   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,
@@ -128,10 +126,10 @@ spec:
                   of them can be set at a time.
                 x-kubernetes-int-or-string: true
                 x-kubernetes-validations:
-                - message: If supplied value is String should match regex '^(100|[0-9]{1,2}%)$'
+                - message: If supplied value is String should match regex '^(100|[0-9]{1,2})%$'
                     or If supplied value is Integer must be greater than or equal
                     to 0
-                  rule: 'type(self) == string ? self.matches(''^(100|[0-9]{1,2}%)$'')
+                  rule: 'type(self) == string ? self.matches(''^(100|[0-9]{1,2})%$'')
                     : self >= 0'
             type: object
             x-kubernetes-validations:

config/crd/bases/placement.kubernetes-fleet.io_clusterresourceplacementevictions.yaml

@@ -49,6 +49,11 @@ spec:
           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.

test/apis/placement/v1alpha1/api_validation_integration_test.go

@@ -37,7 +37,31 @@ var _ = Describe("Test placement v1alpha1 API validation", func() {
 			Expect(hubClient.Create(ctx, &crpdb)).Should(Succeed())
 		})
 
-		It("should allow creation of ClusterPlacementDisruptionBudget with valid maxUnavailable - string", func() {
+		It("should allow creation of ClusterPlacementDisruptionBudget with valid maxUnavailable less than 10% specified as one digit - string", func() {
+			crpdb := placementv1alpha1.ClusterResourcePlacementDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: fmt.Sprintf(crpdbNameTemplate, GinkgoParallelProcess()),
+				},
+				Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{
+					MaxUnavailable: &intstr.IntOrString{Type: intstr.String, StrVal: "2%"},
+				},
+			}
+			Expect(hubClient.Create(ctx, &crpdb)).Should(Succeed())
+		})
+
+		It("should allow creation of ClusterPlacementDisruptionBudget with valid maxUnavailable less than 10% specified as two digits - string", func() {
+			crpdb := placementv1alpha1.ClusterResourcePlacementDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: fmt.Sprintf(crpdbNameTemplate, GinkgoParallelProcess()),
+				},
+				Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{
+					MaxUnavailable: &intstr.IntOrString{Type: intstr.String, StrVal: "02%"},
+				},
+			}
+			Expect(hubClient.Create(ctx, &crpdb)).Should(Succeed())
+		})
+
+		It("should allow creation of ClusterPlacementDisruptionBudget with valid maxUnavailable greater than or equal to 10% and less than 100% - string", func() {
 			crpdb := placementv1alpha1.ClusterResourcePlacementDisruptionBudget{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: fmt.Sprintf(crpdbNameTemplate, GinkgoParallelProcess()),
@@ -49,6 +73,18 @@ var _ = Describe("Test placement v1alpha1 API validation", func() {
 			Expect(hubClient.Create(ctx, &crpdb)).Should(Succeed())
 		})
 
+		It("should allow creation of ClusterPlacementDisruptionBudget with valid maxUnavailable equal to 100% - string", func() {
+			crpdb := placementv1alpha1.ClusterResourcePlacementDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: fmt.Sprintf(crpdbNameTemplate, GinkgoParallelProcess()),
+				},
+				Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{
+					MaxUnavailable: &intstr.IntOrString{Type: intstr.String, StrVal: "100%"},
+				},
+			}
+			Expect(hubClient.Create(ctx, &crpdb)).Should(Succeed())
+		})
+
 		It("should allow creation of ClusterPlacementDisruptionBudget with valid minAvailable - int", func() {
 			crpdb := placementv1alpha1.ClusterResourcePlacementDisruptionBudget{
 				ObjectMeta: metav1.ObjectMeta{
@@ -61,7 +97,31 @@ var _ = Describe("Test placement v1alpha1 API validation", func() {
 			Expect(hubClient.Create(ctx, &crpdb)).Should(Succeed())
 		})
 
-		It("should allow creation of ClusterPlacementDisruptionBudget with valid minAvailable - string", func() {
+		It("should allow creation of ClusterPlacementDisruptionBudget with valid minAvailable less than 10% specified as one digit - string", func() {
+			crpdb := placementv1alpha1.ClusterResourcePlacementDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: fmt.Sprintf(crpdbNameTemplate, GinkgoParallelProcess()),
+				},
+				Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{
+					MinAvailable: &intstr.IntOrString{Type: intstr.String, StrVal: "5%"},
+				},
+			}
+			Expect(hubClient.Create(ctx, &crpdb)).Should(Succeed())
+		})
+
+		It("should allow creation of ClusterPlacementDisruptionBudget with valid minAvailable less than 10% specified as two digits - string", func() {
+			crpdb := placementv1alpha1.ClusterResourcePlacementDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: fmt.Sprintf(crpdbNameTemplate, GinkgoParallelProcess()),
+				},
+				Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{
+					MinAvailable: &intstr.IntOrString{Type: intstr.String, StrVal: "05%"},
+				},
+			}
+			Expect(hubClient.Create(ctx, &crpdb)).Should(Succeed())
+		})
+
+		It("should allow creation of ClusterPlacementDisruptionBudget with valid minAvailable greater than or equal to 10% and less than 100% - string", func() {
 			crpdb := placementv1alpha1.ClusterResourcePlacementDisruptionBudget{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: fmt.Sprintf(crpdbNameTemplate, GinkgoParallelProcess()),
@@ -73,6 +133,18 @@ var _ = Describe("Test placement v1alpha1 API validation", func() {
 			Expect(hubClient.Create(ctx, &crpdb)).Should(Succeed())
 		})
 
+		It("should allow creation of ClusterPlacementDisruptionBudget with valid minAvailable equal to 100% - string", func() {
+			crpdb := placementv1alpha1.ClusterResourcePlacementDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: fmt.Sprintf(crpdbNameTemplate, GinkgoParallelProcess()),
+				},
+				Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{
+					MinAvailable: &intstr.IntOrString{Type: intstr.String, StrVal: "100%"},
+				},
+			}
+			Expect(hubClient.Create(ctx, &crpdb)).Should(Succeed())
+		})
+
 		AfterEach(func() {
 			crpdb := placementv1alpha1.ClusterResourcePlacementDisruptionBudget{
 				ObjectMeta: metav1.ObjectMeta{