Skip to content

Commit b43d206

Browse files
pedjakpedjak
committed
WIP Introduce Progressing condition
1 parent 5b2d1da commit b43d206

File tree

5 files changed

+117
-76
lines changed

5 files changed

+117
-76
lines changed

‎api/v1/clusterextensionrevision_types.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@ const (
4141
ClusterExtensionRevisionReasonIncomplete = "Incomplete"
4242
ClusterExtensionRevisionReasonProgressing = "Progressing"
4343
ClusterExtensionRevisionReasonArchived = "Archived"
44-
ClusterExtensionRevisionReasonRolloutInProgress = "RolloutInProgress"
44+
ClusterExtensionRevisionReasonRolloutInProgress = "RollingOut"
45+
ClusterExtensionRevisionReasonRolloutError = "RolloutError"
46+
ClusterExtensionRevisionReasonRolledOut = "RolledOut"
4547
)
4648

4749
// ClusterExtensionRevisionSpec defines the desired state of ClusterExtensionRevision.
@@ -152,6 +154,7 @@ type ClusterExtensionRevisionStatus struct {
152154

153155
// ClusterExtensionRevision is the Schema for the clusterextensionrevisions API
154156
// +kubebuilder:printcolumn:name="Available",type=string,JSONPath=`.status.conditions[?(@.type=='Available')].status`
157+
// +kubebuilder:printcolumn:name="Progressing",type=string,JSONPath=`.status.conditions[?(@.type=='Progressing')].status`
155158
// +kubebuilder:printcolumn:name=Age,type=date,JSONPath=`.metadata.creationTimestamp`
156159
type ClusterExtensionRevision struct {
157160
metav1.TypeMeta `json:",inline"`

‎helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ spec:
1919
- jsonPath: .status.conditions[?(@.type=='Available')].status
2020
name: Available
2121
type: string
22+
- jsonPath: .status.conditions[?(@.type=='Progressing')].status
23+
name: Progressing
24+
type: string
2225
- jsonPath: .metadata.creationTimestamp
2326
name: Age
2427
type: date

‎internal/operator-controller/controllers/clusterextensionrevision_controller.go‎

Lines changed: 104 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -126,36 +126,51 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
126126
// Reconcile
127127
//
128128
if err := c.ensureFinalizer(ctx, rev, clusterExtensionRevisionTeardownFinalizer); err != nil {
129-
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
130-
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
131-
Status: metav1.ConditionFalse,
132-
Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure,
133-
Message: err.Error(),
134-
ObservedGeneration: rev.Generation,
135-
})
136129
return ctrl.Result{}, fmt.Errorf("error ensuring teardown finalizer: %v", err)
137130
}
138131

139-
if err := c.establishWatch(ctx, rev, revision); err != nil {
132+
inRollout := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) == nil
133+
if inRollout {
134+
if err := c.establishWatch(ctx, rev, revision); err != nil {
135+
werr := fmt.Errorf("establish watch: %v", err)
136+
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
137+
Type: ocv1.ClusterExtensionRevisionTypeProgressing,
138+
Status: metav1.ConditionTrue,
139+
Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure,
140+
Message: werr.Error(),
141+
ObservedGeneration: rev.Generation,
142+
})
143+
return ctrl.Result{}, werr
144+
}
140145
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
141-
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
142-
Status: metav1.ConditionFalse,
143-
Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure,
144-
Message: err.Error(),
146+
Type: ocv1.ClusterExtensionRevisionTypeProgressing,
147+
Status: metav1.ConditionTrue,
148+
Reason: ocv1.ClusterExtensionRevisionReasonRolloutInProgress,
149+
Message: "Revision is being rolled out.",
145150
ObservedGeneration: rev.Generation,
146151
})
147-
return ctrl.Result{}, fmt.Errorf("establish watch: %v", err)
148152
}
149153

150154
rres, err := c.RevisionEngine.Reconcile(ctx, *revision, opts...)
151155
if err != nil {
152-
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
153-
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
154-
Status: metav1.ConditionFalse,
155-
Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure,
156-
Message: err.Error(),
157-
ObservedGeneration: rev.Generation,
158-
})
156+
if inRollout {
157+
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
158+
Type: ocv1.ClusterExtensionRevisionTypeProgressing,
159+
Status: metav1.ConditionTrue,
160+
Reason: ocv1.ClusterExtensionRevisionReasonRolloutError,
161+
Message: err.Error(),
162+
ObservedGeneration: rev.Generation,
163+
})
164+
} else {
165+
// it is a probably transient error, and we do not know if the revision is available or not
166+
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
167+
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
168+
Status: metav1.ConditionUnknown,
169+
Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure,
170+
Message: err.Error(),
171+
ObservedGeneration: rev.Generation,
172+
})
173+
}
159174
return ctrl.Result{}, fmt.Errorf("revision reconcile: %v", err)
160175
}
161176
l.Info("reconcile report", "report", rres.String())
@@ -165,32 +180,51 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
165180
if verr := rres.GetValidationError(); verr != nil {
166181
l.Info("preflight error, retrying after 10s", "err", verr.String())
167182

168-
// if we take Availability as strictly being "all availability probes pass"
169-
// we should probably be at an Unknown state (or not posted it at all here)
170-
// and post this up in the Progressing condition as False with a failed rollout reason / message
171-
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
172-
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
173-
Status: metav1.ConditionFalse,
174-
Reason: ocv1.ClusterExtensionRevisionReasonRevisionValidationFailure,
175-
Message: fmt.Sprintf("revision validation error: %s", verr),
176-
ObservedGeneration: rev.Generation,
177-
})
183+
if inRollout {
184+
// given that we retry, we are not going to keep Progressing condition True
185+
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
186+
Type: ocv1.ClusterExtensionRevisionTypeProgressing,
187+
Status: metav1.ConditionTrue,
188+
Reason: ocv1.ClusterExtensionRevisionReasonRevisionValidationFailure,
189+
Message: fmt.Sprintf("revision validation error: %s", verr),
190+
ObservedGeneration: rev.Generation,
191+
})
192+
} else {
193+
// it is a probably transient error, and we do not know if the revision is available or not
194+
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
195+
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
196+
Status: metav1.ConditionUnknown,
197+
Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure,
198+
Message: fmt.Sprintf("revision validation error: %s", verr),
199+
ObservedGeneration: rev.Generation,
200+
})
201+
}
178202
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
179203
}
180204

181205
for i, pres := range rres.GetPhases() {
182206
if verr := pres.GetValidationError(); verr != nil {
183207
l.Info("preflight error, retrying after 10s", "err", verr.String())
184208

185-
// we probably want to either eat this error and leave Progressing as True with a rolling out reason and implement timeout
186-
// or surface this error through a Degraded-type condition that can flap as roll out continues
187-
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
188-
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
189-
Status: metav1.ConditionFalse,
190-
Reason: ocv1.ClusterExtensionRevisionReasonPhaseValidationError,
191-
Message: fmt.Sprintf("phase %d validation error: %s", i, verr),
192-
ObservedGeneration: rev.Generation,
193-
})
209+
if inRollout {
210+
// given that we retry, we are not going to keep Progressing condition True
211+
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
212+
Type: ocv1.ClusterExtensionRevisionTypeProgressing,
213+
Status: metav1.ConditionTrue,
214+
Reason: ocv1.ClusterExtensionRevisionReasonPhaseValidationError,
215+
Message: fmt.Sprintf("phase %d validation error: %s", i, verr),
216+
ObservedGeneration: rev.Generation,
217+
})
218+
} else {
219+
// it is a probably transient error, and we do not know if the revision is available or not
220+
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
221+
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
222+
Status: metav1.ConditionUnknown,
223+
Reason: ocv1.ClusterExtensionRevisionReasonPhaseValidationError,
224+
Message: fmt.Sprintf("phase %d validation error: %s", i, verr),
225+
ObservedGeneration: rev.Generation,
226+
})
227+
}
194228
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
195229
}
196230

@@ -205,19 +239,32 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
205239
l.Info("object collision error, retrying after 10s", "collisions", collidingObjs)
206240
// collisions are probably stickier than phase roll out probe failures - so we'd probably want to set
207241
// Progressing to false here due to the collision
208-
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
209-
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
210-
Status: metav1.ConditionFalse,
211-
Reason: ocv1.ClusterExtensionRevisionReasonObjectCollisions,
212-
Message: fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")),
213-
ObservedGeneration: rev.Generation,
214-
})
215-
216-
// idk if we want to return yet or check the Availability probes on a best-effort basis
217-
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
242+
if inRollout {
243+
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
244+
Type: ocv1.ClusterExtensionRevisionTypeProgressing,
245+
Status: metav1.ConditionFalse,
246+
Reason: ocv1.ClusterExtensionRevisionReasonObjectCollisions,
247+
Message: fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")),
248+
ObservedGeneration: rev.Generation,
249+
})
250+
251+
// not sure if we want to retry here - collisions are probably not transient?
252+
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
253+
}
218254
}
219255
}
220256

257+
if !rres.InTransistion() {
258+
// we have rolled out all objects in all phases, not interested in probes here
259+
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
260+
Type: ocv1.ClusterExtensionRevisionTypeProgressing,
261+
Status: metav1.ConditionFalse,
262+
Reason: ocv1.ClusterExtensionRevisionReasonRolledOut,
263+
Message: "Revision is rolled out.",
264+
ObservedGeneration: rev.Generation,
265+
})
266+
}
267+
221268
//nolint:nestif
222269
if rres.IsComplete() {
223270
// Archive other revisions.
@@ -231,30 +278,26 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
231278
}
232279
}
233280

234-
// Report status.
235-
// We probably want to set Progressing to False here with a rollout success reason and message
236281
// It would be good to understand from Nico how we can distinguish between progression and availability probes
237282
// and how to best check that all Availability probes are passing
238283
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
239284
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
240285
Status: metav1.ConditionTrue,
241286
Reason: ocv1.ClusterExtensionRevisionReasonAvailable,
242-
Message: "Object is available and passes all probes.",
287+
Message: "Objects are available and passes all probes.",
243288
ObservedGeneration: rev.Generation,
244289
})
245290

246291
// We'll probably only want to remove this once we are done updating the ClusterExtension conditions
247292
// as its one of the interfaces between the revision and the extension. If we still have the Succeeded for now
248293
// that's fine.
249-
if !meta.IsStatusConditionTrue(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded) {
250-
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
251-
Type: ocv1.ClusterExtensionRevisionTypeSucceeded,
252-
Status: metav1.ConditionTrue,
253-
Reason: ocv1.ClusterExtensionRevisionReasonRolloutSuccess,
254-
Message: "Revision succeeded rolling out.",
255-
ObservedGeneration: rev.Generation,
256-
})
257-
}
294+
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
295+
Type: ocv1.ClusterExtensionRevisionTypeSucceeded,
296+
Status: metav1.ConditionTrue,
297+
Reason: ocv1.ClusterExtensionRevisionReasonRolloutSuccess,
298+
Message: "Revision succeeded rolling out.",
299+
ObservedGeneration: rev.Generation,
300+
})
258301
} else {
259302
var probeFailureMsgs []string
260303
for _, pres := range rres.GetPhases() {
@@ -290,7 +333,6 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
290333
ObservedGeneration: rev.Generation,
291334
})
292335
} else {
293-
// either to nothing or set the Progressing condition to True with rolling out reason / message
294336
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
295337
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
296338
Status: metav1.ConditionFalse,
@@ -300,19 +342,6 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
300342
})
301343
}
302344
}
303-
if rres.InTransistion() {
304-
// seems to be the right thing XD
305-
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
306-
Type: ocv1.TypeProgressing,
307-
Status: metav1.ConditionTrue,
308-
Reason: ocv1.ClusterExtensionRevisionReasonProgressing,
309-
Message: "Rollout in progress.",
310-
ObservedGeneration: rev.Generation,
311-
})
312-
} else {
313-
// we may not want to remove it but rather set it to False?
314-
meta.RemoveStatusCondition(&rev.Status.Conditions, ocv1.TypeProgressing)
315-
}
316345

317346
return ctrl.Result{}, nil
318347
}

‎manifests/experimental-e2e.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,9 @@ spec:
610610
- jsonPath: .status.conditions[?(@.type=='Available')].status
611611
name: Available
612612
type: string
613+
- jsonPath: .status.conditions[?(@.type=='Progressing')].status
614+
name: Progressing
615+
type: string
613616
- jsonPath: .metadata.creationTimestamp
614617
name: Age
615618
type: date

‎manifests/experimental.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,9 @@ spec:
575575
- jsonPath: .status.conditions[?(@.type=='Available')].status
576576
name: Available
577577
type: string
578+
- jsonPath: .status.conditions[?(@.type=='Progressing')].status
579+
name: Progressing
580+
type: string
578581
- jsonPath: .metadata.creationTimestamp
579582
name: Age
580583
type: date

0 commit comments

Comments
 (0)