Repository: submariner-io/submariner-operator
Pull request #1912 (2022-03-07 08:26:21)

Fix extraneous uninstall pods present after cleanup complete
After the finalizer is finalizer, it can take a little time before K8s
deletes the object. In the mean time, the controller may get triggered
again in which case it will observe the DeletionTimeStamp non-zero and
re-create the uninstall daemon sets. To avoid this, elide running
cleanup if the finalizer isn't present.

Also increased the requeue interval to 500 ms.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
(cherry picked from commit a1dbdeebe3c7f39a14784d353c81569307dc740f)

<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our developer guide: https://submariner.io/development/
2. Ensure you have added the appropriate tests for your PR: https://submariner.io/development/code-review/#test-new-functionality
3. Read the code review guide to ease the review process: https://submariner.io/development/code-review/
4. If the PR is unfinished, mark it as a draft: https://submariner.io/development/code-review/#mark-work-in-progress-prs-as-drafts
5. If you are using CI to debug, use your private fork: https://submariner.io/development/code-review/#use-private-forks-for-debugging-prs-by-running-ci
6. Add labels to the PR as appropriate.

This template is based on the K8s/K8s template:

https://github.com/kubernetes/kubernetes/blob/master/.github/PULL_REQUEST_TEMPLATE.md
-->


submariner-bot: 🤖 Created branch: z_pr1912/skitt/backport-1899-0.12 
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

Patch:
diff --git a/controllers/servicediscovery/cleanup.go b/controllers/servicediscovery/cleanup.go
index a2683683..81efb3d1 100644
--- a/controllers/servicediscovery/cleanup.go
+++ b/controllers/servicediscovery/cleanup.go
@@ -40,6 +40,10 @@ import (
 )
 
 func (r *Reconciler) doCleanup(ctx context.Context, instance *operatorv1alpha1.ServiceDiscovery) (reconcile.Result, error) {
+	if !finalizer.IsPresent(instance, constants.CleanupFinalizer) {
+		return reconcile.Result{}, nil
+	}
+
 	if !uninstall.IsSupportedForVersion(instance.Spec.Version) {
 		log.Info("Deleting ServiceDiscovery version does not support uninstall", "version", instance.Spec.Version)
 		return reconcile.Result{}, r.removeFinalizer(ctx, instance)
@@ -87,7 +91,7 @@ func (r *Reconciler) doCleanup(ctx context.Context, instance *operatorv1alpha1.S
 	}
 
 	if requeue {
-		return reconcile.Result{RequeueAfter: time.Millisecond * 100}, nil
+		return reconcile.Result{RequeueAfter: time.Millisecond * 500}, nil
 	}
 
 	return reconcile.Result{}, r.removeFinalizer(ctx, instance)
diff --git a/controllers/submariner/cleanup.go b/controllers/submariner/cleanup.go
index f047b40b..b779f25f 100644
--- a/controllers/submariner/cleanup.go
+++ b/controllers/submariner/cleanup.go
@@ -35,6 +35,10 @@ import (
 )
 
 func (r *Reconciler) runComponentCleanup(ctx context.Context, instance *operatorv1alpha1.Submariner) (reconcile.Result, error) {
+	if !finalizer.IsPresent(instance, constants.CleanupFinalizer) {
+		return reconcile.Result{}, nil
+	}
+
 	if !uninstall.IsSupportedForVersion(instance.Spec.Version) {
 		log.Info("Deleting Submariner version does not support uninstall", "version", instance.Spec.Version)
 		return reconcile.Result{}, r.removeFinalizer(ctx, instance)
@@ -89,7 +93,7 @@ func (r *Reconciler) runComponentCleanup(ctx context.Context, instance *operator
 	}
 
 	if requeue {
-		return reconcile.Result{RequeueAfter: time.Millisecond * 100}, nil
+		return reconcile.Result{RequeueAfter: time.Millisecond * 500}, nil
 	}
 
 	return reconcile.Result{}, r.removeFinalizer(ctx, instance)
diff --git a/go.mod b/go.mod
index a8fafa76..f7f207cd 100644
--- a/go.mod
+++ b/go.mod
@@ -27,7 +27,7 @@ require (
 	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.12.1
 	github.com/spf13/cobra v1.3.0
-	github.com/submariner-io/admiral v0.12.0-rc0
+	github.com/submariner-io/admiral v0.12.0-rc0.0.20220304174809-981438a81552
 	github.com/submariner-io/cloud-prepare v0.12.0-rc0
 	github.com/submariner-io/lighthouse v0.12.0-rc0
 	github.com/submariner-io/shipyard v0.12.0-rc0
diff --git a/go.sum b/go.sum
index 37dbf9a4..4851bacd 100644
--- a/go.sum
+++ b/go.sum
@@ -1402,8 +1402,9 @@ github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-github.com/submariner-io/admiral v0.12.0-rc0 h1:ttK1eBSz/fXSkrUZPS92hbWCw2AEQKpltGf0ZIDddUQ=
 github.com/submariner-io/admiral v0.12.0-rc0/go.mod h1:D+sR9aN135wKT6cS7NAr/TfCSJOzPqPVMAhsBSqAs1I=
+github.com/submariner-io/admiral v0.12.0-rc0.0.20220304174809-981438a81552 h1:tDwd6snT2cduQMva5ZJQaFxruE5uUzbroBnJMHarLgw=
+github.com/submariner-io/admiral v0.12.0-rc0.0.20220304174809-981438a81552/go.mod h1:D+sR9aN135wKT6cS7NAr/TfCSJOzPqPVMAhsBSqAs1I=
 github.com/submariner-io/cloud-prepare v0.12.0-rc0 h1:QbiVzWqmoY0t72oftW5yf26euytX/OoiHcJXQMlMZEQ=
 github.com/submariner-io/cloud-prepare v0.12.0-rc0/go.mod h1:D+lHuBMKQiZJFSTo2DIPfvvLXiOmKABsfMkDe+bR4ZY=
 github.com/submariner-io/lighthouse v0.12.0-rc0 h1:rD1fdUKcWl7D69LDvCNHcLPq23TcrSobA0XROfqNbeo=


Test patch:
diff --git a/controllers/servicediscovery/servicediscovery_controller_test.go b/controllers/servicediscovery/servicediscovery_controller_test.go
index d8bce300..3504306a 100644
--- a/controllers/servicediscovery/servicediscovery_controller_test.go
+++ b/controllers/servicediscovery/servicediscovery_controller_test.go
@@ -314,6 +314,9 @@ func testDeploymentUninstall() {
 			t.AssertNoDeployment(names.AppendUninstall(names.ServiceDiscoveryComponent))
 
 			t.awaitNoFinalizer()
+
+			t.AssertReconcileSuccess()
+			t.AssertNoDeployment(names.AppendUninstall(names.ServiceDiscoveryComponent))
 		})
 	})
 
diff --git a/controllers/submariner/submariner_controller_test.go b/controllers/submariner/submariner_controller_test.go
index cd06598e..64241793 100644
--- a/controllers/submariner/submariner_controller_test.go
+++ b/controllers/submariner/submariner_controller_test.go
@@ -313,6 +313,9 @@ func testDeletion() {
 			t.AssertNoDeployment(names.AppendUninstall(names.NetworkPluginSyncerComponent))
 
 			t.awaitNoFinalizer()
+
+			t.AssertReconcileSuccess()
+			t.AssertNoDaemonSet(names.AppendUninstall(names.GatewayComponent))
 		})
 	})