Revert "Add option to skip the dryrun from the sync context (#708)" (#730)

* Revert "Add option to skip the dryrun from the sync context (#708)"

This reverts commit 717b8bf.

Signed-off-by: Michael Crenshaw <[email protected]>

* format

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: Michael Crenshaw <[email protected]>
Co-authored-by: Alexandre Gaudreault <[email protected]>
Signed-off-by: Roelof Kuijpers <[email protected]>

Author: 2 people

pkg/sync/common/types.go

@@ -23,8 +23,6 @@ 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,8 +80,7 @@ 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 if CRD resource is missing in the cluster
-- SkipDryRun=true - disables dry run if resource is missing in the cluster
+- SkipDryRunOnMissingResource=true - disables dry run in 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,12 +188,6 @@ func WithReplace(replace bool) SyncOpt {
 	}
 }
 
-func WithSkipDryRun(skipDryRun bool) SyncOpt {
-	return func(ctx *syncContext) {
-		ctx.skipDryRun = skipDryRun
-	}
-}
-
 func WithSkipDryRunOnMissingResource(skipDryRunOnMissingResource bool) SyncOpt {
 	return func(ctx *syncContext) {
 		ctx.skipDryRunOnMissingResource = skipDryRunOnMissingResource
@@ -361,7 +355,6 @@ type syncContext struct {
 	namespace           string
 
 	dryRun                          bool
-	skipDryRun                      bool
 	skipDryRunOnMissingResource     bool
 	force                           bool
 	validate                        bool
@@ -867,13 +860,6 @@ func (sc *syncContext) getSyncTasks() (_ syncTasks, successful bool) {
 				// to be created during app synchronization.
 				sc.log.WithValues("task", task).V(1).Info("Skip dry-run for custom resource")
 				task.skipDryRun = true
-			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

@@ -1198,39 +1198,6 @@ 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 bool, validate bool, serverSideApply bool, manager string) (string, error) {
+func (r *MockResourceOps) ApplyResource(_ context.Context, obj *unstructured.Unstructured, _ cmdutil.DryRunStrategy, force, validate, 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 bool, validate bool, serverSideApply bool, manager string) (string, error)
+	ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, 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)
@@ -302,7 +302,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 bool, validate bool, serverSideApply bool, manager string) (string, error) {
+func (k *kubectlServerSideDiffDryRunApplier) ApplyResource(_ context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) {
 	span := k.tracer.StartSpan("ApplyResource")
 	span.SetBaggageItem("kind", obj.GetKind())
 	span.SetBaggageItem("name", obj.GetName())
@@ -358,7 +358,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 bool, 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, serverSideApply bool, dryRunStrategy cmdutil.DryRunStrategy, manager string) (*apply.ApplyOptions, error) {
 	flags := apply.NewApplyFlags(ioStreams)
 	o := &apply.ApplyOptions{
 		IOStreams:         ioStreams,