diff --git a/internal/controller/ansibletest_controller.go b/internal/controller/ansibletest_controller.go index 67e9c80c..5762ef77 100644 --- a/internal/controller/ansibletest_controller.go +++ b/internal/controller/ansibletest_controller.go @@ -58,7 +58,6 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request) ServiceName: ansibletest.ServiceName, NeedsNetworkAttachments: false, NeedsConfigMaps: false, - NeedsFinalizer: false, SupportsWorkflow: true, BuildPod: func(ctx context.Context, instance *testv1beta1.AnsibleTest, labels, annotations map[string]string, workflowStepIndex int, pvcIndex int) (*corev1.Pod, error) { diff --git a/internal/controller/common.go b/internal/controller/common.go index dffa413d..4bd3fff6 100644 --- a/internal/controller/common.go +++ b/internal/controller/common.go @@ -51,6 +51,8 @@ const ( const ( // ErrConfirmLockOwnership is the error message for lock ownership confirmation failures ErrConfirmLockOwnership = "Can not confirm ownership of %s lock." + // ErrCleanupLock is the error message for failing to clean up the lock during pod deletion + ErrCleanupLock = "Failed to cleanup lock during pod deletion." ) const ( @@ -610,6 +612,22 @@ func (r *Reconciler) ReleaseLock(ctx context.Context, instance client.Object) (b return false, ErrFailedToDeleteLock } +// CleanupLock removes the lock for the given instance without waiting for confirmation +func (r *Reconciler) CleanupLock(ctx context.Context, instance client.Object) { + Log := r.GetLogger(ctx) + + cm, err := r.GetLockInfo(ctx, instance) + if err != nil { + return + } + + if cm.Data[testOperatorLockOwnerField] == string(instance.GetUID()) { + if err := r.Client.Delete(ctx, cm); err != nil { + Log.Info(ErrCleanupLock) + } + } +} + // GetPodIfExists returns the pod for the given instance and workflow step if it exists func (r *Reconciler) GetPodIfExists( ctx context.Context, diff --git a/internal/controller/common_controller.go b/internal/controller/common_controller.go index 41718f97..44ad7194 100644 --- a/internal/controller/common_controller.go +++ b/internal/controller/common_controller.go @@ -51,9 +51,6 @@ type TestResourceConfig[T TestResource] struct { // NeedsConfigMaps indicates if ServiceConfigReadyCondition is needed NeedsConfigMaps bool - // NeedsFinalizer indicates if the controller needs finalizer handling - NeedsFinalizer bool - // SupportsWorkflow indicates if the controller supports workflow feature SupportsWorkflow bool @@ -154,14 +151,15 @@ func CommonReconcile[T TestResource]( // If we're not deleting this and the service object doesn't have our // finalizer, add it. - if config.NeedsFinalizer && instance.GetDeletionTimestamp().IsZero() && + if instance.GetDeletionTimestamp().IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) { return ctrl.Result{}, nil } // Handle service delete - if config.NeedsFinalizer && !instance.GetDeletionTimestamp().IsZero() { + if !instance.GetDeletionTimestamp().IsZero() { Log.Info("Reconciling Service delete") + r.CleanupLock(ctx, instance) controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) Log.Info("Reconciled Service delete successfully") return ctrl.Result{}, nil diff --git a/internal/controller/horizontest_controller.go b/internal/controller/horizontest_controller.go index 8cf48df9..fb1110a9 100644 --- a/internal/controller/horizontest_controller.go +++ b/internal/controller/horizontest_controller.go @@ -58,7 +58,6 @@ func (r *HorizonTestReconciler) Reconcile(ctx context.Context, req ctrl.Request) ServiceName: horizontest.ServiceName, NeedsNetworkAttachments: false, NeedsConfigMaps: true, - NeedsFinalizer: false, SupportsWorkflow: false, GenerateServiceConfigMaps: func(ctx context.Context, helper *helper.Helper, labels map[string]string, instance *testv1beta1.HorizonTest, _ int) error { diff --git a/internal/controller/tempest_controller.go b/internal/controller/tempest_controller.go index 0d70e4a6..bd1e34f2 100644 --- a/internal/controller/tempest_controller.go +++ b/internal/controller/tempest_controller.go @@ -61,7 +61,6 @@ func (r *TempestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct ServiceName: tempest.ServiceName, NeedsNetworkAttachments: true, NeedsConfigMaps: true, - NeedsFinalizer: true, SupportsWorkflow: true, GenerateServiceConfigMaps: func(ctx context.Context, helper *helper.Helper, _ map[string]string, instance *testv1beta1.Tempest, workflowStep int) error { diff --git a/internal/controller/tobiko_controller.go b/internal/controller/tobiko_controller.go index 5ca071c4..ee97de53 100644 --- a/internal/controller/tobiko_controller.go +++ b/internal/controller/tobiko_controller.go @@ -61,7 +61,6 @@ func (r *TobikoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr ServiceName: tobiko.ServiceName, NeedsNetworkAttachments: true, NeedsConfigMaps: true, - NeedsFinalizer: false, SupportsWorkflow: true, GenerateServiceConfigMaps: func(ctx context.Context, helper *helper.Helper, labels map[string]string, instance *testv1beta1.Tobiko, workflowStepIndex int) error { diff --git a/test/functional/ansibletest_controller_test.go b/test/functional/ansibletest_controller_test.go index 83cbaf58..0c6cbca6 100644 --- a/test/functional/ansibletest_controller_test.go +++ b/test/functional/ansibletest_controller_test.go @@ -81,6 +81,12 @@ var _ = Describe("AnsibleTest controller", func() { Expect(ansibleTest.Spec.AnsibleGitRepo).ShouldNot(BeEmpty()) Expect(ansibleTest.Spec.AnsiblePlaybookPath).ShouldNot(BeEmpty()) }) + + It("should have a finalizer", func() { + Eventually(func() []string { + return GetAnsibleTest(ansibleTestName).Finalizers + }, timeout, interval).Should(ContainElement("openstack.org/ansibletest")) + }) }) When("All dependencies are ready", func() { diff --git a/test/functional/horizontest_controller_test.go b/test/functional/horizontest_controller_test.go index 4b94d433..573b84e9 100644 --- a/test/functional/horizontest_controller_test.go +++ b/test/functional/horizontest_controller_test.go @@ -83,6 +83,12 @@ var _ = Describe("HorizonTest controller", func() { Expect(horizonTest.Spec.DashboardUrl).ShouldNot(BeEmpty()) Expect(horizonTest.Spec.AuthUrl).ShouldNot(BeEmpty()) }) + + It("should have a finalizer", func() { + Eventually(func() []string { + return GetHorizonTest(horizonTestName).Finalizers + }, timeout, interval).Should(ContainElement("openstack.org/horizontest")) + }) }) When("All dependencies are ready", func() { diff --git a/test/functional/tempest_controller_test.go b/test/functional/tempest_controller_test.go index d0ee0533..db467c57 100644 --- a/test/functional/tempest_controller_test.go +++ b/test/functional/tempest_controller_test.go @@ -86,8 +86,6 @@ var _ = Describe("Tempest controller", func() { }) It("should have a finalizer", func() { - // the reconciler loop adds the finalizer so we have to wait for - // it to run Eventually(func() []string { return GetTempest(tempestName).Finalizers }, timeout, interval).Should(ContainElement("openstack.org/tempest")) diff --git a/test/functional/tobiko_controller_test.go b/test/functional/tobiko_controller_test.go index 08148082..6138b01e 100644 --- a/test/functional/tobiko_controller_test.go +++ b/test/functional/tobiko_controller_test.go @@ -82,6 +82,11 @@ var _ = Describe("Tobiko controller", func() { Expect(tobiko.Spec.Testenv).Should(Equal("sanity")) }) + It("should have a finalizer", func() { + Eventually(func() []string { + return GetTobiko(tobikoName).Finalizers + }, timeout, interval).Should(ContainElement("openstack.org/tobiko")) + }) }) When("All dependencies are ready", func() {