Skip to content

Commit

Permalink
Merge pull request #687 from stuggi/fix_svc_endpoint_change
Browse files Browse the repository at this point in the history 
Prevent endpoint update if service operator changes condition
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored Mar 4, 2024
2 parents 5ca45db + a518f8f commit 28cd696
Show file tree
Hide file tree
Showing 15 changed files with 336 additions and 334 deletions.
27 changes: 13 additions & 14 deletions pkg/openstack/barbican.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand All @@ -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,
Expand All @@ -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)
Expand Down
27 changes: 13 additions & 14 deletions pkg/openstack/cinder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand All @@ -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,
Expand All @@ -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)
Expand Down
38 changes: 29 additions & 9 deletions pkg/openstack/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
26 changes: 13 additions & 13 deletions pkg/openstack/designate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand All @@ -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,
Expand All @@ -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()
}

Expand Down
73 changes: 40 additions & 33 deletions pkg/openstack/glance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 -
Expand Down Expand Up @@ -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]
Expand All @@ -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,
Expand All @@ -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")
Expand Down
Loading

0 comments on commit 28cd696

Please sign in to comment.