Repository: submariner-io/submariner-operator
Pull request #1899 (2022-03-04 12:40:38)

Fix extraneous uninstall pods present after cleanup complete
After the finalizer is removed, 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.

Depends on https://github.com/submariner-io/admiral/pull/327

submariner-bot: 🤖 Created branch: z_pr1899/tpantelis/extra_uninstall_pods 
🚀 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.
github-actions[bot]: :hourglass_flowing_sand: Alright! Looks like we need to wait for some *dependencies*:

* submariner-io/admiral#327

:bulb: Don't worry, I will continue watching the list above and keep this comment updated. To add or remove a dependency please update this issue/PR description.

Brought to you by **[Dependent Issues](https://github.com/z0al/dependent-issues)** (:robot: ). Happy coding!
<!-- By Dependent Issues (Action) - DO NOT REMOVE -->
sridhargaddam:  
> Depends on [submariner-io/admiral#327](https://github.com/submariner-io/admiral/pull/327)

It says "Depends on" but i dont see that its using the new API from Admiral. 
skitt: > > Depends on [submariner-io/admiral#327](https://github.com/submariner-io/admiral/pull/327)
> 
> It says "Depends on" but i dont see that its using the new API from Admiral.

I imagine Tom wants to rework this to use the Admiral utility (as mentioned in the `TODO`s).
github-actions[bot]: :tada: Great news! Looks like all the *dependencies* have been resolved:

* ~~submariner-io/admiral#327~~

:bulb: To add or remove a dependency please update this issue/PR description.

Brought to you by **[Dependent Issues](https://github.com/z0al/dependent-issues)** (:robot: ). Happy coding!
<!-- By Dependent Issues (Action) - DO NOT REMOVE -->
tpantelis: > > > Depends on [submariner-io/admiral#327](https://github.com/submariner-io/admiral/pull/327)
> > 
> > 
> > It says "Depends on" but i dont see that its using the new API from Admiral.
> 
> I imagine Tom wants to rework this to use the Admiral utility (as mentioned in the `TODO`s).

Yes and done.

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 4bc63576..2d5e259d 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-m3.0.20220211050139-69a40598bdd6
+	github.com/submariner-io/admiral v0.12.0-m3.0.20220304150942-10c88bb61e69
 	github.com/submariner-io/cloud-prepare v0.12.0-m3.0.20220216201630-d2a9f9d88aae
 	github.com/submariner-io/lighthouse v0.12.0-m3.0.20220215144325-3a00dd24acab
 	github.com/submariner-io/shipyard v0.12.0-m3
diff --git a/go.sum b/go.sum
index ce86ed22..ccabc392 100644
--- a/go.sum
+++ b/go.sum
@@ -1408,6 +1408,8 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
 github.com/submariner-io/admiral v0.12.0-m3/go.mod h1:UBcJ547DlOWcX13HtWBS83tmd+DcWDu9FDbmI0iDSU4=
 github.com/submariner-io/admiral v0.12.0-m3.0.20220211050139-69a40598bdd6 h1:RMgMytzHKdn4giyMDnu9oFmT40ea0NX75HcttQCoL/U=
 github.com/submariner-io/admiral v0.12.0-m3.0.20220211050139-69a40598bdd6/go.mod h1:hFPcxH0gAr559/qK7ri10Yh1wyeK6yzow1PIqtBk+30=
+github.com/submariner-io/admiral v0.12.0-m3.0.20220304150942-10c88bb61e69 h1:BmjnngDV0IwUJ4FXM7eWxbvjUCXTBMBxWgzL7bjLxGo=
+github.com/submariner-io/admiral v0.12.0-m3.0.20220304150942-10c88bb61e69/go.mod h1:hFPcxH0gAr559/qK7ri10Yh1wyeK6yzow1PIqtBk+30=
 github.com/submariner-io/cloud-prepare v0.12.0-m3.0.20220216201630-d2a9f9d88aae h1:90Y+wiXPt5T6agzcV0YhWUHGcXRhCmVM5L+y+noOhSw=
 github.com/submariner-io/cloud-prepare v0.12.0-m3.0.20220216201630-d2a9f9d88aae/go.mod h1:yhEfsTN1GbfhGfvZFJOddAh1LOHE4YrhkDNpF8yxKyo=
 github.com/submariner-io/lighthouse v0.12.0-m3.0.20220215144325-3a00dd24acab h1:YIFa6fPm/MSXPMdKRjwUJvemZDDQa1t++phYuaM7PBI=


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))
 		})
 	})