Add support for customizing additional provider deployments

Author: willie-yao

Diff for: api/v1alpha1/provider_conversion.go

@@ -46,6 +46,7 @@ func (src *BootstrapProvider) ConvertTo(dstRaw conversion.Hub) error {
 	}
 
 	dst.Spec.ManifestPatches = restored.Spec.ManifestPatches
+	dst.Spec.AdditionalDeployments = restored.Spec.AdditionalDeployments
 
 	return nil
 }
@@ -107,6 +108,7 @@ func (src *ControlPlaneProvider) ConvertTo(dstRaw conversion.Hub) error {
 	}
 
 	dst.Spec.ManifestPatches = restored.Spec.ManifestPatches
+	dst.Spec.AdditionalDeployments = restored.Spec.AdditionalDeployments
 
 	return nil
 }
@@ -168,6 +170,7 @@ func (src *CoreProvider) ConvertTo(dstRaw conversion.Hub) error {
 	}
 
 	dst.Spec.ManifestPatches = restored.Spec.ManifestPatches
+	dst.Spec.AdditionalDeployments = restored.Spec.AdditionalDeployments
 
 	return nil
 }
@@ -229,6 +232,7 @@ func (src *InfrastructureProvider) ConvertTo(dstRaw conversion.Hub) error {
 	}
 
 	dst.Spec.ManifestPatches = restored.Spec.ManifestPatches
+	dst.Spec.AdditionalDeployments = restored.Spec.AdditionalDeployments
 
 	return nil
 }

Diff for: api/v1alpha1/zz_generated.conversion.go

@@ -73,6 +73,24 @@ type ProviderSpec struct {
 	// This should be an inline yaml blob-string https://datatracker.ietf.org/doc/html/rfc7396
 	// +optional
 	ManifestPatches []string `json:"manifestPatches,omitempty"`
+
+	// AdditionalDeployments is a map of additional deployments that the provider
+	// should manage. The key is the name of the deployment and the value is the
+	// DeploymentSpec.
+	// +optional
+	AdditionalDeployments map[string]AdditionalDeployments `json:"additionalDeployments,omitempty"`
+}
+
+// AdditionalDeployments defines the properties that can be enabled on the controller
+// manager and deployment for the provider if the provider is managing additional deployments.
+type AdditionalDeployments struct {
+	// Manager defines the properties that can be enabled on the controller manager for the additional provider deployment.
+	// +optional
+	Manager *ManagerSpec `json:"manager,omitempty"`
+
+	// Deployment defines the properties that can be enabled on the deployment for the additional provider deployment.
+	// +optional
+	Deployment *DeploymentSpec `json:"deployment,omitempty"`
 }
 
 // ConfigmapReference contains enough information to locate the configmap.

Diff for: api/v1alpha2/provider_types.go

@@ -1,3 +1,13 @@
+{{- define "recursivePrinter" }}
+{{- range $key, $value := . }}
+{{- if kindIs "map" $value }}
+  {{ $key }}:
+  {{- include "recursivePrinter" $value | indent 2 }}
+{{- else }}
+  {{ $key }}: {{ $value }}
+{{- end }}
+{{- end }}
+{{- end }}
 # Infrastructure providers
 {{- if .Values.infrastructure }}
 {{- $infrastructures := split ";" .Values.infrastructure }}
@@ -40,7 +50,7 @@ metadata:
     "helm.sh/hook": "post-install"
     "helm.sh/hook-weight": "2"
     "argocd.argoproj.io/sync-wave": "2"
-{{- if or $infrastructureVersion $.Values.configSecret.name $.Values.manager }}
+{{- if or $infrastructureVersion $.Values.configSecret.name $.Values.manager $.Values.additionalDeployments }}
 spec:
 {{- end }}
 {{- if $infrastructureVersion }}
@@ -66,5 +76,9 @@ spec:
     namespace: {{ $.Values.configSecret.namespace }}
     {{- end }}
 {{- end }}
+{{- if $.Values.additionalDeployments }}
+  additionalDeployments:
+  {{- include "recursivePrinter" $.Values.additionalDeployments | indent 2 }}
+{{- end }}
 {{- end }}
 {{- end }}

Diff for: api/v1alpha2/zz_generated.deepcopy.go

@@ -73,25 +73,41 @@ func customizeObjectsFn(provider operatorv1.GenericProvider) func(objs []unstruc
 					}))
 			}
 
+			//nolint:nestif
 			if o.GetKind() == deploymentKind {
+				d := &appsv1.Deployment{}
+				if err := scheme.Scheme.Convert(&o, d, nil); err != nil {
+					return nil, err
+				}
+
+				providerDeployment := provider.GetSpec().Deployment
+				providerManager := provider.GetSpec().Manager
+
+				// If there are multiple deployments, check if we specify customizations for those deployments.
 				// We need to skip the deployment customization if there are several deployments available
-				// and the deployment name doesn't follow "ca*-controller-manager" pattern.
-				// Currently it is applicable only for CAPZ manifests, which contain 2 deployments:
-				// capz-controller-manager and azureserviceoperator-controller-manager
+				// and the deployment name doesn't follow "ca*-controller-manager" pattern, or the provider
+				// doesn't specify customizations for the deployment.
 				// This is a temporary fix until CAPI provides a contract to distinguish provider deployments.
 				// TODO: replace this check and just compare labels when CAPI provides the contract for that.
 				if isMultipleDeployments && !isProviderManagerDeploymentName(o.GetName()) {
-					results = append(results, o)
-
-					continue
+					additionalDeployments := provider.GetSpec().AdditionalDeployments
+					// Skip the deployment if there are no additional deployments specified.
+					if additionalDeployments == nil {
+						results = append(results, o)
+						continue
+					}
+
+					additionalProviderCustomization, ok := additionalDeployments[o.GetName()]
+					if !ok {
+						results = append(results, o)
+						continue
+					}
+
+					providerDeployment = additionalProviderCustomization.Deployment
+					providerManager = additionalProviderCustomization.Manager
 				}
 
-				d := &appsv1.Deployment{}
-				if err := scheme.Scheme.Convert(&o, d, nil); err != nil {
-					return nil, err
-				}
-
-				if err := customizeDeployment(provider.GetSpec(), d); err != nil {
+				if err := customizeDeployment(providerDeployment, providerManager, d); err != nil {
 					return nil, err
 				}
 
@@ -108,28 +124,26 @@ func customizeObjectsFn(provider operatorv1.GenericProvider) func(objs []unstruc
 }
 
 // customizeDeployment customize provider deployment base on provider spec input.
-func customizeDeployment(pSpec operatorv1.ProviderSpec, d *appsv1.Deployment) error {
+func customizeDeployment(dSpec *operatorv1.DeploymentSpec, mSpec *operatorv1.ManagerSpec, d *appsv1.Deployment) error {
 	// Customize deployment spec first.
-	if pSpec.Deployment != nil {
-		customizeDeploymentSpec(pSpec, d)
+	if dSpec != nil {
+		customizeDeploymentSpec(*dSpec, d)
 	}
 
 	// Run the customizeManagerContainer after, so it overrides anything in the deploymentSpec.
-	if pSpec.Manager != nil {
+	if mSpec != nil {
 		container := findManagerContainer(&d.Spec)
 		if container == nil {
 			return fmt.Errorf("cannot find %q container in deployment %q", managerContainerName, d.Name)
 		}
 
-		customizeManagerContainer(pSpec.Manager, container)
+		customizeManagerContainer(mSpec, container)
 	}
 
 	return nil
 }
 
-func customizeDeploymentSpec(pSpec operatorv1.ProviderSpec, d *appsv1.Deployment) {
-	dSpec := pSpec.Deployment
-
+func customizeDeploymentSpec(dSpec operatorv1.DeploymentSpec, d *appsv1.Deployment) {
 	if dSpec.Replicas != nil {
 		d.Spec.Replicas = ptr.To(int32(*dSpec.Replicas))
 	}

Diff for: config/crd/bases/operator.cluster.x-k8s.io_addonproviders.yaml

@@ -557,10 +557,7 @@ func TestCustomizeDeployment(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			deployment := managerDepl.DeepCopy()
-			if err := customizeDeployment(operatorv1.ProviderSpec{
-				Deployment: tc.inputDeploymentSpec,
-				Manager:    tc.inputManagerSpec,
-			}, deployment); err != nil {
+			if err := customizeDeployment(tc.inputDeploymentSpec, tc.inputManagerSpec, deployment); err != nil {
 				t.Error(err)
 			}
 
@@ -575,6 +572,7 @@ func TestCustomizeMultipleDeployment(t *testing.T) {
 	tests := []struct {
 		name                     string
 		nonManagerDeploymentName string
+		shouldCustomize          bool
 	}{
 		{
 			name:                     "name without suffix and prefix",
@@ -588,6 +586,11 @@ func TestCustomizeMultipleDeployment(t *testing.T) {
 			name:                     "name with suffix",
 			nonManagerDeploymentName: "non-manager-controller-manager",
 		},
+		{
+			name:                     "name with azureserviceoperator controller-manager",
+			nonManagerDeploymentName: "azureserviceoperator-controller-manager",
+			shouldCustomize:          true,
+		},
 		{
 			name:                     "empty name",
 			nonManagerDeploymentName: "",
@@ -603,6 +606,17 @@ func TestCustomizeMultipleDeployment(t *testing.T) {
 				},
 				Spec: appsv1.DeploymentSpec{
 					Replicas: ptr.To(int32(3)),
+					Template: corev1.PodTemplateSpec{
+						Spec: corev1.PodSpec{
+							Containers: []corev1.Container{
+								{
+									Name:  "manager",
+									Image: "registry.k8s.io/a-manager:1.6.2",
+									Args:  []string{},
+								},
+							},
+						},
+					},
 				},
 			}
 
@@ -629,6 +643,23 @@ func TestCustomizeMultipleDeployment(t *testing.T) {
 						Deployment: &operatorv1.DeploymentSpec{
 							Replicas: ptr.To(10),
 						},
+						AdditionalDeployments: map[string]operatorv1.AdditionalDeployments{
+							"azureserviceoperator-controller-manager": {
+								Deployment: &operatorv1.DeploymentSpec{
+									Containers: []operatorv1.ContainerSpec{
+										{
+											Name: "manager",
+											Args: map[string]string{
+												"--crd-pattern": ".*",
+											},
+										},
+									},
+								},
+								Manager: &operatorv1.ManagerSpec{
+									Verbosity: 1,
+								},
+							},
+						},
 					},
 				},
 			}
@@ -657,8 +688,18 @@ func TestCustomizeMultipleDeployment(t *testing.T) {
 				t.Errorf("expected 10 replicas, got %d", *managerDepl.Spec.Replicas)
 			}
 
+			if tc.shouldCustomize {
+				// non-manager container should have been customized
+				container := findManagerContainer(&nonManagerDepl.Spec)
+				if container == nil {
+					t.Error("expected container to be found")
+				} else if container.Args != nil && container.Args[0] != "--crd-pattern=.*" {
+					t.Errorf("expected --crd-pattern=.*, got %s", container.Args[0])
+				}
+			}
+
 			// non-manager deployment should not have been customized
-			if *nonManagerDepl.Spec.Replicas != 3 {
+			if *nonManagerDepl.Spec.Replicas != 3 && !tc.shouldCustomize {
 				t.Errorf("expected 3 replicas, got %d", *nonManagerDepl.Spec.Replicas)
 			}
 		})