fix(kong): upgrade --install and handle error properly (#536)

internal/retry/command.go

@@ -50,11 +50,16 @@ func (c commandDoer) Do(ctx context.Context) error {
 	)
 }
 
+// DoWithErrorHandling executes the command and runs errorFunc passing a resulting err, stdout and stderr to be handled
+// by the caller. The errorFunc is going to be called only when the resulting err != nil.
 func (c commandDoer) DoWithErrorHandling(ctx context.Context, errorFunc ErrorFunc) error {
 	return retry.Do(func() error {
 		cmd, stdout, stderr := c.createCmd(ctx)
 		err := cmd.Run()
-		return errorFunc(err, stdout, stderr)
+		if err != nil {
+			return errorFunc(err, stdout, stderr)
+		}
+		return nil
 	},
 		c.createOpts(ctx)...,
 	)

internal/retry/command_test.go

@@ -0,0 +1,42 @@
+package retry_test
+
+import (
+	"bytes"
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/kong/kubernetes-testing-framework/internal/retry"
+)
+
+func TestDoWithErrorHandling(t *testing.T) {
+	t.Run("succeeded command won't call the errorFunc", func(t *testing.T) {
+		cmd := retry.Command("echo", "test")
+
+		itShouldntGetCalled := func(err error, _ *bytes.Buffer, _ *bytes.Buffer) error {
+			t.Error("this function shouldn't be called because there was no error running command")
+			return err
+		}
+		err := cmd.DoWithErrorHandling(context.Background(), itShouldntGetCalled)
+		require.NoError(t, err)
+	})
+
+	t.Run("failing command will call the errorFunc", func(t *testing.T) {
+		cmd := retry.Command("unknown-command")
+
+		wasCalled := false
+		itShouldBeCalled := func(err error, _ *bytes.Buffer, _ *bytes.Buffer) error {
+			wasCalled = true
+			return err
+		}
+
+		// Wait just a second to not make tests run too long. It's enough to know the errorFunc was called at least once.
+		ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+		defer cancel()
+		err := cmd.DoWithErrorHandling(ctx, itShouldBeCalled)
+		require.Error(t, err)
+		require.True(t, wasCalled, "expected errorFunc to be called because the command has failed")
+	})
+}

pkg/clusters/addons/kong/addon.go

@@ -245,7 +245,7 @@ func (a *Addon) Deploy(ctx context.Context, cluster clusters.Cluster) error {
 	}
 
 	// if the dbmode is postgres, set several related values
-	args := []string{"--kubeconfig", kubeconfig.Name(), "install", DefaultDeploymentName, "kong/kong"}
+	args := []string{"--kubeconfig", kubeconfig.Name(), "upgrade", "--install", DefaultDeploymentName, "kong/kong"}
 	if a.proxyDBMode == PostgreSQL {
 		a.deployArgs = append(a.deployArgs, []string{
 			"--set", "env.database=postgres",
@@ -343,10 +343,11 @@ func (a *Addon) Deploy(ctx context.Context, cluster clusters.Cluster) error {
 	return retry.
 		Command("helm", args...).
 		DoWithErrorHandling(ctx, func(err error, _, stderr *bytes.Buffer) error {
-			if !strings.Contains(stderr.String(), "cannot re-use") {
-				return fmt.Errorf("%s: %w", stderr, err)
+			// ignore if addon is already deployed
+			if strings.Contains(stderr.String(), "cannot re-use") {
+				return nil
 			}
-			return nil
+			return fmt.Errorf("%s: %w", stderr, err)
 		})
 }
 

test/integration/kongaddon_test.go

@@ -83,6 +83,9 @@ func testKongAddonWithCustomImage(t *testing.T, tc customImageTest) {
 	env, err := builder.Build(ctx)
 	require.NoError(t, err)
 
+	err = <-env.WaitForReady(ctx)
+	require.NoError(t, err)
+
 	t.Logf("setting up the environment cleanup for environment %s and cluster %s", env.Name(), env.Cluster().Name())
 	defer func() {
 		t.Logf("cleaning up environment %s and cluster %s", env.Name(), env.Cluster().Name())