Skip to content

Commit 83668e9

Browse files
abaysClaude (Anthropic)
andcommitted
[OSPRH-20452] 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-20452 Co-authored-by: Claude (Anthropic) <[email protected]>
1 parent fb90e2d commit 83668e9

File tree

5 files changed

+47
-18
lines changed

5 files changed

+47
-18
lines changed

controllers/aodh_controller.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -536,10 +536,12 @@ func (r *AutoscalingReconciler) reconcileNormalAodh(
536536
)
537537
if err != nil {
538538
if k8s_errors.IsNotFound(err) {
539+
// Since the CA cert secret should have been manually created by the user and provided in the spec,
540+
// we treat this as a warning because it means that the service will not be able to start.
539541
instance.Status.Conditions.Set(condition.FalseCondition(
540542
condition.TLSInputReadyCondition,
541-
condition.RequestedReason,
542-
condition.SeverityInfo,
543+
condition.ErrorReason,
544+
condition.SeverityWarning,
543545
condition.TLSInputReadyWaitingMessage, instance.Spec.Aodh.TLS.CaBundleSecretName))
544546
return ctrl.Result{}, nil
545547
}

controllers/autoscaling_controller.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -373,11 +373,16 @@ func (r *AutoscalingReconciler) reconcileNormal(
373373
memcached, err := memcachedv1.GetMemcachedByName(ctx, helper, instance.Spec.Aodh.MemcachedInstance, instance.Namespace)
374374
if err != nil {
375375
if k8s_errors.IsNotFound(err) {
376+
// Memcached should be automatically created by the encompassing OpenStackControlPlane,
377+
// but we don't propagate its name into the "memcachedInstance" field of other sub-resources,
378+
// so if it is missing at this point, it *could* be because there's a mismatch between the
379+
// name of the Memcached CR and the name of the Memcached instance referenced by this CR.
380+
// Since that situation would block further reconciliation, we treat it as a warning.
376381
Log.Info(fmt.Sprintf("memcached %s not found", instance.Spec.Aodh.MemcachedInstance))
377382
instance.Status.Conditions.Set(condition.FalseCondition(
378383
condition.MemcachedReadyCondition,
379-
condition.RequestedReason,
380-
condition.SeverityInfo,
384+
condition.ErrorReason,
385+
condition.SeverityWarning,
381386
condition.MemcachedReadyWaitingMessage))
382387
return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
383388
}
@@ -410,11 +415,16 @@ func (r *AutoscalingReconciler) reconcileNormal(
410415
heat, err := r.getAutoscalingHeat(ctx, helper, instance)
411416
if err != nil {
412417
if k8s_errors.IsNotFound(err) {
418+
// Heat should be automatically created by the encompassing OpenStackControlPlane,
419+
// but if it is missing at this point, it could be because there's a mismatch between the
420+
// name of the Heat CR and the name referenced in the autoscaling instance spec, or
421+
// that the user somehow disabled the Heat service. Since that situation would block
422+
// further reconciliation, we treat it as a warning.
413423
Log.Info(fmt.Sprintf("heat %s not found", instance.Spec.HeatInstance))
414424
instance.Status.Conditions.Set(condition.FalseCondition(
415425
telemetryv1.HeatReadyCondition,
416-
condition.RequestedReason,
417-
condition.SeverityInfo,
426+
condition.ErrorReason,
427+
condition.SeverityWarning,
418428
telemetryv1.HeatReadyNotFoundMessage))
419429
return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
420430
}

controllers/ceilometer_controller.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -617,10 +617,12 @@ func (r *CeilometerReconciler) reconcileCeilometer(
617617
)
618618
if err != nil {
619619
if k8s_errors.IsNotFound(err) {
620+
// Since the CA cert secret should have been manually created by the user and provided in the spec,
621+
// we treat this as a warning because it means that the service will not be able to start.
620622
instance.Status.Conditions.Set(condition.FalseCondition(
621623
condition.TLSInputReadyCondition,
622-
condition.RequestedReason,
623-
condition.SeverityInfo,
624+
condition.ErrorReason,
625+
condition.SeverityWarning,
624626
condition.TLSInputReadyWaitingMessage, instance.Spec.TLS.CaBundleSecretName))
625627
return ctrl.Result{}, nil
626628
}
@@ -847,10 +849,12 @@ func (r *CeilometerReconciler) reconcileMysqldExporter(
847849
)
848850
if err != nil {
849851
if k8s_errors.IsNotFound(err) {
852+
// Since the CA cert secret should have been manually created by the user and provided in the spec,
853+
// we treat this as a warning because it means that the service will not be able to start.
850854
instance.Status.Conditions.Set(condition.FalseCondition(
851855
telemetryv1.MysqldExporterTLSInputReadyCondition,
852-
condition.RequestedReason,
853-
condition.SeverityInfo,
856+
condition.ErrorReason,
857+
condition.SeverityWarning,
854858
condition.TLSInputReadyWaitingMessage, instance.Spec.MysqldExporterTLS.CaBundleSecretName))
855859
return ctrl.Result{}, nil
856860
}
@@ -873,6 +877,8 @@ func (r *CeilometerReconciler) reconcileMysqldExporter(
873877
hash, err := instance.Spec.MysqldExporterTLS.ValidateCertSecret(ctx, helper, instance.Namespace)
874878
if err != nil {
875879
if k8s_errors.IsNotFound(err) {
880+
// Since the TLS cert secret should have been automatically created by the encompassing OpenStackControlPlane,
881+
// we treat this as an info.
876882
instance.Status.Conditions.Set(condition.FalseCondition(
877883
telemetryv1.MysqldExporterTLSInputReadyCondition,
878884
condition.RequestedReason,
@@ -1026,10 +1032,12 @@ func (r *CeilometerReconciler) reconcileKSM(
10261032
)
10271033
if err != nil {
10281034
if k8s_errors.IsNotFound(err) {
1035+
// Since the CA cert secret should have been manually created by the user and provided in the spec,
1036+
// we treat this as a warning because it means that the service will not be able to start.
10291037
instance.Status.Conditions.Set(condition.FalseCondition(
10301038
telemetryv1.KSMTLSInputReadyCondition,
1031-
condition.RequestedReason,
1032-
condition.SeverityInfo,
1039+
condition.ErrorReason,
1040+
condition.SeverityWarning,
10331041
condition.TLSInputReadyWaitingMessage, instance.Spec.KSMTLS.CaBundleSecretName))
10341042
return ctrl.Result{}, nil
10351043
}

controllers/metricstorage_controller.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -481,10 +481,12 @@ func (r *MetricStorageReconciler) reconcileNormal(
481481
)
482482
if err != nil {
483483
if k8s_errors.IsNotFound(err) {
484+
// Since the CA cert secret should have been manually created by the user and provided in the spec,
485+
// we treat this as a warning because it means that the service will not be able to start.
484486
instance.Status.Conditions.Set(condition.FalseCondition(
485487
condition.TLSInputReadyCondition,
486-
condition.RequestedReason,
487-
condition.SeverityInfo,
488+
condition.ErrorReason,
489+
condition.SeverityWarning,
488490
condition.TLSInputReadyWaitingMessage, instance.Spec.PrometheusTLS.CaBundleSecretName))
489491
return ctrl.Result{}, nil
490492
}
@@ -503,6 +505,8 @@ func (r *MetricStorageReconciler) reconcileNormal(
503505
_, err := instance.Spec.PrometheusTLS.ValidateCertSecret(ctx, helper, instance.Namespace)
504506
if err != nil {
505507
if k8s_errors.IsNotFound(err) {
508+
// Since the TLS cert secret should have been automatically created by the encompassing OpenStackControlPlane,
509+
// we treat this as an info.
506510
instance.Status.Conditions.Set(condition.FalseCondition(
507511
condition.TLSInputReadyCondition,
508512
condition.RequestedReason,
@@ -534,10 +538,12 @@ func (r *MetricStorageReconciler) reconcileNormal(
534538
nad, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace)
535539
if err != nil {
536540
if k8s_errors.IsNotFound(err) {
541+
// Since the net-attach-def CR should have been manually created by the user and referenced in the spec,
542+
// we treat this as a warning because it means that the service will not be able to start.
537543
instance.Status.Conditions.Set(condition.FalseCondition(
538544
condition.NetworkAttachmentsReadyCondition,
539-
condition.RequestedReason,
540-
condition.SeverityInfo,
545+
condition.ErrorReason,
546+
condition.SeverityWarning,
541547
condition.NetworkAttachmentsReadyWaitingMessage,
542548
netAtt))
543549
return ctrl.Result{RequeueAfter: time.Second * 10}, fmt.Errorf("%w: %s", telemetry.ErrNetworkAttachmentDefinitionNotFound, netAtt)

controllers/telemetry_common.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,14 @@ func ensureSecret(
118118
err.Error()))
119119
return "", res, err
120120
} else if (res != ctrl.Result{}) {
121+
// Since some of the secrets for which the function is used should have been manually
122+
// created by the user and referenced in the spec, we treat this as a warning because
123+
// it means that the associated service will not be able to start.
121124
log.FromContext(ctx).Info(fmt.Sprintf("OpenStack secret %s not found", secretName))
122125
conditionUpdater.Set(condition.FalseCondition(
123126
condition.InputReadyCondition,
124-
condition.RequestedReason,
125-
condition.SeverityInfo,
127+
condition.ErrorReason,
128+
condition.SeverityWarning,
126129
condition.InputReadyWaitingMessage))
127130
return "", res, nil
128131
}

0 commit comments

Comments
 (0)