Skip to content

Commit

Permalink
Add configurable CRDPattern field for manager.
Browse files Browse the repository at this point in the history
  • Loading branch information
willie-yao committed Jun 3, 2024
1 parent fab98f2 commit 7c0f00d
Show file tree
Hide file tree
Showing 15 changed files with 169 additions and 13 deletions.
12 changes: 12 additions & 0 deletions api/v1alpha1/provider_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ func (src *BootstrapProvider) ConvertTo(dstRaw conversion.Hub) error {
}

dst.Spec.ManifestPatches = restored.Spec.ManifestPatches
if restored.Spec.Manager != nil {
dst.Spec.Manager.CRDPattern = restored.Spec.Manager.CRDPattern
}

return nil
}
Expand Down Expand Up @@ -107,6 +110,9 @@ func (src *ControlPlaneProvider) ConvertTo(dstRaw conversion.Hub) error {
}

dst.Spec.ManifestPatches = restored.Spec.ManifestPatches
if restored.Spec.Manager != nil {
dst.Spec.Manager.CRDPattern = restored.Spec.Manager.CRDPattern
}

return nil
}
Expand Down Expand Up @@ -168,6 +174,9 @@ func (src *CoreProvider) ConvertTo(dstRaw conversion.Hub) error {
}

dst.Spec.ManifestPatches = restored.Spec.ManifestPatches
if restored.Spec.Manager != nil {
dst.Spec.Manager.CRDPattern = restored.Spec.Manager.CRDPattern
}

return nil
}
Expand Down Expand Up @@ -229,6 +238,9 @@ func (src *InfrastructureProvider) ConvertTo(dstRaw conversion.Hub) error {
}

dst.Spec.ManifestPatches = restored.Spec.ManifestPatches
if restored.Spec.Manager != nil {
dst.Spec.Manager.CRDPattern = restored.Spec.Manager.CRDPattern
}

return nil
}
Expand Down
1 change: 1 addition & 0 deletions api/v1alpha1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions api/v1alpha2/provider_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ type ManagerSpec struct {
// in as container args to the provider's controller manager.
// Controller Manager flag is --feature-gates.
FeatureGates map[string]bool `json:"featureGates,omitempty"`

// CRDPattern is a regular expression that matches the CRDs that the provider
// is expected to manage.
// +optional
CRDPattern *string `json:"crdPattern,omitempty"`
}

// DeploymentSpec defines the properties that can be enabled on the Deployment for the provider.
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -1369,6 +1369,11 @@ spec:
description: RecoverPanic indicates if panics should be recovered.
type: boolean
type: object
crdPattern:
description: |-
CRDPattern is a regular expression that matches the CRDs that the provider
is expected to manage.
type: string
featureGates:
additionalProperties:
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2954,6 +2954,11 @@ spec:
description: RecoverPanic indicates if panics should be recovered.
type: boolean
type: object
crdPattern:
description: |-
CRDPattern is a regular expression that matches the CRDs that the provider
is expected to manage.
type: string
featureGates:
additionalProperties:
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2956,6 +2956,11 @@ spec:
description: RecoverPanic indicates if panics should be recovered.
type: boolean
type: object
crdPattern:
description: |-
CRDPattern is a regular expression that matches the CRDs that the provider
is expected to manage.
type: string
featureGates:
additionalProperties:
type: boolean
Expand Down
5 changes: 5 additions & 0 deletions config/crd/bases/operator.cluster.x-k8s.io_coreproviders.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2954,6 +2954,11 @@ spec:
description: RecoverPanic indicates if panics should be recovered.
type: boolean
type: object
crdPattern:
description: |-
CRDPattern is a regular expression that matches the CRDs that the provider
is expected to manage.
type: string
featureGates:
additionalProperties:
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2956,6 +2956,11 @@ spec:
description: RecoverPanic indicates if panics should be recovered.
type: boolean
type: object
crdPattern:
description: |-
CRDPattern is a regular expression that matches the CRDs that the provider
is expected to manage.
type: string
featureGates:
additionalProperties:
type: boolean
Expand Down
5 changes: 5 additions & 0 deletions config/crd/bases/operator.cluster.x-k8s.io_ipamproviders.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1369,6 +1369,11 @@ spec:
description: RecoverPanic indicates if panics should be recovered.
type: boolean
type: object
crdPattern:
description: |-
CRDPattern is a regular expression that matches the CRDs that the provider
is expected to manage.
type: string
featureGates:
additionalProperties:
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1371,6 +1371,11 @@ spec:
description: RecoverPanic indicates if panics should be recovered.
type: boolean
type: object
crdPattern:
description: |-
CRDPattern is a regular expression that matches the CRDs that the provider
is expected to manage.
type: string
featureGates:
additionalProperties:
type: boolean
Expand Down
3 changes: 3 additions & 0 deletions hack/charts/cluster-api-operator/templates/infra.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ spec:
{{- end }}
{{- end }}
{{- end }}
{{- if $.Values.manager.crdPattern }}
crdPattern: {{ $.Values.manager.crdPattern }}
{{- end }}
{{- end }}
{{- if $.Values.configSecret.name }}
configSecret:
Expand Down
54 changes: 42 additions & 12 deletions internal/controller/component_customizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,26 +73,34 @@ 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
}

// 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
// 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
}

d := &appsv1.Deployment{}
if err := scheme.Scheme.Convert(&o, d, nil); err != nil {
return nil, err
}

if err := customizeDeployment(provider.GetSpec(), d); err != nil {
return nil, err
// If the deployment is azureserviceoperator-controller-manager, customize the deployment
// to support configurable CRD Pattern. This is to enable installing additional ASO CRDs.
if strings.HasPrefix(o.GetName(), "azureserviceoperator") {
if err := customizeASODeployment(provider.GetSpec(), d); err != nil {
return nil, err
}
} else {
results = append(results, o)
continue
}
} else {
if err := customizeDeployment(provider.GetSpec(), d); err != nil {
return nil, err
}
}

if err := scheme.Scheme.Convert(d, &o, nil); err != nil {
Expand Down Expand Up @@ -127,6 +135,21 @@ func customizeDeployment(pSpec operatorv1.ProviderSpec, d *appsv1.Deployment) er
return nil
}

// customizeASODeployment customize ASO provider deployment base on provider spec input.
func customizeASODeployment(pSpec operatorv1.ProviderSpec, d *appsv1.Deployment) error {
// Run the customizeManagerContainer after, so it overrides anything in the deploymentSpec.
if pSpec.Manager != nil {
container := findManagerContainer(&d.Spec)
if container == nil {
return fmt.Errorf("cannot find %q container in deployment %q", managerContainerName, d.Name)
}

customizeASOManagerContainer(pSpec.Manager, container)
}

return nil
}

func customizeDeploymentSpec(pSpec operatorv1.ProviderSpec, d *appsv1.Deployment) {
dSpec := pSpec.Deployment

Expand Down Expand Up @@ -253,6 +276,13 @@ func customizeManagerContainer(mSpec *operatorv1.ManagerSpec, c *corev1.Containe
}
}

// customizeManagerContainer customize ASO manager container base on provider spec input.
func customizeASOManagerContainer(mSpec *operatorv1.ManagerSpec, c *corev1.Container) {
if mSpec.CRDPattern != nil {
c.Args = setArgs(c.Args, "--crd-pattern", *mSpec.CRDPattern)
}
}

// customizeContainer customize provider container base on provider spec input.
func customizeContainer(cSpec operatorv1.ContainerSpec, d *appsv1.Deployment) {
for j, c := range d.Spec.Template.Spec.Containers {
Expand Down
32 changes: 31 additions & 1 deletion internal/controller/component_customizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,7 @@ func TestCustomizeMultipleDeployment(t *testing.T) {
tests := []struct {
name string
nonManagerDeploymentName string
shouldCustomize bool
}{
{
name: "name without suffix and prefix",
Expand All @@ -588,6 +589,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: "",
Expand All @@ -603,6 +609,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{},
},
},
},
},
},
}

Expand All @@ -629,6 +646,9 @@ func TestCustomizeMultipleDeployment(t *testing.T) {
Deployment: &operatorv1.DeploymentSpec{
Replicas: ptr.To(10),
},
Manager: &operatorv1.ManagerSpec{
CRDPattern: ptr.To(".*"),
},
},
},
}
Expand Down Expand Up @@ -657,8 +677,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 != nil && *nonManagerDepl.Spec.Replicas != 3 && !tc.shouldCustomize {
t.Errorf("expected 3 replicas, got %d", *nonManagerDepl.Spec.Replicas)
}
})
Expand Down
Loading

0 comments on commit 7c0f00d

Please sign in to comment.