Apply provider manifests with SSA

pkg/controllers/capiinstaller/capi_installer_controller.go

@@ -34,21 +34,18 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
-	"k8s.io/utils/clock"
 
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	configv1 "github.com/openshift/api/config/v1"
 	"github.com/openshift/cluster-capi-operator/pkg/controllers"
 	"github.com/openshift/cluster-capi-operator/pkg/operatorstatus"
-	"github.com/openshift/library-go/pkg/operator/events"
-	"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
-	"github.com/openshift/library-go/pkg/operator/resource/resourcemerge"
 
 	"github.com/klauspost/compress/zstd"
 )
@@ -75,7 +72,6 @@ const (
 
 var (
 	errEmptyProviderConfigMap = errors.New("provider configmap has no components data")
-	errResourceNotFound       = errors.New("resource not found")
 )
 
 // CapiInstallerController reconciles a ClusterOperator object.
@@ -177,55 +173,19 @@ func (r *CapiInstallerController) reconcile(ctx context.Context, log logr.Logger
 // applyProviderComponents applies the provider components to the cluster.
 // It does so by differentiating between static components and dynamic components (i.e. Deployments).
 func (r *CapiInstallerController) applyProviderComponents(ctx context.Context, components []string) error {
-	componentsFilenames, componentsAssets, deploymentsFilenames, deploymentsAssets, err := getProviderComponents(r.Scheme, components)
+	providerObjects, err := getProviderObjects(r.Scheme, components)
 	if err != nil {
 		return fmt.Errorf("error getting provider components: %w", err)
 	}
 
-	// Perform a Direct apply of the static components.
-	res := resourceapply.ApplyDirectly(
-		ctx,
-		resourceapply.NewKubeClientHolder(r.ApplyClient).WithAPIExtensionsClient(r.APIExtensionsClient),
-		events.NewInMemoryRecorder("cluster-capi-operator-capi-installer-apply-client", clock.RealClock{}),
-		resourceapply.NewResourceCache(),
-		assetFn(componentsAssets),
-		componentsFilenames...,
-	)
-
-	// For each of the Deployment components perform a Deployment-specific apply.
-	for _, d := range deploymentsFilenames {
-		deploymentManifest, ok := deploymentsAssets[d]
-		if !ok {
-			panic("error finding deployment manifest")
-		}
-
-		obj, err := yamlToRuntimeObject(r.Scheme, deploymentManifest)
-		if err != nil {
-			return fmt.Errorf("error parsing CAPI provider deployment manifets %q: %w", d, err)
-		}
-
-		// TODO: Deployments State/Conditions should influence the overall ClusterOperator Status.
-		deployment, ok := obj.(*appsv1.Deployment)
-		if !ok {
-			return fmt.Errorf("error casting object to Deployment: %w", err)
-		}
-
-		if _, _, err := resourceapply.ApplyDeployment(
-			ctx,
-			r.ApplyClient.AppsV1(),
-			events.NewInMemoryRecorder("cluster-capi-operator-capi-installer-apply-client", clock.RealClock{}),
-			deployment,
-			resourcemerge.ExpectedDeploymentGeneration(deployment, nil),
-		); err != nil {
-			return fmt.Errorf("error applying CAPI provider deployment %q: %w", deployment.Name, err)
-		}
-	}
-
 	var errs error
 
-	for i, r := range res {
-		if r.Error != nil {
-			errs = errors.Join(errs, fmt.Errorf("error applying CAPI provider component %q at position %d: %w", r.File, i, r.Error))
+	for i, providerObject := range providerObjects {
+		err := r.Patch(ctx, providerObject, client.Apply, client.ForceOwnership, client.FieldOwner("cluster-capi-operator.openshift.io/installer"))
+		if err != nil {
+			gvk := providerObject.GroupVersionKind()
+			name := strings.Join([]string{gvk.Group, gvk.Version, gvk.Kind, providerObject.GetName()}, "/")
+			errs = errors.Join(errs, fmt.Errorf("error applying CAPI provider component %q at position %d: %w", name, i, err))
 		}
 	}
 
@@ -234,38 +194,20 @@ func (r *CapiInstallerController) applyProviderComponents(ctx context.Context, c
 
 // getProviderComponents parses the provided list of components into a map of filenames and assets.
 // Deployments are handled separately so are returned in a separate map.
-func getProviderComponents(scheme *runtime.Scheme, components []string) ([]string, map[string]string, []string, map[string]string, error) {
-	componentsFilenames := []string{}
-	componentsAssets := make(map[string]string)
-
-	deploymentsFilenames := []string{}
-	deploymentsAssets := make(map[string]string)
+func getProviderObjects(scheme *runtime.Scheme, components []string) ([]*unstructured.Unstructured, error) {
+	objects := make([]*unstructured.Unstructured, len(components))
 
 	for i, m := range components {
 		// Parse the YAML manifests into unstructure objects.
 		u, err := yamlToUnstructured(scheme, m)
 		if err != nil {
-			return nil, nil, nil, nil, fmt.Errorf("error parsing provider component at position %d to unstructured: %w", i, err)
+			return nil, fmt.Errorf("error parsing provider component at position %d to unstructured: %w", i, err)
 		}
 
-		name := fmt.Sprintf("%s/%s/%s - %s",
-			u.GroupVersionKind().Group,
-			u.GroupVersionKind().Version,
-			u.GroupVersionKind().Kind,
-			getResourceName(u.GetNamespace(), u.GetName()),
-		)
-
-		// Divide manifests into static vs deployment components.
-		if u.GroupVersionKind().Kind == "Deployment" {
-			deploymentsFilenames = append(deploymentsFilenames, name)
-			deploymentsAssets[name] = m
-		} else {
-			componentsFilenames = append(componentsFilenames, name)
-			componentsAssets[name] = m
-		}
+		objects[i] = u
 	}
 
-	return componentsFilenames, componentsAssets, deploymentsFilenames, deploymentsAssets, nil
+	return objects, nil
 }
 
 // setAvailableCondition sets the ClusterOperator status condition to Available.
@@ -327,14 +269,20 @@ func (r *CapiInstallerController) SetupWithManager(mgr ctrl.Manager) error {
 			&corev1.ConfigMap{},
 			handler.EnqueueRequestsFromMapFunc(toClusterOperator),
 			builder.WithPredicates(configMapPredicate(r.ManagedNamespace, r.Platform)),
+		).
+		// We reconcile all Deployment changes because we intend to reflect the
+		// status of any created Deployment in the ClusterOperator status.
+		Watches(
+			&appsv1.Deployment{},
+			handler.EnqueueRequestsFromMapFunc(toClusterOperator),
+			builder.WithPredicates(ownedPlatformLabelPredicate(r.ManagedNamespace, r.Platform)),
 		)
 
 	// All of the following watches share the ownedPlatformLabelPredicate.
 	watches := []struct {
 		obj       client.Object
 		namespace string
 	}{
-		{&appsv1.Deployment{}, r.ManagedNamespace},
 		{&admissionregistrationv1.ValidatingWebhookConfiguration{}, notNamespaced},
 		{&admissionregistrationv1.MutatingWebhookConfiguration{}, notNamespaced},
 		{&admissionregistrationv1.ValidatingAdmissionPolicy{}, notNamespaced},
@@ -352,7 +300,16 @@ func (r *CapiInstallerController) SetupWithManager(mgr ctrl.Manager) error {
 		build = build.Watches(
 			w.obj,
 			handler.EnqueueRequestsFromMapFunc(toClusterOperator),
-			builder.WithPredicates(ownedPlatformLabelPredicate(w.namespace, r.Platform)),
+			builder.WithPredicates(
+				ownedPlatformLabelPredicate(w.namespace, r.Platform),
+
+				// We're only interested in changes which affect an object's spec
+				predicate.Or(
+					predicate.AnnotationChangedPredicate{},
+					predicate.LabelChangedPredicate{},
+					predicate.GenerationChangedPredicate{},
+				),
+			),
 		)
 	}
 
@@ -454,28 +411,6 @@ func platformToInfraProviderComponentName(platform configv1.PlatformType) string
 	return strings.ToLower(fmt.Sprintf("infrastructure-%s", platform))
 }
 
-// getResourceName returns a "namespace/name" string or a "name" string if namespace is empty.
-func getResourceName(namespace, name string) string {
-	resourceName := fmt.Sprintf("%s/%s", namespace, name)
-	if namespace == "" {
-		resourceName = name
-	}
-
-	return resourceName
-}
-
-// assetsFn is a resourceapply.AssetFunc.
-func assetFn(assetsMap map[string]string) resourceapply.AssetFunc {
-	return func(name string) ([]byte, error) {
-		o, ok := assetsMap[name]
-		if !ok {
-			return nil, fmt.Errorf("error fetching resource %s: %w", name, errResourceNotFound)
-		}
-
-		return []byte(o), nil
-	}
-}
-
 // yamlToRuntimeObject parses a YAML manifest into a runtime.Object.
 func yamlToRuntimeObject(sch *runtime.Scheme, m string) (runtime.Object, error) {
 	decode := serializer.NewCodecFactory(sch).UniversalDeserializer().Decode

pkg/controllers/capiinstaller/watch_predicates.go

@@ -32,11 +32,12 @@ func clusterOperatorPredicates() predicate.Funcs {
 		return ok && clusterOperator.GetName() == clusterOperatorName
 	}
 
+	// We only want to be reconciled on creation of the cluster operator,
+	// because we wait for it before reconciling. The Create event also fires
+	// when the manager is started, so this will additionally ensure we are
+	// called at least once at startup.
 	return predicate.Funcs{
-		CreateFunc:  func(e event.CreateEvent) bool { return isClusterOperator(e.Object) },
-		UpdateFunc:  func(e event.UpdateEvent) bool { return isClusterOperator(e.ObjectNew) },
-		GenericFunc: func(e event.GenericEvent) bool { return isClusterOperator(e.Object) },
-		DeleteFunc:  func(e event.DeleteEvent) bool { return isClusterOperator(e.Object) },
+		CreateFunc: func(e event.CreateEvent) bool { return isClusterOperator(e.Object) },
 	}
 }