Skip to content

Commit 190f286

Browse files
Merge pull request #1801 from deads2k/apply-03-staticpodloops
API-1835: update the revision controller to use the type moved latestrevision
2 parents 856f475 + 0c1a7b5 commit 190f286

39 files changed

+509
-282
lines changed

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ require (
2020
github.com/opencontainers/go-digest v1.0.0
2121
github.com/opencontainers/runc v1.1.10
2222
github.com/opencontainers/selinux v1.11.0
23-
github.com/openshift/api v0.0.0-20240918014254-07bccfd9266f
23+
github.com/openshift/api v0.0.0-20240924220842-3c700b6cb32b
2424
github.com/openshift/build-machinery-go v0.0.0-20240419090851-af9c868bcf52
25-
github.com/openshift/client-go v0.0.0-20240918182115-6a8ead8397fd
25+
github.com/openshift/client-go v0.0.0-20240925210910-aaed17e719c5
2626
github.com/pkg/errors v0.9.1
2727
github.com/pkg/profile v1.3.0
2828
github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.74.0

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -193,12 +193,12 @@ github.com/opencontainers/runc v1.1.10 h1:EaL5WeO9lv9wmS6SASjszOeQdSctvpbu0DdBQB
193193
github.com/opencontainers/runc v1.1.10/go.mod h1:+/R6+KmDlh+hOO8NkjmgkG9Qzvypzk0yXxAPYYR65+M=
194194
github.com/opencontainers/selinux v1.11.0 h1:+5Zbo97w3Lbmb3PeqQtpmTkMwsW5nRI3YaLpt7tQ7oU=
195195
github.com/opencontainers/selinux v1.11.0/go.mod h1:E5dMC3VPuVvVHDYmi78qvhJp8+M586T4DlDRYpFkyec=
196-
github.com/openshift/api v0.0.0-20240918014254-07bccfd9266f h1:jw869n5Wfttrj56lRolhl7+loudoEvxJt4oa7CbO+QM=
197-
github.com/openshift/api v0.0.0-20240918014254-07bccfd9266f/go.mod h1:OOh6Qopf21pSzqNVCB5gomomBXb8o5sGKZxG2KNpaXM=
196+
github.com/openshift/api v0.0.0-20240924220842-3c700b6cb32b h1:3CDA+4Ed9JWKNs3czWoq1DcI2rjWMShIpoIiPFey11o=
197+
github.com/openshift/api v0.0.0-20240924220842-3c700b6cb32b/go.mod h1:OOh6Qopf21pSzqNVCB5gomomBXb8o5sGKZxG2KNpaXM=
198198
github.com/openshift/build-machinery-go v0.0.0-20240419090851-af9c868bcf52 h1:bqBwrXG7sbJUqP1Og1bR8FvVh7qb7CrMgy9saKmOZFs=
199199
github.com/openshift/build-machinery-go v0.0.0-20240419090851-af9c868bcf52/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
200-
github.com/openshift/client-go v0.0.0-20240918182115-6a8ead8397fd h1:Gd0+bYdcfGIsDOJ8BwTJJjQeXoziyIsTwqp/s38rKyM=
201-
github.com/openshift/client-go v0.0.0-20240918182115-6a8ead8397fd/go.mod h1:EB7GeA/vpf9AHklMgnnT0+uG6l/3f8cChtCFbJFrk4g=
200+
github.com/openshift/client-go v0.0.0-20240925210910-aaed17e719c5 h1:1yz2alsLpp8L99THG6tWXzeds45hf6jXI4bNoTxEim8=
201+
github.com/openshift/client-go v0.0.0-20240925210910-aaed17e719c5/go.mod h1:axlsYEU3WeMRlIvHdsSKl/wP17k8oZgHCtPy9WgAMts=
202202
github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
203203
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
204204
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=

pkg/operator/apiserver/controllerset/apiservercontrollerset.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,16 +310,17 @@ func (cs *APIServerControllerSet) WithRevisionController(
310310
configMaps []revisioncontroller.RevisionResource,
311311
secrets []revisioncontroller.RevisionResource,
312312
kubeInformersForTargetNamespace kubeinformers.SharedInformerFactory,
313-
revisionClient revisioncontroller.LatestRevisionClient,
313+
operatorClient v1helpers.OperatorClient,
314314
configMapGetter corev1client.ConfigMapsGetter,
315315
secretGetter corev1client.SecretsGetter,
316316
) *APIServerControllerSet {
317317
cs.revisionController.controller = revisioncontroller.NewRevisionController(
318+
cs.name,
318319
targetNamespace,
319320
configMaps,
320321
secrets,
321322
kubeInformersForTargetNamespace,
322-
revisionClient,
323+
operatorClient,
323324
configMapGetter,
324325
secretGetter,
325326
cs.eventRecorder,

pkg/operator/configobserver/node/suppress_config_updates_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,10 @@ func TestSuppressConfigUpdateUntilSameProfileFunc(t *testing.T) {
212212
}
213213
}
214214
staticPodOperatorStatus := operatorv1.StaticPodOperatorStatus{
215-
LatestAvailableRevision: int32(len(testCase.observedConfigRevisions) + 1),
216-
NodeStatuses: nodesStatuses,
215+
OperatorStatus: operatorv1.OperatorStatus{
216+
LatestAvailableRevision: int32(len(testCase.observedConfigRevisions) + 1),
217+
},
218+
NodeStatuses: nodesStatuses,
217219
}
218220

219221
operatorObservedConfig, err := json.Marshal(testCase.operatorSpecObservedConfig)

pkg/operator/revisioncontroller/revision_controller.go

Lines changed: 83 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -7,46 +7,37 @@ import (
77
"strings"
88
"time"
99

10-
"k8s.io/klog/v2"
11-
1210
operatorv1 "github.com/openshift/api/operator/v1"
13-
corev1 "k8s.io/api/core/v1"
14-
"k8s.io/apimachinery/pkg/api/equality"
15-
apierrors "k8s.io/apimachinery/pkg/api/errors"
16-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
17-
"k8s.io/client-go/informers"
18-
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
19-
11+
applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
2012
"github.com/openshift/library-go/pkg/controller/factory"
2113
"github.com/openshift/library-go/pkg/operator/condition"
2214
"github.com/openshift/library-go/pkg/operator/events"
2315
"github.com/openshift/library-go/pkg/operator/management"
2416
"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
2517
"github.com/openshift/library-go/pkg/operator/v1helpers"
18+
corev1 "k8s.io/api/core/v1"
19+
"k8s.io/apimachinery/pkg/api/equality"
20+
apierrors "k8s.io/apimachinery/pkg/api/errors"
21+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
22+
"k8s.io/client-go/informers"
23+
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
24+
"k8s.io/klog/v2"
2625
)
2726

28-
// LatestRevisionClient is an operator client for an operator status with a latest revision field.
29-
type LatestRevisionClient interface {
30-
v1helpers.OperatorClient
31-
32-
// GetLatestRevisionState returns the spec, status and latest revision.
33-
GetLatestRevisionState() (spec *operatorv1.OperatorSpec, status *operatorv1.OperatorStatus, rev int32, rv string, err error)
34-
// UpdateLatestRevisionOperatorStatus updates the status with the given latestAvailableRevision and the by applying the given updateFuncs.
35-
UpdateLatestRevisionOperatorStatus(ctx context.Context, latestAvailableRevision int32, updateFuncs ...v1helpers.UpdateStatusFunc) (*operatorv1.OperatorStatus, bool, error)
36-
}
37-
3827
// RevisionController is a controller that watches a set of configmaps and secrets and them against a revision snapshot
3928
// of them. If the original resources changes, the revision counter is increased, stored in LatestAvailableRevision
4029
// field of the operator config and new snapshots suffixed by the revision are created.
4130
type RevisionController struct {
31+
controllerInstanceName string
32+
4233
targetNamespace string
4334
// configMaps is the list of configmaps that are directly copied.A different actor/controller modifies these.
4435
// the first element should be the configmap that contains the static pod manifest
4536
configMaps []RevisionResource
4637
// secrets is a list of secrets that are directly copied for the current values. A different actor/controller modifies these.
4738
secrets []RevisionResource
4839

49-
operatorClient LatestRevisionClient
40+
operatorClient v1helpers.OperatorClient
5041
configMapGetter corev1client.ConfigMapsGetter
5142
secretGetter corev1client.SecretsGetter
5243

@@ -63,11 +54,12 @@ type PreconditionFunc func(ctx context.Context) (bool, error)
6354

6455
// NewRevisionController create a new revision controller.
6556
func NewRevisionController(
57+
instanceName string,
6658
targetNamespace string,
6759
configMaps []RevisionResource,
6860
secrets []RevisionResource,
6961
kubeInformersForTargetNamespace informers.SharedInformerFactory,
70-
operatorClient LatestRevisionClient,
62+
operatorClient v1helpers.OperatorClient,
7163
configMapGetter corev1client.ConfigMapsGetter,
7264
secretGetter corev1client.SecretsGetter,
7365
eventRecorder events.Recorder,
@@ -80,9 +72,10 @@ func NewRevisionController(
8072
}
8173

8274
c := &RevisionController{
83-
targetNamespace: targetNamespace,
84-
configMaps: configMaps,
85-
secrets: secrets,
75+
controllerInstanceName: factory.ControllerInstanceName(instanceName, "RevisionController"),
76+
targetNamespace: targetNamespace,
77+
configMaps: configMaps,
78+
secrets: secrets,
8679

8780
operatorClient: operatorClient,
8881
configMapGetter: configMapGetter,
@@ -106,18 +99,17 @@ func NewRevisionController(
10699

107100
// createRevisionIfNeeded takes care of creating content for the static pods to use.
108101
// returns whether or not requeue and if an error happened when updating status. Normally it updates status itself.
109-
func (c RevisionController) createRevisionIfNeeded(ctx context.Context, recorder events.Recorder, latestAvailableRevision int32, resourceVersion string) (bool, error) {
110-
isLatestRevisionCurrent, requiredIsNotFound, reason := c.isLatestRevisionCurrent(ctx, latestAvailableRevision)
102+
func (c RevisionController) createRevisionIfNeeded(ctx context.Context, recorder events.Recorder, currentLastAvailableRevision int32) (wroteStatus bool, requeue bool, err error) {
103+
isLatestRevisionCurrent, requiredIsNotFound, reason := c.isLatestRevisionCurrent(ctx, currentLastAvailableRevision)
111104

112105
// 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
113106
if isLatestRevisionCurrent {
114-
klog.V(4).Infof("Returning early, %d triggered and up to date", latestAvailableRevision)
115-
return false, nil
107+
klog.V(4).Infof("Returning early, %d triggered and up to date", currentLastAvailableRevision)
108+
return false, false, nil
116109
}
117110

118-
nextRevision := latestAvailableRevision + 1
111+
nextRevision := currentLastAvailableRevision + 1
119112
var createdNewRevision bool
120-
var err error
121113
// check to make sure no new revision is created when a required object is missing
122114
if requiredIsNotFound {
123115
err = fmt.Errorf("%v", reason)
@@ -127,36 +119,41 @@ func (c RevisionController) createRevisionIfNeeded(ctx context.Context, recorder
127119
}
128120

129121
if err != nil {
130-
cond := operatorv1.OperatorCondition{
131-
Type: "RevisionControllerDegraded",
132-
Status: operatorv1.ConditionTrue,
133-
Reason: "ContentCreationError",
134-
Message: err.Error(),
135-
}
136-
if _, _, updateError := v1helpers.UpdateStatus(ctx, c.operatorClient, v1helpers.UpdateConditionFn(cond)); updateError != nil {
122+
status := applyoperatorv1.OperatorStatus().
123+
WithConditions(applyoperatorv1.OperatorCondition().
124+
WithType(condition.RevisionControllerDegradedConditionType).
125+
WithStatus(operatorv1.ConditionTrue).
126+
WithReason("ContentCreationError").
127+
WithMessage(err.Error()),
128+
).
129+
WithLatestAvailableRevision(currentLastAvailableRevision)
130+
updateError := c.operatorClient.ApplyOperatorStatus(ctx, c.controllerInstanceName, status)
131+
if updateError != nil {
137132
recorder.Warningf("RevisionCreateFailed", "Failed to create revision %d: %v", nextRevision, err.Error())
138-
return true, updateError
133+
return true, true, updateError
139134
}
140-
return true, nil
135+
return true, true, nil
141136
}
142137

143138
if !createdNewRevision {
144139
klog.V(4).Infof("Revision %v not created", nextRevision)
145-
return false, nil
140+
return false, false, nil
146141
}
142+
147143
recorder.Eventf("RevisionTriggered", "new revision %d triggered by %q", nextRevision, reason)
148144

149-
cond := operatorv1.OperatorCondition{
150-
Type: "RevisionControllerDegraded",
151-
Status: operatorv1.ConditionFalse,
152-
}
153-
if _, updated, updateError := c.operatorClient.UpdateLatestRevisionOperatorStatus(ctx, nextRevision, v1helpers.UpdateConditionFn(cond)); updateError != nil {
154-
return true, updateError
155-
} else if updated {
156-
recorder.Eventf("RevisionCreate", "Revision %d created because %s", nextRevision, reason)
145+
status := applyoperatorv1.OperatorStatus().
146+
WithConditions(applyoperatorv1.OperatorCondition().
147+
WithType(condition.RevisionControllerDegradedConditionType).
148+
WithStatus(operatorv1.ConditionFalse),
149+
).
150+
WithLatestAvailableRevision(nextRevision)
151+
updateError := c.operatorClient.ApplyOperatorStatus(ctx, c.controllerInstanceName, status)
152+
if updateError != nil {
153+
return true, true, updateError
157154
}
158155

159-
return false, nil
156+
return true, false, nil
160157
}
161158

162159
func nameFor(name string, revision int32) string {
@@ -327,6 +324,11 @@ func (c RevisionController) getLatestAvailableRevision(ctx context.Context) (int
327324
var latestRevision int32
328325
for _, configMap := range configMaps.Items {
329326
if !strings.HasPrefix(configMap.Name, "revision-status-") {
327+
// if it's not a revision status configmap, skip
328+
continue
329+
}
330+
if configMap.Annotations["operator.openshift.io/revision-ready"] != "true" {
331+
// we only want to include ready revisions.
330332
continue
331333
}
332334
if revision, ok := configMap.Data["revision"]; ok {
@@ -344,7 +346,7 @@ func (c RevisionController) getLatestAvailableRevision(ctx context.Context) (int
344346
}
345347

346348
func (c RevisionController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
347-
operatorSpec, _, latestAvailableRevisionSeenByOperator, resourceVersion, err := c.operatorClient.GetLatestRevisionState()
349+
operatorSpec, operatorStatus, _, err := c.operatorClient.GetOperatorState()
348350
if err != nil {
349351
return err
350352
}
@@ -353,42 +355,54 @@ func (c RevisionController) sync(ctx context.Context, syncCtx factory.SyncContex
353355
return nil
354356
}
355357

356-
if shouldCreateNewRevision, err := c.revisionPrecondition(ctx); err != nil || !shouldCreateNewRevision {
357-
return err
358-
}
359-
360358
// If the operator status's latest available revision is not the same as the observed latest revision, update the operator.
359+
// This needs to be done even if the revision precondition is not required because it ensures our operator status is
360+
// correct for all consumers.
361+
// This is what is going to allow us to move where the state is stored in authentications.openshift.io.
361362
latestObservedRevision, err := c.getLatestAvailableRevision(ctx)
362363
if err != nil {
363364
return err
364365
}
365-
if latestObservedRevision != 0 && latestAvailableRevisionSeenByOperator != latestObservedRevision {
366+
if latestObservedRevision != 0 && operatorStatus.LatestAvailableRevision != latestObservedRevision {
366367
// Then make sure that revision number is what's in the operator status
367-
_, _, err := c.operatorClient.UpdateLatestRevisionOperatorStatus(ctx, latestObservedRevision)
368+
status := applyoperatorv1.OperatorStatus().
369+
WithConditions(applyoperatorv1.OperatorCondition().
370+
WithType(condition.RevisionControllerDegradedConditionType).
371+
WithStatus(operatorv1.ConditionFalse),
372+
).
373+
WithLatestAvailableRevision(latestObservedRevision)
374+
err := c.operatorClient.ApplyOperatorStatus(ctx, c.controllerInstanceName, status)
368375
if err != nil {
369376
return err
370377
}
371378
// regardless of whether we made a change, requeue to rerun the sync with updated status
372379
return factory.SyntheticRequeueError
373380
}
374381

375-
requeue, syncErr := c.createRevisionIfNeeded(ctx, syncCtx.Recorder(), latestAvailableRevisionSeenByOperator, resourceVersion)
376-
if requeue && syncErr == nil {
382+
if shouldCreateNewRevision, err := c.revisionPrecondition(ctx); err != nil || !shouldCreateNewRevision {
383+
return err
384+
}
385+
386+
wroteStatus, requeue, syncErr := c.createRevisionIfNeeded(ctx, syncCtx.Recorder(), operatorStatus.LatestAvailableRevision)
387+
switch {
388+
case requeue && syncErr == nil:
377389
return factory.SyntheticRequeueError
390+
case syncErr != nil:
391+
// this is updated in status inside of createRevisionIfNeeded
392+
return syncErr
393+
case wroteStatus:
394+
return syncErr
378395
}
379-
err = syncErr
380396

381397
// update failing condition
382-
cond := operatorv1.OperatorCondition{
383-
Type: condition.RevisionControllerDegradedConditionType,
384-
Status: operatorv1.ConditionFalse,
385-
}
386-
if err != nil {
387-
cond.Status = operatorv1.ConditionTrue
388-
cond.Reason = "Error"
389-
cond.Message = err.Error()
390-
}
391-
if _, _, updateError := v1helpers.UpdateStatus(ctx, c.operatorClient, v1helpers.UpdateConditionFn(cond)); updateError != nil {
398+
status := applyoperatorv1.OperatorStatus().
399+
WithConditions(applyoperatorv1.OperatorCondition().
400+
WithType(condition.RevisionControllerDegradedConditionType).
401+
WithStatus(operatorv1.ConditionFalse),
402+
).
403+
WithLatestAvailableRevision(operatorStatus.LatestAvailableRevision)
404+
updateError := c.operatorClient.ApplyOperatorStatus(ctx, c.controllerInstanceName, status)
405+
if updateError != nil {
392406
if err == nil {
393407
return updateError
394408
}

0 commit comments

Comments
 (0)