update the revision controller to use the type moved latestrevision

Author: deads2k

pkg/operator/apiserver/controllerset/apiservercontrollerset.go

@@ -315,6 +315,7 @@ func (cs *APIServerControllerSet) WithRevisionController(
 	secretGetter corev1client.SecretsGetter,
 ) *APIServerControllerSet {
 	cs.revisionController.controller = revisioncontroller.NewRevisionController(
+		cs.name,
 		targetNamespace,
 		configMaps,
 		secrets,

pkg/operator/configobserver/node/suppress_config_updates_test.go

@@ -212,8 +212,10 @@ func TestSuppressConfigUpdateUntilSameProfileFunc(t *testing.T) {
 				}
 			}
 			staticPodOperatorStatus := operatorv1.StaticPodOperatorStatus{
-				LatestAvailableRevision: int32(len(testCase.observedConfigRevisions) + 1),
-				NodeStatuses:            nodesStatuses,
+				OperatorStatus: operatorv1.OperatorStatus{
+					LatestAvailableRevision: int32(len(testCase.observedConfigRevisions) + 1),
+				},
+				NodeStatuses: nodesStatuses,
 			}
 
 			operatorObservedConfig, err := json.Marshal(testCase.operatorSpecObservedConfig)

pkg/operator/revisioncontroller/revision_controller.go

@@ -7,25 +7,25 @@ import (
 	"strings"
 	"time"
 
-	"k8s.io/klog/v2"
-
 	operatorv1 "github.com/openshift/api/operator/v1"
-	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/equality"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/informers"
-	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
-
+	applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
 	"github.com/openshift/library-go/pkg/controller/factory"
 	"github.com/openshift/library-go/pkg/operator/condition"
 	"github.com/openshift/library-go/pkg/operator/events"
 	"github.com/openshift/library-go/pkg/operator/management"
 	"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
 	"github.com/openshift/library-go/pkg/operator/v1helpers"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/informers"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
+	"k8s.io/klog/v2"
 )
 
 // LatestRevisionClient is an operator client for an operator status with a latest revision field.
+// Deprecated
 type LatestRevisionClient interface {
 	v1helpers.OperatorClient
 
@@ -39,14 +39,16 @@ type LatestRevisionClient interface {
 // of them. If the original resources changes, the revision counter is increased, stored in LatestAvailableRevision
 // field of the operator config and new snapshots suffixed by the revision are created.
 type RevisionController struct {
+	controllerInstanceName string
+
 	targetNamespace string
 	// configMaps is the list of configmaps that are directly copied.A different actor/controller modifies these.
 	// the first element should be the configmap that contains the static pod manifest
 	configMaps []RevisionResource
 	// secrets is a list of secrets that are directly copied for the current values.  A different actor/controller modifies these.
 	secrets []RevisionResource
 
-	operatorClient  LatestRevisionClient
+	operatorClient  v1helpers.OperatorClient
 	configMapGetter corev1client.ConfigMapsGetter
 	secretGetter    corev1client.SecretsGetter
 
@@ -63,11 +65,12 @@ type PreconditionFunc func(ctx context.Context) (bool, error)
 
 // NewRevisionController create a new revision controller.
 func NewRevisionController(
+	instanceName string,
 	targetNamespace string,
 	configMaps []RevisionResource,
 	secrets []RevisionResource,
 	kubeInformersForTargetNamespace informers.SharedInformerFactory,
-	operatorClient LatestRevisionClient,
+	operatorClient v1helpers.OperatorClient,
 	configMapGetter corev1client.ConfigMapsGetter,
 	secretGetter corev1client.SecretsGetter,
 	eventRecorder events.Recorder,
@@ -80,9 +83,10 @@ func NewRevisionController(
 	}
 
 	c := &RevisionController{
-		targetNamespace: targetNamespace,
-		configMaps:      configMaps,
-		secrets:         secrets,
+		controllerInstanceName: factory.ControllerInstanceName(instanceName, "RevisionController"),
+		targetNamespace:        targetNamespace,
+		configMaps:             configMaps,
+		secrets:                secrets,
 
 		operatorClient:       operatorClient,
 		configMapGetter:      configMapGetter,
@@ -106,18 +110,17 @@ func NewRevisionController(
 
 // createRevisionIfNeeded takes care of creating content for the static pods to use.
 // returns whether or not requeue and if an error happened when updating status.  Normally it updates status itself.
-func (c RevisionController) createRevisionIfNeeded(ctx context.Context, recorder events.Recorder, latestAvailableRevision int32, resourceVersion string) (bool, error) {
-	isLatestRevisionCurrent, requiredIsNotFound, reason := c.isLatestRevisionCurrent(ctx, latestAvailableRevision)
+func (c RevisionController) createRevisionIfNeeded(ctx context.Context, recorder events.Recorder, currentLastAvailableRevision int32) (wroteStatus bool, requeue bool, err error) {
+	isLatestRevisionCurrent, requiredIsNotFound, reason := c.isLatestRevisionCurrent(ctx, currentLastAvailableRevision)
 
 	// check to make sure that the latestRevision has the exact content we expect.  No mutation here, so we start creating the next Revision only when it is required
 	if isLatestRevisionCurrent {
-		klog.V(4).Infof("Returning early, %d triggered and up to date", latestAvailableRevision)
-		return false, nil
+		klog.V(4).Infof("Returning early, %d triggered and up to date", currentLastAvailableRevision)
+		return false, false, nil
 	}
 
-	nextRevision := latestAvailableRevision + 1
+	nextRevision := currentLastAvailableRevision + 1
 	var createdNewRevision bool
-	var err error
 	// check to make sure no new revision is created when a required object is missing
 	if requiredIsNotFound {
 		err = fmt.Errorf("%v", reason)
@@ -127,36 +130,43 @@ func (c RevisionController) createRevisionIfNeeded(ctx context.Context, recorder
 	}
 
 	if err != nil {
-		cond := operatorv1.OperatorCondition{
-			Type:    "RevisionControllerDegraded",
-			Status:  operatorv1.ConditionTrue,
-			Reason:  "ContentCreationError",
-			Message: err.Error(),
-		}
-		if _, _, updateError := v1helpers.UpdateStatus(ctx, c.operatorClient, v1helpers.UpdateConditionFn(cond)); updateError != nil {
+		status := applyoperatorv1.OperatorStatus().
+			WithConditions(applyoperatorv1.OperatorCondition().
+				WithType(condition.RevisionControllerDegradedConditionType).
+				WithStatus(operatorv1.ConditionTrue).
+				WithReason("ContentCreationError").
+				WithMessage(err.Error()),
+			).
+			WithLatestAvailableRevision(currentLastAvailableRevision)
+		// different fieldmanager is used to conflict. We could also choose to *always* set the lastavailablerevision
+		// update this tomorrow, that's probably better.
+		updateError := c.operatorClient.ApplyOperatorStatus(ctx, c.controllerInstanceName, status)
+		if updateError != nil {
 			recorder.Warningf("RevisionCreateFailed", "Failed to create revision %d: %v", nextRevision, err.Error())
-			return true, updateError
+			return true, true, updateError
 		}
-		return true, nil
+		return true, true, nil
 	}
 
 	if !createdNewRevision {
 		klog.V(4).Infof("Revision %v not created", nextRevision)
-		return false, nil
+		return false, false, nil
 	}
+
 	recorder.Eventf("RevisionTriggered", "new revision %d triggered by %q", nextRevision, reason)
 
-	cond := operatorv1.OperatorCondition{
-		Type:   "RevisionControllerDegraded",
-		Status: operatorv1.ConditionFalse,
-	}
-	if _, updated, updateError := c.operatorClient.UpdateLatestRevisionOperatorStatus(ctx, nextRevision, v1helpers.UpdateConditionFn(cond)); updateError != nil {
-		return true, updateError
-	} else if updated {
-		recorder.Eventf("RevisionCreate", "Revision %d created because %s", nextRevision, reason)
+	status := applyoperatorv1.OperatorStatus().
+		WithConditions(applyoperatorv1.OperatorCondition().
+			WithType(condition.RevisionControllerDegradedConditionType).
+			WithStatus(operatorv1.ConditionFalse),
+		).
+		WithLatestAvailableRevision(nextRevision)
+	updateError := c.operatorClient.ApplyOperatorStatus(ctx, c.controllerInstanceName, status)
+	if updateError != nil {
+		return true, true, updateError
 	}
 
-	return false, nil
+	return true, false, nil
 }
 
 func nameFor(name string, revision int32) string {
@@ -344,7 +354,8 @@ func (c RevisionController) getLatestAvailableRevision(ctx context.Context) (int
 }
 
 func (c RevisionController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
-	operatorSpec, _, latestAvailableRevisionSeenByOperator, resourceVersion, err := c.operatorClient.GetLatestRevisionState()
+	//operatorSpec, _, latestAvailableRevisionSeenByOperator, resourceVersion, err := c.operatorClient.GetLatestRevisionState()
+	operatorSpec, operatorStatus, _, err := c.operatorClient.GetOperatorState()
 	if err != nil {
 		return err
 	}
@@ -362,33 +373,47 @@ func (c RevisionController) sync(ctx context.Context, syncCtx factory.SyncContex
 	if err != nil {
 		return err
 	}
-	if latestObservedRevision != 0 && latestAvailableRevisionSeenByOperator != latestObservedRevision {
+	if latestObservedRevision != 0 && operatorStatus.LatestAvailableRevision != latestObservedRevision {
 		// Then make sure that revision number is what's in the operator status
-		_, _, err := c.operatorClient.UpdateLatestRevisionOperatorStatus(ctx, latestObservedRevision)
+		status := applyoperatorv1.OperatorStatus().
+			WithConditions(applyoperatorv1.OperatorCondition().
+				WithType(condition.RevisionControllerDegradedConditionType).
+				WithStatus(operatorv1.ConditionFalse),
+			).
+			WithLatestAvailableRevision(latestObservedRevision)
+		err := c.operatorClient.ApplyOperatorStatus(ctx, c.controllerInstanceName, status)
 		if err != nil {
 			return err
 		}
 		// regardless of whether we made a change, requeue to rerun the sync with updated status
 		return factory.SyntheticRequeueError
 	}
 
-	requeue, syncErr := c.createRevisionIfNeeded(ctx, syncCtx.Recorder(), latestAvailableRevisionSeenByOperator, resourceVersion)
+	wroteStatus, requeue, syncErr := c.createRevisionIfNeeded(ctx, syncCtx.Recorder(), operatorStatus.LatestAvailableRevision)
+	switch {
+	case requeue && syncErr == nil:
+		return factory.SyntheticRequeueError
+	case syncErr != nil:
+		// this is updated in status inside of createRevisionIfNeeded
+		return syncErr
+
+	}
 	if requeue && syncErr == nil {
 		return factory.SyntheticRequeueError
 	}
-	err = syncErr
+	if wroteStatus {
+		return syncErr
+	}
 
 	// update failing condition
-	cond := operatorv1.OperatorCondition{
-		Type:   condition.RevisionControllerDegradedConditionType,
-		Status: operatorv1.ConditionFalse,
-	}
-	if err != nil {
-		cond.Status = operatorv1.ConditionTrue
-		cond.Reason = "Error"
-		cond.Message = err.Error()
-	}
-	if _, _, updateError := v1helpers.UpdateStatus(ctx, c.operatorClient, v1helpers.UpdateConditionFn(cond)); updateError != nil {
+	status := applyoperatorv1.OperatorStatus().
+		WithConditions(applyoperatorv1.OperatorCondition().
+			WithType(condition.RevisionControllerDegradedConditionType).
+			WithStatus(operatorv1.ConditionFalse),
+		).
+		WithLatestAvailableRevision(operatorStatus.LatestAvailableRevision)
+	updateError := c.operatorClient.ApplyOperatorStatus(ctx, c.controllerInstanceName, status)
+	if updateError != nil {
 		if err == nil {
 			return updateError
 		}