Add option to skip the dryrun from the sync context (argoproj#708)

* Add option to skip the dryrun from the sync context

Signed-off-by: Nick Heijmink <[email protected]>

* Fix test by mocking the discovery

Signed-off-by: Nick Heijmink <[email protected]>

* Fix linting errors

Signed-off-by: Nick Heijmink <[email protected]>

* Fix skip dryrun const

---------

Signed-off-by: Nick Heijmink <[email protected]>

Author: Nheijmink19

pkg/sync/common/types.go

@@ -20,6 +20,8 @@ const (
 
 	// Sync option that disables dry run in resource is missing in the cluster
 	SyncOptionSkipDryRunOnMissingResource = "SkipDryRunOnMissingResource=true"
+	// Sync option that disables dry run for applying resources
+	SyncOptionSkipDryRun = "SkipDryRun=true"
 	// Sync option that disables resource pruning
 	SyncOptionDisablePrune = "Prune=false"
 	// Sync option that disables resource validation

pkg/sync/doc.go

@@ -80,7 +80,8 @@ that runs before all other resources. The `argocd.argoproj.io/sync-wave` annotat
 The sync options allows customizing the synchronization of selected resources. The options are specified using the
 annotation 'argocd.argoproj.io/sync-options'. Following sync options are supported:
 
-- SkipDryRunOnMissingResource=true - disables dry run in resource is missing in the cluster
+- SkipDryRunOnMissingResource=true - disables dry run if CRD resource is missing in the cluster
+- SkipDryRun=true - disables dry run if resource is missing in the cluster
 - Prune=false - disables resource pruning
 - Validate=false - disables resource validation (equivalent to 'kubectl apply --validate=false')
 

pkg/sync/sync_context.go

@@ -188,6 +188,12 @@ func WithReplace(replace bool) SyncOpt {
 	}
 }
 
+func WithSkipDryRun(skipDryRun bool) SyncOpt {
+	return func(ctx *syncContext) {
+		ctx.skipDryRun = skipDryRun
+	}
+}
+
 func WithServerSideApply(serverSideApply bool) SyncOpt {
 	return func(ctx *syncContext) {
 		ctx.serverSideApply = serverSideApply
@@ -335,6 +341,7 @@ type syncContext struct {
 	namespace           string
 
 	dryRun                 bool
+	skipDryRun             bool
 	force                  bool
 	validate               bool
 	skipHooks              bool
@@ -812,16 +819,24 @@ func (sc *syncContext) getSyncTasks() (_ syncTasks, successful bool) {
 		}
 
 		if err != nil {
-			// Special case for custom resources: if CRD is not yet known by the K8s API server,
-			// and the CRD is part of this sync or the resource is annotated with SkipDryRunOnMissingResource=true,
-			// then skip verification during `kubectl apply --dry-run` since we expect the CRD
-			// to be created during app synchronization.
-			if apierrors.IsNotFound(err) &&
+			switch {
+			case apierrors.IsNotFound(err) &&
 				((task.targetObj != nil && resourceutil.HasAnnotationOption(task.targetObj, common.AnnotationSyncOptions, common.SyncOptionSkipDryRunOnMissingResource)) ||
-					sc.hasCRDOfGroupKind(task.group(), task.kind())) {
+					sc.hasCRDOfGroupKind(task.group(), task.kind())):
+				// Special case for custom resources: if CRD is not yet known by the K8s API server,
+				// and the CRD is part of this sync or the resource is annotated with SkipDryRunOnMissingResource=true,
+				// then skip verification during `kubectl apply --dry-run` since we expect the CRD
+				// to be created during app synchronization.
 				sc.log.WithValues("task", task).V(1).Info("Skip dry-run for custom resource")
 				task.skipDryRun = true
-			} else {
+			case sc.skipDryRun:
+				// Skip dryrun for task if the sync context is in skip dryrun mode
+				// This can be useful when resource creation is depending on the creation of other resources
+				// like namespaces that need to be created first before the resources in the namespace can be created
+				// For CRD's one can also use the SkipDryRunOnMissingResource annotation.
+				sc.log.WithValues("task", task).V(1).Info("Skipping dry-run for task because skipDryRun is set in the sync context")
+				task.skipDryRun = true
+			default:
 				sc.setResourceResult(task, common.ResultCodeSyncFailed, "", err.Error())
 				successful = false
 			}

pkg/sync/sync_context_test.go

@@ -1189,6 +1189,39 @@ func TestNamespaceAutoCreationForNonExistingNs(t *testing.T) {
 			waveOverride:   nil,
 		}, tasks[0])
 	})
+
+	t.Run("pre-sync task error should be ignored if skip dryrun is true", func(t *testing.T) {
+		syncCtx.resources = groupResources(ReconciliationResult{
+			Live:   []*unstructured.Unstructured{nil},
+			Target: []*unstructured.Unstructured{pod},
+		})
+
+		fakeDisco := syncCtx.disco.(*fakedisco.FakeDiscovery)
+		fakeDisco.Resources = []*metav1.APIResourceList{}
+		syncCtx.disco = fakeDisco
+
+		syncCtx.skipDryRun = true
+		creatorCalled := false
+		syncCtx.syncNamespace = func(_, _ *unstructured.Unstructured) (bool, error) {
+			creatorCalled = true
+			return true, errors.New("some error")
+		}
+		tasks, successful := syncCtx.getSyncTasks()
+
+		assert.True(t, creatorCalled)
+		assert.True(t, successful)
+		assert.Len(t, tasks, 2)
+		assert.Equal(t, &syncTask{
+			phase:          synccommon.SyncPhasePreSync,
+			liveObj:        nil,
+			targetObj:      tasks[0].targetObj,
+			skipDryRun:     true,
+			syncStatus:     synccommon.ResultCodeSyncFailed,
+			operationState: synccommon.OperationError,
+			message:        "namespaceModifier error: some error",
+			waveOverride:   nil,
+		}, tasks[0])
+	})
 }
 
 func createNamespaceTask(namespace string) (*syncTask, error) {

pkg/utils/kube/kubetest/mock_resource_operations.go

@@ -106,7 +106,7 @@ func (r *MockResourceOps) GetLastResourceCommand(key kube.ResourceKey) string {
 	return r.lastCommandPerResource[key]
 }
 
-func (r *MockResourceOps) ApplyResource(_ context.Context, obj *unstructured.Unstructured, _ cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) {
+func (r *MockResourceOps) ApplyResource(_ context.Context, obj *unstructured.Unstructured, _ cmdutil.DryRunStrategy, force bool, validate bool, serverSideApply bool, manager string) (string, error) {
 	r.SetLastValidate(validate)
 	r.SetLastServerSideApply(serverSideApply)
 	r.SetLastServerSideApplyManager(manager)

pkg/utils/kube/resource_ops.go

@@ -39,7 +39,7 @@ import (
 
 // ResourceOperations provides methods to manage k8s resources
 type ResourceOperations interface {
-	ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error)
+	ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force bool, validate bool, serverSideApply bool, manager string) (string, error)
 	ReplaceResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force bool) (string, error)
 	CreateResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, validate bool) (string, error)
 	UpdateResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy) (*unstructured.Unstructured, error)
@@ -301,7 +301,7 @@ func (k *kubectlResourceOperations) UpdateResource(ctx context.Context, obj *uns
 }
 
 // ApplyResource performs an apply of a unstructured resource
-func (k *kubectlServerSideDiffDryRunApplier) ApplyResource(_ context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) {
+func (k *kubectlServerSideDiffDryRunApplier) ApplyResource(_ context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force bool, validate bool, serverSideApply bool, manager string) (string, error) {
 	span := k.tracer.StartSpan("ApplyResource")
 	span.SetBaggageItem("kind", obj.GetKind())
 	span.SetBaggageItem("name", obj.GetName())
@@ -357,7 +357,7 @@ func (k *kubectlResourceOperations) ApplyResource(ctx context.Context, obj *unst
 	})
 }
 
-func newApplyOptionsCommon(config *rest.Config, fact cmdutil.Factory, ioStreams genericclioptions.IOStreams, obj *unstructured.Unstructured, fileName string, validate bool, force, serverSideApply bool, dryRunStrategy cmdutil.DryRunStrategy, manager string) (*apply.ApplyOptions, error) {
+func newApplyOptionsCommon(config *rest.Config, fact cmdutil.Factory, ioStreams genericclioptions.IOStreams, obj *unstructured.Unstructured, fileName string, validate bool, force bool, serverSideApply bool, dryRunStrategy cmdutil.DryRunStrategy, manager string) (*apply.ApplyOptions, error) {
 	flags := apply.NewApplyFlags(ioStreams)
 	o := &apply.ApplyOptions{
 		IOStreams:         ioStreams,