Drop hypershift-operator util and consolidate with support

Following up to openshift#1592, this keeps refactoring dropping hypershif-operator/util in favour of support/ and consolidates legacy HO reconciliation for mapprover and autoscaler with PCO reconciliation.
gnufied · Aug 4, 2022 · 250d021 · 250d021
1 parent 0b95b70
commit 250d021
Show file tree

Hide file tree

Showing 16 changed files with 225 additions and 1,208 deletions.
diff --git a/control-plane-operator/controllers/hostedcontrolplane/autoscaler/reconcile.go b/control-plane-operator/controllers/hostedcontrolplane/autoscaler/reconcile.go
@@ -1,11 +1,15 @@
 package autoscaler
 
 import (
+	"context"
 	"fmt"
 
 	hyperv1 "github.com/openshift/hypershift/api/v1alpha1"
 	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/kas"
+	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests"
+	"github.com/openshift/hypershift/hypershift-operator/controllers/manifests/controlplaneoperator"
 	"github.com/openshift/hypershift/support/config"
+	"github.com/openshift/hypershift/support/upsert"
 	"github.com/openshift/hypershift/support/util"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -14,6 +18,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	k8sutilspointer "k8s.io/utils/pointer"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 func ReconcileAutoscalerDeployment(deployment *appsv1.Deployment, hcp *hyperv1.HostedControlPlane, sa *corev1.ServiceAccount, kubeConfigSecret *corev1.Secret, options hyperv1.ClusterAutoscaling, clusterAutoscalerImage, availabilityProberImage string, setDefaultSecurityContext bool) error {
@@ -218,3 +223,58 @@ func ReconcileAutoscalerRoleBinding(binding *rbacv1.RoleBinding, role *rbacv1.Ro
 
 	return nil
 }
+
+// ReconcileAutoscaler orchestrates reconciliation of autoscaler components.
+func ReconcileAutoscaler(ctx context.Context, c client.Client, hcp *hyperv1.HostedControlPlane, autoscalerImage, availabilityProberImage string, createOrUpdate upsert.CreateOrUpdateFN, setDefaultSecurityContext bool) error {
+	autoscalerRole := manifests.AutoscalerRole(hcp.Namespace)
+	_, err := createOrUpdate(ctx, c, autoscalerRole, func() error {
+		return ReconcileAutoscalerRole(autoscalerRole, config.OwnerRefFrom(hcp))
+	})
+	if err != nil {
+		return fmt.Errorf("failed to reconcile autoscaler role: %w", err)
+	}
+
+	autoscalerServiceAccount := manifests.AutoscalerServiceAccount(hcp.Namespace)
+	_, err = createOrUpdate(ctx, c, autoscalerServiceAccount, func() error {
+		util.EnsurePullSecret(autoscalerServiceAccount, controlplaneoperator.PullSecret("").Name)
+		config.OwnerRefFrom(hcp).ApplyTo(autoscalerServiceAccount)
+		return nil
+	})
+	if err != nil {
+		return fmt.Errorf("failed to reconcile autoscaler service account: %w", err)
+	}
+
+	autoscalerRoleBinding := manifests.AutoscalerRoleBinding(hcp.Namespace)
+	_, err = createOrUpdate(ctx, c, autoscalerRoleBinding, func() error {
+		return ReconcileAutoscalerRoleBinding(autoscalerRoleBinding, autoscalerRole, autoscalerServiceAccount, config.OwnerRefFrom(hcp))
+	})
+	if err != nil {
+		return fmt.Errorf("failed to reconcile autoscaler role binding: %w", err)
+	}
+
+	// The deployment depends on the kubeconfig being reported.
+	if hcp.Status.KubeConfig != nil {
+		// Resolve the kubeconfig secret for CAPI which the
+		// autoscaler is deployed alongside of.
+		capiKubeConfigSecret := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: hcp.Namespace,
+				Name:      fmt.Sprintf("%s-kubeconfig", hcp.Spec.InfraID),
+			},
+		}
+		err = c.Get(ctx, client.ObjectKeyFromObject(capiKubeConfigSecret), capiKubeConfigSecret)
+		if err != nil {
+			return fmt.Errorf("failed to get hosted controlplane kubeconfig secret %q: %w", capiKubeConfigSecret.Name, err)
+		}
+
+		autoscalerDeployment := manifests.AutoscalerDeployment(hcp.Namespace)
+		_, err = createOrUpdate(ctx, c, autoscalerDeployment, func() error {
+			return ReconcileAutoscalerDeployment(autoscalerDeployment, hcp, autoscalerServiceAccount, capiKubeConfigSecret, hcp.Spec.Autoscaling, autoscalerImage, availabilityProberImage, setDefaultSecurityContext)
+		})
+		if err != nil {
+			return fmt.Errorf("failed to reconcile autoscaler deployment: %w", err)
+		}
+	}
+
+	return nil
+}
diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
@@ -40,7 +40,6 @@ import (
 	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/olm"
 	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/pki"
 	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/scheduler"
-	"github.com/openshift/hypershift/hypershift-operator/controllers/manifests/controlplaneoperator"
 	"github.com/openshift/hypershift/support/capabilities"
 	"github.com/openshift/hypershift/support/config"
 	"github.com/openshift/hypershift/support/events"
@@ -74,6 +73,9 @@ const (
 	finalizer                  = "hypershift.openshift.io/finalizer"
 	DefaultAdminKubeconfigName = "admin-kubeconfig"
 	DefaultAdminKubeconfigKey  = "kubeconfig"
+
+	ImageStreamAutoscalerImage             = "cluster-autoscaler"
+	ImageStreamClusterMachineApproverImage = "cluster-machine-approver"
 )
 
 type InfrastructureStatus struct {
@@ -2715,120 +2717,29 @@ func (r *HostedControlPlaneReconciler) reconcileCloudControllerManager(ctx conte
 
 // reconcileAutoscaler orchestrates reconciliation of autoscaler components using
 func (r *HostedControlPlaneReconciler) reconcileAutoscaler(ctx context.Context, hcp *hyperv1.HostedControlPlane, images map[string]string, createOrUpdate upsert.CreateOrUpdateFN) error {
-	autoscalerRole := manifests.AutoscalerRole(hcp.Namespace)
-	_, err := createOrUpdate(ctx, r.Client, autoscalerRole, func() error {
-		return autoscaler.ReconcileAutoscalerRole(autoscalerRole, config.OwnerRefFrom(hcp))
-	})
-	if err != nil {
-		return fmt.Errorf("failed to reconcile autoscaler role: %w", err)
-	}
-
-	autoscalerServiceAccount := manifests.AutoscalerServiceAccount(hcp.Namespace)
-	_, err = createOrUpdate(ctx, r.Client, autoscalerServiceAccount, func() error {
-		util.EnsurePullSecret(autoscalerServiceAccount, controlplaneoperator.PullSecret("").Name)
-		config.OwnerRefFrom(hcp).ApplyTo(autoscalerServiceAccount)
-		return nil
-	})
-	if err != nil {
-		return fmt.Errorf("failed to reconcile autoscaler service account: %w", err)
-	}
-
-	autoscalerRoleBinding := manifests.AutoscalerRoleBinding(hcp.Namespace)
-	_, err = createOrUpdate(ctx, r.Client, autoscalerRoleBinding, func() error {
-		return autoscaler.ReconcileAutoscalerRoleBinding(autoscalerRoleBinding, autoscalerRole, autoscalerServiceAccount, config.OwnerRefFrom(hcp))
-	})
-	if err != nil {
-		return fmt.Errorf("failed to reconcile autoscaler role binding: %w", err)
+	autoscalerImage, ok := images[ImageStreamAutoscalerImage]
+	if !ok {
+		return fmt.Errorf("autoscaler image not found")
 	}
 
-	// The deployment depends on the kubeconfig being reported.
-	if hcp.Status.KubeConfig != nil {
-		// Resolve the kubeconfig secret for CAPI which the
-		// autoscaler is deployed alongside of.
-		capiKubeConfigSecret := &corev1.Secret{
-			ObjectMeta: metav1.ObjectMeta{
-				Namespace: hcp.Namespace,
-				Name:      fmt.Sprintf("%s-kubeconfig", hcp.Spec.InfraID),
-			},
-		}
-		err = r.Client.Get(ctx, client.ObjectKeyFromObject(capiKubeConfigSecret), capiKubeConfigSecret)
-		if err != nil {
-			return fmt.Errorf("failed to get hosted controlplane kubeconfig secret %q: %w", capiKubeConfigSecret.Name, err)
-		}
-
-		autoscalerImage, ok := images["cluster-autoscaler"]
-		if !ok {
-			return fmt.Errorf("autoscaler image not found")
-		}
-
-		availabilityProberImage, ok := images[util.AvailabilityProberImageName]
-		if !ok {
-			return fmt.Errorf("availability prober image not found")
-		}
-
-		autoscalerDeployment := manifests.AutoscalerDeployment(hcp.Namespace)
-		_, err = createOrUpdate(ctx, r.Client, autoscalerDeployment, func() error {
-			return autoscaler.ReconcileAutoscalerDeployment(autoscalerDeployment, hcp, autoscalerServiceAccount, capiKubeConfigSecret, hcp.Spec.Autoscaling, autoscalerImage, availabilityProberImage, r.SetDefaultSecurityContext)
-		})
-		if err != nil {
-			return fmt.Errorf("failed to reconcile autoscaler deployment: %w", err)
-		}
+	availabilityProberImage, ok := images[util.AvailabilityProberImageName]
+	if !ok {
+		return fmt.Errorf("availability prober image not found")
 	}
 
-	return nil
+	return autoscaler.ReconcileAutoscaler(ctx, r.Client, hcp, autoscalerImage, availabilityProberImage, createOrUpdate, r.SetDefaultSecurityContext)
 }
 
 func (r *HostedControlPlaneReconciler) reconcileMachineApprover(ctx context.Context, hcp *hyperv1.HostedControlPlane, images map[string]string, createOrUpdate upsert.CreateOrUpdateFN) error {
-	role := manifests.MachineApproverRole(hcp.Namespace)
-	if _, err := createOrUpdate(ctx, r.Client, role, func() error {
-		return machineapprover.ReconcileMachineApproverRole(role, config.OwnerRefFrom(hcp))
-	}); err != nil {
-		return fmt.Errorf("failed to reconcile machine-approver role: %w", err)
-	}
-
-	sa := manifests.MachineApproverServiceAccount(hcp.Namespace)
-	if _, err := createOrUpdate(ctx, r.Client, sa, func() error {
-		util.EnsurePullSecret(sa, controlplaneoperator.PullSecret("").Name)
-		config.OwnerRefFrom(hcp).ApplyTo(sa)
-		return nil
-	}); err != nil {
-		return fmt.Errorf("failed to reconcile machine-approver service account: %w", err)
-	}
-
-	roleBinding := manifests.MachineApproverRoleBinding(hcp.Namespace)
-	if _, err := createOrUpdate(ctx, r.Client, roleBinding, func() error {
-		return machineapprover.ReconcileMachineApproverRoleBinding(roleBinding, role, sa, config.OwnerRefFrom(hcp))
-	}); err != nil {
-		return fmt.Errorf("failed to reconcile machine-approver role binding: %w", err)
-	}
-	cm := manifests.ConfigMap(hcp.Namespace)
-	if _, err := createOrUpdate(ctx, r.Client, cm, func() error {
-		return machineapprover.ReconcileMachineApproverConfig(cm, config.OwnerRefFrom(hcp))
-	}); err != nil {
-		return fmt.Errorf("failed to reconcile machine-approver config: %w", err)
+	machineApproverImage, ok := images[ImageStreamClusterMachineApproverImage]
+	if !ok {
+		return fmt.Errorf("autoscaler image not found")
 	}
 
-	if hcp.Status.KubeConfig != nil {
-		// Resolve the kubeconfig secret for machine-approver
-		kubeconfigSecretName := manifests.KASServiceKubeconfigSecret(hcp.Namespace).Name
-
-		machineApproverImage, ok := images["cluster-machine-approver"]
-		if !ok {
-			return fmt.Errorf("autoscaler image not found")
-		}
-
-		availabilityProberImage, ok := images[util.AvailabilityProberImageName]
-		if !ok {
-			return fmt.Errorf("availability prober image not found")
-		}
-
-		deployment := manifests.MachineApproverDeployment(hcp.Namespace)
-		if _, err := createOrUpdate(ctx, r.Client, deployment, func() error {
-			return machineapprover.ReconcileMachineApproverDeployment(deployment, hcp, sa, kubeconfigSecretName, cm, machineApproverImage, availabilityProberImage, r.SetDefaultSecurityContext)
-		}); err != nil {
-			return fmt.Errorf("failed to reconcile machine-approver deployment: %w", err)
-		}
+	availabilityProberImage, ok := images[util.AvailabilityProberImageName]
+	if !ok {
+		return fmt.Errorf("availability prober image not found")
 	}
 
-	return nil
+	return machineapprover.ReconcileMachineApprover(ctx, r.Client, hcp, machineApproverImage, availabilityProberImage, createOrUpdate, r.SetDefaultSecurityContext)
 }
diff --git a/control-plane-operator/controllers/hostedcontrolplane/ignitionserver/ignitionserver.go b/control-plane-operator/controllers/hostedcontrolplane/ignitionserver/ignitionserver.go
@@ -13,7 +13,6 @@ import (
 	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/ingress"
 	"github.com/openshift/hypershift/hypershift-operator/controllers/manifests/controlplaneoperator"
 	"github.com/openshift/hypershift/hypershift-operator/controllers/manifests/ignitionserver"
-	hyperutil "github.com/openshift/hypershift/hypershift-operator/controllers/util"
 	"github.com/openshift/hypershift/support/certs"
 	"github.com/openshift/hypershift/support/config"
 	"github.com/openshift/hypershift/support/proxy"
@@ -378,9 +377,10 @@ func ReconcileIgnitionServer(ctx context.Context,
 				RunAsUser: utilpointer.Int64Ptr(config.DefaultSecurityContextUser),
 			}
 		}
-		hyperutil.SetRestartAnnotation(hcp.ObjectMeta, ignitionServerDeployment)
-		hyperutil.SetDefaultPriorityClass(ignitionServerDeployment)
+
 		deploymentConfig := config.DeploymentConfig{}
+		deploymentConfig.Scheduling.PriorityClass = config.DefaultPriorityClass
+		deploymentConfig.SetRestartAnnotation(hcp.ObjectMeta)
 		deploymentConfig.SetDefaults(hcp, ignitionServerLabels, nil)
 		deploymentConfig.ApplyTo(ignitionServerDeployment)
 

diff --git a/control-plane-operator/controllers/hostedcontrolplane/machineapprover/reconcile.go b/control-plane-operator/controllers/hostedcontrolplane/machineapprover/reconcile.go
@@ -1,9 +1,15 @@
 package machineapprover
 
 import (
+	"context"
+	"fmt"
+
 	hyperv1 "github.com/openshift/hypershift/api/v1alpha1"
 	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/kas"
+	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests"
+	"github.com/openshift/hypershift/hypershift-operator/controllers/manifests/controlplaneoperator"
 	"github.com/openshift/hypershift/support/config"
+	"github.com/openshift/hypershift/support/upsert"
 	"github.com/openshift/hypershift/support/util"
 	"gopkg.in/yaml.v2"
 	appsv1 "k8s.io/api/apps/v1"
@@ -13,6 +19,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	k8sutilspointer "k8s.io/utils/pointer"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 func ReconcileMachineApproverConfig(cm *corev1.ConfigMap, owner config.OwnerRef) error {
@@ -207,3 +214,47 @@ func ReconcileMachineApproverDeployment(deployment *appsv1.Deployment, hcp *hype
 
 	return nil
 }
+
+func ReconcileMachineApprover(ctx context.Context, c client.Client, hcp *hyperv1.HostedControlPlane, machineApproverImage, availabilityProberImage string, createOrUpdate upsert.CreateOrUpdateFN, setDefaultSecurityContext bool) error {
+	role := manifests.MachineApproverRole(hcp.Namespace)
+	if _, err := createOrUpdate(ctx, c, role, func() error {
+		return ReconcileMachineApproverRole(role, config.OwnerRefFrom(hcp))
+	}); err != nil {
+		return fmt.Errorf("failed to reconcile machine-approver role: %w", err)
+	}
+
+	sa := manifests.MachineApproverServiceAccount(hcp.Namespace)
+	if _, err := createOrUpdate(ctx, c, sa, func() error {
+		util.EnsurePullSecret(sa, controlplaneoperator.PullSecret("").Name)
+		config.OwnerRefFrom(hcp).ApplyTo(sa)
+		return nil
+	}); err != nil {
+		return fmt.Errorf("failed to reconcile machine-approver service account: %w", err)
+	}
+
+	roleBinding := manifests.MachineApproverRoleBinding(hcp.Namespace)
+	if _, err := createOrUpdate(ctx, c, roleBinding, func() error {
+		return ReconcileMachineApproverRoleBinding(roleBinding, role, sa, config.OwnerRefFrom(hcp))
+	}); err != nil {
+		return fmt.Errorf("failed to reconcile machine-approver role binding: %w", err)
+	}
+	cm := manifests.ConfigMap(hcp.Namespace)
+	if _, err := createOrUpdate(ctx, c, cm, func() error {
+		return ReconcileMachineApproverConfig(cm, config.OwnerRefFrom(hcp))
+	}); err != nil {
+		return fmt.Errorf("failed to reconcile machine-approver config: %w", err)
+	}
+
+	if hcp.Status.KubeConfig != nil {
+		// Resolve the kubeconfig secret for machine-approver
+		kubeconfigSecretName := manifests.KASServiceKubeconfigSecret(hcp.Namespace).Name
+		deployment := manifests.MachineApproverDeployment(hcp.Namespace)
+		if _, err := createOrUpdate(ctx, c, deployment, func() error {
+			return ReconcileMachineApproverDeployment(deployment, hcp, sa, kubeconfigSecretName, cm, machineApproverImage, availabilityProberImage, setDefaultSecurityContext)
+		}); err != nil {
+			return fmt.Errorf("failed to reconcile machine-approver deployment: %w", err)
+		}
+	}
+
+	return nil
+}