Skip to content

Commit

Permalink
Better handling of Topology Finalizers
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
fmount committed Jan 10, 2025
1 parent 0fc5288 commit 57cd522
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 11 deletions.
2 changes: 2 additions & 0 deletions api/bases/glance.openstack.org_glanceapis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,8 @@ spec:
additionalProperties:
type: string
type: object
lastAppliedTopology:
type: string
networkAttachments:
additionalProperties:
items:
Expand Down
3 changes: 3 additions & 0 deletions api/v1beta1/glanceapi_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions config/crd/bases/glance.openstack.org_glanceapis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,8 @@ spec:
additionalProperties:
type: string
type: object
lastAppliedTopology:
type: string
networkAttachments:
additionalProperties:
items:
Expand Down
32 changes: 25 additions & 7 deletions controllers/glanceapi_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand All @@ -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 applied Topology name, which will be
// used to handle finalizers
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
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
)

Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ github.com/evanphx/json-patch v5.7.0+incompatible h1:vgGkfT/9f8zE6tvSCe74nfpAVDQ
github.com/evanphx/json-patch v5.7.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg=
github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ=
github.com/fmount/infra-operator/apis v0.0.0-20241217210734-91376cf9eb3f h1:7k78qT8ZdGsHbYnL6eBnlxf3D1KSJlDrF08nlJ0CXQ4=
github.com/fmount/infra-operator/apis v0.0.0-20241217210734-91376cf9eb3f/go.mod h1:8HmHTSXUAUt2jBk1QodnNxt2h96LM1YQsbC1YOkzNTE=
github.com/fmount/lib-common/modules/common v0.0.0-20241217100632-a2c8ea43c395 h1:FTnFgkzbg5agJorJB4wXfU4LtW8xfGAsozo8It1i6vU=
github.com/fmount/lib-common/modules/common v0.0.0-20241217100632-a2c8ea43c395/go.mod h1:YpNTuJhDWhbXM50O3qBkhO7M+OOyRmWkNVmJ4y3cyFs=
github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
Expand Down Expand Up @@ -80,12 +84,8 @@ github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094 h1:J1wuGhVxpsHykZBa6
github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4=
github.com/openstack-k8s-operators/cinder-operator/api v0.5.1-0.20241217072755-fb4d39411ad2 h1:iY0RYw5hyeaou7ujKCrkYpgebIyh5O5FRsvkMwSZdNk=
github.com/openstack-k8s-operators/cinder-operator/api v0.5.1-0.20241217072755-fb4d39411ad2/go.mod h1:GoZVwi0mFiqivii2K+EBYW0meEv/X8txqqpBRjxDVbc=
github.com/openstack-k8s-operators/infra-operator/apis v0.5.1-0.20241217184302-c302f3d72ada h1:5rkkywGCNBimAR7WWHpMQtJgWlZf3O++5JquvFM4GB0=
github.com/openstack-k8s-operators/infra-operator/apis v0.5.1-0.20241217184302-c302f3d72ada/go.mod h1:gznNWtIOdZLwyv3/LmWbDqtwRgtyzCw616Rwrn51DT0=
github.com/openstack-k8s-operators/keystone-operator/api v0.5.1-0.20241217165019-8e243bd36596 h1:JKeShCY9BQj6cYDk44bgEIm8jPcvggodGxrW4ECzsv4=
github.com/openstack-k8s-operators/keystone-operator/api v0.5.1-0.20241217165019-8e243bd36596/go.mod h1:CyuEOM1TpXKNUR1n8cudNtRzTEwkzv90JFkpDPPId8E=
github.com/openstack-k8s-operators/lib-common/modules/common v0.5.1-0.20241216113837-d172b3ac0f4e h1:hf4kVQBkyG79WcHBxdQ25QrDBbGFdarebS1Tc0Xclq4=
github.com/openstack-k8s-operators/lib-common/modules/common v0.5.1-0.20241216113837-d172b3ac0f4e/go.mod h1:YpNTuJhDWhbXM50O3qBkhO7M+OOyRmWkNVmJ4y3cyFs=
github.com/openstack-k8s-operators/lib-common/modules/openstack v0.5.1-0.20241216113837-d172b3ac0f4e h1:HFo4OqPY0x4ZQeaWI2YGonTXAGTQFt+rOEJlfZVhS7s=
github.com/openstack-k8s-operators/lib-common/modules/openstack v0.5.1-0.20241216113837-d172b3ac0f4e/go.mod h1:IASoGvp5QM/tBJUd/8i8uIjj4DBnI+64Ydh4r7pmnvA=
github.com/openstack-k8s-operators/lib-common/modules/storage v0.5.1-0.20241216113837-d172b3ac0f4e h1:Qz0JFEoRDUyjEWorNY3LggwxTsmpMtQkcpmZDQulGHQ=
Expand Down

0 comments on commit 57cd522

Please sign in to comment.