Merge pull request #2024 from jingyih/customize_ratelimit

feat: simplify the schema of NamespacedControllerReconciler

operator/config/crd/bases/customize.core.cnrm.cloud.google.com_namespacedcontrollerreconcilers.yaml

@@ -41,40 +41,22 @@ spec:
           spec:
             description: NamespacedControllerReconciler is the specification of NamespacedControllerReconciler.
             properties:
-              containers:
-                description: The list of containers whose reconciliation related parameters
-                  to be customized.
-                items:
-                  description: |-
-                    ContainerReconcilerSpec is the specification of the reconciliation related customization for
-                    a container of a config connector controller.
-                  properties:
-                    name:
-                      description: |-
-                        The name of the container.
-                        Required
-                      enum:
-                      - manager
-                      type: string
-                    rateLimit:
-                      description: |-
-                        RateLimit describes the token bucket rate limit to the kubernetes client.
-                        Please note this rate limit is shared among all the Config Connector resources' requests.
-                        If not specified, the default will be Token Bucket with qps 20, burst 30.
-                      properties:
-                        burst:
-                          description: The burst of the token bucket rate limit for
-                            all the requests to the kubernetes client.
-                          type: integer
-                        qps:
-                          description: The QPS of the token bucket rate limit for
-                            all the requests to the kubernetes client.
-                          type: integer
-                      type: object
-                  required:
-                  - name
-                  type: object
-                type: array
+              rateLimit:
+                description: |-
+                  RateLimit configures the token bucket rate limit to the kubernetes client used
+                  by the manager container of the config connector namespaced controller manager.
+                  Please note this rate limit is shared among all the Config Connector resources' requests.
+                  If not specified, the default will be Token Bucket with qps 20, burst 30.
+                properties:
+                  burst:
+                    description: The burst of the token bucket rate limit for all
+                      the requests to the kubernetes client.
+                    type: integer
+                  qps:
+                    description: The QPS of the token bucket rate limit for all the
+                      requests to the kubernetes client.
+                    type: integer
+                type: object
             type: object
           status:
             description: NamespacedControllerReconcilerStatus defines the observed

operator/config/samples/namespaced_controller_reconciler_customization_sample.yaml

@@ -18,8 +18,6 @@ metadata:
   name: cnrm-controller-manager # name should not contain the namespace ID suffix
   namespace: config-control
 spec:
-  containers:
-    - name: manager
-      rateLimit:
-        qps: 80
-        burst: 30
+  rateLimit:
+    qps: 80
+    burst: 30

operator/pkg/apis/core/customize/v1alpha1/controllerreconciler_types.go

@@ -35,19 +35,8 @@ type NamespacedControllerReconciler struct {
 
 // NamespacedControllerReconciler is the specification of NamespacedControllerReconciler.
 type NamespacedControllerReconcilerSpec struct {
-	// The list of containers whose reconciliation related parameters to be customized.
-	// +optional
-	Containers []ContainerReconcilerSpec `json:"containers,omitempty"`
-}
-
-// ContainerReconcilerSpec is the specification of the reconciliation related customization for
-// a container of a config connector controller.
-type ContainerReconcilerSpec struct {
-	// The name of the container.
-	// +kubebuilder:validation:Enum=manager
-	// Required
-	Name string `json:"name"`
-	// RateLimit describes the token bucket rate limit to the kubernetes client.
+	// RateLimit configures the token bucket rate limit to the kubernetes client used
+	// by the manager container of the config connector namespaced controller manager.
 	// Please note this rate limit is shared among all the Config Connector resources' requests.
 	// If not specified, the default will be Token Bucket with qps 20, burst 30.
 	// +optional
@@ -81,6 +70,10 @@ func (c *NamespacedControllerReconciler) SetCommonStatus(s addonv1alpha1.CommonS
 	c.Status.CommonStatus = s
 }
 
+var SupportedNamespacedControllers = []string{
+	"cnrm-controller-manager",
+}
+
 func init() {
 	SchemeBuilder.Register(&NamespacedControllerReconciler{}, &NamespacedControllerReconcilerList{})
 }

operator/pkg/controllers/configconnectorcontext/configconnectorcontext_controller.go

@@ -535,12 +535,7 @@ func (r *Reconciler) fetchAndApplyAllNamespacedControllerReconcilers(ctx context
 
 // applyNamespacedControllerReconciler applies customizations specified in NamespacedControllerReconciler CR.
 func (r *Reconciler) applyNamespacedControllerReconciler(ctx context.Context, cr *customizev1alpha1.NamespacedControllerReconciler, m *manifest.Objects) error {
-	if cr.Name != "cnrm-controller-manager" {
-		msg := fmt.Sprintf("controller reconiler customization for %s is not supported", cr.Name)
-		r.log.Info(msg)
-		return r.handleApplyNamespacedControllerReconcilerFailed(ctx, cr.Namespace, cr.Name, msg)
-	}
-	if err := controllers.ApplyContainerRateLimit(m, cr.Name, cr.Spec.Containers); err != nil {
+	if err := controllers.ApplyContainerRateLimit(m, cr.Name, cr.Spec.RateLimit); err != nil {
 		msg := fmt.Sprintf("failed to apply rate limit customization %s: %v", cr.Name, err)
 		return r.handleApplyNamespacedControllerReconcilerFailed(ctx, cr.Namespace, cr.Name, msg)
 	}

operator/pkg/controllers/utils.go

@@ -491,58 +491,54 @@ func validateContainerResourceCustomizationValues(r customizev1beta1.ResourceReq
 	return nil
 }
 
-func ApplyContainerRateLimit(m *manifest.Objects, targetName string, containers []customizev1alpha1.ContainerReconcilerSpec) error {
-	if containers == nil || len(containers) == 0 {
+func ApplyContainerRateLimit(m *manifest.Objects, targetControllerName string, ratelimit *customizev1alpha1.RateLimit) error {
+	if ratelimit == nil {
 		return nil
 	}
-	// TODO: check for duplicated containers
 
-	// rateLimitMap is a map of container name to its corresponding rate limit customization.
-	rateLimitMap := make(map[string]*customizev1alpha1.RateLimit)
-	for _, c := range containers {
-		rateLimitMap[c.Name] = c.RateLimit
-	}
-	targetGVK := schema.GroupVersionKind{
-		Group:   appsv1.SchemeGroupVersion.Group,
-		Version: appsv1.SchemeGroupVersion.Version,
-		Kind:    "StatefulSet",
+	var (
+		targetContainerName string
+		targetControllerGVK schema.GroupVersionKind
+	)
+	switch targetControllerName {
+	case "cnrm-controller-manager":
+		targetContainerName = "manager"
+		targetControllerGVK = schema.GroupVersionKind{
+			Group:   appsv1.SchemeGroupVersion.Group,
+			Version: appsv1.SchemeGroupVersion.Version,
+			Kind:    "StatefulSet",
+		}
+	default:
+		return fmt.Errorf("rate limit customization for %s is not supported. "+
+			"Supported controllers: %s",
+			targetControllerName, strings.Join(customizev1alpha1.SupportedNamespacedControllers, ", "))
 	}
+
 	for _, item := range m.Items {
-		if item.GroupVersionKind() != targetGVK {
+		if item.GroupVersionKind() != targetControllerGVK {
 			continue
 		}
-		if !strings.HasPrefix(item.GetName(), targetName) {
+		if !strings.HasPrefix(item.GetName(), targetControllerName) {
 			continue
 		}
-		if err := item.MutateContainers(customizeRateLimitFn(rateLimitMap)); err != nil {
+		if err := item.MutateContainers(customizeRateLimitFn(targetContainerName, ratelimit)); err != nil {
 			return err
 		}
 		break // we already found the matching controller, no need to keep looking.
 	}
-
-	// check if all container resource customizations are applied
-	var notApplied []string
-	for c := range rateLimitMap {
-		notApplied = append(notApplied, c)
-	}
-	if len(notApplied) > 0 {
-		return fmt.Errorf("the following containers were not found in the manifest: %s", strings.Join(notApplied, ", "))
-	}
 	return nil
 }
 
-func customizeRateLimitFn(rateLimitMap map[string]*customizev1alpha1.RateLimit) func(container map[string]interface{}) error {
+func customizeRateLimitFn(target string, rateLimit *customizev1alpha1.RateLimit) func(container map[string]interface{}) error {
 	return func(container map[string]interface{}) error {
 		name, _, err := unstructured.NestedString(container, "name")
 		if err != nil {
 			return fmt.Errorf("error reading container name: %w", err)
 		}
-		rl, found := rateLimitMap[name]
-		if !found {
+		if name != target {
 			return nil
 		}
-		delete(rateLimitMap, name)
-		return applyRateLimitToContainerArg(container, rl)
+		return applyRateLimitToContainerArg(container, rateLimit)
 	}
 }
 

operator/pkg/test/controller/customization.go

@@ -16,6 +16,7 @@ package controller
 
 import (
 	"fmt"
+	"strings"
 
 	customizev1alpha1 "github.com/GoogleCloudPlatform/k8s-config-connector/operator/pkg/apis/core/customize/v1alpha1"
 	customizev1beta1 "github.com/GoogleCloudPlatform/k8s-config-connector/operator/pkg/apis/core/customize/v1beta1"
@@ -133,14 +134,9 @@ var (
 			Namespace: "foo-ns",
 		},
 		Spec: customizev1alpha1.NamespacedControllerReconcilerSpec{
-			Containers: []customizev1alpha1.ContainerReconcilerSpec{
-				{
-					Name: "manager",
-					RateLimit: &customizev1alpha1.RateLimit{
-						Burst: 30,
-						QPS:   80,
-					},
-				},
+			RateLimit: &customizev1alpha1.RateLimit{
+				Burst: 30,
+				QPS:   80,
 			},
 		},
 	}
@@ -248,14 +244,9 @@ var (
 			Namespace: "does-not-match",
 		},
 		Spec: customizev1alpha1.NamespacedControllerReconcilerSpec{
-			Containers: []customizev1alpha1.ContainerReconcilerSpec{
-				{
-					Name: "manager",
-					RateLimit: &customizev1alpha1.RateLimit{
-						Burst: 30,
-						QPS:   80,
-					},
-				},
+			RateLimit: &customizev1alpha1.RateLimit{
+				Burst: 30,
+				QPS:   80,
 			},
 		},
 	}
@@ -310,18 +301,16 @@ var (
 			Namespace: "foo-ns",
 		},
 		Spec: customizev1alpha1.NamespacedControllerReconcilerSpec{
-			Containers: []customizev1alpha1.ContainerReconcilerSpec{
-				{
-					Name: "manager",
-					RateLimit: &customizev1alpha1.RateLimit{
-						Burst: 30,
-						QPS:   80,
-					},
-				},
+			RateLimit: &customizev1alpha1.RateLimit{
+				Burst: 30,
+				QPS:   80,
 			},
 		},
 	}
-	ErrUnsupportedController = fmt.Sprintf("controller reconiler customization for %s is not supported", unsupportedControllerName)
+	ErrUnsupportedController = fmt.Sprintf("failed to apply rate limit customization %s: "+
+		"rate limit customization for %s is not supported. "+
+		"Supported controllers: %s",
+		unsupportedControllerName, unsupportedControllerName, strings.Join(customizev1alpha1.SupportedNamespacedControllers, ", "))
 )
 
 var ClusterModeComponents = []string{`