Replace custom deployment controller

Use DeploymentController to manage Deployment. Remove the custom
controller.

Ad move functions from the original controller to global functions, so they
can be used by the hooks.

Author: jsafrane

pkg/operator/csidriveroperator/deploymentcontroller.go

@@ -2,165 +2,31 @@ package csidriveroperator
 
 import (
 	"bytes"
-	"context"
 	"fmt"
 	"strconv"
 	"strings"
 	"time"
 
 	appsv1 "k8s.io/api/apps/v1"
-	"k8s.io/client-go/kubernetes"
-	"k8s.io/klog/v2"
 
 	configv1 "github.com/openshift/api/config/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	configv1listers "github.com/openshift/client-go/config/listers/config/v1"
+	"github.com/openshift/cluster-storage-operator/assets"
+	"github.com/openshift/cluster-storage-operator/pkg/csoclients"
+	"github.com/openshift/cluster-storage-operator/pkg/operator/configobservation/util"
+	"github.com/openshift/cluster-storage-operator/pkg/operator/csidriveroperator/csioperatorclient"
 	"github.com/openshift/library-go/pkg/controller/factory"
 	"github.com/openshift/library-go/pkg/operator/deploymentcontroller"
 	"github.com/openshift/library-go/pkg/operator/events"
 	"github.com/openshift/library-go/pkg/operator/loglevel"
-	"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
-	"github.com/openshift/library-go/pkg/operator/resource/resourcemerge"
 	"github.com/openshift/library-go/pkg/operator/status"
-	"github.com/openshift/library-go/pkg/operator/v1helpers"
-
-	"github.com/openshift/cluster-storage-operator/pkg/csoclients"
-	"github.com/openshift/cluster-storage-operator/pkg/operator/configobservation/util"
-	"github.com/openshift/cluster-storage-operator/pkg/operator/csidriveroperator/csioperatorclient"
-	csoutils "github.com/openshift/cluster-storage-operator/pkg/utils"
 )
 
 const (
 	deploymentControllerName = "CSIDriverOperatorDeployment"
 )
 
-type CommonCSIDeploymentController struct {
-	name              string
-	operatorClient    v1helpers.OperatorClient
-	commonClients     *csoclients.Clients
-	csiOperatorConfig csioperatorclient.CSIOperatorConfig
-	kubeClient        kubernetes.Interface
-	versionGetter     status.VersionGetter
-	targetVersion     string
-	eventRecorder     events.Recorder
-	infraLister       configv1listers.InfrastructureLister
-	resyncInterval    time.Duration
-	factory           *factory.Factory
-	replacers         []*strings.Replacer
-	manifestHooks     []deploymentcontroller.ManifestHookFunc
-	deploymentHooks   []deploymentcontroller.DeploymentHookFunc
-}
-
-func (c *CommonCSIDeploymentController) initController(factoryHookFunc func(*factory.Factory)) *factory.Factory {
-	f := factory.New()
-	f = f.ResyncEvery(c.resyncInterval)
-	f = f.WithSyncDegradedOnError(c.operatorClient)
-	// Necessary to do initial Sync after the controller starts.
-	f = f.WithPostStartHooks(initalSync)
-	// Add informers to the factory now, but the actual event handlers
-	// are added later in CSIDriverOperatorDeploymentController.Run(),
-	// when we're 100% sure the controller is going to start (because it
-	// depends on the platform).
-	// If we added the event handlers now, all events would pile up in the
-	// controller queue, without anything reading it.
-	f = f.WithInformers(
-		c.commonClients.OperatorClient.Informer())
-	factoryHookFunc(f)
-	return f
-}
-
-func (c *CommonCSIDeploymentController) preCheckSync(
-	ctx context.Context,
-	syncCtx factory.SyncContext) (bool, *operatorv1.OperatorStatus, *operatorv1.OperatorSpec, error) {
-
-	opSpec, opStatus, _, err := c.operatorClient.GetOperatorState()
-	if err != nil {
-		return false, opStatus, opSpec, err
-	}
-	if opSpec.ManagementState != operatorv1.Managed {
-		return false, opStatus, opSpec, nil
-	}
-	return true, opStatus, opSpec, nil
-}
-
-func (c *CommonCSIDeploymentController) postSync(ctx context.Context, deployment *appsv1.Deployment) error {
-	progressingCondition := operatorv1.OperatorCondition{
-		Type:   c.name + operatorv1.OperatorStatusTypeProgressing,
-		Status: operatorv1.ConditionFalse,
-	}
-
-	if ok, msg := isProgressing(deployment); ok {
-		progressingCondition.Status = operatorv1.ConditionTrue
-		progressingCondition.Message = msg
-		progressingCondition.Reason = "Deploying"
-	}
-
-	updateStatusFn := func(newStatus *operatorv1.OperatorStatus) error {
-		resourcemerge.SetDeploymentGeneration(&newStatus.Generations, deployment)
-		return nil
-	}
-
-	_, _, err := v1helpers.UpdateStatus(
-		ctx,
-		c.operatorClient,
-		updateStatusFn,
-		v1helpers.UpdateConditionFn(progressingCondition),
-	)
-	return err
-}
-
-func initCommonDeploymentParams(
-	client *csoclients.Clients,
-	csiOperatorConfig csioperatorclient.CSIOperatorConfig,
-	resyncInterval time.Duration,
-	versionGetter status.VersionGetter,
-	targetVersion string,
-	eventRecorder events.Recorder) CommonCSIDeploymentController {
-	c := CommonCSIDeploymentController{
-		name:              csiOperatorConfig.ConditionPrefix,
-		operatorClient:    client.OperatorClient,
-		kubeClient:        client.KubeClient,
-		csiOperatorConfig: csiOperatorConfig,
-		commonClients:     client,
-		versionGetter:     versionGetter,
-		targetVersion:     targetVersion,
-		resyncInterval:    resyncInterval,
-		eventRecorder:     eventRecorder.WithComponentSuffix(csiOperatorConfig.ConditionPrefix),
-		infraLister:       client.ConfigInformers.Config().V1().Infrastructures().Lister(),
-	}
-	c.manifestHooks = []deploymentcontroller.ManifestHookFunc{
-		c.getReplacerHook(),
-		c.getLogLevelHook(),
-	}
-	c.deploymentHooks = []deploymentcontroller.DeploymentHookFunc{
-		c.getProxyHook(),
-	}
-
-	// Common replacers
-	c.replacers = []*strings.Replacer{sidecarReplacer}
-	if csiOperatorConfig.ImageReplacer != nil {
-		c.replacers = append(c.replacers, csiOperatorConfig.ImageReplacer)
-	}
-
-	return c
-}
-
-// This CSIDriverStarterController installs and syncs CSI driver operator Deployment.
-// It replace ${LOG_LEVEL} in the Deployment with current log level.
-// It replaces images in the Deployment using  CSIOperatorConfig.ImageReplacer.
-// It produces following Conditions:
-// <CSI driver name>CSIDriverOperatorDeploymentProgressing
-// <CSI driver name>CSIDriverOperatorDeploymentDegraded
-// This controller doesn't set the Available condition to avoid prematurely cascading
-// up to the clusteroperator CR a potential Available=false. On the other hand it
-// does a better in making sure the Degraded condition is properly set if
-// Deployment isn't healthy.
-type CSIDriverOperatorDeploymentController struct {
-	CommonCSIDeploymentController
-}
-
-var _ factory.Controller = &CSIDriverOperatorDeploymentController{}
-
 func NewCSIDriverOperatorDeploymentController(
 	clients *csoclients.Clients,
 	csiOperatorConfig csioperatorclient.CSIOperatorConfig,
@@ -169,28 +35,65 @@ func NewCSIDriverOperatorDeploymentController(
 	eventRecorder events.Recorder,
 	resyncInterval time.Duration,
 ) factory.Controller {
-	c := &CSIDriverOperatorDeploymentController{
-		CommonCSIDeploymentController: initCommonDeploymentParams(clients, csiOperatorConfig, resyncInterval, versionGetter, targetVersion, eventRecorder),
+
+	deploymentBytes, err := assets.ReadFile(csiOperatorConfig.DeploymentAsset)
+	if err != nil {
+		panic(err)
+	}
+
+	manifestHooks, deploymentHooks := getCommonHooks(getCommonReplacers(csiOperatorConfig))
+	deploymentHooks = append(deploymentHooks, getStandaloneNodeSelectorHook(clients.ConfigInformers.Config().V1().Infrastructures().Lister()))
+
+	c, err := deploymentcontroller.NewDeploymentControllerBuilder(
+		csiOperatorConfig.ConditionPrefix+deploymentControllerName,
+		deploymentBytes,
+		eventRecorder,
+		clients.OperatorClient,
+		clients.KubeClient,
+		clients.KubeInformers.InformersFor(csoclients.CSIOperatorNamespace).Apps().V1().Deployments(),
+	).WithConditions(
+		// Explicitly disable Available condition to avoid prematurely cascading
+		// up to the clusteroperator CR a potential Available=false.
+		operatorv1.OperatorStatusTypeProgressing,
+		operatorv1.OperatorStatusTypeDegraded,
+	).WithExtraInformers(
+		clients.ConfigInformers.Config().V1().Infrastructures().Informer(),
+	).WithManifestHooks(
+		manifestHooks...,
+	).WithDeploymentHooks(
+		deploymentHooks...,
+	).WithPostStartHooks(
+		initalSync,
+	).ToController()
+	if err != nil {
+		panic(err)
 	}
-	f := c.initController(func(f *factory.Factory) {
-		f.WithInformers(
-			c.commonClients.KubeInformers.InformersFor(csoclients.CSIOperatorNamespace).Apps().V1().Deployments().Informer(),
-			c.commonClients.ConfigInformers.Config().V1().Infrastructures().Informer())
-	})
+	return c
+}
 
-	// Standalone specific deployment hooks
-	c.deploymentHooks = append(c.deploymentHooks, c.getStandaloneNodeSelectorHook())
+func getCommonReplacers(csiOperatorConfig csioperatorclient.CSIOperatorConfig) []*strings.Replacer {
+	replacers := []*strings.Replacer{sidecarReplacer}
+	if csiOperatorConfig.ImageReplacer != nil {
+		replacers = append(replacers, csiOperatorConfig.ImageReplacer)
+	}
+	return replacers
+}
 
-	c.factory = f
-	return c
+func getCommonHooks(replacers []*strings.Replacer) ([]deploymentcontroller.ManifestHookFunc, []deploymentcontroller.DeploymentHookFunc) {
+	return []deploymentcontroller.ManifestHookFunc{
+			getReplacerHook(replacers),
+			getLogLevelHook(),
+		}, []deploymentcontroller.DeploymentHookFunc{
+			getProxyHook(),
+		}
 }
 
-func (c *CommonCSIDeploymentController) getReplacerHook() deploymentcontroller.ManifestHookFunc {
+func getReplacerHook(replacers []*strings.Replacer) deploymentcontroller.ManifestHookFunc {
 	return func(spec *operatorv1.OperatorSpec, deploymentBytes []byte) ([]byte, error) {
 		deploymentString := string(deploymentBytes)
 
 		// Replace images
-		for _, replacer := range c.replacers {
+		for _, replacer := range replacers {
 			// Replace images
 			if replacer != nil {
 				deploymentString = replacer.Replace(deploymentString)
@@ -201,23 +104,23 @@ func (c *CommonCSIDeploymentController) getReplacerHook() deploymentcontroller.M
 	}
 }
 
-func (c *CommonCSIDeploymentController) getLogLevelHook() deploymentcontroller.ManifestHookFunc {
+func getLogLevelHook() deploymentcontroller.ManifestHookFunc {
 	return func(spec *operatorv1.OperatorSpec, deploymentBytes []byte) ([]byte, error) {
 		logLevel := loglevel.LogLevelToVerbosity(spec.LogLevel)
 		deploymentBytes = bytes.ReplaceAll(deploymentBytes, []byte("${LOG_LEVEL}"), []byte(strconv.Itoa(logLevel)))
 		return deploymentBytes, nil
 	}
 }
 
-func (c *CommonCSIDeploymentController) getProxyHook() deploymentcontroller.DeploymentHookFunc {
+func getProxyHook() deploymentcontroller.DeploymentHookFunc {
 	return func(spec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error {
 		return util.InjectObservedProxyInDeploymentContainers(deployment, spec)
 	}
 }
 
-func (c *CSIDriverOperatorDeploymentController) getStandaloneNodeSelectorHook() deploymentcontroller.DeploymentHookFunc {
+func getStandaloneNodeSelectorHook(infraLister configv1listers.InfrastructureLister) deploymentcontroller.DeploymentHookFunc {
 	return func(spec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error {
-		infra, err := c.infraLister.Get(infraConfigName)
+		infra, err := infraLister.Get(infraConfigName)
 		if err != nil {
 			return fmt.Errorf("failed to get infrastructure resource: %w", err)
 		}
@@ -227,66 +130,3 @@ func (c *CSIDriverOperatorDeploymentController) getStandaloneNodeSelectorHook()
 		return nil
 	}
 }
-
-func (c *CSIDriverOperatorDeploymentController) Sync(ctx context.Context, syncCtx factory.SyncContext) error {
-	klog.V(4).Infof("CSIDriverOperatorDeploymentController sync started")
-	defer klog.V(4).Infof("CSIDriverOperatorDeploymentController sync finished")
-
-	runSync, opStatus, opSpec, err := c.preCheckSync(ctx, syncCtx)
-	if err != nil {
-		return err
-	}
-
-	if !runSync {
-		return nil
-	}
-
-	required, err := csoutils.GetRequiredDeployment(c.csiOperatorConfig.DeploymentAsset, opSpec, c.manifestHooks, nil)
-	if err != nil {
-		return fmt.Errorf("failed to generate required Deployment: %s", err)
-	}
-
-	requiredCopy := required.DeepCopy()
-
-	lastGeneration := resourcemerge.ExpectedDeploymentGeneration(requiredCopy, opStatus.Generations)
-	deployment, _, err := resourceapply.ApplyDeployment(ctx, c.kubeClient.AppsV1(), c.eventRecorder, requiredCopy, lastGeneration)
-	if err != nil {
-		return err
-	}
-
-	err = c.postSync(ctx, deployment)
-	if err != nil {
-		return err
-	}
-	return checkDeploymentHealth(ctx, c.kubeClient.AppsV1(), deployment)
-}
-
-func (c *CSIDriverOperatorDeploymentController) Run(ctx context.Context, workers int) {
-	// This adds event handlers to informers.
-	ctrl := c.factory.WithSync(c.Sync).ToController(c.Name(), c.eventRecorder)
-	ctrl.Run(ctx, workers)
-}
-
-func (c *CSIDriverOperatorDeploymentController) Name() string {
-	return c.name + deploymentControllerName
-}
-
-// TODO: create a common function in library-go
-func isProgressing(deployment *appsv1.Deployment) (bool, string) {
-	var deploymentExpectedReplicas int32
-	if deployment.Spec.Replicas != nil {
-		deploymentExpectedReplicas = *deployment.Spec.Replicas
-	}
-
-	switch {
-	case deployment.Generation != deployment.Status.ObservedGeneration:
-		return true, "Waiting for Deployment to act on changes"
-	case deployment.Status.UnavailableReplicas > 0:
-		return true, "Waiting for Deployment to deploy pods"
-	case deployment.Status.UpdatedReplicas < deploymentExpectedReplicas:
-		return true, "Waiting for Deployment to update pods"
-	case deployment.Status.AvailableReplicas < deploymentExpectedReplicas:
-		return true, "Waiting for Deployment to deploy pods"
-	}
-	return false, ""
-}