From 262fd7409f178e30abcf4f67e114681ac0da146f Mon Sep 17 00:00:00 2001 From: Francesco Pantano Date: Thu, 9 Jan 2025 10:32:06 +0100 Subject: [PATCH] Better handling of Topology Finalizers When a GlanceAPI references a Topology, a finalizer is added to such resource. By doing this, we can clearly identify which topology is applied to which API. For this reason, and because Glance is potentially made by a list of APIs that can independently reference the same Topology CR, the finalizer string is based on the concatenation between the Glance service (prefix), and the API name. It is possible, as a day2 operation, to add, update or remove a Topology reference. When doing this, especially for update and removal operations, a pointer to the CR must be updated. To properly support this action, the information about the LastAppliedTopology is saved in the Status, and it allows to properly clean up unused resources and not locking them indefinitely. Signed-off-by: Francesco Pantano --- .../glance.openstack.org_glanceapis.yaml | 2 ++ api/go.mod | 2 +- api/v1beta1/glanceapi_types.go | 3 ++ .../glance.openstack.org_glanceapis.yaml | 2 ++ controllers/glanceapi_controller.go | 32 +++++++++++++++---- go.mod | 2 +- pkg/glanceapi/statefulset.go | 20 ++++++------ 7 files changed, 44 insertions(+), 19 deletions(-) diff --git a/api/bases/glance.openstack.org_glanceapis.yaml b/api/bases/glance.openstack.org_glanceapis.yaml index a10bafff..b0e0f6cd 100644 --- a/api/bases/glance.openstack.org_glanceapis.yaml +++ b/api/bases/glance.openstack.org_glanceapis.yaml @@ -759,6 +759,8 @@ spec: additionalProperties: type: string type: object + lastAppliedTopology: + type: string networkAttachments: additionalProperties: items: diff --git a/api/go.mod b/api/go.mod index bf401314..ca6f3b53 100644 --- a/api/go.mod +++ b/api/go.mod @@ -71,6 +71,6 @@ require ( // must consistent within modules and service operators replace github.com/openshift/api => github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094 //allow-merging -replace github.com/openstack-k8s-operators/infra-operator/apis => github.com/fmount/infra-operator/apis v0.0.0-20241217210734-91376cf9eb3f //allow-merging +replace github.com/openstack-k8s-operators/infra-operator/apis => github.com/fmount/infra-operator/apis v0.0.0-20250109124018-4262fdefc70b //allow-merging replace github.com/openstack-k8s-operators/lib-common/modules/common => github.com/fmount/lib-common/modules/common v0.0.0-20241217100632-a2c8ea43c395 //allow-merging diff --git a/api/v1beta1/glanceapi_types.go b/api/v1beta1/glanceapi_types.go index 5c164300..1a54fb28 100644 --- a/api/v1beta1/glanceapi_types.go +++ b/api/v1beta1/glanceapi_types.go @@ -115,6 +115,9 @@ type GlanceAPIStatus struct { // then the controller has not processed the latest changes injected by // the opentack-operator in the top-level CR (e.g. the ContainerImage) ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // LastAppliedTopology - the last applied Topology + LastAppliedTopology string `json:"lastAppliedTopology,omitempty"` } // +kubebuilder:object:root=true diff --git a/config/crd/bases/glance.openstack.org_glanceapis.yaml b/config/crd/bases/glance.openstack.org_glanceapis.yaml index a10bafff..b0e0f6cd 100644 --- a/config/crd/bases/glance.openstack.org_glanceapis.yaml +++ b/config/crd/bases/glance.openstack.org_glanceapis.yaml @@ -759,6 +759,8 @@ spec: additionalProperties: type: string type: object + lastAppliedTopology: + type: string networkAttachments: additionalProperties: items: diff --git a/controllers/glanceapi_controller.go b/controllers/glanceapi_controller.go index a7f40d46..09ecd18e 100644 --- a/controllers/glanceapi_controller.go +++ b/controllers/glanceapi_controller.go @@ -347,7 +347,6 @@ func (r *GlanceAPIReconciler) SetupWithManager(mgr ctrl.Manager) error { topologyFn := func(_ context.Context, o client.Object) []reconcile.Request { result := []reconcile.Request{} - // get all GlanceAPIs CRs glanceAPIs := &glancev1.GlanceAPIList{} listOpts := []client.ListOption{ @@ -855,6 +854,15 @@ func (r *GlanceAPIReconciler) reconcileNormal( instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) var topology *topologyv1.Topology + // When the Topology CR reference is updated and the current GlanceAPI + // switches to a new Topology, remove the finalizer from the previous + // Topology + if instance.Status.LastAppliedTopology != "" { + _, err = r.ensureDeletedTopology(ctx, instance, helper) + if err != nil { + return ctrl.Result{}, err + } + } if instance.Spec.Topology != nil { topology, err = r.ensureGlanceAPITopology(ctx, helper, instance) if err != nil { @@ -867,6 +875,9 @@ func (r *GlanceAPIReconciler) reconcileNormal( r.Log.Info("Glance config is waiting for Topology requirements, requeueing...") return ctrl.Result{}, err } + // update the Status with the last retrieved Topology name as the finalizer + // is now removed from the previous topologyRef + instance.Status.LastAppliedTopology = instance.Spec.Topology.Name } // At this point Glance has a Topology CR (or not in case it's not referenced in the @@ -1532,7 +1543,7 @@ func (r *GlanceAPIReconciler) ensureGlanceAPITopology( // Add finalizer to the resource because it is going to be consumed by GlanceAPI if !controllerutil.ContainsFinalizer(topology, h.GetFinalizer()) { - controllerutil.AddFinalizer(topology, h.GetFinalizer()) + controllerutil.AddFinalizer(topology, fmt.Sprintf("%s-%s", h.GetFinalizer(), instance.APIName())) } // Update the resource if err := h.GetClient().Update(ctx, topology); err != nil { @@ -1553,17 +1564,24 @@ func (r *GlanceAPIReconciler) ensureDeletedTopology( ) (ctrl.Result, error) { ns := instance.Namespace - if instance.Spec.Topology == nil { + // no Topology is currently passed to the GlanceAPI, and it was not used + // before + if instance.Spec.Topology == nil && instance.Status.LastAppliedTopology == "" { return ctrl.Result{}, nil } - if instance.Spec.Topology.Namespace != "" { - ns = instance.Spec.Topology.Namespace + if instance.Spec.Topology != nil { + // Check namespace and set name + if instance.Spec.Topology.Namespace != "" { + ns = instance.Spec.Topology.Namespace + } } + + name := instance.Status.LastAppliedTopology // Remove the finalizer from the Topology CR topology, _, err := topologyv1.GetTopologyByName( ctx, h, - instance.Spec.Topology.Name, + name, ns, ) @@ -1574,7 +1592,7 @@ func (r *GlanceAPIReconciler) ensureDeletedTopology( return ctrl.Result{}, err } if err == nil { - if controllerutil.RemoveFinalizer(topology, h.GetFinalizer()) { + if controllerutil.RemoveFinalizer(topology, fmt.Sprintf("%s-%s", h.GetFinalizer(), instance.APIName())) { err = r.Update(ctx, topology) if err != nil && !k8s_errors.IsNotFound(err) { return ctrl.Result{}, err diff --git a/go.mod b/go.mod index 240f67a5..a33bfe15 100644 --- a/go.mod +++ b/go.mod @@ -90,6 +90,6 @@ replace github.com/openstack-k8s-operators/glance-operator/api => ./api // must consistent within modules and service operators replace github.com/openshift/api => github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094 //allow-merging -replace github.com/openstack-k8s-operators/infra-operator/apis => github.com/fmount/infra-operator/apis v0.0.0-20241217210734-91376cf9eb3f //allow-merging +replace github.com/openstack-k8s-operators/infra-operator/apis => github.com/fmount/infra-operator/apis v0.0.0-20250109124018-4262fdefc70b //allow-merging replace github.com/openstack-k8s-operators/lib-common/modules/common => github.com/fmount/lib-common/modules/common v0.0.0-20241217100632-a2c8ea43c395 //allow-merging diff --git a/pkg/glanceapi/statefulset.go b/pkg/glanceapi/statefulset.go index acb6d64b..3e79e267 100644 --- a/pkg/glanceapi/statefulset.go +++ b/pkg/glanceapi/statefulset.go @@ -76,6 +76,16 @@ func StatefulSet( glanceURIScheme := corev1.URISchemeHTTP tlsEnabled := instance.Spec.TLS.API.Enabled(service.EndpointPublic) + // initialize an Affinity Object + aff := &affinity.OverrideSpec{ + PodAffinity: nil, + PodAntiAffinity: nil, + NodeAffinity: nil, + } + + // initialize a Topology Object + tplg := []corev1.TopologySpreadConstraint{} + if instance.Spec.APIType == glancev1.APIInternal || instance.Spec.APIType == glancev1.APIEdge { port = int32(glance.GlanceInternalPort) @@ -297,16 +307,6 @@ func StatefulSet( statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector } - // initialize an Affinity Object - aff := &affinity.OverrideSpec{ - PodAffinity: nil, - PodAntiAffinity: nil, - NodeAffinity: nil, - } - - // initialize a Topology Object - tplg := []corev1.TopologySpreadConstraint{} - if topologyOverride != nil { ts := topologyOverride.Spec if ts.TopologySpreadConstraint == nil {