From 966da9c3c5ad1fc9dc81621b47deecffebafaafd Mon Sep 17 00:00:00 2001
From: Balazs Gibizer <gibi@redhat.com>
Date: Mon, 12 Feb 2024 12:47:36 +0100
Subject: [PATCH] Fix reconcileDelete during namespace delete

`oc delete namespace openstack` hangs forever because KeystoneAPI is
reconciled in the state when the namespace already being terminated but
the KeystoneAPI CR has no delete timestamp set yet. In this state
KeystoneAPI's reconciler tries to manipulate things in the namespace
(service account, TransportURL, etc) and that is rejected by k8s. This
cause KeystoneAPI to become Ready=False. This then prevent
KeystoneEndpoint and KeystoneService deletion to proceed as they are
waiting for KeystoneAPI to become Ready=true.

This patch implements one possible solution. It assumes if the
KeystoneAPI is being deleted then there is no point removing the
endpoints and users from the keystone DB as the DB will be deleted too.
Therefore the KeystoneEndpoint and KeystoneService simply removes the
finalizers if they detect that KeystoneAPI deleted timestamp is set.

This partially resolves the namespace deletion hangs. (There is another
issue with DNSMasq CR that still blocks the namespace deletion)

Fixes: OSP-27793
---
 controllers/keystoneendpoint_controller.go | 46 ++++++++++++++++++++++
 controllers/keystoneservice_controller.go  | 32 +++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/controllers/keystoneendpoint_controller.go b/controllers/keystoneendpoint_controller.go
index 78c230a3..62620e0a 100644
--- a/controllers/keystoneendpoint_controller.go
+++ b/controllers/keystoneendpoint_controller.go
@@ -157,6 +157,15 @@ func (r *KeystoneEndpointReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		return ctrl.Result{}, err
 	}
 
+	// If both the endpoint and the KeystoneAPI is deleted then we can skip
+	// the cleanup of the endpoint in the DB as the DB is going away as well.
+	// Moreover if KeystoneAPI is being deleted then we cannot talk to the
+	// keystone REST API any more. This happens for example during namespace
+	// deletion.
+	if !instance.DeletionTimestamp.IsZero() && !keystoneAPI.DeletionTimestamp.IsZero() {
+		return r.reconcileDeleteFinalizersOnly(ctx, instance, helper, keystoneAPI)
+	}
+
 	// If this KeystoneEndpoint CR is being deleted and it has not registered any actual
 	// endpoints on the OpenStack side, just redirect execution to the "reconcileDelete()"
 	// logic to avoid potentially hanging on waiting for the KeystoneAPI to be ready
@@ -303,6 +312,43 @@ func (r *KeystoneEndpointReconciler) reconcileDelete(
 	return ctrl.Result{}, nil
 }
 
+func (r *KeystoneEndpointReconciler) reconcileDeleteFinalizersOnly(
+	ctx context.Context,
+	instance *keystonev1.KeystoneEndpoint,
+	helper *helper.Helper,
+	keystoneAPI *keystonev1.KeystoneAPI,
+) (ctrl.Result, error) {
+	l := GetLog(ctx)
+	l.Info("Reconciling Endpoint delete while KeystoneAPI is being deleted")
+
+	ksSvc, err := keystonev1.GetKeystoneServiceWithName(ctx, helper, instance.Spec.ServiceName, instance.Namespace)
+	if err == nil {
+		// Remove the finalizer for this endpoint from the Service
+		if controllerutil.RemoveFinalizer(ksSvc, fmt.Sprintf("%s-%s", helper.GetFinalizer(), instance.Name)) {
+			err := r.Update(ctx, ksSvc)
+
+			if err != nil {
+				return ctrl.Result{}, err
+			}
+		}
+	} else if !k8s_errors.IsNotFound(err) {
+		return ctrl.Result{}, err
+	}
+
+	if controllerutil.RemoveFinalizer(keystoneAPI, fmt.Sprintf("%s-%s", helper.GetFinalizer(), instance.Name)) {
+		err := r.Update(ctx, keystoneAPI)
+
+		if err != nil {
+			return ctrl.Result{}, err
+		}
+	}
+
+	controllerutil.RemoveFinalizer(instance, helper.GetFinalizer())
+	l.Info("Reconciled Endpoint delete successfully")
+
+	return ctrl.Result{}, nil
+}
+
 func (r *KeystoneEndpointReconciler) reconcileNormal(
 	ctx context.Context,
 	instance *keystonev1.KeystoneEndpoint,
diff --git a/controllers/keystoneservice_controller.go b/controllers/keystoneservice_controller.go
index 4ced743e..d9bbb12d 100644
--- a/controllers/keystoneservice_controller.go
+++ b/controllers/keystoneservice_controller.go
@@ -170,6 +170,15 @@ func (r *KeystoneServiceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return ctrl.Result{}, err
 	}
 
+	// If both the service and the KeystoneAPI is deleted then we can skip
+	// the cleanup of the service in the DB as the DB is going away as well.
+	// Moreover if KeystoneAPI is being deleted then we cannot talk to the
+	// keystone REST API any more. This happens for example during namespace
+	// deletion.
+	if !instance.DeletionTimestamp.IsZero() && !keystoneAPI.DeletionTimestamp.IsZero() {
+		return r.reconcileDeleteFinalizersOnly(ctx, instance, helper, keystoneAPI)
+	}
+
 	// If this KeystoneService CR is being deleted and it has not registered any actual
 	// service on the OpenStack side, just redirect execution to the "reconcileDelete()"
 	// logic to avoid potentially hanging on waiting for the KeystoneAPI to be ready
@@ -301,6 +310,29 @@ func (r *KeystoneServiceReconciler) reconcileDelete(
 	return ctrl.Result{}, nil
 }
 
+func (r *KeystoneServiceReconciler) reconcileDeleteFinalizersOnly(
+	ctx context.Context,
+	instance *keystonev1.KeystoneService,
+	helper *helper.Helper,
+	keystoneAPI *keystonev1.KeystoneAPI,
+) (ctrl.Result, error) {
+	l := GetLog(ctx)
+	l.Info("Reconciling Service delete while KeystoneAPI is being deleted")
+
+	if controllerutil.RemoveFinalizer(keystoneAPI, fmt.Sprintf("%s-%s", helper.GetFinalizer(), instance.Name)) {
+		err := r.Update(ctx, keystoneAPI)
+
+		if err != nil {
+			return ctrl.Result{}, err
+		}
+	}
+
+	controllerutil.RemoveFinalizer(instance, helper.GetFinalizer())
+	l.Info("Reconciled Service delete successfully")
+
+	return ctrl.Result{}, nil
+}
+
 func (r *KeystoneServiceReconciler) reconcileNormal(
 	ctx context.Context,
 	instance *keystonev1.KeystoneService,