fix: add suspend column alias and improve test coverage for review feedback

Signed-off-by: Mario Apostolov <mario.apostolov@mariadb.com>

Author: mariadb-MarioEApostolov

docs/offloading-large-workflows.md

@@ -4,7 +4,7 @@
 
 Argo stores workflows as Kubernetes resources (i.e. within EtcD). This creates a limit to their size as resources must be under 1MB. Each resource includes the status of each node, which is stored in the `/status/nodes` field for the resource. This can be over 1MB. If this happens, we try and compress the node status and store it in `/status/compressedNodes`. If the status is still too large, we then try and store it in an SQL database.
 
-To enable this feature, configure a Postgres or MySQL database under `persistence` in [your configuration](workflow-controller-configmap.yaml) and set `nodeStatusOffLoad: true`.
+To enable this feature, configure a Postgres, MySQL, or MariaDB database under `persistence` in [your configuration](workflow-controller-configmap.yaml) and set `nodeStatusOffLoad: true`.
 
 ## FAQ
 

docs/synchronization-config.md

@@ -31,7 +31,7 @@ Database-based limits allow multiple workflow controllers (typically across diff
 
 Before you can manage database limits via the API, you must:
 
-1. Configure a PostgreSQL or MySQL database for synchronization (see [workflow synchronization](synchronization.md#database-configuration))
+1. Configure a PostgreSQL, MySQL, or MariaDB database for synchronization (see [workflow synchronization](synchronization.md#database-configuration))
 2. Enable the synchronization API in your workflow controller configuration
 
 ### Enable the API

docs/synchronization.md

@@ -68,7 +68,7 @@ spec:
 Multiple controllers can share locks using a database as an intermediary.
 This would normally be used to share locks across multiple clusters, but can also be used to share locks across multiple controllers in the same cluster.
 
-To configure multiple controller locks, you need to set up a database (either PostgreSQL or MySQL) and [configure it](#database-configuration) in the workflow-controller-configmap ConfigMap.
+To configure multiple controller locks, you need to set up a database (either PostgreSQL, MySQL, or MariaDB) and [configure it](#database-configuration) in the workflow-controller-configmap ConfigMap.
 All controllers which want to share locks must share all of these tables.
 If you do not configure the database you will get an error if you try to use database locks.
 

persist/sqldb/workflow_archive.go

@@ -187,8 +187,8 @@ func (r *workflowArchive) ListWorkflows(ctx context.Context, options sutils.List
 					db.Raw("coalesce(JSON_EXTRACT(workflow, '$.metadata.labels'), '{}') as labels"),
 					db.Raw("coalesce(JSON_EXTRACT(workflow, '$.metadata.annotations'), '{}') as annotations"),
 					db.Raw("coalesce(JSON_UNQUOTE(JSON_EXTRACT(workflow, '$.status.progress')), '') as progress"),
-					db.Raw("JSON_UNQUOTE(JSON_EXTRACT(workflow, '$.spec.suspend'))"),
-					db.Raw("coalesce(JSON_UNQUOTE(JSON_EXTRACT(workflow, '$.spec.arguments')), '{}') as arguments"),
+					db.Raw("JSON_UNQUOTE(JSON_EXTRACT(workflow, '$.spec.suspend')) as suspend"),
+					db.Raw("coalesce(JSON_EXTRACT(workflow, '$.spec.arguments'), '{}') as arguments"),
 					db.Raw("coalesce(JSON_UNQUOTE(JSON_EXTRACT(workflow, '$.status.message')), '') as message"),
 					db.Raw("coalesce(JSON_UNQUOTE(JSON_EXTRACT(workflow, '$.status.estimatedDuration')), '0') as estimatedduration"),
 					db.Raw("coalesce(JSON_EXTRACT(workflow, '$.status.resourcesDuration'), '{}') as resourcesduration"),

persist/sqldb/workflow_archive_mysql_test.go

@@ -1,8 +1,9 @@
+//go:build !windows
+
 package sqldb
 
 import (
 	"context"
-	"runtime"
 	"strconv"
 	"testing"
 	"time"
@@ -14,6 +15,7 @@ import (
 	"github.com/testcontainers/testcontainers-go/wait"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/ptr"
 
 	"github.com/argoproj/argo-workflows/v4/config"
 	wfv1 "github.com/argoproj/argo-workflows/v4/pkg/apis/workflow/v1alpha1"
@@ -23,31 +25,27 @@ import (
 	usqldb "github.com/argoproj/argo-workflows/v4/util/sqldb"
 )
 
-type mysqlVariant struct {
-	image       string
-	waitMessage string
-}
-
-var mysqlVariants = map[string]mysqlVariant{
-	"MySQL":   {image: "mysql:8.4", waitMessage: "port: 3306  MySQL Community Server"},
-	"MariaDB": {image: "mariadb:11.4", waitMessage: "mariadbd: ready for connections"},
-}
-
-func setupMySQLArchiveTest(ctx context.Context, t *testing.T, v mysqlVariant) (WorkflowArchive, func()) {
+// setupMySQLArchiveTest starts a MySQL or MariaDB container, runs migrations, and returns a WorkflowArchive.
+func setupMySQLArchiveTest(ctx context.Context, t *testing.T, v usqldb.MySQLVariant) WorkflowArchive {
 	t.Helper()
 
 	c, err := testmysql.Run(ctx,
-		v.image,
+		v.Image,
 		testmysql.WithDatabase("argo"),
 		testmysql.WithUsername("argo"),
 		testmysql.WithPassword("argo"),
 		testcontainers.WithWaitStrategy(
 			wait.ForAll(
-				wait.ForLog(v.waitMessage).WithStartupTimeout(60*time.Second),
+				wait.ForLog(v.WaitMessage).WithStartupTimeout(60*time.Second),
 				wait.ForListeningPort("3306/tcp"),
 			)),
 	)
 	require.NoError(t, err)
+	t.Cleanup(func() {
+		if err := testcontainers.TerminateContainer(c); err != nil {
+			t.Logf("failed to terminate container: %s", err)
+		}
+	})
 
 	host, err := c.Host(ctx)
 	require.NoError(t, err)
@@ -74,24 +72,18 @@ func setupMySQLArchiveTest(ctx context.Context, t *testing.T, v mysqlVariant) (W
 	err = Migrate(ctx, proxy.Session(), "test", "argo_workflows", proxy.DBType())
 	require.NoError(t, err)
 
-	return NewWorkflowArchive(proxy, "test", "", instanceid.NewService("")), func() {
-		proxy.Close()
-		testcontainers.TerminateContainer(c) //nolint:errcheck
-	}
+	t.Cleanup(func() { proxy.Close() })
+
+	return NewWorkflowArchive(proxy, "test", "", instanceid.NewService(""))
 }
 
 // TestMySQLListWorkflows verifies that JSON_EXTRACT/JSON_UNQUOTE queries in
 // ListWorkflows execute correctly against both MySQL and MariaDB.
 func TestMySQLListWorkflows(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("test requires Linux container")
-	}
-
-	for name, variant := range mysqlVariants {
+	for name, variant := range usqldb.MySQLVariants {
 		t.Run(name, func(t *testing.T) {
 			ctx := logging.TestContext(t.Context())
-			archive, cleanup := setupMySQLArchiveTest(ctx, t, variant)
-			defer cleanup()
+			archive := setupMySQLArchiveTest(ctx, t, variant)
 
 			now := metav1.Now()
 			err := archive.ArchiveWorkflow(ctx, &wfv1.Workflow{
@@ -100,24 +92,24 @@ func TestMySQLListWorkflows(t *testing.T) {
 					Namespace:         "default",
 					UID:               types.UID("test-uid-001"),
 					CreationTimestamp: now,
-					Labels: map[string]string{
-						"workflows.argoproj.io/archive-strategy": "Persisted",
-						"env":                                    "test",
-					},
+					Labels:            map[string]string{"env": "test"},
+					Annotations:       map[string]string{"note": "integration-test"},
 				},
 				Spec: wfv1.WorkflowSpec{
+					Suspend: ptr.To(true),
 					Arguments: wfv1.Arguments{
 						Parameters: []wfv1.Parameter{
 							{Name: "msg", Value: wfv1.AnyStringPtr("hello")},
 						},
 					},
 				},
 				Status: wfv1.WorkflowStatus{
-					Phase:      wfv1.WorkflowSucceeded,
-					StartedAt:  now,
-					FinishedAt: now,
-					Progress:   "1/1",
-					Message:    "completed",
+					Phase:             wfv1.WorkflowSucceeded,
+					StartedAt:         now,
+					FinishedAt:        now,
+					Progress:          "1/1",
+					Message:           "completed",
+					EstimatedDuration: wfv1.EstimatedDuration(30),
 				},
 			})
 			require.NoError(t, err)
@@ -132,6 +124,10 @@ func TestMySQLListWorkflows(t *testing.T) {
 			assert.Equal(t, wfv1.Progress("1/1"), wf.Status.Progress)
 			assert.Equal(t, "completed", wf.Status.Message)
 			assert.Equal(t, "test", wf.GetLabels()["env"])
+			assert.Equal(t, "integration-test", wf.GetAnnotations()["note"])
+			assert.Equal(t, ptr.To(true), wf.Spec.Suspend)
+			assert.Equal(t, "hello", wf.Spec.Arguments.Parameters[0].Value.String())
+			assert.Equal(t, wfv1.EstimatedDuration(30), wf.Status.EstimatedDuration)
 		})
 	}
 }

util/sqldb/mysql_test_helper.go

@@ -0,0 +1,13 @@
+package sqldb
+
+// MySQLVariant defines a MySQL-compatible database image for integration testing.
+type MySQLVariant struct {
+	Image       string
+	WaitMessage string
+}
+
+// MySQLVariants contains the set of MySQL-compatible databases to test against.
+var MySQLVariants = map[string]MySQLVariant{
+	"MySQL":   {Image: "mysql:8.4", WaitMessage: "port: 3306  MySQL Community Server"},
+	"MariaDB": {Image: "mariadb:11.4", WaitMessage: "mariadbd: ready for connections"},
+}

util/sqldb/sqldb.go

@@ -202,7 +202,7 @@ func createMySQLDBSessionWithCreds(cfg *config.MySQLConfig, persistPool *config.
 		Addr:                 cfg.GetHostname(),
 		DBName:               cfg.Database,
 		ParseTime:            true,
-		AllowNativePasswords: true,
+		AllowNativePasswords: true, // Required for MariaDB which uses mysql_native_password by default
 		Params:               cfg.Options,
 	}
 	dsn := mysqlCfg.FormatDSN()

util/sqldb/sqldb_test.go

@@ -1,8 +1,9 @@
+//go:build !windows
+
 package sqldb
 
 import (
 	"context"
-	"runtime"
 	"strconv"
 	"testing"
 	"time"
@@ -16,31 +17,27 @@ import (
 	"github.com/argoproj/argo-workflows/v4/util/logging"
 )
 
-type mysqlVariant struct {
-	image       string
-	waitMessage string
-}
-
-var mysqlVariants = map[string]mysqlVariant{
-	"MySQL":   {image: "mysql:8.4", waitMessage: "port: 3306  MySQL Community Server"},
-	"MariaDB": {image: "mariadb:11.4", waitMessage: "mariadbd: ready for connections"},
-}
-
-func setupMySQLContainer(ctx context.Context, t *testing.T, v mysqlVariant) (config.DBConfig, func()) {
+// setupMySQLContainer starts a MySQL or MariaDB container and returns the corresponding DBConfig.
+func setupMySQLContainer(ctx context.Context, t *testing.T, v MySQLVariant) config.DBConfig {
 	t.Helper()
 
 	c, err := testmysql.Run(ctx,
-		v.image,
+		v.Image,
 		testmysql.WithDatabase("argo"),
 		testmysql.WithUsername("argo"),
 		testmysql.WithPassword("argo"),
 		testcontainers.WithWaitStrategy(
 			wait.ForAll(
-				wait.ForLog(v.waitMessage).WithStartupTimeout(60*time.Second),
+				wait.ForLog(v.WaitMessage).WithStartupTimeout(60*time.Second),
 				wait.ForListeningPort("3306/tcp"),
 			)),
 	)
 	require.NoError(t, err)
+	t.Cleanup(func() {
+		if err := testcontainers.TerminateContainer(c); err != nil {
+			t.Logf("failed to terminate container: %s", err)
+		}
+	})
 
 	host, err := c.Host(ctx)
 	require.NoError(t, err)
@@ -57,27 +54,21 @@ func setupMySQLContainer(ctx context.Context, t *testing.T, v mysqlVariant) (con
 				Port:     port,
 			},
 		},
-	}, func() { testcontainers.TerminateContainer(c) } //nolint:errcheck
+	}
 }
 
 // TestMySQLSessionConnect verifies that CreateDBSessionWithCreds can connect
 // to both MySQL and MariaDB. MariaDB requires AllowNativePasswords.
 func TestMySQLSessionConnect(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("test requires Linux container")
-	}
-
-	for name, variant := range mysqlVariants {
+	for name, variant := range MySQLVariants {
 		t.Run(name, func(t *testing.T) {
 			ctx := logging.TestContext(t.Context())
-			dbConfig, cleanup := setupMySQLContainer(ctx, t, variant)
-			defer cleanup()
+			dbConfig := setupMySQLContainer(ctx, t, variant)
 
-			sess, dbType, err := CreateDBSessionWithCreds(dbConfig, "argo", "argo")
+			sess, _, err := CreateDBSessionWithCreds(dbConfig, "argo", "argo")
 			require.NoError(t, err)
 			defer sess.Close()
 
-			require.Equal(t, MySQL, dbType)
 			require.NoError(t, sess.Ping())
 		})
 	}