Skip to content

Commit 717b8bf

Browse files
Nheijmink19Nheijmink19
authored
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 <nick.heijmink@alliander.com> * Fix test by mocking the discovery Signed-off-by: Nick Heijmink <nick.heijmink@alliander.com> * Fix linting errors Signed-off-by: Nick Heijmink <nick.heijmink@alliander.com> * Fix skip dryrun const --------- Signed-off-by: Nick Heijmink <nick.heijmink@alliander.com>
1 parent c617562 commit 717b8bf

File tree

6 files changed

+63
-12
lines changed

6 files changed

+63
-12
lines changed

‎pkg/sync/common/types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ const (
2020

2121
// Sync option that disables dry run in resource is missing in the cluster
2222
SyncOptionSkipDryRunOnMissingResource = "SkipDryRunOnMissingResource=true"
23+
// Sync option that disables dry run for applying resources
24+
SyncOptionSkipDryRun = "SkipDryRun=true"
2325
// Sync option that disables resource pruning
2426
SyncOptionDisablePrune = "Prune=false"
2527
// Sync option that disables resource validation

‎pkg/sync/doc.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ that runs before all other resources. The `argocd.argoproj.io/sync-wave` annotat
8080
The sync options allows customizing the synchronization of selected resources. The options are specified using the
8181
annotation 'argocd.argoproj.io/sync-options'. Following sync options are supported:
8282
83-
- SkipDryRunOnMissingResource=true - disables dry run in resource is missing in the cluster
83+
- SkipDryRunOnMissingResource=true - disables dry run if CRD resource is missing in the cluster
84+
- SkipDryRun=true - disables dry run if resource is missing in the cluster
8485
- Prune=false - disables resource pruning
8586
- Validate=false - disables resource validation (equivalent to 'kubectl apply --validate=false')
8687

‎pkg/sync/sync_context.go

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,12 @@ func WithReplace(replace bool) SyncOpt {
188188
}
189189
}
190190

191+
func WithSkipDryRun(skipDryRun bool) SyncOpt {
192+
return func(ctx *syncContext) {
193+
ctx.skipDryRun = skipDryRun
194+
}
195+
}
196+
191197
func WithServerSideApply(serverSideApply bool) SyncOpt {
192198
return func(ctx *syncContext) {
193199
ctx.serverSideApply = serverSideApply
@@ -335,6 +341,7 @@ type syncContext struct {
335341
namespace string
336342

337343
dryRun bool
344+
skipDryRun bool
338345
force bool
339346
validate bool
340347
skipHooks bool
@@ -812,16 +819,24 @@ func (sc *syncContext) getSyncTasks() (_ syncTasks, successful bool) {
812819
}
813820

814821
if err != nil {
815-
// Special case for custom resources: if CRD is not yet known by the K8s API server,
816-
// and the CRD is part of this sync or the resource is annotated with SkipDryRunOnMissingResource=true,
817-
// then skip verification during `kubectl apply --dry-run` since we expect the CRD
818-
// to be created during app synchronization.
819-
if apierrors.IsNotFound(err) &&
822+
switch {
823+
case apierrors.IsNotFound(err) &&
820824
((task.targetObj != nil && resourceutil.HasAnnotationOption(task.targetObj, common.AnnotationSyncOptions, common.SyncOptionSkipDryRunOnMissingResource)) ||
821-
sc.hasCRDOfGroupKind(task.group(), task.kind())) {
825+
sc.hasCRDOfGroupKind(task.group(), task.kind())):
826+
// Special case for custom resources: if CRD is not yet known by the K8s API server,
827+
// and the CRD is part of this sync or the resource is annotated with SkipDryRunOnMissingResource=true,
828+
// then skip verification during `kubectl apply --dry-run` since we expect the CRD
829+
// to be created during app synchronization.
822830
sc.log.WithValues("task", task).V(1).Info("Skip dry-run for custom resource")
823831
task.skipDryRun = true
824-
} else {
832+
case sc.skipDryRun:
833+
// Skip dryrun for task if the sync context is in skip dryrun mode
834+
// This can be useful when resource creation is depending on the creation of other resources
835+
// like namespaces that need to be created first before the resources in the namespace can be created
836+
// For CRD's one can also use the SkipDryRunOnMissingResource annotation.
837+
sc.log.WithValues("task", task).V(1).Info("Skipping dry-run for task because skipDryRun is set in the sync context")
838+
task.skipDryRun = true
839+
default:
825840
sc.setResourceResult(task, common.ResultCodeSyncFailed, "", err.Error())
826841
successful = false
827842
}

‎pkg/sync/sync_context_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,6 +1189,39 @@ func TestNamespaceAutoCreationForNonExistingNs(t *testing.T) {
11891189
waveOverride: nil,
11901190
}, tasks[0])
11911191
})
1192+
1193+
t.Run("pre-sync task error should be ignored if skip dryrun is true", func(t *testing.T) {
1194+
syncCtx.resources = groupResources(ReconciliationResult{
1195+
Live: []*unstructured.Unstructured{nil},
1196+
Target: []*unstructured.Unstructured{pod},
1197+
})
1198+
1199+
fakeDisco := syncCtx.disco.(*fakedisco.FakeDiscovery)
1200+
fakeDisco.Resources = []*metav1.APIResourceList{}
1201+
syncCtx.disco = fakeDisco
1202+
1203+
syncCtx.skipDryRun = true
1204+
creatorCalled := false
1205+
syncCtx.syncNamespace = func(_, _ *unstructured.Unstructured) (bool, error) {
1206+
creatorCalled = true
1207+
return true, errors.New("some error")
1208+
}
1209+
tasks, successful := syncCtx.getSyncTasks()
1210+
1211+
assert.True(t, creatorCalled)
1212+
assert.True(t, successful)
1213+
assert.Len(t, tasks, 2)
1214+
assert.Equal(t, &syncTask{
1215+
phase: synccommon.SyncPhasePreSync,
1216+
liveObj: nil,
1217+
targetObj: tasks[0].targetObj,
1218+
skipDryRun: true,
1219+
syncStatus: synccommon.ResultCodeSyncFailed,
1220+
operationState: synccommon.OperationError,
1221+
message: "namespaceModifier error: some error",
1222+
waveOverride: nil,
1223+
}, tasks[0])
1224+
})
11921225
}
11931226

11941227
func createNamespaceTask(namespace string) (*syncTask, error) {

‎pkg/utils/kube/kubetest/mock_resource_operations.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func (r *MockResourceOps) GetLastResourceCommand(key kube.ResourceKey) string {
106106
return r.lastCommandPerResource[key]
107107
}
108108

109-
func (r *MockResourceOps) ApplyResource(_ context.Context, obj *unstructured.Unstructured, _ cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) {
109+
func (r *MockResourceOps) ApplyResource(_ context.Context, obj *unstructured.Unstructured, _ cmdutil.DryRunStrategy, force bool, validate bool, serverSideApply bool, manager string) (string, error) {
110110
r.SetLastValidate(validate)
111111
r.SetLastServerSideApply(serverSideApply)
112112
r.SetLastServerSideApplyManager(manager)

‎pkg/utils/kube/resource_ops.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import (
3939

4040
// ResourceOperations provides methods to manage k8s resources
4141
type ResourceOperations interface {
42-
ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error)
42+
ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force bool, validate bool, serverSideApply bool, manager string) (string, error)
4343
ReplaceResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force bool) (string, error)
4444
CreateResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, validate bool) (string, error)
4545
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
301301
}
302302

303303
// ApplyResource performs an apply of a unstructured resource
304-
func (k *kubectlServerSideDiffDryRunApplier) ApplyResource(_ context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) {
304+
func (k *kubectlServerSideDiffDryRunApplier) ApplyResource(_ context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force bool, validate bool, serverSideApply bool, manager string) (string, error) {
305305
span := k.tracer.StartSpan("ApplyResource")
306306
span.SetBaggageItem("kind", obj.GetKind())
307307
span.SetBaggageItem("name", obj.GetName())
@@ -357,7 +357,7 @@ func (k *kubectlResourceOperations) ApplyResource(ctx context.Context, obj *unst
357357
})
358358
}
359359

360-
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) {
360+
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) {
361361
flags := apply.NewApplyFlags(ioStreams)
362362
o := &apply.ApplyOptions{
363363
IOStreams: ioStreams,

0 commit comments

Comments
 (0)