Improve external E2E test logic (#377)

This change does the following:
 - Converts individual external E2E tests to subtests. This now
   matches the standard E2E test flow and provides better test
   logging output.
 - Split Mattermost version upgrade test out from base test.
 - Improve reconciliation logging with more status output.
 - Improve reconciliation check with ObservedGeneration validation.
 - Update external E2E postgres container image to version 14.

resources/postgres.yaml

@@ -50,7 +50,7 @@ spec:
         tier: postgreSQL
     spec:
       containers:
-        - image: postgres:12-alpine
+        - image: postgres:14-alpine
           name: postgresql
           env:
             - name: POSTGRES_USER

test/e2e-external/external_db_filestore_test.go

@@ -7,16 +7,34 @@ import (
 
 	mmv1beta "github.com/mattermost/mattermost-operator/apis/mattermost/v1beta1"
 	ptrUtil "github.com/mattermost/mattermost-operator/pkg/utils"
-	operatortest "github.com/mattermost/mattermost-operator/test"
 	"github.com/mattermost/mattermost-operator/test/e2e"
 	"github.com/stretchr/testify/require"
-	appsv1 "k8s.io/api/apps/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 )
 
-func Test_MattermostExternalServices(t *testing.T) {
-	namespace := "e2e-test-external-db-file-store"
+func TestMattermostExternalServices(t *testing.T) {
+	t.Log("Running tests with external Mattermost Services")
+
+	t.Run("mattermost base test", func(t *testing.T) {
+		mattermostBaseTest(t)
+	})
+
+	t.Run("mattermost upgrade test", func(t *testing.T) {
+		mattermostUpgradeTest(t)
+	})
+
+	t.Run("mattermost size test", func(t *testing.T) {
+		mattermostSizeTest(t)
+	})
+
+	t.Run("mattermost ingress test", func(t *testing.T) {
+		mattermostIngressTest(t)
+	})
+}
+
+func mattermostBaseTest(t *testing.T) {
+	namespace := "e2e-test-external-services"
 
 	testEnv, err := SetupTestEnv(k8sClient, namespace)
 	require.NoError(t, err)
@@ -28,7 +46,6 @@ func Test_MattermostExternalServices(t *testing.T) {
 			Namespace: namespace,
 		},
 		Spec: mmv1beta.MattermostSpec{
-			Version: operatortest.PreviousStableMattermostVersion,
 			Ingress: &mmv1beta.Ingress{
 				Host: "e2e-test-example.mattermost.dev",
 			},
@@ -43,7 +60,6 @@ func Test_MattermostExternalServices(t *testing.T) {
 	}
 
 	expectValidMattermostInstance(t, mattermost)
-	checkVersionUpgrade(t, mattermost)
 }
 
 func expectValidMattermostInstance(t *testing.T, mattermost *mmv1beta.Mattermost) {
@@ -62,31 +78,3 @@ func expectValidMattermostInstance(t *testing.T, mattermost *mmv1beta.Mattermost
 	// this most likely needs to be done from inside the cluster
 	// by running some job.
 }
-
-func checkVersionUpgrade(t *testing.T, mattermost *mmv1beta.Mattermost) {
-	mmNamespaceName := types.NamespacedName{Namespace: mattermost.Namespace, Name: mattermost.Name}
-
-	newMattermost := &mmv1beta.Mattermost{}
-	err := k8sClient.Get(context.TODO(), mmNamespaceName, newMattermost)
-	require.NoError(t, err)
-
-	// Upgrade to new version
-	newMattermost.Spec.Version = operatortest.LatestStableMattermostVersion
-	err = k8sClient.Update(context.TODO(), newMattermost)
-	require.NoError(t, err)
-
-	// Wait for reconciliation start.
-	err = e2e.WaitForMattermostToReconcile(t, k8sClient, mmNamespaceName, 3*time.Minute)
-	require.NoError(t, err)
-
-	// Wait for mattermost to be stable again.
-	err = e2e.WaitForMattermostStable(t, k8sClient, mmNamespaceName, 3*time.Minute)
-	require.NoError(t, err)
-
-	var mmDeployment appsv1.Deployment
-	err = k8sClient.Get(context.TODO(), mmNamespaceName, &mmDeployment)
-	require.NoError(t, err)
-	// check if deployment has the new version
-	require.Equal(t, "mattermost/mattermost-enterprise-edition:"+operatortest.LatestStableMattermostVersion,
-		mmDeployment.Spec.Template.Spec.Containers[0].Image)
-}

test/e2e-external/ingress_test.go

@@ -13,9 +13,9 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// TestMattermostIngress Check that setting custom values in the mattermost ingress are set on the k8s
+// mattermostIngressTest checks that setting custom values in the mattermost ingress are set on the k8s
 // ingress and that disabling it and updating the instance removes the k8s ingress from the cluster
-func TestMattermostIngress(t *testing.T) {
+func mattermostIngressTest(t *testing.T) {
 	namespace := "e2e-test-custom-ingress"
 	name := "test-mm"
 

test/e2e-external/size_test.go

@@ -12,8 +12,8 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 )
 
-// TestMattermostSize checks defaulting & updating replicas & resources from size.
-func TestMattermostSize(t *testing.T) {
+// mattermostSizeTest checks defaulting & updating replicas & resources from size.
+func mattermostSizeTest(t *testing.T) {
 	namespace := "e2e-test-size"
 	name := "test-mm"
 	mmNamespaceName := types.NamespacedName{Namespace: namespace, Name: name}

test/e2e-external/upgrade_test.go

@@ -0,0 +1,68 @@
+package e2e
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	mmv1beta "github.com/mattermost/mattermost-operator/apis/mattermost/v1beta1"
+	ptrUtil "github.com/mattermost/mattermost-operator/pkg/utils"
+	operatortest "github.com/mattermost/mattermost-operator/test"
+	"github.com/mattermost/mattermost-operator/test/e2e"
+	"github.com/stretchr/testify/require"
+	appsv1 "k8s.io/api/apps/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+)
+
+func mattermostUpgradeTest(t *testing.T) {
+	namespace := "e2e-test-external-services-upgrade"
+
+	testEnv, err := SetupTestEnv(k8sClient, namespace)
+	require.NoError(t, err)
+	defer testEnv.CleanupFunc()
+
+	mattermost := &mmv1beta.Mattermost{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-mm",
+			Namespace: namespace,
+		},
+		Spec: mmv1beta.MattermostSpec{
+			Version: operatortest.PreviousStableMattermostVersion,
+			Ingress: &mmv1beta.Ingress{
+				Host: "e2e-test-example.mattermost.dev",
+			},
+			Replicas: ptrUtil.NewInt32(1),
+			FileStore: mmv1beta.FileStore{
+				External: &testEnv.FileStoreConfig,
+			},
+			Database: mmv1beta.Database{
+				External: &testEnv.DBConfig,
+			},
+		},
+	}
+
+	expectValidMattermostInstance(t, mattermost)
+
+	mmNamespaceName := types.NamespacedName{Namespace: mattermost.Namespace, Name: mattermost.Name}
+
+	newMattermost := &mmv1beta.Mattermost{}
+	err = k8sClient.Get(context.TODO(), mmNamespaceName, newMattermost)
+	require.NoError(t, err)
+
+	// Upgrade to new version
+	newMattermost.Spec.Version = operatortest.LatestStableMattermostVersion
+	err = k8sClient.Update(context.TODO(), newMattermost)
+	require.NoError(t, err)
+
+	// Wait for mattermost to be stable again.
+	err = e2e.WaitForMattermostStable(t, k8sClient, mmNamespaceName, 3*time.Minute)
+	require.NoError(t, err)
+
+	var mmDeployment appsv1.Deployment
+	err = k8sClient.Get(context.TODO(), mmNamespaceName, &mmDeployment)
+	require.NoError(t, err)
+	// check if deployment has the new version
+	require.Equal(t, "mattermost/mattermost-enterprise-edition:"+operatortest.LatestStableMattermostVersion,
+		mmDeployment.Spec.Template.Spec.Containers[0].Image)
+}

test/e2e/mattermost_test.go

@@ -212,7 +212,7 @@ func mattermostUpgradeTest(t *testing.T, k8sClient client.Client, k8sTypedClient
 	require.NoError(t, err)
 }
 
-func mattermostWithMySQLReplicas(t *testing.T, client client.Client, typedClient kubernetes.Interface) {
+func mattermostWithMySQLReplicas(t *testing.T, client client.Client, _ kubernetes.Interface) {
 	testName := "test-mm3"
 
 	exampleMattermost := &operator.Mattermost{

test/e2e/utils.go

@@ -2,13 +2,14 @@ package e2e
 
 import (
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 
-	operator "github.com/mattermost/mattermost-operator/apis/mattermost/v1beta1"
+	mmv1beta1 "github.com/mattermost/mattermost-operator/apis/mattermost/v1beta1"
 	mysqlv1alpha1 "github.com/mattermost/mattermost-operator/pkg/database/mysql_operator/v1alpha1"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -40,45 +41,34 @@ func waitForMySQLStatusReady(t *testing.T, dynclient client.Client, namespace, n
 }
 
 func WaitForMattermostStable(t *testing.T, k8sClient client.Client, mmKey types.NamespacedName, timeout time.Duration) error {
-	newMattermost := &operator.Mattermost{}
-	err := wait.Poll(3*time.Second, timeout, func() (done bool, err error) {
-		errClient := k8sClient.Get(context.TODO(), mmKey, newMattermost)
+	mattermost := &mmv1beta1.Mattermost{}
+	err := wait.Poll(5*time.Second, timeout, func() (done bool, err error) {
+		errClient := k8sClient.Get(context.TODO(), mmKey, mattermost)
 		if errClient != nil {
 			return false, errClient
 		}
 
-		if newMattermost.Status.State == operator.Stable {
+		if mattermost.Status.State == mmv1beta1.Stable && mattermost.Generation == mattermost.Status.ObservedGeneration {
 			return true, nil
 		}
-		t.Logf("Waiting for Reconcilication finish (Status:%s)\n", newMattermost.Status.State)
+		t.Logf("Waiting for reconcilication (%s)", prettyPrintStatus(mattermost.Status))
 		return false, nil
 	})
 	if err != nil {
 		return err
 	}
-	t.Logf("Reconcilication completed (%s)\n", newMattermost.Status.State)
+
+	t.Logf("Reconcilication finished (%s)", prettyPrintStatus(mattermost.Status))
 	return nil
 }
 
-func WaitForMattermostToReconcile(t *testing.T, k8sClient client.Client, mmKey types.NamespacedName, timeout time.Duration) error {
-	newMattermost := &operator.Mattermost{}
-	err := wait.Poll(3*time.Second, timeout, func() (done bool, err error) {
-		errClient := k8sClient.Get(context.TODO(), mmKey, newMattermost)
-		if errClient != nil {
-			return false, errClient
-		}
-
-		if newMattermost.Status.State == operator.Reconciling {
-			return true, nil
-		}
-		t.Logf("Waiting for Reconcilication to start (Status:%s)\n", newMattermost.Status.State)
-		return false, nil
-	})
-	if err != nil {
-		return err
-	}
-	t.Logf("Reconcilication started (%s)\n", newMattermost.Status.State)
-	return nil
+func prettyPrintStatus(status mmv1beta1.MattermostStatus) string {
+	return fmt.Sprintf("State:%s, UpdatedReplicas:%d, Version:%s, ObservedGeneration:%d",
+		status.State,
+		status.UpdatedReplicas,
+		status.Version,
+		status.ObservedGeneration,
+	)
 }
 
 func waitForStatefulSet(t *testing.T, dynclient client.Client, namespace, name string, replicas int, retryInterval, timeout time.Duration) error {