From b4dc4a34ba1a32209224b46e6957611bdaa7997f Mon Sep 17 00:00:00 2001 From: Anand Francis Joseph Date: Fri, 19 Jan 2024 13:11:57 +0530 Subject: [PATCH] Revert "feat: retry with client side dry run if server one was failed (#548)" This reverts commit c0c2dd1f6f487c46a25d3d3caaad7fec06667728. --- pkg/sync/sync_context.go | 50 ++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index c2ed67691..fd03d11b3 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -921,39 +921,29 @@ func (sc *syncContext) applyObject(t *syncTask, dryRun, force, validate bool) (c var err error var message string shouldReplace := sc.replace || resourceutil.HasAnnotationOption(t.targetObj, common.AnnotationSyncOptions, common.SyncOptionReplace) - applyFn := func(dryRunStrategy cmdutil.DryRunStrategy) (string, error) { - if !shouldReplace { - return sc.resourceOps.ApplyResource(context.TODO(), t.targetObj, dryRunStrategy, force, validate, serverSideApply, sc.serverSideApplyManager, false) - } - if t.liveObj == nil { - return sc.resourceOps.CreateResource(context.TODO(), t.targetObj, dryRunStrategy, validate) - } - // Avoid using `kubectl replace` for CRDs since 'replace' might recreate resource and so delete all CRD instances. - // The same thing applies for namespaces, which would delete the namespace as well as everything within it, - // so we want to avoid using `kubectl replace` in that case as well. - if kube.IsCRD(t.targetObj) || t.targetObj.GetKind() == kubeutil.NamespaceKind { - update := t.targetObj.DeepCopy() - update.SetResourceVersion(t.liveObj.GetResourceVersion()) - _, err = sc.resourceOps.UpdateResource(context.TODO(), update, dryRunStrategy) - if err != nil { - return fmt.Sprintf("error when updating: %v", err.Error()), err + if shouldReplace { + if t.liveObj != nil { + // Avoid using `kubectl replace` for CRDs since 'replace' might recreate resource and so delete all CRD instances. + // The same thing applies for namespaces, which would delete the namespace as well as everything within it, + // so we want to avoid using `kubectl replace` in that case as well. + if kube.IsCRD(t.targetObj) || t.targetObj.GetKind() == kubeutil.NamespaceKind { + update := t.targetObj.DeepCopy() + update.SetResourceVersion(t.liveObj.GetResourceVersion()) + _, err = sc.resourceOps.UpdateResource(context.TODO(), update, dryRunStrategy) + if err == nil { + message = fmt.Sprintf("%s/%s updated", t.targetObj.GetKind(), t.targetObj.GetName()) + } else { + message = fmt.Sprintf("error when updating: %v", err.Error()) + } + } else { + message, err = sc.resourceOps.ReplaceResource(context.TODO(), t.targetObj, dryRunStrategy, force) } - return fmt.Sprintf("%s/%s updated", t.targetObj.GetKind(), t.targetObj.GetName()), nil - + } else { + message, err = sc.resourceOps.CreateResource(context.TODO(), t.targetObj, dryRunStrategy, validate) } - return sc.resourceOps.ReplaceResource(context.TODO(), t.targetObj, dryRunStrategy, force) - } - - message, err = applyFn(dryRunStrategy) - - // DryRunServer fails with "Kind does not support fieldValidation" error for kubernetes server < 1.25 - // it fails inside apply.go , o.DryRunVerifier.HasSupport(info.Mapping.GroupVersionKind) line - // so we retry with DryRunClient that works for all cases, but cause issues with hooks - // Details: https://github.com/argoproj/argo-cd/issues/16177 - if dryRunStrategy == cmdutil.DryRunServer && err != nil { - message, err = applyFn(cmdutil.DryRunClient) + } else { + message, err = sc.resourceOps.ApplyResource(context.TODO(), t.targetObj, dryRunStrategy, force, validate, serverSideApply, sc.serverSideApplyManager, false) } - if err != nil { return common.ResultCodeSyncFailed, err.Error() }