Merge pull request #401 from abays/fix_finalizer_lockup

Only KeystoneAPI finalizer from sub-resources when actually needed

controllers/keystoneendpoint_controller.go

@@ -175,19 +175,6 @@ func (r *KeystoneEndpointReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		return r.reconcileDelete(ctx, instance, helper, nil, keystoneAPI)
 	}
 
-	//
-	// Add a finalizer to the KeystoneAPI for this endpoint instance, as we do not want
-	// the KeystoneAPI to disappear before this endpoint in the case where this endpoint
-	// is deleted
-	//
-	if controllerutil.AddFinalizer(keystoneAPI, fmt.Sprintf("%s-%s", helper.GetFinalizer(), instance.Name)) {
-		err := r.Update(ctx, keystoneAPI)
-
-		if err != nil {
-			return ctrl.Result{}, err
-		}
-	}
-
 	if !keystoneAPI.IsReady() {
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			keystonev1.KeystoneAPIReadyCondition,
@@ -233,7 +220,7 @@ func (r *KeystoneEndpointReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	}
 
 	// Handle non-deleted clusters
-	return r.reconcileNormal(ctx, instance, helper, os)
+	return r.reconcileNormal(ctx, instance, helper, os, keystoneAPI)
 }
 
 // SetupWithManager sets up the controller with the Manager.
@@ -279,6 +266,9 @@ func (r *KeystoneEndpointReconciler) reconcileDelete(
 		}
 	}
 
+	// Remove endpoints from status
+	instance.Status.EndpointIDs = map[string]string{}
+
 	ksSvc, err := keystonev1.GetKeystoneServiceWithName(ctx, helper, instance.Spec.ServiceName, instance.Namespace)
 	if err == nil {
 		// Remove the finalizer for this endpoint from the Service
@@ -354,6 +344,7 @@ func (r *KeystoneEndpointReconciler) reconcileNormal(
 	instance *keystonev1.KeystoneEndpoint,
 	helper *helper.Helper,
 	os *openstack.OpenStack,
+	keystoneAPI *keystonev1.KeystoneAPI,
 ) (ctrl.Result, error) {
 	l := GetLog(ctx)
 	l.Info("Reconciling Endpoint normal")
@@ -385,6 +376,20 @@ func (r *KeystoneEndpointReconciler) reconcileNormal(
 
 	instance.Status.ServiceID = ksSvc.Status.ServiceID
 
+	//
+	// Add a finalizer to the KeystoneAPI for this endpoint instance, as we do not want the
+	// KeystoneAPI to disappear before this endpoint in the case where this endpoint is deleted
+	// (so that we can properly remove the endpoint from the Keystone database on the OpenStack
+	// side)
+	//
+	if controllerutil.AddFinalizer(keystoneAPI, fmt.Sprintf("%s-%s", helper.GetFinalizer(), instance.Name)) {
+		err := r.Update(ctx, keystoneAPI)
+
+		if err != nil {
+			return ctrl.Result{}, err
+		}
+	}
+
 	//
 	// Add a finalizer to KeystoneService, because KeystoneEndpoint is dependent on
 	// the service entry created by KeystoneService

controllers/keystoneservice_controller.go

@@ -188,19 +188,6 @@ func (r *KeystoneServiceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return r.reconcileDelete(ctx, instance, helper, nil, keystoneAPI)
 	}
 
-	//
-	// Add a finalizer to the KeystoneAPI for this service instance, as we do not want
-	// the KeystoneAPI to disappear before this service in the case where this service
-	// is deleted
-	//
-	if controllerutil.AddFinalizer(keystoneAPI, fmt.Sprintf("%s-%s", helper.GetFinalizer(), instance.Name)) {
-		err := r.Update(ctx, keystoneAPI)
-
-		if err != nil {
-			return ctrl.Result{}, err
-		}
-	}
-
 	if !keystoneAPI.IsReady() {
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			keystonev1.KeystoneAPIReadyCondition,
@@ -245,7 +232,7 @@ func (r *KeystoneServiceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	}
 
 	// Handle non-deleted clusters
-	return r.reconcileNormal(ctx, instance, helper, os)
+	return r.reconcileNormal(ctx, instance, helper, os, keystoneAPI)
 
 }
 
@@ -287,6 +274,11 @@ func (r *KeystoneServiceReconciler) reconcileDelete(
 			return ctrl.Result{}, err
 		}
 
+		// Clear the service ID so that any potential requeues after this reconcile
+		// will know that there is no need to worry about cleaning up the OpenStack
+		// side of things anymore (deferred PatchInstance call will persist this to
+		// etcd)
+		instance.Status.ServiceID = ""
 	} else {
 		l.Info("Not deleting service as there is no stores service ID", "KeystoneService", instance.Spec.ServiceName)
 	}
@@ -338,10 +330,25 @@ func (r *KeystoneServiceReconciler) reconcileNormal(
 	instance *keystonev1.KeystoneService,
 	helper *helper.Helper,
 	os *openstack.OpenStack,
+	keystoneAPI *keystonev1.KeystoneAPI,
 ) (ctrl.Result, error) {
 	l := GetLog(ctx)
 	l.Info("Reconciling Service")
 
+	//
+	// Add a finalizer to the KeystoneAPI for this service instance, as we do not want the
+	// KeystoneAPI to disappear before this service in the case where this service is deleted
+	// (so that we can properly remove the service and user from the Keystone database on the
+	// OpenStack side)
+	//
+	if controllerutil.AddFinalizer(keystoneAPI, fmt.Sprintf("%s-%s", helper.GetFinalizer(), instance.Name)) {
+		err := r.Update(ctx, keystoneAPI)
+
+		if err != nil {
+			return ctrl.Result{}, err
+		}
+	}
+
 	//
 	// Create new service if ServiceID is not already set
 	//