From 717b8bfd69482e09a77294ba873570c6ead514b9 Mon Sep 17 00:00:00 2001 From: Nick Heijmink <75807895+Nheijmink19@users.noreply.github.com> Date: Thu, 10 Apr 2025 23:12:36 +0200 Subject: [PATCH] Add option to skip the dryrun from the sync context (#708) * Add option to skip the dryrun from the sync context Signed-off-by: Nick Heijmink * Fix test by mocking the discovery Signed-off-by: Nick Heijmink * Fix linting errors Signed-off-by: Nick Heijmink * Fix skip dryrun const --------- Signed-off-by: Nick Heijmink --- pkg/sync/common/types.go | 2 ++ pkg/sync/doc.go | 3 +- pkg/sync/sync_context.go | 29 ++++++++++++---- pkg/sync/sync_context_test.go | 33 +++++++++++++++++++ .../kube/kubetest/mock_resource_operations.go | 2 +- pkg/utils/kube/resource_ops.go | 6 ++-- 6 files changed, 63 insertions(+), 12 deletions(-) diff --git a/pkg/sync/common/types.go b/pkg/sync/common/types.go index 002bb23..060f793 100644 --- a/pkg/sync/common/types.go +++ b/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 diff --git a/pkg/sync/doc.go b/pkg/sync/doc.go index f4f5d87..32080d6 100644 --- a/pkg/sync/doc.go +++ b/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') diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index c2cc8b3..a3d4468 100644 --- a/pkg/sync/sync_context.go +++ b/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 } diff --git a/pkg/sync/sync_context_test.go b/pkg/sync/sync_context_test.go index 5f543b6..88e2e1a 100644 --- a/pkg/sync/sync_context_test.go +++ b/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) { diff --git a/pkg/utils/kube/kubetest/mock_resource_operations.go b/pkg/utils/kube/kubetest/mock_resource_operations.go index 8f42428..a97c4d0 100644 --- a/pkg/utils/kube/kubetest/mock_resource_operations.go +++ b/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) diff --git a/pkg/utils/kube/resource_ops.go b/pkg/utils/kube/resource_ops.go index 35dfd27..039749a 100644 --- a/pkg/utils/kube/resource_ops.go +++ b/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,