Skip to content

Commit cc490bf

Browse files
pedjakpedjak
committed
Add ClusterExtensionRevision MarkAs(Progressing|NotProgressing|Available|Unavailable) helper methods for improved code readability
1 parent b43d206 commit cc490bf

File tree

2 files changed

+60
-73
lines changed

2 files changed

+60
-73
lines changed

‎api/v1/clusterextensionrevision_types.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package v1
1818

1919
import (
20+
"k8s.io/apimachinery/pkg/api/meta"
2021
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2122
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2223
"k8s.io/apimachinery/pkg/types"
@@ -38,6 +39,7 @@ const (
3839
ClusterExtensionRevisionReasonObjectCollisions = "ObjectCollisions"
3940
ClusterExtensionRevisionReasonRolloutSuccess = "RolloutSuccess"
4041
ClusterExtensionRevisionReasonProbeFailure = "ProbeFailure"
42+
ClusterExtensionRevisionReasonProbesSucceeded = "ProbesSucceeded"
4143
ClusterExtensionRevisionReasonIncomplete = "Incomplete"
4244
ClusterExtensionRevisionReasonProgressing = "Progressing"
4345
ClusterExtensionRevisionReasonArchived = "Archived"
@@ -169,6 +171,46 @@ type ClusterExtensionRevision struct {
169171
Status ClusterExtensionRevisionStatus `json:"status,omitempty"`
170172
}
171173

174+
func (cer *ClusterExtensionRevision) MarkAsProgressing(reason, message string) {
175+
meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{
176+
Type: ClusterExtensionRevisionTypeProgressing,
177+
Status: metav1.ConditionTrue,
178+
Reason: reason,
179+
Message: message,
180+
ObservedGeneration: cer.Generation,
181+
})
182+
}
183+
184+
func (cer *ClusterExtensionRevision) MarkAsNotProgressing(reason, message string) {
185+
meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{
186+
Type: ClusterExtensionRevisionTypeProgressing,
187+
Status: metav1.ConditionFalse,
188+
Reason: reason,
189+
Message: message,
190+
ObservedGeneration: cer.Generation,
191+
})
192+
}
193+
194+
func (cer *ClusterExtensionRevision) MarkAsAvailable(reason, message string) {
195+
meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{
196+
Type: ClusterExtensionRevisionTypeAvailable,
197+
Status: metav1.ConditionTrue,
198+
Reason: reason,
199+
Message: message,
200+
ObservedGeneration: cer.Generation,
201+
})
202+
}
203+
204+
func (cer *ClusterExtensionRevision) MarkAsUnavailable(reason, message string) {
205+
meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{
206+
Type: ClusterExtensionRevisionTypeAvailable,
207+
Status: metav1.ConditionFalse,
208+
Reason: reason,
209+
Message: message,
210+
ObservedGeneration: cer.Generation,
211+
})
212+
}
213+
172214
// +kubebuilder:object:root=true
173215

174216
// ClusterExtensionRevisionList contains a list of ClusterExtensionRevision

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

Lines changed: 18 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -129,40 +129,25 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
129129
return ctrl.Result{}, fmt.Errorf("error ensuring teardown finalizer: %v", err)
130130
}
131131

132+
// If the Available condition is not present, we are still rolling out the objects
132133
inRollout := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) == nil
133134
if inRollout {
134135
if err := c.establishWatch(ctx, rev, revision); err != nil {
135136
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-
})
137+
// this error is very likely transient, so we should keep revision as progressing
138+
rev.MarkAsProgressing(ocv1.ClusterExtensionRevisionReasonReconcileFailure, werr.Error())
143139
return ctrl.Result{}, werr
144140
}
145-
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
146-
Type: ocv1.ClusterExtensionRevisionTypeProgressing,
147-
Status: metav1.ConditionTrue,
148-
Reason: ocv1.ClusterExtensionRevisionReasonRolloutInProgress,
149-
Message: "Revision is being rolled out.",
150-
ObservedGeneration: rev.Generation,
151-
})
141+
rev.MarkAsProgressing(ocv1.ClusterExtensionRevisionReasonRolloutInProgress, "Revision is being rolled out.")
152142
}
153143

154144
rres, err := c.RevisionEngine.Reconcile(ctx, *revision, opts...)
155145
if err != nil {
156146
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-
})
147+
rev.MarkAsProgressing(ocv1.ClusterExtensionRevisionReasonRolloutError, err.Error())
164148
} else {
165149
// it is a probably transient error, and we do not know if the revision is available or not
150+
// perhaps we should not report it at all, hoping that it is going to be mitigated in the next reconcile?
166151
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
167152
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
168153
Status: metav1.ConditionUnknown,
@@ -181,16 +166,11 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
181166
l.Info("preflight error, retrying after 10s", "err", verr.String())
182167

183168
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-
})
169+
// given that we retry, we are going to keep Progressing condition True
170+
rev.MarkAsProgressing(ocv1.ClusterExtensionRevisionReasonRevisionValidationFailure, fmt.Sprintf("revision validation error: %s", verr))
192171
} else {
193172
// it is a probably transient error, and we do not know if the revision is available or not
173+
// perhaps we should not report it at all, hoping that it is going to be mitigated in the next reconcile?
194174
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
195175
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
196176
Status: metav1.ConditionUnknown,
@@ -207,16 +187,11 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
207187
l.Info("preflight error, retrying after 10s", "err", verr.String())
208188

209189
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-
})
190+
// given that we retry, we are going to keep Progressing condition True
191+
rev.MarkAsProgressing(ocv1.ClusterExtensionRevisionReasonPhaseValidationError, fmt.Sprintf("phase %d validation error: %s", i, verr))
218192
} else {
219193
// it is a probably transient error, and we do not know if the revision is available or not
194+
// perhaps we should not report it at all, hoping that it is going to be mitigated in the next reconcile?
220195
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
221196
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
222197
Status: metav1.ConditionUnknown,
@@ -240,29 +215,17 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
240215
// collisions are probably stickier than phase roll out probe failures - so we'd probably want to set
241216
// Progressing to false here due to the collision
242217
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-
})
218+
rev.MarkAsNotProgressing(ocv1.ClusterExtensionRevisionReasonObjectCollisions, fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")))
250219

251-
// not sure if we want to retry here - collisions are probably not transient?
220+
// NOTE(pedjak): not sure if we want to retry here - collisions are probably not transient?
252221
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
253222
}
254223
}
255224
}
256225

257226
if !rres.InTransistion() {
258227
// 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-
})
228+
rev.MarkAsNotProgressing(ocv1.ClusterExtensionRevisionReasonRolledOut, "Revision is rolled out.")
266229
}
267230

268231
//nolint:nestif
@@ -280,13 +243,7 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
280243

281244
// It would be good to understand from Nico how we can distinguish between progression and availability probes
282245
// and how to best check that all Availability probes are passing
283-
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
284-
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
285-
Status: metav1.ConditionTrue,
286-
Reason: ocv1.ClusterExtensionRevisionReasonAvailable,
287-
Message: "Objects are available and passes all probes.",
288-
ObservedGeneration: rev.Generation,
289-
})
246+
rev.MarkAsAvailable(ocv1.ClusterExtensionRevisionReasonProbesSucceeded, "Objects are available and pass all probes.")
290247

291248
// We'll probably only want to remove this once we are done updating the ClusterExtension conditions
292249
// as its one of the interfaces between the revision and the extension. If we still have the Succeeded for now
@@ -325,21 +282,9 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
325282
}
326283
}
327284
if len(probeFailureMsgs) > 0 {
328-
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
329-
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
330-
Status: metav1.ConditionFalse,
331-
Reason: ocv1.ClusterExtensionRevisionReasonProbeFailure,
332-
Message: strings.Join(probeFailureMsgs, "\n"),
333-
ObservedGeneration: rev.Generation,
334-
})
285+
rev.MarkAsUnavailable(ocv1.ClusterExtensionRevisionReasonProbeFailure, strings.Join(probeFailureMsgs, "\n"))
335286
} else {
336-
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
337-
Type: ocv1.ClusterExtensionRevisionTypeAvailable,
338-
Status: metav1.ConditionFalse,
339-
Reason: ocv1.ClusterExtensionRevisionReasonIncomplete,
340-
Message: "Revision has not been rolled out completely.",
341-
ObservedGeneration: rev.Generation,
342-
})
287+
rev.MarkAsUnavailable(ocv1.ClusterExtensionRevisionReasonIncomplete, "Revision has not been rolled out completely.")
343288
}
344289
}
345290

0 commit comments

Comments
 (0)