Skip to content

Commit 9ad9305

Browse files
Merge pull request #1599 from abays/OSPRH-19828
[OSPRH-19828] Improve consistency of condition severity usage
2 parents 8b12772 + 376aad8 commit 9ad9305

10 files changed

Lines changed: 93 additions & 56 deletions

File tree

controllers/client/openstackclient_controller.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,8 @@ func (r *OpenStackClientReconciler) Reconcile(ctx context.Context, req ctrl.Requ
219219
_, configMapHash, err := configmap.GetConfigMapAndHashWithName(ctx, helper, *instance.Spec.OpenStackConfigMap, instance.Namespace)
220220
if err != nil {
221221
if k8s_errors.IsNotFound(err) {
222+
// Since the OpenStackConfigMap config map is usually automatically created by the Keystone operator,
223+
// we treat this as an info (because the user is usually not responsible for manually creating it).
222224
instance.Status.Conditions.Set(condition.FalseCondition(
223225
clientv1.OpenStackClientReadyCondition,
224226
condition.RequestedReason,
@@ -239,6 +241,8 @@ func (r *OpenStackClientReconciler) Reconcile(ctx context.Context, req ctrl.Requ
239241
_, secretHash, err := secret.GetSecret(ctx, helper, *instance.Spec.OpenStackConfigSecret, instance.Namespace)
240242
if err != nil {
241243
if k8s_errors.IsNotFound(err) {
244+
// Since the OpenStackConfigSecret secret is usually automatically created by the Keystone operator,
245+
// we treat this as an info (because the user is usually not responsible for manually creating it).
242246
instance.Status.Conditions.Set(condition.FalseCondition(
243247
clientv1.OpenStackClientReadyCondition,
244248
condition.RequestedReason,
@@ -267,10 +271,12 @@ func (r *OpenStackClientReconciler) Reconcile(ctx context.Context, req ctrl.Requ
267271
)
268272
if err != nil {
269273
if k8s_errors.IsNotFound(err) {
274+
// Since the CA cert secret should have been manually created by the user when provided in the spec,
275+
// we treat this as a warning because it means that reconciliation will not be able to continue.
270276
instance.Status.Conditions.Set(condition.FalseCondition(
271277
condition.TLSInputReadyCondition,
272-
condition.RequestedReason,
273-
condition.SeverityInfo,
278+
condition.ErrorReason,
279+
condition.SeverityWarning,
274280
fmt.Sprintf(condition.TLSInputReadyWaitingMessage, instance.Spec.CaBundleSecretName)))
275281
return ctrl.Result{}, nil
276282
}

controllers/core/openstackversion_controller.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -354,18 +354,22 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
354354
Log.Info("Minor update for Controlplane in progress")
355355
return ctrl.Result{}, nil
356356
}
357+
357358
// ctlplane is ready, lets make sure all images match newly deployed versions
358359
ctlplaneImagesMatch, badMatches := openstack.ControlplaneContainerImageMatch(ctx, controlPlane, instance)
359360
if !ctlplaneImagesMatch {
361+
// Since we need the images to match and we cannot proceed without it,
362+
// we treat this as a warning because it means that reconciliation will not be able to continue.
360363
errMsgBadMatches := "Controlplane images do not match the target version for the following services: " + strings.Join(badMatches, ", ")
361364
instance.Status.Conditions.Set(condition.FalseCondition(
362365
corev1beta1.OpenStackVersionMinorUpdateControlplane,
363-
condition.RequestedReason,
364-
condition.SeverityInfo,
366+
condition.ErrorReason,
367+
condition.SeverityWarning,
365368
corev1beta1.OpenStackVersionMinorUpdateReadyErrorMessage,
366369
errMsgBadMatches))
367-
370+
return ctrl.Result{}, nil
368371
}
372+
369373
instance.Status.Conditions.MarkTrue(
370374
corev1beta1.OpenStackVersionMinorUpdateControlplane,
371375
corev1beta1.OpenStackVersionMinorUpdateReadyMessage)

controllers/dataplane/openstackdataplanenodeset_controller.go

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -280,15 +280,17 @@ func (r *OpenStackDataPlaneNodeSetReconciler) Reconcile(ctx context.Context, req
280280
if err != nil {
281281
instance.Status.Conditions.MarkFalse(
282282
condition.InputReadyCondition,
283-
condition.RequestedReason,
283+
condition.ErrorReason,
284284
condition.SeverityError,
285285
err.Error())
286286
return result, err
287287
} else if (result != ctrl.Result{}) {
288+
// Since the the private key secret should have been manually created by the user when provided in the spec,
289+
// we treat this as a warning because it means that reconciliation will not be able to continue.
288290
instance.Status.Conditions.MarkFalse(
289291
condition.InputReadyCondition,
290-
condition.RequestedReason,
291-
condition.SeverityInfo,
292+
condition.ErrorReason,
293+
condition.SeverityWarning,
292294
dataplanev1.InputReadyWaitingMessage,
293295
"secret/"+ansibleSSHPrivateKeySecret)
294296
return result, nil
@@ -424,8 +426,10 @@ func (r *OpenStackDataPlaneNodeSetReconciler) Reconcile(ctx context.Context, req
424426
// Handles the case where the NodeSet is created, but not yet deployed.
425427
if instance.Status.Conditions.IsUnknown(condition.DeploymentReadyCondition) {
426428
Log.Info("Set NodeSet DeploymentReadyCondition false")
427-
instance.Status.Conditions.MarkFalse(condition.DeploymentReadyCondition,
428-
condition.RequestedReason, condition.SeverityInfo,
429+
instance.Status.Conditions.MarkFalse(
430+
condition.DeploymentReadyCondition,
431+
condition.RequestedReason,
432+
condition.SeverityInfo,
429433
dataplanev1.NodeSetDeploymentReadyWaitingMessage)
430434
}
431435

@@ -437,8 +441,10 @@ func (r *OpenStackDataPlaneNodeSetReconciler) Reconcile(ctx context.Context, req
437441
} else if isDeploymentRunning {
438442
Log.Info("Deployment still running...", "instance", instance)
439443
Log.Info("Set NodeSet DeploymentReadyCondition false")
440-
instance.Status.Conditions.MarkFalse(condition.DeploymentReadyCondition,
441-
condition.RequestedReason, condition.SeverityInfo,
444+
instance.Status.Conditions.MarkFalse(
445+
condition.DeploymentReadyCondition,
446+
condition.RequestedReason,
447+
condition.SeverityInfo,
442448
condition.DeploymentReadyRunningMessage)
443449
} else if isDeploymentFailed {
444450
podsInterface := r.Kclient.CoreV1().Pods(instance.Namespace)
@@ -459,8 +465,10 @@ func (r *OpenStackDataPlaneNodeSetReconciler) Reconcile(ctx context.Context, req
459465
if err != nil {
460466
deployErrorMsg = err.Error()
461467
}
462-
instance.Status.Conditions.MarkFalse(condition.DeploymentReadyCondition,
463-
condition.ErrorReason, condition.SeverityError,
468+
instance.Status.Conditions.MarkFalse(
469+
condition.DeploymentReadyCondition,
470+
condition.ErrorReason,
471+
condition.SeverityError,
464472
deployErrorMsg)
465473
}
466474

pkg/dataplane/baremetal.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,8 @@ func DeployBaremetalSet(
113113
if err != nil {
114114
instance.Status.Conditions.MarkFalse(
115115
dataplanev1.NodeSetBareMetalProvisionReadyCondition,
116-
condition.ErrorReason, condition.SeverityError,
116+
condition.ErrorReason,
117+
condition.SeverityError,
117118
dataplanev1.NodeSetBaremetalProvisionErrorMessage,
118119
err.Error())
119120
return ProvisionResult{}, err
@@ -124,7 +125,8 @@ func DeployBaremetalSet(
124125
utils.LogForObject(helper, "BaremetalSet not ready, waiting...", instance)
125126
instance.Status.Conditions.MarkFalse(
126127
dataplanev1.NodeSetBareMetalProvisionReadyCondition,
127-
condition.RequestedReason, condition.SeverityInfo,
128+
condition.RequestedReason,
129+
condition.SeverityInfo,
128130
dataplanev1.NodeSetBaremetalProvisionReadyWaitingMessage)
129131
return ProvisionResult{}, nil
130132
}

pkg/dataplane/ipam.go

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -195,14 +195,16 @@ func EnsureDNSData(ctx context.Context, helper *helper.Helper,
195195
if err != nil {
196196
instance.Status.Conditions.MarkFalse(
197197
dataplanev1.NodeSetDNSDataReadyCondition,
198-
condition.ErrorReason, condition.SeverityError,
198+
condition.ErrorReason,
199+
condition.SeverityError,
199200
err.Error())
200201
return dnsDetails, err
201202
}
202203
if dnsDetails.ClusterAddresses == nil {
203204
instance.Status.Conditions.MarkFalse(
204205
dataplanev1.NodeSetDNSDataReadyCondition,
205-
condition.RequestedReason, condition.SeverityInfo,
206+
condition.RequestedReason,
207+
condition.SeverityInfo,
206208
dataplanev1.NodeSetDNSDataReadyWaitingMessage)
207209
return dnsDetails, nil
208210
}
@@ -213,7 +215,8 @@ func EnsureDNSData(ctx context.Context, helper *helper.Helper,
213215
if err != nil {
214216
instance.Status.Conditions.MarkFalse(
215217
dataplanev1.NodeSetDNSDataReadyCondition,
216-
condition.ErrorReason, condition.SeverityError,
218+
condition.ErrorReason,
219+
condition.SeverityError,
217220
dataplanev1.NodeSetDNSDataReadyErrorMessage,
218221
err.Error())
219222
return dnsDetails, err
@@ -230,7 +233,8 @@ func EnsureDNSData(ctx context.Context, helper *helper.Helper,
230233
if err != nil {
231234
instance.Status.Conditions.MarkFalse(
232235
dataplanev1.NodeSetDNSDataReadyCondition,
233-
condition.ErrorReason, condition.SeverityError,
236+
condition.ErrorReason,
237+
condition.SeverityError,
234238
dataplanev1.NodeSetDNSDataReadyErrorMessage,
235239
err.Error())
236240
return dnsDetails, err
@@ -239,7 +243,8 @@ func EnsureDNSData(ctx context.Context, helper *helper.Helper,
239243
util.LogForObject(helper, "DNSData not ready yet waiting", instance)
240244
instance.Status.Conditions.MarkFalse(
241245
dataplanev1.NodeSetDNSDataReadyCondition,
242-
condition.RequestedReason, condition.SeverityInfo,
246+
condition.RequestedReason,
247+
condition.SeverityInfo,
243248
dataplanev1.NodeSetDNSDataReadyWaitingMessage)
244249
return dnsDetails, nil
245250
}
@@ -261,7 +266,8 @@ func EnsureIPSets(ctx context.Context, helper *helper.Helper,
261266
util.LogErrorForObject(helper, err, "Could not cleanup stale IP Reservations", instance)
262267
instance.Status.Conditions.MarkFalse(
263268
dataplanev1.NodeSetIPReservationReadyCondition,
264-
condition.ErrorReason, condition.SeverityError,
269+
condition.ErrorReason,
270+
condition.SeverityError,
265271
dataplanev1.NodeSetIPReservationReadyErrorMessage,
266272
err.Error())
267273
return nil, nil, false, err
@@ -270,7 +276,8 @@ func EnsureIPSets(ctx context.Context, helper *helper.Helper,
270276
if err != nil {
271277
instance.Status.Conditions.MarkFalse(
272278
dataplanev1.NodeSetIPReservationReadyCondition,
273-
condition.ErrorReason, condition.SeverityError,
279+
condition.ErrorReason,
280+
condition.SeverityError,
274281
dataplanev1.NodeSetIPReservationReadyErrorMessage,
275282
err.Error())
276283
return nil, netServiceNetMap, false, err
@@ -280,7 +287,8 @@ func EnsureIPSets(ctx context.Context, helper *helper.Helper,
280287
if s.Status.Conditions.IsFalse(condition.ReadyCondition) {
281288
instance.Status.Conditions.MarkFalse(
282289
dataplanev1.NodeSetIPReservationReadyCondition,
283-
condition.RequestedReason, condition.SeverityInfo,
290+
condition.RequestedReason,
291+
condition.SeverityInfo,
284292
dataplanev1.NodeSetIPReservationReadyWaitingMessage)
285293
return nil, netServiceNetMap, false, nil
286294
}

pkg/openstack/ca.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -406,8 +406,6 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
406406
}
407407
}
408408

409-
instance.Status.Conditions.MarkTrue(corev1.OpenStackControlPlaneCAReadyCondition, corev1.OpenStackControlPlaneCAReadyMessage)
410-
411409
// create/update combined CA secret
412410
if instance.Spec.TLS.CaBundleSecretName != "" {
413411
caSecret, _, err := secret.GetSecret(ctx, helper, instance.Spec.TLS.CaBundleSecretName, instance.Namespace)
@@ -421,6 +419,14 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
421419
instance.Spec.TLS.CaBundleSecretName,
422420
err.Error()))
423421
if k8s_errors.IsNotFound(err) {
422+
// Since the CA cert secret should have been manually created by the user when provided in the spec,
423+
// we treat this as a warning because it means that reconciliation will not be able to continue.
424+
instance.Status.Conditions.Set(condition.FalseCondition(
425+
corev1.OpenStackControlPlaneCAReadyCondition,
426+
condition.ErrorReason,
427+
condition.SeverityWarning,
428+
fmt.Sprintf(condition.TLSInputReadyWaitingMessage, instance.Spec.TLS.CaBundleSecretName)))
429+
424430
timeout := time.Second * 10
425431
Log.Info(fmt.Sprintf("Certificate %s not found, reconcile in %s", instance.Spec.TLS.CaBundleSecretName, timeout.String()))
426432

@@ -438,6 +444,8 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
438444
}
439445
}
440446

447+
instance.Status.Conditions.MarkTrue(corev1.OpenStackControlPlaneCAReadyCondition, corev1.OpenStackControlPlaneCAReadyMessage)
448+
441449
// get CA bundle from operator image. Downstream and upstream build use a different
442450
// base image, so the ca bundle cert file can be in different locations
443451
caBundle, err := getOperatorCABundle(tls.DownstreamTLSCABundlePath)

pkg/openstack/common.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,10 +261,12 @@ func EnsureEndpointConfig(
261261
_, err := validateSecret.ValidateCertSecret(ctx, helper, instance.GetNamespace())
262262
if err != nil {
263263
if k8s_errors.IsNotFound(err) {
264+
// Since the CA cert secret should have been manually created by the user when provided in the spec,
265+
// we treat this as a warning because it means that reconciliation will not be able to continue.
264266
instance.Status.Conditions.Set(condition.FalseCondition(
265267
corev1.OpenStackControlPlaneCustomTLSReadyCondition,
266-
condition.RequestedReason,
267-
condition.SeverityInfo,
268+
condition.ErrorReason,
269+
condition.SeverityWarning,
268270
corev1.OpenStackControlPlaneCustomTLSReadyWaitingMessage,
269271
ingressOverride.TLS.SecretName))
270272
return endpoints, ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil

pkg/openstack/version.go

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -284,80 +284,79 @@ func stringPointersEqual(a, b *string) bool {
284284
// only enabled services are checked
285285
func ControlplaneContainerImageMatch(ctx context.Context, controlPlane *corev1beta1.OpenStackControlPlane, version *corev1beta1.OpenStackVersion) (bool, []string) {
286286
failedMatches := []string{}
287-
if BarbicanImageMatch(ctx, controlPlane, version) {
287+
if !BarbicanImageMatch(ctx, controlPlane, version) {
288288
failedMatches = append(failedMatches, "Barbican")
289289
}
290-
if CinderImageMatch(ctx, controlPlane, version) {
290+
if !CinderImageMatch(ctx, controlPlane, version) {
291291
failedMatches = append(failedMatches, "Cinder")
292292
}
293-
if DesignateImageMatch(ctx, controlPlane, version) {
293+
if !DesignateImageMatch(ctx, controlPlane, version) {
294294
failedMatches = append(failedMatches, "Designate")
295295
}
296-
if DnsmasqImageMatch(ctx, controlPlane, version) {
296+
if !DnsmasqImageMatch(ctx, controlPlane, version) {
297297
failedMatches = append(failedMatches, "Dnsmasq")
298298
}
299-
if GaleraImageMatch(ctx, controlPlane, version) {
299+
if !GaleraImageMatch(ctx, controlPlane, version) {
300300
failedMatches = append(failedMatches, "Galera")
301301
}
302-
if GlanceImageMatch(ctx, controlPlane, version) {
302+
if !GlanceImageMatch(ctx, controlPlane, version) {
303303
failedMatches = append(failedMatches, "Glance")
304304
}
305-
if HeatImageMatch(ctx, controlPlane, version) {
305+
if !HeatImageMatch(ctx, controlPlane, version) {
306306
failedMatches = append(failedMatches, "Heat")
307307
}
308-
if HorizonImageMatch(ctx, controlPlane, version) {
308+
if !HorizonImageMatch(ctx, controlPlane, version) {
309309
failedMatches = append(failedMatches, "Horizon")
310310
}
311-
if IronicImageMatch(ctx, controlPlane, version) {
311+
if !IronicImageMatch(ctx, controlPlane, version) {
312312
failedMatches = append(failedMatches, "Ironic")
313313
}
314-
if KeystoneImageMatch(ctx, controlPlane, version) {
314+
if !KeystoneImageMatch(ctx, controlPlane, version) {
315315
failedMatches = append(failedMatches, "Keystone")
316316
}
317-
if ManilaImageMatch(ctx, controlPlane, version) {
317+
if !ManilaImageMatch(ctx, controlPlane, version) {
318318
failedMatches = append(failedMatches, "Manila")
319319
}
320-
if MemcachedImageMatch(ctx, controlPlane, version) {
320+
if !MemcachedImageMatch(ctx, controlPlane, version) {
321321
failedMatches = append(failedMatches, "Memcached")
322322
}
323-
if RedisImageMatch(ctx, controlPlane, version) {
323+
if !RedisImageMatch(ctx, controlPlane, version) {
324324
failedMatches = append(failedMatches, "Redis")
325325
}
326-
if NeutronImageMatch(ctx, controlPlane, version) {
326+
if !NeutronImageMatch(ctx, controlPlane, version) {
327327
failedMatches = append(failedMatches, "Neutron")
328328
}
329-
if NovaImageMatch(ctx, controlPlane, version) {
329+
if !NovaImageMatch(ctx, controlPlane, version) {
330330
failedMatches = append(failedMatches, "Nova")
331331
}
332-
if OctaviaImageMatch(ctx, controlPlane, version) {
332+
if !OctaviaImageMatch(ctx, controlPlane, version) {
333333
failedMatches = append(failedMatches, "Octavia")
334334
}
335-
if ClientImageMatch(ctx, controlPlane, version) {
335+
if !ClientImageMatch(ctx, controlPlane, version) {
336336
failedMatches = append(failedMatches, "OpenstackClient")
337337
}
338-
if OVNControllerImageMatch(ctx, controlPlane, version) {
338+
if !OVNControllerImageMatch(ctx, controlPlane, version) {
339339
failedMatches = append(failedMatches, "OVNController")
340340
}
341-
if OVNNorthImageMatch(ctx, controlPlane, version) {
341+
if !OVNNorthImageMatch(ctx, controlPlane, version) {
342342
failedMatches = append(failedMatches, "OVNNorth")
343343
}
344-
if OVNDbClusterImageMatch(ctx, controlPlane, version) {
344+
if !OVNDbClusterImageMatch(ctx, controlPlane, version) {
345345
failedMatches = append(failedMatches, "OVNDbCluster")
346346
}
347-
if PlacementImageMatch(ctx, controlPlane, version) {
347+
if !PlacementImageMatch(ctx, controlPlane, version) {
348348
failedMatches = append(failedMatches, "Placement")
349349
}
350-
if RabbitmqImageMatch(ctx, controlPlane, version) {
350+
if !RabbitmqImageMatch(ctx, controlPlane, version) {
351351
failedMatches = append(failedMatches, "Rabbitmq")
352352
}
353-
if SwiftImageMatch(ctx, controlPlane, version) {
353+
if !SwiftImageMatch(ctx, controlPlane, version) {
354354
failedMatches = append(failedMatches, "Swift")
355355
}
356-
if TelemetryImageMatch(ctx, controlPlane, version) {
356+
if !TelemetryImageMatch(ctx, controlPlane, version) {
357357
failedMatches = append(failedMatches, "Telemetry")
358358
}
359-
360-
if WatcherImageMatch(ctx, controlPlane, version) {
359+
if !WatcherImageMatch(ctx, controlPlane, version) {
361360
failedMatches = append(failedMatches, "Watcher")
362361
}
363362

tests/functional/ctlplane/openstackoperator_controller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1438,7 +1438,7 @@ var _ = Describe("OpenStackOperator controller", func() {
14381438
ConditionGetterFunc(OpenStackControlPlaneConditionGetter),
14391439
corev1.OpenStackControlPlaneCustomTLSReadyCondition,
14401440
k8s_corev1.ConditionFalse,
1441-
condition.RequestedReason,
1441+
condition.ErrorReason,
14421442
fmt.Sprintf(corev1.OpenStackControlPlaneCustomTLSReadyWaitingMessage, names.CustomServiceCertSecretName.Name),
14431443
)
14441444
})

tests/functional/ctlplane/openstackversion_controller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -626,7 +626,7 @@ var _ = Describe("OpenStackOperator controller", func() {
626626
// this would occur automatically via the watch on the DataPlaneNodeSet's by openstackcontrolplane
627627
// so once the administrator executes the DataplaneDeployment and that finishes the controlplane will update the images immediately
628628
SimulateControlplaneReady()
629-
// now we check that the rest of the container images got updated
629+
// now we check that the container images for the aforementioned services got updated
630630
Eventually(func(g Gomega) {
631631
th.ExpectCondition(
632632
names.OpenStackVersionName,

0 commit comments

Comments
 (0)