diff --git a/pkg/operator/revisioncontroller/revision_controller.go b/pkg/operator/revisioncontroller/revision_controller.go index 8c4e39e52f..4d835dd1e2 100644 --- a/pkg/operator/revisioncontroller/revision_controller.go +++ b/pkg/operator/revisioncontroller/revision_controller.go @@ -7,10 +7,8 @@ import ( "strings" "time" - operatorv1 "github.com/openshift/api/operator/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" @@ -90,26 +88,28 @@ func NewRevisionController( kubeInformersForTargetNamespace.Core().V1().Secrets().Informer(), ). WithSync(c.sync). + WithSyncDegradedOnError(operatorClient). ResyncEvery(1*time.Minute). ToController( - "RevisionController", // don't change what is passed here unless you also remove the old FooDegraded condition + "RevisionController", eventRecorder, ) } // 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, currentLastAvailableRevision int32) (wroteStatus bool, requeue bool, err error) { +func (c RevisionController) createRevisionIfNeeded(ctx context.Context, recorder events.Recorder, currentLastAvailableRevision int32) 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", currentLastAvailableRevision) - return false, false, nil + return nil } 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) @@ -119,41 +119,24 @@ func (c RevisionController) createRevisionIfNeeded(ctx context.Context, recorder } if err != nil { - status := applyoperatorv1.OperatorStatus(). - WithConditions(applyoperatorv1.OperatorCondition(). - WithType(condition.RevisionControllerDegradedConditionType). - WithStatus(operatorv1.ConditionTrue). - WithReason("ContentCreationError"). - WithMessage(err.Error()), - ). - WithLatestAvailableRevision(currentLastAvailableRevision) - 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, true, updateError - } - return true, true, nil + return err } if !createdNewRevision { klog.V(4).Infof("Revision %v not created", nextRevision) - return false, false, nil + return nil } recorder.Eventf("RevisionTriggered", "new revision %d triggered by %q", 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 updateError } - return true, false, nil + return nil } func nameFor(name string, revision int32) string { @@ -394,47 +377,23 @@ func (c RevisionController) sync(ctx context.Context, syncCtx factory.SyncContex if latestObservedRevision != 0 && operatorStatus.LatestAvailableRevision != latestObservedRevision { // Then make sure that revision number is what's in the operator status 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 + // we will be called again because the update will self-feed the informer + return nil } if shouldCreateNewRevision, err := c.revisionPrecondition(ctx); err != nil || !shouldCreateNewRevision { return err } - 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 + syncErr := c.createRevisionIfNeeded(ctx, syncCtx.Recorder(), operatorStatus.LatestAvailableRevision) + if syncErr != nil { return syncErr - case wroteStatus: - return syncErr - } - - // update failing condition - 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 - } } - return err + return nil } diff --git a/pkg/operator/revisioncontroller/revision_controller_test.go b/pkg/operator/revisioncontroller/revision_controller_test.go index f45e661105..2968ca996f 100644 --- a/pkg/operator/revisioncontroller/revision_controller_test.go +++ b/pkg/operator/revisioncontroller/revision_controller_test.go @@ -220,17 +220,8 @@ func TestRevisionController(t *testing.T) { ), testConfigs: []RevisionResource{{Name: "test-config"}}, testSecrets: []RevisionResource{{Name: "test-secret"}}, - expectSyncError: "synthetic requeue request", + expectSyncError: `configmaps "test-config" not found`, validateStatus: func(t *testing.T, status *operatorv1.StaticPodOperatorStatus) { - if status.Conditions[0].Type != "RevisionControllerDegraded" { - t.Errorf("expected status condition to be 'RevisionControllerFailing', got %v", status.Conditions[0].Type) - } - if status.Conditions[0].Reason != "ContentCreationError" { - t.Errorf("expected status condition reason to be 'ContentCreationError', got %v", status.Conditions[0].Reason) - } - if !strings.Contains(status.Conditions[0].Message, `configmaps "test-config" not found`) { - t.Errorf("expected status to be 'configmaps test-config not found', got: %s", status.Conditions[0].Message) - } }, validateActions: func(t *testing.T, actions []clienttesting.Action, kclient *fake.Clientset) { createdObjects := filterCreateActions(actions)