Improve Helm uninstallation cleanup

test/cloud-slack-dev-e2e/cloud_slack_dev_e2e_test.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -90,7 +91,8 @@ func TestCloudSlackE2E(t *testing.T) {
 	cfg.Slack.Tester.CloudBasedTestEnabled = false // override property used only in the Cloud Slack E2E tests
 
 	authHeaderValue := ""
-	helmChartUninstalled := false
+	var botkubeDeploymentUninstalled atomic.Bool
+	botkubeDeploymentUninstalled.Store(true) // not yet installed
 	gqlEndpoint := fmt.Sprintf("%s/%s", cfg.BotkubeCloud.APIBaseURL, cfg.BotkubeCloud.APIGraphQLEndpoint)
 
 	if cfg.ScreenshotsDir != "" {
@@ -319,18 +321,7 @@ func TestCloudSlackE2E(t *testing.T) {
 		t.Log("Creating deployment...")
 		deployment := gqlCli.MustCreateBasicDeploymentWithCloudSlack(t, channel.Name(), slackWorkspace.TeamID, channel.Name())
 		t.Cleanup(func() {
-			// We have a glitch on backend side and the logic below is a workaround for that.
-			// Tl;dr uninstalling Helm chart reports "DISCONNECTED" status, and deplyment deletion reports "DELETED" status.
-			// If we do these two things too quickly, we'll run into resource version mismatch in repository logic.
-			// Read more here: https://github.com/kubeshop/botkube-cloud/pull/486#issuecomment-1604333794
-
-			for !helmChartUninstalled {
-				t.Log("Waiting for Helm chart uninstallation, in order to proceed with deleting the first deployment...")
-				time.Sleep(1 * time.Second)
-			}
-
-			t.Log("Helm chart uninstalled. Waiting a bit...")
-			time.Sleep(3 * time.Second) // ugly, but at least we will be pretty sure we won't run into the resource version mismatch
+			helmx.WaitForUninstallation(context.Background(), t, &botkubeDeploymentUninstalled)
 
 			t.Log("Deleting first deployment...")
 			gqlCli.MustDeleteDeployment(t, graphql.ID(deployment.ID))
@@ -351,11 +342,12 @@ func TestCloudSlackE2E(t *testing.T) {
 			Command:       *deployment.HelmCommand,
 			PluginRepoURL: cfg.BotkubeCloud.PluginRepoURL,
 		}
+		botkubeDeploymentUninstalled.Store(false) // it is being installed now
 		helmInstallCallback := helmx.InstallChart(t, params)
 		t.Cleanup(func() {
 			t.Log("Uninstalling Helm chart...")
 			helmInstallCallback(t)
-			helmChartUninstalled = true
+			botkubeDeploymentUninstalled.Store(true)
 		})
 
 		t.Log("Waiting for help message...")

test/e2e/bots_test.go

@@ -15,6 +15,8 @@ import (
 	"time"
 	"unicode"
 
+	"botkube.io/botube/test/helmx"
+
 	"botkube.io/botube/test/botkubex"
 	"botkube.io/botube/test/commplatform"
 	"botkube.io/botube/test/diff"
@@ -202,7 +204,8 @@ func runBotTest(t *testing.T,
 	deployEnvSecondaryChannelIDName,
 	deployEnvRbacChannelIDName string,
 ) {
-	botkubeDeploymentUninstalled := &atomic.Bool{}
+	var botkubeDeploymentUninstalled atomic.Bool
+	botkubeDeploymentUninstalled.Store(true) // not yet installed
 
 	t.Logf("Creating API client with provided token for %s...", driverType)
 	botDriver, err := newBotDriver(appCfg, driverType)
@@ -261,18 +264,7 @@ func runBotTest(t *testing.T,
 			gqlCli.MustCreateAlias(t, alias[0], alias[1], alias[2], deployment.ID)
 		}
 		t.Cleanup(func() {
-			ctx := context.WithTimeout(context.Background(), )
-			// We have a glitch on backend side and the logic below is a workaround for that.
-			// Tl;dr uninstalling Helm chart reports "DISCONNECTED" status, and deployment deletion reports "DELETED" status.
-			// If we do these two things too quickly, we'll run into resource version mismatch in repository logic.
-			// Read more here: https://github.com/kubeshop/botkube-cloud/pull/486#issuecomment-1604333794
-			for !botkubeDeploymentUninstalled.Load() {
-
-				case :
-				default:
-					t.Log("Waiting for Helm chart uninstallation, in order to proceed with deleting Botkube Cloud instance...")
-					time.Sleep(1 * time.Second)
-			}
+			helmx.WaitForUninstallation(context.Background(), t, &botkubeDeploymentUninstalled)
 
 			t.Log("Helm chart uninstalled. Waiting a bit...")
 			time.Sleep(3 * time.Second) // ugly, but at least we will be pretty sure we won't run into the resource version mismatch
@@ -281,6 +273,7 @@ func runBotTest(t *testing.T,
 			gqlCli.MustDeleteDeployment(t, graphql.ID(deployment.ID))
 		})
 
+		botkubeDeploymentUninstalled.Store(false)
 		err = botkubex.Install(t, botkubex.InstallParams{
 			BinaryPath:                              appCfg.ConfigProvider.BotkubeCliBinaryPath,
 			HelmRepoDirectory:                       appCfg.ConfigProvider.HelmRepoDirectory,
@@ -297,7 +290,7 @@ func runBotTest(t *testing.T,
 		t.Cleanup(func() {
 			t.Log("Uninstalling Helm chart...")
 			botkubex.Uninstall(t, appCfg.ConfigProvider.BotkubeCliBinaryPath)
-			botkubeDeploymentUninstalled = true
+			botkubeDeploymentUninstalled.Store(true)
 		})
 	}
 

test/helmx/helm_helpers.go

@@ -1,12 +1,15 @@
 package helmx
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"os/exec"
 	"regexp"
 	"strings"
+	"sync/atomic"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 )
@@ -108,3 +111,32 @@ func redactAPIKey(in []string) []string {
 	}
 	return dst
 }
+
+const (
+	uninstallTimeout = 30 * time.Second
+)
+
+// WaitForUninstallation waits until a Helm chart is uninstalled, based on the atomic value.
+func WaitForUninstallation(ctx context.Context, t *testing.T, alreadyUninstalled *atomic.Bool) {
+	ctxWithTimeout, cancelFunc := context.WithTimeout(ctx, uninstallTimeout)
+	defer cancelFunc()
+
+	// We have a glitch on backend side and the logic below is a workaround for that.
+	// Tl;dr uninstalling Helm chart reports "DISCONNECTED" status, and deployment deletion reports "DELETED" status.
+	// If we do these two things too quickly, we'll run into resource version mismatch in repository logic.
+	// Read more here: https://github.com/kubeshop/botkube-cloud/pull/486#issuecomment-1604333794
+wait:
+	for !alreadyUninstalled.Load() {
+		select {
+		case <-ctxWithTimeout.Done():
+			t.Log("Force uninstalling the Helm chart ")
+			break wait
+		default:
+			t.Log("Waiting for Helm chart uninstallation, in order to proceed with deleting Botkube Cloud instance...")
+			time.Sleep(1 * time.Second)
+		}
+	}
+
+	t.Log("Waiting a bit more...")
+	time.Sleep(3 * time.Second) // ugly, but at least we will be pretty sure we won't run into the resource version mismatch
+}

test/helmx/redact_test.go

@@ -7,7 +7,6 @@ import (
 )
 
 func TestRedactAPIKey(t *testing.T) {
-
 	tests := []struct {
 		name     string
 		input    []string