Prevent endpoint update if service operator changes condition

Previously to apply service overrides to service CRs there
were checks that services have reached a specific condition
state. E.g. for glance that glancev1.GlanceAPIReadyCondition
is true. This conditon resulted in the behavior that when
after a successful deployment glance flipped to
glancev1.GlanceAPIReadyCondition == False e.g. seen when ceph
is not healty and glance api pods manually restarted, that the
service overrides got removed (with route hostname information)
and the keystoneendpoint updated to be the k8s service instead
of the route.

With this change there is no wait for a condition, instead
checking the endpoint k8s services matching the label selector
applied by the openstack-operator. If no services are found, the user
provided overrides + the label selector get set. If the expected
number of k8s services are identified, endpointdetails get
evaluated and e.g. the routes hostname applied.

Initially the conditions were used to prevent rerunning bootstrap
jobs while there is still one in progress. With the changes
to the DoJob [1] this should no longer be required since the job
handling will wait for a running job to complete before rerunning
it with a new config.

[1] https://github.com/openstack-k8s-operators/lib-common/blob/main/modules/common/job/job.go#L115

Author: stuggi

pkg/openstack/barbican.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/openstack-k8s-operators/lib-common/modules/common"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/service"
@@ -42,7 +41,7 @@ func ReconcileBarbican(ctx context.Context, instance *corev1beta1.OpenStackContr
 		if instance.Spec.Barbican.Template.BarbicanAPI.Override.Service == nil {
 			instance.Spec.Barbican.Template.BarbicanAPI.Override.Service = map[service.Endpoint]service.RoutedOverrideSpec{}
 		}
-		instance.Spec.Barbican.Template.BarbicanAPI.Override.Service[endpointType] = AddServiceComponentLabel(
+		instance.Spec.Barbican.Template.BarbicanAPI.Override.Service[endpointType] = AddServiceOpenStackOperatorLabel(
 			instance.Spec.Barbican.Template.BarbicanAPI.Override.Service[endpointType],
 			barbican.Name)
 	}
@@ -60,17 +59,18 @@ func ReconcileBarbican(ctx context.Context, instance *corev1beta1.OpenStackContr
 	}
 	instance.Spec.Barbican.Template.BarbicanAPI.TLS.CaBundleSecretName = instance.Status.TLS.CaBundleSecretName
 
-	if barbican.Status.Conditions.IsTrue(barbicanv1.BarbicanAPIReadyCondition) {
-		svcs, err := service.GetServicesListWithLabel(
-			ctx,
-			helper,
-			instance.Namespace,
-			map[string]string{common.AppSelector: barbican.Name},
-		)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
+	svcs, err := service.GetServicesListWithLabel(
+		ctx,
+		helper,
+		instance.Namespace,
+		GetServiceOpenStackOperatorLabel(barbican.Name),
+	)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
 
+	// make sure to get to EndpointConfig when all service got created
+	if len(svcs.Items) == len(instance.Spec.Barbican.Template.BarbicanAPI.Override.Service) {
 		endpointDetails, ctrlResult, err := EnsureEndpointConfig(
 			ctx,
 			instance,
@@ -87,9 +87,8 @@ func ReconcileBarbican(ctx context.Context, instance *corev1beta1.OpenStackContr
 		} else if (ctrlResult != ctrl.Result{}) {
 			return ctrlResult, nil
 		}
-
+		// set service overrides
 		instance.Spec.Barbican.Template.BarbicanAPI.Override.Service = endpointDetails.GetEndpointServiceOverrides()
-
 		// update TLS settings with cert secret
 		instance.Spec.Barbican.Template.BarbicanAPI.TLS.API.Public.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointPublic)
 		instance.Spec.Barbican.Template.BarbicanAPI.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)

pkg/openstack/cinder.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/openstack-k8s-operators/lib-common/modules/common"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/service"
@@ -44,7 +43,7 @@ func ReconcileCinder(ctx context.Context, instance *corev1beta1.OpenStackControl
 			instance.Spec.Cinder.Template.CinderAPI.Override.Service = map[service.Endpoint]service.RoutedOverrideSpec{}
 		}
 		instance.Spec.Cinder.Template.CinderAPI.Override.Service[endpointType] =
-			AddServiceComponentLabel(
+			AddServiceOpenStackOperatorLabel(
 				instance.Spec.Cinder.Template.CinderAPI.Override.Service[endpointType],
 				cinder.Name)
 	}
@@ -62,17 +61,18 @@ func ReconcileCinder(ctx context.Context, instance *corev1beta1.OpenStackControl
 	}
 	instance.Spec.Cinder.Template.CinderAPI.TLS.CaBundleSecretName = instance.Status.TLS.CaBundleSecretName
 
-	if cinder.Status.Conditions.IsTrue(cinderv1.CinderAPIReadyCondition) {
-		svcs, err := service.GetServicesListWithLabel(
-			ctx,
-			helper,
-			instance.Namespace,
-			map[string]string{common.AppSelector: cinder.Name},
-		)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
+	svcs, err := service.GetServicesListWithLabel(
+		ctx,
+		helper,
+		instance.Namespace,
+		GetServiceOpenStackOperatorLabel(cinder.Name),
+	)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
 
+	// make sure to get to EndpointConfig when all service got created
+	if len(svcs.Items) == len(instance.Spec.Cinder.Template.CinderAPI.Override.Service) {
 		endpointDetails, ctrlResult, err := EnsureEndpointConfig(
 			ctx,
 			instance,
@@ -89,9 +89,8 @@ func ReconcileCinder(ctx context.Context, instance *corev1beta1.OpenStackControl
 		} else if (ctrlResult != ctrl.Result{}) {
 			return ctrlResult, nil
 		}
-
+		// set service overrides
 		instance.Spec.Cinder.Template.CinderAPI.Override.Service = endpointDetails.GetEndpointServiceOverrides()
-
 		// update TLS settings with cert secret
 		instance.Spec.Cinder.Template.CinderAPI.TLS.API.Public.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointPublic)
 		instance.Spec.Cinder.Template.CinderAPI.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)

pkg/openstack/common.go

@@ -18,7 +18,6 @@ import (
 	ironicv1 "github.com/openstack-k8s-operators/ironic-operator/api/v1beta1"
 	keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1"
 	"github.com/openstack-k8s-operators/lib-common/modules/certmanager"
-	"github.com/openstack-k8s-operators/lib-common/modules/common"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/route"
@@ -47,6 +46,17 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
+const (
+	// ooSelector is used as a selector to the labels to differentiate from
+	// any other possible labels added to services, because  e.g. the common.AppSelector
+	// is also used by service operators.
+	ooSelector = "osctlplane"
+
+	// ooAppSelector service selector label added by the openstack-operator to service
+	// overrides
+	ooAppSelector = "osctlplane-service"
+)
+
 // GetLog returns a logger object with a prefix of "controller.name" and aditional controller context fields
 func GetLogger(ctx context.Context) logr.Logger {
 	return log.FromContext(ctx).WithName("Controllers").WithName("OpenstackControlPlane")
@@ -71,10 +81,20 @@ func EnsureDeleted(ctx context.Context, helper *helper.Helper, obj client.Object
 
 }
 
-// AddServiceComponentLabel - adds component label to the service override to be able to query
+// GetServiceOpenStackOperatorLabel - returns the labels to be added to service override
+func GetServiceOpenStackOperatorLabel(value string) map[string]string {
+	return map[string]string{
+		ooAppSelector: value,
+		ooSelector:    "",
+	}
+}
+
+// AddServiceOpenStackOperatorLabel - adds labels to the service override to be able to query
 // the service labels to check for any route creation
-func AddServiceComponentLabel(svcOverride service.RoutedOverrideSpec, value string) service.RoutedOverrideSpec {
-	svcOverride.AddLabel(map[string]string{common.AppSelector: value})
+func AddServiceOpenStackOperatorLabel(svcOverride service.RoutedOverrideSpec, value string) service.RoutedOverrideSpec {
+	for s, v := range GetServiceOpenStackOperatorLabel(value) {
+		svcOverride.AddLabel(map[string]string{s: v})
+	}
 
 	return svcOverride
 }
@@ -315,13 +335,13 @@ func (ed *EndpointDetail) ensureRoute(
 	owner metav1.Object,
 	condType condition.Type,
 ) (ctrl.Result, error) {
-	// check if there is already a route with common.AppSelector from the service
-	if svcLabelVal, ok := svc.Labels[common.AppSelector]; ok {
+	// check if there is already a route with ooAppSelector from the service
+	if svcLabelVal, ok := svc.Labels[ooAppSelector]; ok {
 		routes, err := GetRoutesListWithLabel(
 			ctx,
 			helper,
 			instance.Namespace,
-			map[string]string{common.AppSelector: svcLabelVal},
+			map[string]string{ooAppSelector: svcLabelVal},
 		)
 		if err != nil {
 			return ctrl.Result{}, err
@@ -376,8 +396,8 @@ func (ed *EndpointDetail) ensureRoute(
 			ed.Service.OverrideSpec.EmbeddedLabelsAnnotations = &service.EmbeddedLabelsAnnotations{}
 		}
 
-		if labelVal, ok := ed.Service.OverrideSpec.EmbeddedLabelsAnnotations.Labels[common.AppSelector]; ok {
-			ed.Labels = map[string]string{common.AppSelector: labelVal}
+		if labelVal, ok := ed.Service.OverrideSpec.EmbeddedLabelsAnnotations.Labels[ooAppSelector]; ok {
+			ed.Labels = map[string]string{ooAppSelector: labelVal}
 		}
 
 		ctrlResult, err := ed.CreateRoute(ctx, helper, owner)

pkg/openstack/designate.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/openstack-k8s-operators/lib-common/modules/common"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/service"
@@ -43,7 +42,7 @@ func ReconcileDesignate(ctx context.Context, instance *corev1beta1.OpenStackCont
 			instance.Spec.Designate.Template.DesignateAPI.Override.Service = map[service.Endpoint]service.RoutedOverrideSpec{}
 		}
 		instance.Spec.Designate.Template.DesignateAPI.Override.Service[endpointType] =
-			AddServiceComponentLabel(
+			AddServiceOpenStackOperatorLabel(
 				instance.Spec.Designate.Template.DesignateAPI.Override.Service[endpointType],
 				designate.Name)
 	}
@@ -55,17 +54,18 @@ func ReconcileDesignate(ctx context.Context, instance *corev1beta1.OpenStackCont
 		}
 	}
 
-	if designate.Status.Conditions.IsTrue(designatev1.DesignateAPIReadyCondition) {
-		svcs, err := service.GetServicesListWithLabel(
-			ctx,
-			helper,
-			instance.Namespace,
-			map[string]string{common.AppSelector: designate.Name},
-		)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
+	svcs, err := service.GetServicesListWithLabel(
+		ctx,
+		helper,
+		instance.Namespace,
+		GetServiceOpenStackOperatorLabel(designate.Name),
+	)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
 
+	// make sure to get to EndpointConfig when all service got created
+	if len(svcs.Items) == len(instance.Spec.Designate.Template.DesignateAPI.Override.Service) {
 		endpointDetails, ctrlResult, err := EnsureEndpointConfig(
 			ctx,
 			instance,
@@ -82,7 +82,7 @@ func ReconcileDesignate(ctx context.Context, instance *corev1beta1.OpenStackCont
 		} else if (ctrlResult != ctrl.Result{}) {
 			return ctrlResult, nil
 		}
-
+		// set service overrides
 		instance.Spec.Designate.Template.DesignateAPI.Override.Service = endpointDetails.GetEndpointServiceOverrides()
 	}
 

pkg/openstack/glance.go

@@ -8,6 +8,7 @@ import (
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/service"
+	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 	corev1beta1 "github.com/openstack-k8s-operators/openstack-operator/apis/core/v1beta1"
 	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -18,8 +19,11 @@ import (
 
 const (
 	// svcSelector is used as selector to get the list of "Services" associated
-	// to a specific glanceAPI instance
-	svcSelector = "glanceAPI"
+	// to a specific glanceAPI instance. It must be different from glanceAPI
+	// label set by the service operator as in case of split glanceAPI type the
+	// the label on public svc gets set to -external and internal instance svc
+	// to -internal instead of the glance top level glanceType split
+	svcSelector = "tlGlanceAPI"
 )
 
 // ReconcileGlance -
@@ -55,7 +59,7 @@ func ReconcileGlance(ctx context.Context, instance *corev1beta1.OpenStackControl
 			if glanceAPI.Override.Service == nil {
 				glanceAPI.Override.Service = map[service.Endpoint]service.RoutedOverrideSpec{}
 			}
-			glanceAPI.Override.Service[endpointType] = AddServiceComponentLabel(
+			glanceAPI.Override.Service[endpointType] = AddServiceOpenStackOperatorLabel(
 				glanceAPI.Override.Service[endpointType], glance.Name)
 
 			svcOverride := glanceAPI.Override.Service[endpointType]
@@ -71,32 +75,36 @@ func ReconcileGlance(ctx context.Context, instance *corev1beta1.OpenStackControl
 		instance.Spec.Glance.Template.GlanceAPIs[name] = glanceAPI
 	}
 
-	if glance.Status.Conditions.IsTrue(glancev1.GlanceAPIReadyCondition) {
-		// initialize the main APIOverride struct
-		if instance.Spec.Glance.APIOverride == nil {
-			instance.Spec.Glance.APIOverride = map[string]corev1beta1.Override{}
-		}
+	// initialize the main APIOverride struct
+	if instance.Spec.Glance.APIOverride == nil {
+		instance.Spec.Glance.APIOverride = map[string]corev1beta1.Override{}
+	}
 
-		var changed bool = false
-		for name, glanceAPI := range instance.Spec.Glance.Template.GlanceAPIs {
-			if _, ok := instance.Spec.Glance.APIOverride[name]; !ok {
-				instance.Spec.Glance.APIOverride[name] = corev1beta1.Override{}
-			}
-			// Retrieve the services by Label and filter on glanceAPI: for
-			// each instance we should get **only** the associated `SVCs`
-			// and not the whole list. As per the Glance design doc we know
-			// that a given instance name is made in the form: "<service>
-			// <apiName> <apiType>", so we build the filter accordingly
-			// to resolve the label as <service>-<apiName>
-			svcs, err := service.GetServicesListWithLabel(
-				ctx,
-				helper,
-				instance.Namespace,
+	var changed = false
+	for name, glanceAPI := range instance.Spec.Glance.Template.GlanceAPIs {
+		if _, ok := instance.Spec.Glance.APIOverride[name]; !ok {
+			instance.Spec.Glance.APIOverride[name] = corev1beta1.Override{}
+		}
+		// Retrieve the services by Label and filter on glanceAPI: for
+		// each instance we should get **only** the associated `SVCs`
+		// and not the whole list. As per the Glance design doc we know
+		// that a given instance name is made in the form: "<service>
+		// <apiName> <apiType>", so we build the filter accordingly
+		// to resolve the label as <service>-<apiName>
+		svcs, err := service.GetServicesListWithLabel(
+			ctx,
+			helper,
+			instance.Namespace,
+			util.MergeMaps(
+				GetServiceOpenStackOperatorLabel(glance.Name),
 				getGlanceAPILabelMap(glance.Name, name, glanceAPI.Type),
-			)
-			if err != nil {
-				return ctrl.Result{}, err
-			}
+			),
+		)
+		if err != nil {
+			return ctrl.Result{}, err
+		}
+		// make sure to get to EndpointConfig when all service got created
+		if len(svcs.Items) == len(glanceAPI.Override.Service) {
 			endpointDetails, ctrlResult, err := EnsureEndpointConfig(
 				ctx,
 				instance,
@@ -111,23 +119,22 @@ func ReconcileGlance(ctx context.Context, instance *corev1beta1.OpenStackControl
 			if err != nil {
 				return ctrlResult, err
 			}
+			// set service overrides
 			glanceAPI.Override.Service = endpointDetails.GetEndpointServiceOverrides()
-
 			// update TLS cert secret
 			glanceAPI.TLS.API.Public.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointPublic)
 			glanceAPI.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)
-			instance.Spec.Glance.Template.GlanceAPIs[name] = glanceAPI
 
 			// let's keep track of changes for any instance, but return
 			// only when the iteration on the whole APIList is over
 			if (ctrlResult != ctrl.Result{}) {
 				changed = true
 			}
 		}
-		// if one of the API changed, return
-		if changed {
-			return ctrl.Result{}, nil
-		}
+		instance.Spec.Glance.Template.GlanceAPIs[name] = glanceAPI
+	}
+	if changed {
+		return ctrl.Result{}, nil
 	}
 
 	Log.Info("Reconciling Glance", "Glance.Namespace", instance.Namespace, "Glance.Name", "glance")