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,8 @@ 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
+			err := helmx.WaitForUninstallation(context.Background(), t, &botkubeDeploymentUninstalled)
+			assert.NoError(t, err)
 
 			t.Log("Deleting first deployment...")
 			gqlCli.MustDeleteDeployment(t, graphql.ID(deployment.ID))
@@ -343,6 +335,7 @@ func TestCloudSlackE2E(t *testing.T) {
 			gqlCli.MustDeleteDeployment(t, graphql.ID(deployment2.ID))
 		})
 
+		botkubeDeploymentUninstalled.Store(false) // about to be installed
 		params := helmx.InstallChartParams{
 			RepoURL:       "https://storage.googleapis.com/botkube-latest-main-charts",
 			RepoName:      "botkube",
@@ -355,7 +348,7 @@ func TestCloudSlackE2E(t *testing.T) {
 		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

@@ -10,10 +10,13 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 	"unicode"
 
+	"botkube.io/botube/test/helmx"
+
 	"botkube.io/botube/test/botkubex"
 	"botkube.io/botube/test/commplatform"
 	"botkube.io/botube/test/diff"
@@ -201,7 +204,9 @@ func runBotTest(t *testing.T,
 	deployEnvSecondaryChannelIDName,
 	deployEnvRbacChannelIDName string,
 ) {
-	botkubeDeploymentUninstalled := false
+	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)
 	require.NoError(t, err)
@@ -259,22 +264,14 @@ func runBotTest(t *testing.T,
 			gqlCli.MustCreateAlias(t, alias[0], alias[1], alias[2], deployment.ID)
 		}
 		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 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 {
-				t.Log("Waiting for Helm chart uninstallation, in order to proceed with deleting Botkube Cloud instance...")
-				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
+			err := helmx.WaitForUninstallation(context.Background(), t, &botkubeDeploymentUninstalled)
+			assert.NoError(t, err)
 
 			t.Log("Deleting Botkube Cloud instance...")
 			gqlCli.MustDeleteDeployment(t, graphql.ID(deployment.ID))
 		})
 
+		botkubeDeploymentUninstalled.Store(false) // about to be installed
 		err = botkubex.Install(t, botkubex.InstallParams{
 			BinaryPath:                              appCfg.ConfigProvider.BotkubeCliBinaryPath,
 			HelmRepoDirectory:                       appCfg.ConfigProvider.HelmRepoDirectory,
@@ -291,7 +288,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,17 @@
 package helmx
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"os/exec"
 	"regexp"
 	"strings"
+	"sync/atomic"
 	"testing"
+	"time"
+
+	"k8s.io/apimachinery/pkg/util/wait"
 
 	"github.com/stretchr/testify/require"
 )
@@ -108,3 +113,34 @@ func redactAPIKey(in []string) []string {
 	}
 	return dst
 }
+
+const (
+	uninstallPollInterval = 1 * time.Second
+	uninstallTimeout      = 30 * time.Second
+)
+
+// WaitForUninstallation waits until a Helm chart is uninstalled, based on the atomic value.
+// It's a workaround for the Helm chart uninstallation issue.
+// 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
+func WaitForUninstallation(ctx context.Context, t *testing.T, alreadyUninstalled *atomic.Bool) error {
+	t.Helper()
+	t.Log("Waiting for Helm chart uninstallation, in order to proceed with deleting Botkube Cloud instance...")
+	err := wait.PollUntilContextTimeout(ctx, uninstallPollInterval, uninstallTimeout, false, func(ctx context.Context) (done bool, err error) {
+		return alreadyUninstalled.Load(), nil
+	})
+	waitInterrupted := wait.Interrupted(err)
+	if err != nil && !waitInterrupted {
+		return err
+	}
+
+	if waitInterrupted {
+		t.Log("Force uninstalling the Helm chart after a wait timeout")
+	}
+
+	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
+	return nil
+}

test/helmx/redact_test.go

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