diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index 7c916067faf3b..518ab6f4bbf0c 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -353,7 +353,7 @@ func NewApplicationGetCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com command := &cobra.Command{ Use: "get APPNAME", Short: "Get application details", - Example: templates.Examples(` + Example: templates.Examples(` # Get basic details about the application "my-app" in wide format argocd app get my-app -o wide @@ -383,7 +383,7 @@ func NewApplicationGetCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com # Get application details and display them in a tree format argocd app get my-app --output tree - + # Get application details and display them in a detailed tree format argocd app get my-app --output tree=detailed `), @@ -541,7 +541,7 @@ func NewApplicationLogsCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co command := &cobra.Command{ Use: "logs APPNAME", Short: "Get logs of application pods", - Example: templates.Examples(` + Example: templates.Examples(` # Get logs of pods associated with the application "my-app" argocd app logs my-app @@ -855,7 +855,7 @@ func NewApplicationSetCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com command := &cobra.Command{ Use: "set APPNAME", Short: "Set application parameters", - Example: templates.Examples(` + Example: templates.Examples(` # Set application parameters for the application "my-app" argocd app set my-app --parameter key1=value1 --parameter key2=value2 @@ -2520,18 +2520,19 @@ func getResourceStates(app *argoappv1.Application, selectedResources []*argoappv // print most resources info along with most recent operation results if app.Status.OperationState != nil && app.Status.OperationState.SyncResult != nil { for _, res := range app.Status.OperationState.SyncResult.Resources { - sync := string(res.HookPhase) - health := string(res.Status) + status := string(res.Status) + health := "" key := kube.NewResourceKey(res.Group, res.Kind, res.Namespace, res.Name) - if resource, ok := resourceByKey[key]; ok && res.HookType == "" { - health = "" + if resource, ok := resourceByKey[key]; ok { if resource.Health != nil { health = string(resource.Health.Status) } - sync = string(resource.Status) + } + if res.HookType != "" { + status = string(res.HookPhase) } states = append(states, &resourceState{ - Group: res.Group, Kind: res.Kind, Namespace: res.Namespace, Name: res.Name, Status: sync, Health: health, Hook: string(res.HookType), Message: res.Message, + Group: res.Group, Kind: res.Kind, Namespace: res.Namespace, Name: res.Name, Status: status, Health: health, Hook: string(res.HookType), Message: res.Message, }) delete(resourceByKey, kube.NewResourceKey(res.Group, res.Kind, res.Namespace, res.Name)) } @@ -3484,7 +3485,7 @@ func NewApplicationRemoveSourceCommand(clientOpts *argocdclient.ClientOptions) * Short: "Remove a source from multiple sources application.", Example: ` # Remove the source at position 1 from application's sources. Counting starts at 1. argocd app remove-source myapplication --source-position 1 - + # Remove the source named "test" from application's sources. argocd app remove-source myapplication --source-name test`, Run: func(c *cobra.Command, args []string) { diff --git a/controller/sync.go b/controller/sync.go index 398c0e4ed644e..f03da48d1d8b2 100644 --- a/controller/sync.go +++ b/controller/sync.go @@ -124,7 +124,9 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, project *v1alp state.Message = fmt.Sprintf("Failed to generate sync ID: %v", err) return } - logEntry := log.WithFields(applog.GetAppLogFields(app)).WithField("syncId", syncId) + logger := logutils.NewWithCurrentConfig() + logger.SetLevel(log.TraceLevel) + logEntry := logger.WithFields(applog.GetAppLogFields(app)).WithField("syncId", syncId) if state.Operation.Sync == nil { state.Phase = common.OperationError diff --git a/docs/user-guide/commands/argocd_app_remove-source.md b/docs/user-guide/commands/argocd_app_remove-source.md index c04950b550c69..8353ddb70fc94 100644 --- a/docs/user-guide/commands/argocd_app_remove-source.md +++ b/docs/user-guide/commands/argocd_app_remove-source.md @@ -13,7 +13,7 @@ argocd app remove-source APPNAME [flags] ``` # Remove the source at position 1 from application's sources. Counting starts at 1. argocd app remove-source myapplication --source-position 1 - + # Remove the source named "test" from application's sources. argocd app remove-source myapplication --source-name test ``` diff --git a/docs/user-guide/sync-waves.md b/docs/user-guide/sync-waves.md index 56d45edeb175c..8146683eb0add 100644 --- a/docs/user-guide/sync-waves.md +++ b/docs/user-guide/sync-waves.md @@ -4,14 +4,14 @@ Sync phases and hooks define when resources are applied such as before or after Argo CD has the following hook types: -| Hook | Description | -|------|-------------| -| `PreSync` | Executes prior to the application of the manifests. | -| `Sync` | Executes after all `PreSync` hooks completed and were successful, at the same time as the application of the manifests. | -| `Skip` | Indicates to Argo CD to skip the application of the manifest. | -| `PostSync` | Executes after all `Sync` hooks completed and were successful, a successful application, and all resources in a `Healthy` state. | -| `SyncFail` | Executes when the sync operation fails. | -| `PostDelete` | Executes after all Application resources are deleted. _Available starting in v2.10._ | +| Hook | Description | +| ------------ | -------------------------------------------------------------------------------------------------------------------------------- | +| `PreSync` | Executes prior to the application of the manifests. | +| `Sync` | Executes after all `PreSync` hooks completed and were successful, at the same time as the application of the manifests. | +| `Skip` | Indicates to Argo CD to skip the application of the manifest. | +| `PostSync` | Executes after all `Sync` hooks completed and were successful, a successful application, and all resources in a `Healthy` state. | +| `SyncFail` | Executes when the sync operation fails. | +| `PostDelete` | Executes after all Application resources are deleted. _Available starting in v2.10._ | Adding the argocd.argoproj.io/hook annotation to a resource will assign it to a specific phase. During a Sync operation, Argo CD will apply the resource during the appropriate phase of the deployment. Hooks can be any type of Kubernetes resource kind, but tend to be Pod, Job or Argo Workflows. Multiple hooks can be specified as a comma separated list. @@ -40,13 +40,12 @@ Argo CD offers several methods to clean up hooks and decide how much history wil In the most basic case you can use the argocd.argoproj.io/hook-delete-policy to decide when a hook will be deleted. This can take the following values: -| Policy | Description | -|--------|-------------| -| `HookSucceeded` | The hook resource is deleted after the hook succeeded (e.g. Job/Workflow completed successfully). | -| `HookFailed` | The hook resource is deleted after the hook failed. | +| Policy | Description | +| -------------------- | ------------------------------------------------------------------------------------------------------------------------------- | +| `HookSucceeded` | The hook resource is deleted if the sync succeeds (e.g. All `PreSync` completed successfully). | +| `HookFailed` | The hook resource is deleted after the sync fails. | | `BeforeHookCreation` | Any existing hook resource is deleted before the new one is created (since v1.3). It is meant to be used with `/metadata/name`. | - ## How sync waves work? Argo CD also offers an alternative method of changing the sync order of resources. These are sync waves. They are defined by the argocd.argoproj.io/sync-wave annotation. The value is an integer that defines the ordering (ArgoCD will start deploying from the lowest number and finish with the highest number). @@ -99,7 +98,7 @@ Specify the wave using the following annotation: ```yaml metadata: annotations: - argocd.argoproj.io/sync-wave: "5" + argocd.argoproj.io/sync-wave: '5' ``` Hooks and resources are assigned to wave zero by default. The wave can be negative, so you can create a wave that runs before all other resources. @@ -138,6 +137,7 @@ spec: ``` The following example runs a db migration command before the main sync operation (also in wave -1): + ```yaml apiVersion: batch/v1 kind: Job @@ -173,20 +173,20 @@ spec: Upgrading ingress-nginx controller (managed by helm) with ArgoCD 2.x sometimes fails to work resulting in: -.|. --|- -OPERATION|Sync -PHASE|Running -MESSAGE|waiting for completion of hook batch/Job/ingress-nginx-admission-create - -.|. --|- -KIND |batch/v1/Job -NAMESPACE|ingress-nginx -NAME |ingress-nginx-admission-create -STATUS |Running -HOOK |PreSync -MESSAGE |Pending deletion +| . | . | +| --------- | ----------------------------------------------------------------------- | +| OPERATION | Sync | +| PHASE | Running | +| MESSAGE | waiting for completion of hook batch/Job/ingress-nginx-admission-create | + +| . | . | +| --------- | ------------------------------ | +| KIND | batch/v1/Job | +| NAMESPACE | ingress-nginx | +| NAME | ingress-nginx-admission-create | +| STATUS | Running | +| HOOK | PreSync | +| MESSAGE | Pending deletion | To work around this, a helm user can add: diff --git a/go.mod b/go.mod index 1a4e2749e80a0..9992a0c436989 100644 --- a/go.mod +++ b/go.mod @@ -298,6 +298,8 @@ require ( ) replace ( + github.com/argoproj/gitops-engine => github.com/agaudreault/gitops-engine v0.7.1-0.20250902171136-ef00d914b5e3 + github.com/golang/protobuf => github.com/golang/protobuf v1.5.4 github.com/grpc-ecosystem/grpc-gateway => github.com/grpc-ecosystem/grpc-gateway v1.16.0 golang.org/x/tools => golang.org/x/tools v0.35.0 diff --git a/go.sum b/go.sum index 67a32673020aa..498e0346b5fb2 100644 --- a/go.sum +++ b/go.sum @@ -102,6 +102,8 @@ github.com/RocketChat/Rocket.Chat.Go.SDK v0.0.0-20240116134246-a8cbe886bab0 h1:z github.com/RocketChat/Rocket.Chat.Go.SDK v0.0.0-20240116134246-a8cbe886bab0/go.mod h1:rjP7sIipbZcagro/6TCk6X0ZeFT2eyudH5+fve/cbBA= github.com/TomOnTime/utfutil v1.0.0 h1:/0Ivgo2OjXJxo8i7zgvs7ewSFZMLwCRGm3P5Umowb90= github.com/TomOnTime/utfutil v1.0.0/go.mod h1:l9lZmOniizVSuIliSkEf87qivMRlSNzbdBFKjuLRg1c= +github.com/agaudreault/gitops-engine v0.7.1-0.20250902171136-ef00d914b5e3 h1:3n8H+yRovy2hzmRCn4qLrNRIASf9hKkqwxSS4//OCpY= +github.com/agaudreault/gitops-engine v0.7.1-0.20250902171136-ef00d914b5e3/go.mod h1:aIBEG3ohgaC1gh/sw2On6knkSnXkqRLDoBj234Dqczw= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= @@ -113,8 +115,6 @@ github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be h1:9AeTilPcZAjCFI github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be/go.mod h1:ySMOLuWl6zY27l47sB3qLNK6tF2fkHG55UZxx8oIVo4= github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= github.com/appscode/go v0.0.0-20191119085241-0887d8ec2ecc/go.mod h1:OawnOmAL4ZX3YaPdN+8HTNwBveT1jMsqP74moa9XUbE= -github.com/argoproj/gitops-engine v0.7.1-0.20250724135328-38f73a75c3cf h1:T6GGt8vYN2aRNnOwdY3DIzMX1AhGIEo+8SNhSlNBdvs= -github.com/argoproj/gitops-engine v0.7.1-0.20250724135328-38f73a75c3cf/go.mod h1:aIBEG3ohgaC1gh/sw2On6knkSnXkqRLDoBj234Dqczw= github.com/argoproj/notifications-engine v0.4.1-0.20250710034121-3ec18d4536a7 h1:DwrJWH9kySJgaJ6n5U543QSeFTNdMHDz5Iy+goyfQUw= github.com/argoproj/notifications-engine v0.4.1-0.20250710034121-3ec18d4536a7/go.mod h1:d1RazGXWvKRFv9//rg4MRRR7rbvbE7XLgTSMT5fITTE= github.com/argoproj/pkg v0.13.6 h1:36WPD9MNYECHcO1/R1pj6teYspiK7uMQLCgLGft2abM= diff --git a/test/e2e/admin_test.go b/test/e2e/admin_test.go index 6514598ae579a..0d4ead2ff5bbd 100644 --- a/test/e2e/admin_test.go +++ b/test/e2e/admin_test.go @@ -22,6 +22,8 @@ func TestBackupExportImport(t *testing.T) { appctx := appfixture.GivenWithSameState(t) // Create application in test namespace + var appTestNamespace Application + var appOtherNamespace Application appctx. Path(guestbookPath). Name("exported-app1"). @@ -29,8 +31,9 @@ func TestBackupExportImport(t *testing.T) { CreateApp(). Then(). And(func(app *Application) { - assert.Equal(t, "exported-app1", app.Name) - assert.Equal(t, fixture.TestNamespace(), app.Namespace) + assert.Equal(t, appctx.AppName(), app.Name) + assert.Equal(t, appctx.AppNamespace(), app.Namespace) + appTestNamespace = *app }) // Create app in other namespace @@ -42,8 +45,9 @@ func TestBackupExportImport(t *testing.T) { CreateApp(). Then(). And(func(app *Application) { - assert.Equal(t, "exported-app-other-namespace", app.Name) - assert.Equal(t, fixture.AppNamespace(), app.Namespace) + assert.Equal(t, appctx.AppName(), app.Name) + assert.Equal(t, appctx.AppNamespace(), app.Namespace) + appOtherNamespace = *app }) ctx. @@ -57,8 +61,8 @@ func TestBackupExportImport(t *testing.T) { AndExportedResources(func(exportResources *ExportedResources, err error) { require.NoError(t, err, "export format not valid") assert.True(t, exportResources.HasResource(kube.NewResourceKey("", "ConfigMap", "", "argocd-cm")), "argocd-cm not found in export") - assert.True(t, exportResources.HasResource(kube.NewResourceKey(ApplicationSchemaGroupVersionKind.Group, ApplicationSchemaGroupVersionKind.Kind, "", "exported-app1")), "test namespace application not in export") - assert.True(t, exportResources.HasResource(kube.NewResourceKey(ApplicationSchemaGroupVersionKind.Group, ApplicationSchemaGroupVersionKind.Kind, fixture.AppNamespace(), "exported-app-other-namespace")), "app namespace application not in export") + assert.True(t, exportResources.HasResource(kube.NewResourceKey(ApplicationSchemaGroupVersionKind.Group, ApplicationSchemaGroupVersionKind.Kind, "", appTestNamespace.GetName())), "test namespace application not in export") + assert.True(t, exportResources.HasResource(kube.NewResourceKey(ApplicationSchemaGroupVersionKind.Group, ApplicationSchemaGroupVersionKind.Kind, appOtherNamespace.GetNamespace(), appOtherNamespace.GetName())), "app namespace application not in export") }) // Test import - clean state @@ -70,9 +74,9 @@ func TestBackupExportImport(t *testing.T) { Then(). AndCLIOutput(func(_ string, err error) { require.NoError(t, err, "import finished with error") - _, err = fixture.AppClientset.ArgoprojV1alpha1().Applications(fixture.TestNamespace()).Get(t.Context(), "exported-app1", metav1.GetOptions{}) + _, err = fixture.AppClientset.ArgoprojV1alpha1().Applications(appTestNamespace.GetNamespace()).Get(t.Context(), appTestNamespace.GetName(), metav1.GetOptions{}) require.NoError(t, err, "failed getting test namespace application after import") - _, err = fixture.AppClientset.ArgoprojV1alpha1().Applications(fixture.AppNamespace()).Get(t.Context(), "exported-app-other-namespace", metav1.GetOptions{}) + _, err = fixture.AppClientset.ArgoprojV1alpha1().Applications(appOtherNamespace.GetNamespace()).Get(t.Context(), appOtherNamespace.GetName(), metav1.GetOptions{}) require.NoError(t, err, "failed getting app namespace application after import") }) } diff --git a/test/e2e/app_management_test.go b/test/e2e/app_management_test.go index df302e8641911..8551ac5e30bf2 100644 --- a/test/e2e/app_management_test.go +++ b/test/e2e/app_management_test.go @@ -808,7 +808,7 @@ func TestAppWithSecrets(t *testing.T) { assert.Empty(t, diffOutput) // make sure resource update error does not print secret details - _, err = fixture.RunCli("app", "patch-resource", "test-app-with-secrets", "--resource-name", "test-secret", + _, err = fixture.RunCli("app", "patch-resource", app.Name, "--resource-name", "test-secret", "--kind", "Secret", "--patch", `{"op": "add", "path": "/data", "value": "hello"}'`, "--patch-type", "application/json-patch+json") require.ErrorContains(t, err, fmt.Sprintf("failed to patch Secret %s/test-secret", fixture.DeploymentNamespace())) @@ -1534,10 +1534,9 @@ func assertResourceActions(t *testing.T, appName string, successful bool) { func TestPermissions(t *testing.T) { appCtx := Given(t) - projName := "argo-project" - projActions := projectFixture. - GivenWithSameState(t). - Name(projName). + projCtx := projectFixture.GivenWithSameState(t) + projActions := projCtx. + Name("argo-project"). When(). Create() @@ -1546,7 +1545,7 @@ func TestPermissions(t *testing.T) { appCtx. Path("guestbook-logs"). - Project(projName). + Project(projCtx.GetName()). When(). IgnoreErrors(). // ensure app is not created if project permissions are missing @@ -1604,8 +1603,8 @@ func TestPermissions(t *testing.T) { Refresh(RefreshTypeNormal). Then(). // make sure application resource actiions are failing - And(func(_ *Application) { - assertResourceActions(t, "test-permissions", false) + And(func(a *Application) { + assertResourceActions(t, a.GetName(), false) }) } @@ -3000,37 +2999,46 @@ func TestInstallationID(t *testing.T) { func TestDeletionConfirmation(t *testing.T) { ctx := Given(t) - ctx. - And(func() { - _, err := fixture.KubeClientset.CoreV1().ConfigMaps(fixture.DeploymentNamespace()).Create( - t.Context(), &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-configmap", - Labels: map[string]string{ - common.LabelKeyAppInstance: ctx.AppName(), - }, - Annotations: map[string]string{ - AnnotationSyncOptions: "Prune=confirm", - }, + ctx.And(func() { + _, err := fixture.KubeClientset.CoreV1().ConfigMaps(fixture.DeploymentNamespace()).Create( + t.Context(), &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-configmap", + Labels: map[string]string{ + common.LabelKeyAppInstance: ctx.AppName(), }, - }, metav1.CreateOptions{}) - require.NoError(t, err) - }). + Annotations: map[string]string{ + AnnotationSyncOptions: "Prune=confirm", + }, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err) + }). Path(guestbookPath). Async(true). When(). PatchFile("guestbook-ui-deployment.yaml", `[{ "op": "add", "path": "/metadata/annotations", "value": { "argocd.argoproj.io/sync-options": "Delete=confirm" }}]`). - CreateApp().Sync(). - Then().Expect(OperationPhaseIs(OperationRunning)). - When().ConfirmDeletion(). - Then().Expect(OperationPhaseIs(OperationSucceeded)). - When().Delete(true). + CreateApp(). + Sync(). + Then(). + Expect(OperationPhaseIs(OperationRunning)). + When(). + ConfirmDeletion(). + Then(). + Expect(OperationPhaseIs(OperationSucceeded)). + Given(). + // Make sure to sleep a bit so the existing confirmed pruning annotation is after the deletion request + Sleep(3). + When(). + Delete(true). Then(). And(func(app *Application) { assert.NotNil(t, app.DeletionTimestamp) }). - When().ConfirmDeletion(). - Then().Expect(DoesNotExist()) + When(). + ConfirmDeletion(). + Then(). + Expect(DoesNotExist()) } func TestLastTransitionTimeUnchangedError(t *testing.T) { diff --git a/test/e2e/cli_test.go b/test/e2e/cli_test.go index 54098517c2d11..2790ef4f2e516 100644 --- a/test/e2e/cli_test.go +++ b/test/e2e/cli_test.go @@ -35,16 +35,16 @@ func createTestPlugin(t *testing.T, name, content string) string { // TestCliAppCommand verifies the basic Argo CD CLI commands for app synchronization and listing. func TestCliAppCommand(t *testing.T) { - Given(t). - Path("hook"). + ctx := Given(t) + ctx.Path("hook"). When(). CreateApp(). And(func() { - output, err := RunCli("app", "sync", Name(), "--timeout", "90") + output, err := RunCli("app", "sync", ctx.AppName(), "--timeout", "90") require.NoError(t, err) - vars := map[string]any{"Name": Name(), "Namespace": DeploymentNamespace()} + vars := map[string]any{"Name": ctx.AppName(), "Namespace": DeploymentNamespace()} assert.Contains(t, NormalizeOutput(output), Tmpl(t, `Pod {{.Namespace}} pod Synced Progressing pod/pod created`, vars)) - assert.Contains(t, NormalizeOutput(output), Tmpl(t, `Pod {{.Namespace}} hook Succeeded Sync pod/hook created`, vars)) + assert.Contains(t, NormalizeOutput(output), Tmpl(t, `Pod {{.Namespace}} hook Succeeded Healthy Sync pod/hook created`, vars)) }). Then(). Expect(OperationPhaseIs(OperationSucceeded)). @@ -75,19 +75,19 @@ func TestNormalArgoCDCommandsExecuteOverPluginsWithSameName(t *testing.T) { }) t.Setenv("PATH", filepath.Dir(pluginPath)+":"+origPath) - Given(t). - Path("hook"). + ctx := Given(t) + ctx.Path("hook"). When(). CreateApp(). And(func() { - output, err := RunCli("app", "sync", Name(), "--timeout", "90") + output, err := RunCli("app", "sync", ctx.AppName(), "--timeout", "90") require.NoError(t, err) assert.NotContains(t, NormalizeOutput(output), "I am a plugin, not Argo CD!") - vars := map[string]any{"Name": Name(), "Namespace": DeploymentNamespace()} + vars := map[string]any{"Name": ctx.AppName(), "Namespace": DeploymentNamespace()} assert.Contains(t, NormalizeOutput(output), Tmpl(t, `Pod {{.Namespace}} pod Synced Progressing pod/pod created`, vars)) - assert.Contains(t, NormalizeOutput(output), Tmpl(t, `Pod {{.Namespace}} hook Succeeded Sync pod/hook created`, vars)) + assert.Contains(t, NormalizeOutput(output), Tmpl(t, `Pod {{.Namespace}} hook Succeeded Healthy Sync pod/hook created`, vars)) }). Then(). Expect(OperationPhaseIs(OperationSucceeded)). @@ -101,7 +101,7 @@ func TestNormalArgoCDCommandsExecuteOverPluginsWithSameName(t *testing.T) { expected := Tmpl( t, `{{.Name}} https://kubernetes.default.svc {{.Namespace}} default Synced Healthy Manual `, - map[string]any{"Name": Name(), "Namespace": DeploymentNamespace()}) + map[string]any{"Name": ctx.AppName(), "Namespace": DeploymentNamespace()}) assert.Contains(t, NormalizeOutput(output), expected) }) } diff --git a/test/e2e/cluster_test.go b/test/e2e/cluster_test.go index 53cb6a5bf148a..dff5a5d7afa5e 100644 --- a/test/e2e/cluster_test.go +++ b/test/e2e/cluster_test.go @@ -56,9 +56,8 @@ https://kubernetes.default.svc in-cluster %v Successful `, fixtu } func TestClusterAdd(t *testing.T) { - clusterFixture. - Given(t). - Project(fixture.ProjectName). + ctx := clusterFixture.Given(t) + ctx.Project(fixture.ProjectName). Upsert(true). Server(KubernetesInternalAPIServerAddr). When(). @@ -66,8 +65,7 @@ func TestClusterAdd(t *testing.T) { List(). Then(). AndCLIOutput(func(output string, _ error) { - assert.Equal(t, fmt.Sprintf(`SERVER NAME VERSION STATUS MESSAGE PROJECT -https://kubernetes.default.svc test-cluster-add %v Successful %s`, fixture.GetVersions(t).ServerVersion, fixture.ProjectName), output) + assert.Contains(t, fixture.NormalizeOutput(output), fmt.Sprintf(`https://kubernetes.default.svc %s %v Successful %s`, ctx.GetName(), fixture.GetVersions(t).ServerVersion, fixture.ProjectName)) }) } @@ -112,9 +110,8 @@ func TestClusterAddAllowed(t *testing.T) { }, }, "org-admin") - clusterFixture. - GivenWithSameState(t). - Project(fixture.ProjectName). + ctx := clusterFixture.GivenWithSameState(t) + ctx.Project(fixture.ProjectName). Upsert(true). Server(KubernetesInternalAPIServerAddr). When(). @@ -122,8 +119,7 @@ func TestClusterAddAllowed(t *testing.T) { List(). Then(). AndCLIOutput(func(output string, _ error) { - assert.Equal(t, fmt.Sprintf(`SERVER NAME VERSION STATUS MESSAGE PROJECT -https://kubernetes.default.svc test-cluster-add-allowed %v Successful argo-project`, fixture.GetVersions(t).ServerVersion), output) + assert.Contains(t, fixture.NormalizeOutput(output), fmt.Sprintf(`https://kubernetes.default.svc %s %v Successful %s`, ctx.GetName(), fixture.GetVersions(t).ServerVersion, fixture.ProjectName)) }) } diff --git a/test/e2e/fixture/app/actions.go b/test/e2e/fixture/app/actions.go index 28ff2420c4097..7c1b9c95f2581 100644 --- a/test/e2e/fixture/app/actions.go +++ b/test/e2e/fixture/app/actions.go @@ -437,9 +437,7 @@ func (a *Actions) Sync(args ...string) *Actions { func (a *Actions) ConfirmDeletion() *Actions { a.context.t.Helper() - a.runCli("app", "confirm-deletion", a.context.AppQualifiedName()) - return a } diff --git a/test/e2e/fixture/app/context.go b/test/e2e/fixture/app/context.go index 59370d67161d6..8cdad5b971731 100644 --- a/test/e2e/fixture/app/context.go +++ b/test/e2e/fixture/app/context.go @@ -1,12 +1,16 @@ package app import ( + "fmt" + "strings" "testing" "time" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1" + "github.com/argoproj/argo-cd/v3/test" "github.com/argoproj/argo-cd/v3/test/e2e/fixture" "github.com/argoproj/argo-cd/v3/test/e2e/fixture/certs" "github.com/argoproj/argo-cd/v3/test/e2e/fixture/gpgkeys" @@ -93,8 +97,15 @@ func GivenWithSameState(t *testing.T) *Context { } } +// AppName returns the unique application name for the test context. +// Unique application names prevents from potential conflicts between test run +// caused by the tracking annotation on existing objects func (c *Context) AppName() string { - return c.name + suffix := "-" + fixture.ShortId() + if strings.HasSuffix(c.name, suffix) { + return c.name + } + return fixture.DnsFriendly(c.name, suffix) } func (c *Context) AppQualifiedName() string { @@ -470,3 +481,16 @@ func (c *Context) RegisterKustomizeVersion(version, path string) *Context { require.NoError(c.t, fixture.RegisterKustomizeVersion(version, path)) return c } + +func (c *Context) Resource(content string) *Context { + c.t.Helper() + u := test.YamlToUnstructured(content) + mapping, err := fixture.Mapper.RESTMapping(u.GroupVersionKind().GroupKind(), u.GroupVersionKind().Version) + require.NoError(c.t, err) + if mapping == nil { + require.NoError(c.t, fmt.Errorf("cannot find mapping for %s", u.GroupVersionKind().String())) + } + _, err = fixture.DynamicClientset.Resource(mapping.Resource).Namespace(fixture.DeploymentNamespace()).Apply(c.t.Context(), u.GetName(), u, metav1.ApplyOptions{FieldManager: "e2e-given-step"}) + require.NoError(c.t, err) + return c +} diff --git a/test/e2e/fixture/app/expectation.go b/test/e2e/fixture/app/expectation.go index 0ca0b67b4de1c..eb75a822c1442 100644 --- a/test/e2e/fixture/app/expectation.go +++ b/test/e2e/fixture/app/expectation.go @@ -28,6 +28,26 @@ const ( type Expectation func(c *Consequences) (state state, message string) +func Or(e1 Expectation, e2 Expectation) Expectation { + return func(c *Consequences) (state, string) { + s1, m1 := e1(c) + if s1 == succeeded { + return s1, m1 + } + s2, m2 := e2(c) + if s2 == succeeded { + return s2, m2 + } + if s1 == pending { + return s1, m1 + } + if s2 == pending { + return s2, m2 + } + return failed, fmt.Sprintf("expectations unsuccessful: %s and %s", m1, m2) + } +} + func OperationPhaseIs(expected common.OperationPhase) Expectation { return func(c *Consequences) (state, string) { operationState := c.app().Status.OperationState @@ -177,6 +197,9 @@ func ResourceHealthWithNamespaceIs(kind, resource, namespace string, expected he func ResourceResultNumbering(num int) Expectation { return func(c *Consequences) (state, string) { + if c.app().Status.OperationState == nil || c.app().Status.OperationState.SyncResult == nil { + return pending, "no sync result yet" + } actualNum := len(c.app().Status.OperationState.SyncResult.Resources) if actualNum < num { return pending, fmt.Sprintf("not enough results yet, want %d, got %d", num, actualNum) @@ -189,6 +212,9 @@ func ResourceResultNumbering(num int) Expectation { func ResourceResultIs(result v1alpha1.ResourceResult) Expectation { return func(c *Consequences) (state, string) { + if c.app().Status.OperationState == nil || c.app().Status.OperationState.SyncResult == nil { + return pending, "no sync result yet" + } results := c.app().Status.OperationState.SyncResult.Resources for _, res := range results { if reflect.DeepEqual(*res, result) { @@ -211,6 +237,9 @@ func sameResourceResult(res1, res2 v1alpha1.ResourceResult) bool { func ResourceResultMatches(result v1alpha1.ResourceResult) Expectation { return func(c *Consequences) (state, string) { + if c.app().Status.OperationState == nil || c.app().Status.OperationState.SyncResult == nil { + return pending, "no sync result yet" + } results := c.app().Status.OperationState.SyncResult.Resources for _, res := range results { if sameResourceResult(*res, result) { diff --git a/test/e2e/fixture/fixture.go b/test/e2e/fixture/fixture.go index a75b5e418ab54..7658d46f9daa9 100644 --- a/test/e2e/fixture/fixture.go +++ b/test/e2e/fixture/fixture.go @@ -20,10 +20,13 @@ import ( jsonpatch "github.com/evanphx/json-patch" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/discovery/cached/memory" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + "k8s.io/client-go/restmapper" "k8s.io/client-go/tools/clientcmd" "sigs.k8s.io/yaml" @@ -83,6 +86,7 @@ const ( var ( id string + shortId string deploymentNamespace string name string KubeClientset kubernetes.Interface @@ -90,6 +94,7 @@ var ( DynamicClientset dynamic.Interface AppClientset appclientset.Interface ArgoCDClientset apiclient.Client + Mapper meta.RESTMapper adminUsername string AdminPassword string apiServerAddress string @@ -188,6 +193,7 @@ func init() { AppClientset = appclientset.NewForConfigOrDie(config) KubeClientset = kubernetes.NewForConfigOrDie(config) DynamicClientset = dynamic.NewForConfigOrDie(config) + Mapper = restmapper.NewDeferredDiscoveryRESTMapper(memory.NewMemCacheClient(KubeClientset.Discovery())) KubeConfig = config apiServerAddress = GetEnvWithDefault(apiclient.EnvArgoCDServer, defaultAPIServer) @@ -294,6 +300,10 @@ func Name() string { return name } +func ShortId() string { + return shortId +} + func repoDirectory() string { return path.Join(TmpDir, repoDir) } @@ -997,10 +1007,10 @@ func EnsureCleanState(t *testing.T, opts ...TestOption) { if err != nil { return err } - postFix := "-" + strings.ToLower(randString) - id = t.Name() + postFix - name = DnsFriendly(t.Name(), "") - deploymentNamespace = DnsFriendly("argocd-e2e-"+t.Name(), postFix) + shortId = strings.ToLower(randString) + id = fmt.Sprintf("%s-%s", t.Name(), shortId) + name = DnsFriendly(t.Name(), "-"+shortId) + deploymentNamespace = DnsFriendly("argocd-e2e-"+t.Name(), "-"+shortId) // create namespace _, err = Run("", "kubectl", "create", "ns", DeploymentNamespace()) if err != nil { diff --git a/test/e2e/helm_test.go b/test/e2e/helm_test.go index a492369cd30a1..0e36a49d04f63 100644 --- a/test/e2e/helm_test.go +++ b/test/e2e/helm_test.go @@ -31,7 +31,7 @@ func TestHelmHooksAreCreated(t *testing.T) { Expect(OperationPhaseIs(OperationSucceeded)). Expect(HealthIs(health.HealthStatusHealthy)). Expect(SyncStatusIs(SyncStatusCodeSynced)). - Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: fixture.DeploymentNamespace(), Name: "hook", Message: "pod/hook created", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, HookType: HookTypePreSync, HookPhase: OperationSucceeded, SyncPhase: SyncPhasePreSync})) + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: fixture.DeploymentNamespace(), Name: "hook", Message: "pod/hook created", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, HookType: HookTypePreSync, Status: ResultCodeSynced, HookPhase: OperationSucceeded, SyncPhase: SyncPhasePreSync})) } // make sure we treat Helm weights as a sync wave diff --git a/test/e2e/hook_test.go b/test/e2e/hook_test.go index a350bfaadfa68..363cb4bd01dd4 100644 --- a/test/e2e/hook_test.go +++ b/test/e2e/hook_test.go @@ -10,9 +10,11 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/argoproj/gitops-engine/pkg/health" . "github.com/argoproj/gitops-engine/pkg/sync/common" + "github.com/argoproj/gitops-engine/pkg/sync/hook" . "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1" . "github.com/argoproj/argo-cd/v3/test/e2e/fixture" @@ -49,7 +51,13 @@ func testHookSuccessful(t *testing.T, hookType HookType) { Expect(ResourceSyncStatusIs("Pod", "pod", SyncStatusCodeSynced)). Expect(ResourceHealthIs("Pod", "pod", health.HealthStatusHealthy)). Expect(ResourceResultNumbering(2)). - Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "hook", Message: "pod/hook created", HookType: hookType, HookPhase: OperationSucceeded, SyncPhase: SyncPhase(hookType)})) + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "hook", Status: ResultCodeSynced, Message: "pod/hook created", HookType: hookType, HookPhase: OperationSucceeded, SyncPhase: SyncPhase(hookType)})). + Expect(Pod(func(p corev1.Pod) bool { + // Completed hooks should not have a finalizer + _, isHook := p.GetAnnotations()[AnnotationKeyHook] + hasFinalizer := controllerutil.ContainsFinalizer(&p, hook.HookFinalizer) + return isHook && !hasFinalizer + })) } func TestPostDeleteHook(t *testing.T) { @@ -96,14 +104,19 @@ func TestPreSyncHookFailure(t *testing.T) { IgnoreErrors(). Sync(). Then(). - Expect(Error("hook Failed Synced PreSync container \"main\" failed", "")). - // make sure resource are also printed - Expect(Error("pod OutOfSync Missing", "")). - Expect(OperationPhaseIs(OperationFailed)). // if a pre-sync hook fails, we should not start the main sync Expect(SyncStatusIs(SyncStatusCodeOutOfSync)). + Expect(OperationPhaseIs(OperationFailed)). Expect(ResourceResultNumbering(1)). - Expect(ResourceSyncStatusIs("Pod", "pod", SyncStatusCodeOutOfSync)) + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "hook", Status: ResultCodeSynced, Message: `container "main" failed with exit code 1`, HookType: HookTypePreSync, HookPhase: OperationFailed, SyncPhase: SyncPhase(HookTypePreSync)})). + Expect(ResourceHealthIs("Pod", "pod", health.HealthStatusMissing)). + Expect(ResourceSyncStatusIs("Pod", "pod", SyncStatusCodeOutOfSync)). + Expect(Pod(func(p corev1.Pod) bool { + // Completed hooks should not have a finalizer + _, isHook := p.GetAnnotations()[AnnotationKeyHook] + hasFinalizer := controllerutil.ContainsFinalizer(&p, hook.HookFinalizer) + return isHook && !hasFinalizer + })) } // make sure that if sync fails, we fail the app and we did create the pod @@ -121,7 +134,13 @@ func TestSyncHookFailure(t *testing.T) { // even thought the hook failed, we expect the pod to be in sync Expect(SyncStatusIs(SyncStatusCodeSynced)). Expect(ResourceResultNumbering(2)). - Expect(ResourceSyncStatusIs("Pod", "pod", SyncStatusCodeSynced)) + Expect(ResourceSyncStatusIs("Pod", "pod", SyncStatusCodeSynced)). + Expect(Pod(func(p corev1.Pod) bool { + // Completed hooks should not have a finalizer + _, isHook := p.GetAnnotations()[AnnotationKeyHook] + hasFinalizer := controllerutil.ContainsFinalizer(&p, hook.HookFinalizer) + return isHook && !hasFinalizer + })) } // make sure that if the deployments fails, we still get success and synced @@ -137,7 +156,7 @@ func TestSyncHookResourceFailure(t *testing.T) { Expect(HealthIs(health.HealthStatusProgressing)) } -// make sure that if post-sync fails, we fail the app and we did not create the pod +// make sure that if post-sync fails, we fail the app and we did create the pod func TestPostSyncHookFailure(t *testing.T) { Given(t). Path("hook"). @@ -152,7 +171,13 @@ func TestPostSyncHookFailure(t *testing.T) { Expect(OperationPhaseIs(OperationFailed)). Expect(SyncStatusIs(SyncStatusCodeSynced)). Expect(ResourceResultNumbering(2)). - Expect(ResourceSyncStatusIs("Pod", "pod", SyncStatusCodeSynced)) + Expect(ResourceSyncStatusIs("Pod", "pod", SyncStatusCodeSynced)). + Expect(Pod(func(p corev1.Pod) bool { + // Completed hooks should not have a finalizer + _, isHook := p.GetAnnotations()[AnnotationKeyHook] + hasFinalizer := controllerutil.ContainsFinalizer(&p, hook.HookFinalizer) + return isHook && !hasFinalizer + })) } // make sure that if the pod fails, we do not run the post-sync hook @@ -167,7 +192,7 @@ func TestPostSyncHookPodFailure(t *testing.T) { CreateApp(). Sync(). Then(). - // TODO - I feel like this should be a failure, not success + Expect(OperationPhaseIs(OperationFailed)). Expect(SyncStatusIs(SyncStatusCodeSynced)). Expect(ResourceSyncStatusIs("Pod", "pod", SyncStatusCodeSynced)). Expect(ResourceHealthIs("Pod", "pod", health.HealthStatusDegraded)). @@ -202,8 +227,15 @@ spec: CreateApp(). Sync(). Then(). - Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "sync-fail-hook", Message: "pod/sync-fail-hook created", HookType: HookTypeSyncFail, HookPhase: OperationSucceeded, SyncPhase: SyncPhaseSyncFail})). - Expect(OperationPhaseIs(OperationFailed)) + Expect(ResourceResultNumbering(3)). + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "sync-fail-hook", Status: ResultCodeSynced, Message: "pod/sync-fail-hook created", HookType: HookTypeSyncFail, HookPhase: OperationSucceeded, SyncPhase: SyncPhaseSyncFail})). + Expect(OperationPhaseIs(OperationFailed)). + Expect(Pod(func(p corev1.Pod) bool { + // Completed hooks should not have a finalizer + _, isHook := p.GetAnnotations()[AnnotationKeyHook] + hasFinalizer := controllerutil.ContainsFinalizer(&p, hook.HookFinalizer) + return p.GetName() == "sync-fail-hook" && isHook && !hasFinalizer + })) } func TestSyncFailHookPodFailureSyncFailFailure(t *testing.T) { @@ -249,9 +281,119 @@ spec: CreateApp(). Sync(). Then(). - Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Name: "successful-sync-fail-hook", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Message: "pod/successful-sync-fail-hook created", HookType: HookTypeSyncFail, HookPhase: OperationSucceeded, SyncPhase: SyncPhaseSyncFail})). - Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Name: "failed-sync-fail-hook", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Message: `container "main" failed with exit code 1`, HookType: HookTypeSyncFail, HookPhase: OperationFailed, SyncPhase: SyncPhaseSyncFail})). - Expect(OperationPhaseIs(OperationFailed)) + Expect(ResourceResultNumbering(4)). + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Name: "successful-sync-fail-hook", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Status: ResultCodeSynced, Message: "pod/successful-sync-fail-hook created", HookType: HookTypeSyncFail, HookPhase: OperationSucceeded, SyncPhase: SyncPhaseSyncFail})). + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Name: "failed-sync-fail-hook", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Status: ResultCodeSynced, Message: `container "main" failed with exit code 1`, HookType: HookTypeSyncFail, HookPhase: OperationFailed, SyncPhase: SyncPhaseSyncFail})). + Expect(OperationPhaseIs(OperationFailed)). + Expect(Pod(func(p corev1.Pod) bool { + // Completed hooks should not have a finalizer + _, isHook := p.GetAnnotations()[AnnotationKeyHook] + hasFinalizer := controllerutil.ContainsFinalizer(&p, hook.HookFinalizer) + return p.GetName() == "failed-sync-fail-hook" && isHook && !hasFinalizer + })) +} + +// Make sure that if a hook is invalid (must pass the dry-run client), it fails without affecting other hooks. +func TestInvalidlHookWaitsForOtherHooksToComplete(t *testing.T) { + existingHook := ` +apiVersion: v1 +kind: Pod +metadata: + annotations: + argocd.argoproj.io/hook: Sync + argocd.argoproj.io/hook-delete-policy: HookFailed # To preserve existence before sync + name: invalid-hook +spec: + containers: + - command: + - "true" + image: "quay.io/argoprojlabs/argocd-e2e-container:0.1" + imagePullPolicy: IfNotPresent + name: main + restartPolicy: Never` + + Given(t). + Path("hook"). + Resource(existingHook). + When(). + AddFile("invalid-hook.yaml", existingHook). + // The invalid hook needs to be valid in dry-run, but fail at apply time + // We change an immutable field to make it happen, and hook should already exist since delete policy was HookFailed on last sync + PatchFile("invalid-hook.yaml", `[{"op": "replace", "path": "/spec/containers/0/name", "value": "immutable" }]`). + CreateApp(). + IgnoreErrors(). + Sync(). + Then(). + Expect(ResourceResultNumbering(3)). + Expect(ResourceResultMatches(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "invalid-hook", Status: ResultCodeSyncFailed, Message: `Pod "invalid-hook" is invalid`, HookType: HookTypeSync, HookPhase: OperationFailed, SyncPhase: SyncPhase(HookTypeSync)})). + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "hook", Status: ResultCodeSynced, Message: "pod/hook created", HookType: HookTypeSync, HookPhase: OperationSucceeded, SyncPhase: SyncPhase(HookTypeSync)})). + Expect(OperationPhaseIs(OperationFailed)). + Expect(Pod(func(p corev1.Pod) bool { + // Completed hooks should not have a finalizer + _, isHook := p.GetAnnotations()[AnnotationKeyHook] + hasFinalizer := controllerutil.ContainsFinalizer(&p, hook.HookFinalizer) + return p.GetName() == "hook" && isHook && !hasFinalizer + })) +} + +func TestInvalidSyncFailureHookWaitsForOtherHooksToComplete(t *testing.T) { + existingHook := ` +apiVersion: v1 +kind: Pod +metadata: + annotations: + argocd.argoproj.io/hook: SyncFail + argocd.argoproj.io/hook-delete-policy: HookSucceeded # To preserve existence before sync + name: invalid-sync-fail-hook +spec: + containers: + - command: + - "true" + image: "quay.io/argoprojlabs/argocd-e2e-container:0.1" + imagePullPolicy: IfNotPresent + name: main + restartPolicy: Never` + + Given(t). + Path("hook"). + Resource(existingHook). + When(). + AddFile("successful-sync-fail-hook.yaml", ` +apiVersion: v1 +kind: Pod +metadata: + annotations: + argocd.argoproj.io/hook: SyncFail + name: successful-sync-fail-hook +spec: + containers: + - command: + - "true" + image: "quay.io/argoprojlabs/argocd-e2e-container:0.1" + imagePullPolicy: IfNotPresent + name: main + restartPolicy: Never +`). + AddFile("invalid-sync-fail-hook.yaml", existingHook). + // The invalid hook needs to be valid in dry-run, but fail at apply time + // We change an immutable field to make it happen, and hook should already exist since delete policy was HookFailed on last sync + PatchFile("invalid-sync-fail-hook.yaml", `[{"op": "replace", "path": "/spec/containers/0/name", "value": "immutable" }]`). + // Make the sync fail + PatchFile("hook.yaml", `[{"op": "replace", "path": "/spec/containers/0/command/0", "value": "false"}]`). + CreateApp(). + IgnoreErrors(). + Sync(). + Then(). + Expect(ResourceResultNumbering(4)). + Expect(ResourceResultMatches(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "invalid-sync-fail-hook", Status: ResultCodeSyncFailed, Message: `Pod "invalid-sync-fail-hook" is invalid`, HookType: HookTypeSyncFail, HookPhase: OperationFailed, SyncPhase: SyncPhase(HookTypeSyncFail)})). + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "successful-sync-fail-hook", Status: ResultCodeSynced, Message: "pod/successful-sync-fail-hook created", HookType: HookTypeSyncFail, HookPhase: OperationSucceeded, SyncPhase: SyncPhase(HookTypeSyncFail)})). + Expect(OperationPhaseIs(OperationFailed)). + Expect(Pod(func(p corev1.Pod) bool { + // Completed hooks should not have a finalizer + _, isHook := p.GetAnnotations()[AnnotationKeyHook] + hasFinalizer := controllerutil.ContainsFinalizer(&p, hook.HookFinalizer) + return p.GetName() == "successful-sync-fail-hook" && isHook && !hasFinalizer + })) } // make sure that we delete the hook on success @@ -441,6 +583,8 @@ func TestHookFinalizerPostSync(t *testing.T) { } func testHookFinalizer(t *testing.T, hookType HookType) { + // test that the finalizer prevents hooks from being deleted by Kubernetes without observing + // its health to evaluate completion first. t.Helper() Given(t). And(func() { @@ -478,5 +622,81 @@ func testHookFinalizer(t *testing.T, hookType HookType) { Expect(ResourceSyncStatusIs("Pod", "pod", SyncStatusCodeSynced)). Expect(ResourceHealthIs("Pod", "pod", health.HealthStatusHealthy)). Expect(ResourceResultNumbering(2)). - Expect(ResourceResultIs(ResourceResult{Group: "batch", Version: "v1", Kind: "Job", Namespace: DeploymentNamespace(), Name: "hook", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Message: "Resource has finalizer", HookType: hookType, HookPhase: OperationSucceeded, SyncPhase: SyncPhase(hookType)})) + Expect(ResourceResultIs(ResourceResult{Group: "batch", Version: "v1", Kind: "Job", Namespace: DeploymentNamespace(), Name: "hook", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Status: ResultCodeSynced, Message: "Resource has finalizer", HookType: hookType, HookPhase: OperationSucceeded, SyncPhase: SyncPhase(hookType)})) +} + +// test terminate operation stops running hooks +func TestTerminateWithRunningHooks(t *testing.T) { + newHook := func(name string, deletePolicy HookDeletePolicy, cmd string) string { + return fmt.Sprintf(` +apiVersion: v1 +kind: Pod +metadata: + annotations: + argocd.argoproj.io/hook: PreSync + argocd.argoproj.io/hook-delete-policy: %s + name: %s +spec: + containers: + - command: [ "/bin/sh", "-c", "--" ] + args: [ "%s" ] + image: "quay.io/argoprojlabs/argocd-e2e-container:0.1" + imagePullPolicy: IfNotPresent + name: main + restartPolicy: Never`, deletePolicy, name, cmd) + } + + podDeletedOrTerminatingWithoutFinalizer := func(name string) Expectation { + return Or( + NotPod(func(p corev1.Pod) bool { + return p.GetName() == name + }), + Pod(func(p corev1.Pod) bool { + _, isHook := p.GetAnnotations()[AnnotationKeyHook] + hasFinalizer := controllerutil.ContainsFinalizer(&p, hook.HookFinalizer) + return p.GetName() == name && isHook && !hasFinalizer && p.GetDeletionTimestamp() != nil + })) + } + + podWithoutFinalizer := func(name string) Expectation { + return Pod(func(p corev1.Pod) bool { + _, isHook := p.GetAnnotations()[AnnotationKeyHook] + hasFinalizer := controllerutil.ContainsFinalizer(&p, hook.HookFinalizer) + return p.GetName() == name && isHook && !hasFinalizer + }) + } + + Given(t). + Path("hook"). + Async(true). + When(). + AddFile("running-delete-on-success.yaml", newHook("running-delete-on-success", HookDeletePolicyHookSucceeded, "sleep 300")). + AddFile("running-delete-on-create.yaml", newHook("running-delete-on-create", HookDeletePolicyBeforeHookCreation, "sleep 300")). + AddFile("running-delete-on-failed.yaml", newHook("running-delete-on-failed", HookDeletePolicyHookFailed, "sleep 300")). + AddFile("complete-delete-on-success.yaml", newHook("complete-delete-on-success", HookDeletePolicyHookSucceeded, "true")). + AddFile("complete-delete-on-create.yaml", newHook("complete-delete-on-create", HookDeletePolicyBeforeHookCreation, "true")). + AddFile("complete-delete-on-failed.yaml", newHook("complete-delete-on-failed", HookDeletePolicyHookFailed, "true")). + CreateApp(). + Sync(). + Then(). + Expect(ResourceResultNumbering(6)). + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "complete-delete-on-success", Status: ResultCodeSynced, Message: "pod/complete-delete-on-success created", HookType: HookTypePreSync, HookPhase: OperationSucceeded, SyncPhase: SyncPhase(HookTypePreSync)})). + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "complete-delete-on-create", Status: ResultCodeSynced, Message: "pod/complete-delete-on-create created", HookType: HookTypePreSync, HookPhase: OperationSucceeded, SyncPhase: SyncPhase(HookTypePreSync)})). + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "complete-delete-on-failed", Status: ResultCodeSynced, Message: "pod/complete-delete-on-failed created", HookType: HookTypePreSync, HookPhase: OperationSucceeded, SyncPhase: SyncPhase(HookTypePreSync)})). + Expect(OperationPhaseIs(OperationRunning)). + When(). + TerminateOp(). + Then(). + Expect(OperationPhaseIs(OperationFailed)). + // Running hooks are terminated + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "running-delete-on-success", Status: ResultCodeSynced, Message: "Terminated", HookType: HookTypePreSync, HookPhase: OperationFailed, SyncPhase: SyncPhase(HookTypePreSync)})). + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "running-delete-on-create", Status: ResultCodeSynced, Message: "Terminated", HookType: HookTypePreSync, HookPhase: OperationFailed, SyncPhase: SyncPhase(HookTypePreSync)})). + Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "running-delete-on-failed", Status: ResultCodeSynced, Message: "Terminated", HookType: HookTypePreSync, HookPhase: OperationFailed, SyncPhase: SyncPhase(HookTypePreSync)})). + // terminated hooks finalizer is removed and are deleted successfully + Expect(podDeletedOrTerminatingWithoutFinalizer("running-delete-on-success")). + Expect(podDeletedOrTerminatingWithoutFinalizer("running-delete-on-create")). + Expect(podDeletedOrTerminatingWithoutFinalizer("running-delete-on-failed")). + Expect(podWithoutFinalizer("complete-delete-on-success")). + Expect(podWithoutFinalizer("complete-delete-on-create")). + Expect(podWithoutFinalizer("complete-delete-on-failed")) }