Skip to content

Commit be82af5

Browse files
abaysopenshift-merge-bot[bot]
abays
authored and
openshift-merge-bot[bot]
committed
[OSPRH-20473] Improve consistency of condition severity usage
Use consistent condition severity across repeated patterns found in our operators, and otherwise use an appropriate severity for conditions unique to each operator. Jira: https://issues.redhat.com/browse/OSPRH-20473 Co-authored-by: Claude (Anthropic) claude@anthropic.com
1 parent 9193f53 commit be82af5

7 files changed

Lines changed: 27 additions & 18 deletions

‎controllers/watcher_common.go‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,11 +234,13 @@ func ensureSecret(
234234
err := reader.Get(ctx, secretName, secret)
235235
if err != nil {
236236
if k8s_errors.IsNotFound(err) {
237+
// Since secrets should have been manually created by the user and referenced in the spec,
238+
// we treat this as a warning because it means that the service will not be able to start.
237239
log.FromContext(ctx).Info(fmt.Sprintf("secret %s not found", secretName))
238240
conditionUpdater.Set(condition.FalseCondition(
239241
condition.InputReadyCondition,
240-
condition.RequestedReason,
241-
condition.SeverityInfo,
242+
condition.ErrorReason,
243+
condition.SeverityWarning,
242244
condition.InputReadyWaitingMessage))
243245
return "",
244246
ctrl.Result{RequeueAfter: requeueTimeout},
@@ -338,10 +340,15 @@ func ensureMemcached(
338340
memcached, err := memcachedv1.GetMemcachedByName(ctx, helper, memcachedName, namespaceName)
339341
if err != nil {
340342
if k8s_errors.IsNotFound(err) {
343+
// Memcached should be automatically created by the encompassing OpenStackControlPlane,
344+
// but we don't propagate its name into the "memcachedInstance" field of other sub-resources,
345+
// so if it is missing at this point, it *could* be because there's a mismatch between the
346+
// name of the Memcached CR and the name of the Memcached instance referenced by this CR.
347+
// Since that situation would block further reconciliation, we treat it as a warning.
341348
conditionUpdater.Set(condition.FalseCondition(
342349
condition.MemcachedReadyCondition,
343-
condition.RequestedReason,
344-
condition.SeverityInfo,
350+
condition.ErrorReason,
351+
condition.SeverityWarning,
345352
condition.MemcachedReadyWaitingMessage))
346353
return nil, fmt.Errorf("%w: %s", ErrMemcachedNotFound, memcachedName)
347354
}

‎controllers/watcher_controller.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ func (r *WatcherReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
219219
Log.Info(fmt.Sprintf("Waiting for TransportURL for %s to be created", instance.Name))
220220
instance.Status.Conditions.Set(condition.FalseCondition(
221221
watcherv1beta1.WatcherRabbitMQTransportURLReadyCondition,
222-
condition.RequestedReason,
222+
condition.ErrorReason,
223223
condition.SeverityWarning,
224224
watcherv1beta1.WatcherRabbitMQTransportURLReadyRunningMessage))
225225
return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
@@ -333,7 +333,7 @@ func (r *WatcherReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
333333
// Empty hash means that there is some problem retrieving the key from the secret
334334
instance.Status.Conditions.Set(condition.FalseCondition(
335335
condition.InputReadyCondition,
336-
condition.RequestedReason,
336+
condition.ErrorReason,
337337
condition.SeverityWarning,
338338
watcherv1beta1.WatcherPrometheusSecretErrorMessage))
339339
return ctrl.Result{}, ErrRetrievingPrometheusSecretData
@@ -345,7 +345,7 @@ func (r *WatcherReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
345345
if err != nil {
346346
instance.Status.Conditions.Set(condition.FalseCondition(
347347
condition.InputReadyCondition,
348-
condition.RequestedReason,
348+
condition.ErrorReason,
349349
condition.SeverityWarning,
350350
watcherv1beta1.WatcherPrometheusSecretErrorMessage))
351351
return ctrl.Result{}, err

‎controllers/watcherapi_controller.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ func (r *WatcherAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request)
201201
// Empty hash means that there is some problem retrieving the key from the secret
202202
instance.Status.Conditions.Set(condition.FalseCondition(
203203
condition.InputReadyCondition,
204-
condition.RequestedReason,
204+
condition.ErrorReason,
205205
condition.SeverityWarning,
206206
watcherv1beta1.WatcherPrometheusSecretErrorMessage))
207207
return ctrl.Result{}, ErrRetrievingPrometheusSecretData
@@ -276,10 +276,12 @@ func (r *WatcherAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request)
276276
)
277277
if err != nil {
278278
if k8s_errors.IsNotFound(err) {
279+
// Since the CA cert secret should have been manually created by the user and provided in the spec,
280+
// we treat this as a warning because it means that the service will not be able to start.
279281
instance.Status.Conditions.Set(condition.FalseCondition(
280282
condition.TLSInputReadyCondition,
281-
condition.RequestedReason,
282-
condition.SeverityInfo,
283+
condition.ErrorReason,
284+
condition.SeverityWarning,
283285
condition.TLSInputReadyWaitingMessage, instance.Spec.TLS.CaBundleSecretName,
284286
))
285287
return ctrl.Result{}, nil

‎controllers/watcherdecisionengine_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ func (r *WatcherDecisionEngineReconciler) Reconcile(ctx context.Context, req ctr
194194
// Empty hash means that there is some problem retrieving the key from the secret
195195
instance.Status.Conditions.Set(condition.FalseCondition(
196196
condition.InputReadyCondition,
197-
condition.RequestedReason,
197+
condition.ErrorReason,
198198
condition.SeverityWarning,
199199
watcherv1beta1.WatcherPrometheusSecretErrorMessage))
200200
return ctrl.Result{}, ErrRetrievingPrometheusSecretData

‎tests/functional/watcher_controller_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ var _ = Describe("Watcher controller", func() {
556556
ConditionGetterFunc(WatcherConditionGetter),
557557
condition.InputReadyCondition,
558558
corev1.ConditionFalse,
559-
condition.RequestedReason,
559+
condition.ErrorReason,
560560
"Input data resources missing",
561561
)
562562

@@ -1195,7 +1195,7 @@ var _ = Describe("Watcher controller", func() {
11951195
ConditionGetterFunc(WatcherConditionGetter),
11961196
condition.InputReadyCondition,
11971197
corev1.ConditionFalse,
1198-
condition.RequestedReason,
1198+
condition.ErrorReason,
11991199
"Error with prometheus config secret",
12001200
)
12011201
})

‎tests/functional/watcherapi_controller_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ transport_url =`
372372
ConditionGetterFunc(WatcherAPIConditionGetter),
373373
condition.InputReadyCondition,
374374
corev1.ConditionFalse,
375-
condition.RequestedReason,
375+
condition.ErrorReason,
376376
condition.InputReadyWaitingMessage,
377377
)
378378
})
@@ -406,7 +406,7 @@ transport_url =`
406406
ConditionGetterFunc(WatcherAPIConditionGetter),
407407
condition.MemcachedReadyCondition,
408408
corev1.ConditionFalse,
409-
condition.RequestedReason,
409+
condition.ErrorReason,
410410
condition.MemcachedReadyWaitingMessage,
411411
)
412412
})
@@ -425,7 +425,7 @@ transport_url =`
425425
ConditionGetterFunc(WatcherAPIConditionGetter),
426426
condition.InputReadyCondition,
427427
corev1.ConditionFalse,
428-
condition.RequestedReason,
428+
condition.ErrorReason,
429429
watcherv1beta1.WatcherPrometheusSecretErrorMessage,
430430
)
431431
})

‎tests/functional/watcherapplier_controller_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ transport_url =`
325325
ConditionGetterFunc(WatcherApplierConditionGetter),
326326
condition.InputReadyCondition,
327327
corev1.ConditionFalse,
328-
condition.RequestedReason,
328+
condition.ErrorReason,
329329
condition.InputReadyWaitingMessage,
330330
)
331331
})
@@ -351,7 +351,7 @@ transport_url =`
351351
ConditionGetterFunc(WatcherApplierConditionGetter),
352352
condition.MemcachedReadyCondition,
353353
corev1.ConditionFalse,
354-
condition.RequestedReason,
354+
condition.ErrorReason,
355355
condition.MemcachedReadyWaitingMessage,
356356
)
357357
})

0 commit comments

Comments
 (0)