chore: Parallelize EnsureCleanState for e2e tests, adding timing info…

…rmation (#20998) * chore: Parallelize EnsureCleanState for e2e tests, adding timing information Closes #20968 Closes #20967 A number of cleanup and create steps can be done in parallel, reducing the total runtime. Gather timing information as well and log it. Signed-off-by: Andrii Korotkov <[email protected]> * Add underscores to function map keys Signed-off-by: Andrii Korotkov <[email protected]> * Have separate log statements for timing fields, since everything doesn't fit in Github logs and gets truncated Signed-off-by: Andrii Korotkov <[email protected]> * Re-run tests Signed-off-by: Andrii Korotkov <[email protected]> * Fix duplicate removal attempts, cleanup things in namespaces before removing namespaces Signed-off-by: Andrii Korotkov <[email protected]> * Combine config map cleanups, since they seem to have some kind of locking based on timings Signed-off-by: Andrii Korotkov <[email protected]> * Re-order some cleanups to not wait if something is already covered by no wait commands Signed-off-by: Andrii Korotkov <[email protected]> * Move rm temp dir to be together with re-creation, since otherwise repo server watcher seems to fail Signed-off-by: Andrii Korotkov <[email protected]> * Only update config maps and login as admin if there were any changes Signed-off-by: Andrii Korotkov <[email protected]> * Also check for user being logged in Signed-off-by: Andrii Korotkov <[email protected]> * Improve config maps equivalency check Signed-off-by: Andrii Korotkov <[email protected]> * Remove timing information Signed-off-by: Andrii Korotkov <[email protected]> * Improve loops with sleep Signed-off-by: Andrii Korotkov <[email protected]> * Properly propagate and handle errors for parallel functions Signed-off-by: Andrii Korotkov <[email protected]> * Add missing CheckError Signed-off-by: Andrii Korotkov <[email protected]> * Fix import Signed-off-by: Andrii Korotkov <[email protected]> * Fix imports linter error (try 2) Signed-off-by: Andrii Korotkov <[email protected]> * Parallelize EnsureCleanState for application sets Signed-off-by: Andrii Korotkov <[email protected]> * Fix errors.IsNotFound Signed-off-by: Andrii Korotkov <[email protected]> * Use errgroup to run functions in parallel Signed-off-by: Andrii Korotkov <[email protected]> * Remove name keys for functions as unused Signed-off-by: Andrii Korotkov <[email protected]> * Use apierr directly Signed-off-by: Andrii Korotkov <[email protected]> --------- Signed-off-by: Andrii Korotkov <[email protected]>
argoproj · Dec 2, 2024 · bd5d76f · bd5d76f
1 parent 5fc306e
commit bd5d76f
Show file tree

Hide file tree

Showing 20 changed files with 559 additions and 324 deletions.
diff --git a/test/e2e/accounts_test.go b/test/e2e/accounts_test.go
@@ -120,9 +120,9 @@ func TestCreateAndUseAccountCLI(t *testing.T) {
 	assert.Equal(t, `NAME   ENABLED  CAPABILITIES
 admin  true     login`, output)
 
-	SetAccounts(map[string][]string{
+	errors.CheckError(SetAccounts(map[string][]string{
 		"test": {"login", "apiKey"},
-	})
+	}))
 
 	output, err = RunCli("account", "list")
 	errors.CheckError(err)

diff --git a/test/e2e/app_deletion_test.go b/test/e2e/app_deletion_test.go
@@ -9,18 +9,19 @@ import (
 	. "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	. "github.com/argoproj/argo-cd/v2/test/e2e/fixture"
 	. "github.com/argoproj/argo-cd/v2/test/e2e/fixture/app"
+	"github.com/argoproj/argo-cd/v2/util/errors"
 )
 
 // when a app gets stuck in sync, and we try to delete it, it won't delete, instead we must then terminate it
 // and deletion will then just happen
 func TestDeletingAppStuckInSync(t *testing.T) {
 	Given(t).
 		And(func() {
-			SetResourceOverrides(map[string]ResourceOverride{
+			errors.CheckError(SetResourceOverrides(map[string]ResourceOverride{
 				"ConfigMap": {
 					HealthLua: `return { status = obj.annotations and obj.annotations['health'] or 'Progressing' }`,
 				},
-			})
+			}))
 		}).
 		Async(true).
 		Path("hook-custom-health").

diff --git a/test/e2e/app_management_ns_test.go b/test/e2e/app_management_ns_test.go
@@ -852,14 +852,14 @@ func TestNamespacedKnownTypesInCRDDiffing(t *testing.T) {
 		Expect(SyncStatusIs(SyncStatusCodeOutOfSync)).
 		When().
 		And(func() {
-			SetResourceOverrides(map[string]ResourceOverride{
+			CheckError(SetResourceOverrides(map[string]ResourceOverride{
 				"argoproj.io/Dummy": {
 					KnownTypeFields: []KnownTypeField{{
 						Field: "spec",
 						Type:  "core/v1/ResourceList",
 					}},
 				},
-			})
+			}))
 		}).
 		Refresh(RefreshTypeNormal).
 		Then().
@@ -2232,14 +2232,14 @@ definitions:
 		SetTrackingMethod("annotation").
 		Path("crd-subresource").
 		And(func() {
-			SetResourceOverrides(map[string]ResourceOverride{
+			CheckError(SetResourceOverrides(map[string]ResourceOverride{
 				"argoproj.io/StatusSubResource": {
 					Actions: actions,
 				},
 				"argoproj.io/NonStatusSubResource": {
 					Actions: actions,
 				},
-			})
+			}))
 		}).
 		When().CreateApp().Sync().Then().
 		Expect(OperationPhaseIs(OperationSucceeded)).Expect(SyncStatusIs(SyncStatusCodeSynced)).
@@ -2320,14 +2320,14 @@ func TestNamespacedAppWaitOperationInProgress(t *testing.T) {
 		SetAppNamespace(AppNamespace()).
 		SetTrackingMethod("annotation").
 		And(func() {
-			SetResourceOverrides(map[string]ResourceOverride{
+			CheckError(SetResourceOverrides(map[string]ResourceOverride{
 				"batch/Job": {
 					HealthLua: `return { status = 'Running' }`,
 				},
 				"apps/Deployment": {
 					HealthLua: `return { status = 'Suspended' }`,
 				},
-			})
+			}))
 		}).
 		Async(true).
 		Path("hook-and-deployment").
@@ -2438,9 +2438,9 @@ func TestNamespacedDisableManifestGeneration(t *testing.T) {
 		}).
 		When().
 		And(func() {
-			SetEnableManifestGeneration(map[ApplicationSourceType]bool{
+			CheckError(SetEnableManifestGeneration(map[ApplicationSourceType]bool{
 				ApplicationSourceTypeKustomize: false,
-			})
+			}))
 		}).
 		Refresh(RefreshTypeHard).
 		Then().

diff --git a/test/e2e/app_management_test.go b/test/e2e/app_management_test.go
@@ -998,14 +998,14 @@ func TestKnownTypesInCRDDiffing(t *testing.T) {
 		Expect(SyncStatusIs(SyncStatusCodeOutOfSync)).
 		When().
 		And(func() {
-			SetResourceOverrides(map[string]ResourceOverride{
+			CheckError(SetResourceOverrides(map[string]ResourceOverride{
 				"argoproj.io/Dummy": {
 					KnownTypeFields: []KnownTypeField{{
 						Field: "spec",
 						Type:  "core/v1/ResourceList",
 					}},
 				},
-			})
+			}))
 		}).
 		Refresh(RefreshTypeNormal).
 		Then().
@@ -2360,14 +2360,14 @@ definitions:
 	Given(t).
 		Path("crd-subresource").
 		And(func() {
-			SetResourceOverrides(map[string]ResourceOverride{
+			CheckError(SetResourceOverrides(map[string]ResourceOverride{
 				"argoproj.io/StatusSubResource": {
 					Actions: actions,
 				},
 				"argoproj.io/NonStatusSubResource": {
 					Actions: actions,
 				},
-			})
+			}))
 		}).
 		When().CreateApp().Sync().Then().
 		Expect(OperationPhaseIs(OperationSucceeded)).Expect(SyncStatusIs(SyncStatusCodeSynced)).
@@ -2445,14 +2445,14 @@ func TestAppWaitOperationInProgress(t *testing.T) {
 	ctx := Given(t)
 	ctx.
 		And(func() {
-			SetResourceOverrides(map[string]ResourceOverride{
+			CheckError(SetResourceOverrides(map[string]ResourceOverride{
 				"batch/Job": {
 					HealthLua: `return { status = 'Running' }`,
 				},
 				"apps/Deployment": {
 					HealthLua: `return { status = 'Suspended' }`,
 				},
-			})
+			}))
 		}).
 		Async(true).
 		Path("hook-and-deployment").
@@ -2554,9 +2554,9 @@ func TestDisableManifestGeneration(t *testing.T) {
 		}).
 		When().
 		And(func() {
-			SetEnableManifestGeneration(map[ApplicationSourceType]bool{
+			CheckError(SetEnableManifestGeneration(map[ApplicationSourceType]bool{
 				ApplicationSourceTypeKustomize: false,
-			})
+			}))
 		}).
 		Refresh(RefreshTypeHard).
 		Then().
@@ -2758,7 +2758,7 @@ func TestSwitchTrackingLabel(t *testing.T) {
 func TestAnnotationTrackingExtraResources(t *testing.T) {
 	ctx := Given(t)
 
-	SetTrackingMethod(string(argo.TrackingMethodAnnotation))
+	CheckError(SetTrackingMethod(string(argo.TrackingMethodAnnotation)))
 	ctx.
 		Path("deployment").
 		When().

diff --git a/test/e2e/app_namespaces_test.go b/test/e2e/app_namespaces_test.go
@@ -77,11 +77,11 @@ func TestForbiddenNamespace(t *testing.T) {
 func TestDeletingNamespacedAppStuckInSync(t *testing.T) {
 	ctx := Given(t)
 	ctx.And(func() {
-		SetResourceOverrides(map[string]ResourceOverride{
+		CheckError(SetResourceOverrides(map[string]ResourceOverride{
 			"ConfigMap": {
 				HealthLua: `return { status = obj.annotations and obj.annotations['health'] or 'Progressing' }`,
 			},
-		})
+		}))
 	}).
 		Async(true).
 		SetAppNamespace(AppNamespace()).

diff --git a/test/e2e/deployment_test.go b/test/e2e/deployment_test.go
@@ -18,6 +18,7 @@ import (
 	"github.com/argoproj/argo-cd/v2/common"
 	"github.com/argoproj/argo-cd/v2/util/argo"
 	"github.com/argoproj/argo-cd/v2/util/clusterauth"
+	"github.com/argoproj/argo-cd/v2/util/errors"
 
 	"github.com/argoproj/gitops-engine/pkg/health"
 	. "github.com/argoproj/gitops-engine/pkg/sync/common"
@@ -52,7 +53,7 @@ func TestDeployment(t *testing.T) {
 func TestDeploymentWithAnnotationTrackingMode(t *testing.T) {
 	ctx := Given(t)
 
-	SetTrackingMethod(string(argo.TrackingMethodAnnotation))
+	errors.CheckError(SetTrackingMethod(string(argo.TrackingMethodAnnotation)))
 	ctx.
 		Path("deployment").
 		When().
@@ -75,7 +76,7 @@ func TestDeploymentWithAnnotationTrackingMode(t *testing.T) {
 
 func TestDeploymentWithLabelTrackingMode(t *testing.T) {
 	ctx := Given(t)
-	SetTrackingMethod(string(argo.TrackingMethodLabel))
+	errors.CheckError(SetTrackingMethod(string(argo.TrackingMethodLabel)))
 	ctx.
 		Path("deployment").
 		When().

diff --git a/test/e2e/fixture/account/actions.go b/test/e2e/fixture/account/actions.go
@@ -2,6 +2,7 @@ package project
 
 import (
 	"github.com/argoproj/argo-cd/v2/test/e2e/fixture"
+	"github.com/argoproj/argo-cd/v2/util/errors"
 )
 
 // this implements the "when" part of given/when/then
@@ -46,25 +47,25 @@ func (a *Actions) prepareSetPasswordArgs(account string) []string {
 }
 
 func (a *Actions) Create() *Actions {
-	fixture.SetAccounts(map[string][]string{
+	errors.CheckError(fixture.SetAccounts(map[string][]string{
 		a.context.name: {"login"},
-	})
+	}))
 	_, _ = fixture.RunCli(a.prepareSetPasswordArgs(a.context.name)...)
 	return a
 }
 
 func (a *Actions) SetPermissions(permissions []fixture.ACL, roleName string) *Actions {
-	fixture.SetPermissions(permissions, a.context.name, roleName)
+	errors.CheckError(fixture.SetPermissions(permissions, a.context.name, roleName))
 	return a
 }
 
 func (a *Actions) SetParamInSettingConfigMap(key, value string) *Actions {
-	fixture.SetParamInSettingConfigMap(key, value)
+	errors.CheckError(fixture.SetParamInSettingConfigMap(key, value))
 	return a
 }
 
 func (a *Actions) Login() *Actions {
-	fixture.LoginAs(a.context.name)
+	errors.CheckError(fixture.LoginAs(a.context.name))
 	return a
 }
 

diff --git a/test/e2e/fixture/app/actions.go b/test/e2e/fixture/app/actions.go
@@ -448,7 +448,7 @@ func (a *Actions) Wait(args ...string) *Actions {
 }
 
 func (a *Actions) SetParamInSettingConfigMap(key, value string) *Actions {
-	fixture.SetParamInSettingConfigMap(key, value)
+	errors.CheckError(fixture.SetParamInSettingConfigMap(key, value))
 	return a
 }
 
@@ -477,16 +477,16 @@ func (a *Actions) verifyAction() {
 }
 
 func (a *Actions) SetTrackingMethod(trackingMethod string) *Actions {
-	fixture.SetTrackingMethod(trackingMethod)
+	errors.CheckError(fixture.SetTrackingMethod(trackingMethod))
 	return a
 }
 
 func (a *Actions) SetInstallationID(installationID string) *Actions {
-	fixture.SetInstallationID(installationID)
+	errors.CheckError(fixture.SetInstallationID(installationID))
 	return a
 }
 
 func (a *Actions) SetTrackingLabel(trackingLabel string) *Actions {
-	fixture.SetTrackingLabel(trackingLabel)
+	errors.CheckError(fixture.SetTrackingLabel(trackingLabel))
 	return a
 }
diff --git a/test/e2e/fixture/app/consequences.go b/test/e2e/fixture/app/consequences.go
@@ -38,6 +38,9 @@ func (c *Consequences) Expect(e Expectation) *Consequences {
 	sleepIntervalsIdx := -1
 	timeout := time.Duration(c.timeout) * time.Second
 	for start := time.Now(); time.Since(start) < timeout; time.Sleep(sleepIntervals[sleepIntervalsIdx]) {
+		if sleepIntervalsIdx < len(sleepIntervals)-1 {
+			sleepIntervalsIdx++
+		}
 		state, message = e(c)
 		switch state {
 		case succeeded:
@@ -48,9 +51,6 @@ func (c *Consequences) Expect(e Expectation) *Consequences {
 			return c
 		}
 		log.Infof("pending: %s", message)
-		if sleepIntervalsIdx < len(sleepIntervals)-1 {
-			sleepIntervalsIdx++
-		}
 	}
 	c.context.t.Fatal("timeout waiting for: " + message)
 	return c

diff --git a/test/e2e/fixture/app/context.go b/test/e2e/fixture/app/context.go
@@ -11,6 +11,7 @@ import (
 	"github.com/argoproj/argo-cd/v2/test/e2e/fixture/repos"
 	"github.com/argoproj/argo-cd/v2/util/argo"
 	"github.com/argoproj/argo-cd/v2/util/env"
+	"github.com/argoproj/argo-cd/v2/util/errors"
 	"github.com/argoproj/argo-cd/v2/util/settings"
 )
 
@@ -104,7 +105,7 @@ func (c *Context) AppNamespace() string {
 
 func (c *Context) SetAppNamespace(namespace string) *Context {
 	c.appNamespace = namespace
-	// fixture.SetParamInSettingConfigMap("application.resourceTrackingMethod", "annotation")
+	// errors.CheckError(fixture.SetParamInSettingConfigMap("application.resourceTrackingMethod", "annotation"))
 	return c
 }
 
@@ -211,7 +212,7 @@ func (c *Context) SSHCredentialsAdded() *Context {
 }
 
 func (c *Context) ProjectSpec(spec v1alpha1.AppProjectSpec) *Context {
-	fixture.SetProjectSpec(c.project, spec)
+	errors.CheckError(fixture.SetProjectSpec(c.project, spec))
 	return c
 }
 
@@ -296,12 +297,12 @@ func (c *Context) NameSuffix(nameSuffix string) *Context {
 }
 
 func (c *Context) ResourceOverrides(overrides map[string]v1alpha1.ResourceOverride) *Context {
-	fixture.SetResourceOverrides(overrides)
+	errors.CheckError(fixture.SetResourceOverrides(overrides))
 	return c
 }
 
 func (c *Context) ResourceFilter(filter settings.ResourcesFilter) *Context {
-	fixture.SetResourceFilter(filter)
+	errors.CheckError(fixture.SetResourceFilter(filter))
 	return c
 }
 
@@ -365,12 +366,12 @@ func (c *Context) HelmSkipTests() *Context {
 }
 
 func (c *Context) SetTrackingMethod(trackingMethod string) *Context {
-	fixture.SetTrackingMethod(trackingMethod)
+	errors.CheckError(fixture.SetTrackingMethod(trackingMethod))
 	return c
 }
 
 func (c *Context) SetInstallationID(installationID string) *Context {
-	fixture.SetTrackingMethod(installationID)
+	errors.CheckError(fixture.SetInstallationID(installationID))
 	return c
 }
 

diff --git a/test/e2e/fixture/applicationsets/actions.go b/test/e2e/fixture/applicationsets/actions.go
@@ -455,7 +455,21 @@ func (a *Actions) Update(toUpdate func(*v1alpha1.ApplicationSet)) *Actions {
 
 	var mostRecentError error
 
-	for start := time.Now(); time.Since(start) < timeout; time.Sleep(3 * time.Second) {
+	sleepIntervals := []time.Duration{
+		10 * time.Millisecond,
+		20 * time.Millisecond,
+		50 * time.Millisecond,
+		100 * time.Millisecond,
+		200 * time.Millisecond,
+		300 * time.Millisecond,
+		500 * time.Millisecond,
+		1 * time.Second,
+	}
+	sleepIntervalsIdx := -1
+	for start := time.Now(); time.Since(start) < timeout; time.Sleep(sleepIntervals[sleepIntervalsIdx]) {
+		if sleepIntervalsIdx < len(sleepIntervals)-1 {
+			sleepIntervalsIdx++
+		}
 		appSet, err := a.get()
 		mostRecentError = err
 		if err == nil {

diff --git a/test/e2e/fixture/applicationsets/consequences.go b/test/e2e/fixture/applicationsets/consequences.go
@@ -43,6 +43,9 @@ func (c *Consequences) ExpectWithDuration(e Expectation, timeout time.Duration)
 	}
 	sleepIntervalsIdx := -1
 	for start := time.Now(); time.Since(start) < timeout; time.Sleep(sleepIntervals[sleepIntervalsIdx]) {
+		if sleepIntervalsIdx < len(sleepIntervals)-1 {
+			sleepIntervalsIdx++
+		}
 		state, message = e(c)
 		switch state {
 		case succeeded:
@@ -53,9 +56,6 @@ func (c *Consequences) ExpectWithDuration(e Expectation, timeout time.Duration)
 			return c
 		}
 		log.Infof("expectation pending: %s", message)
-		if sleepIntervalsIdx < len(sleepIntervals)-1 {
-			sleepIntervalsIdx++
-		}
 	}
 	c.context.t.Fatal("timeout waiting for: " + message)
 	return c