Skip to content

Commit

Permalink
Do not use --force with --dry-run
Browse files Browse the repository at this point in the history 
Signed-off-by: rafal-jan <[email protected]>
  • Loading branch information
@rafal-jan
rafal-jan committed Nov 4, 2024
1 parent 9ab0b2e commit b22663c
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 66 deletions.
7 changes: 4 additions & 3 deletions pkg/sync/sync_context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,8 @@ func (sc *syncContext) applyObject(t *syncTask, dryRun, validate bool) (common.R
var err error
var message string
shouldReplace := sc.replace || resourceutil.HasAnnotationOption(t.targetObj, common.AnnotationSyncOptions, common.SyncOptionReplace)
force := sc.force || resourceutil.HasAnnotationOption(t.targetObj, common.AnnotationSyncOptions, common.SyncOptionForce)
// force is not required when running dry-run in client mode
shouldForce := !dryRun && (sc.force || resourceutil.HasAnnotationOption(t.targetObj, common.AnnotationSyncOptions, common.SyncOptionForce))
// if it is a dry run, disable server side apply, as the goal is to validate only the
// yaml correctness of the rendered manifests.
// running dry-run in server mode breaks the auto create namespace feature
Expand All @@ -983,13 +984,13 @@ func (sc *syncContext) applyObject(t *syncTask, dryRun, validate bool) (common.R
message = fmt.Sprintf("error when updating: %v", err.Error())
}
} else {
message, err = sc.resourceOps.ReplaceResource(context.TODO(), t.targetObj, dryRunStrategy, force)
message, err = sc.resourceOps.ReplaceResource(context.TODO(), t.targetObj, dryRunStrategy, shouldForce)
}
} else {
message, err = sc.resourceOps.CreateResource(context.TODO(), t.targetObj, dryRunStrategy, validate)
}
} else {
message, err = sc.resourceOps.ApplyResource(context.TODO(), t.targetObj, dryRunStrategy, force, validate, serverSideApply, sc.serverSideApplyManager, false)
message, err = sc.resourceOps.ApplyResource(context.TODO(), t.targetObj, dryRunStrategy, shouldForce, validate, serverSideApply, sc.serverSideApplyManager, false)
}
if err != nil {
return common.ResultCodeSyncFailed, err.Error()
Expand Down
20 changes: 15 additions & 5 deletions pkg/sync/sync_context_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"k8s.io/client-go/rest"
testcore "k8s.io/client-go/testing"
"k8s.io/klog/v2/textlogger"
cmdutil "k8s.io/kubectl/pkg/cmd/util"

"github.com/argoproj/gitops-engine/pkg/diff"
"github.com/argoproj/gitops-engine/pkg/health"
Expand Down Expand Up @@ -102,7 +103,7 @@ func TestSyncValidate(t *testing.T) {

// kubectl := syncCtx.kubectl.(*kubetest.MockKubectlCmd)
resourceOps, _ := syncCtx.resourceOps.(*kubetest.MockResourceOps)
assert.False(t, resourceOps.GetLastValidate())
assert.False(t, resourceOps.GetLastValidate(kube.GetResourceKey(pod)))
}

func TestSyncNotPermittedNamespace(t *testing.T) {
Expand Down Expand Up @@ -743,7 +744,7 @@ func TestSyncOptionValidate(t *testing.T) {

// kubectl, _ := syncCtx.kubectl.(*kubetest.MockKubectlCmd)
resourceOps, _ := syncCtx.resourceOps.(*kubetest.MockResourceOps)
assert.Equal(t, tt.want, resourceOps.GetLastValidate())
assert.Equal(t, tt.want, resourceOps.GetLastValidate(kube.GetResourceKey(pod)))
})
}
}
Expand Down Expand Up @@ -834,8 +835,8 @@ func TestSync_ServerSideApply(t *testing.T) {
// kubectl, _ := syncCtx.kubectl.(*kubetest.MockKubectlCmd)
resourceOps, _ := syncCtx.resourceOps.(*kubetest.MockResourceOps)
assert.Equal(t, tc.commandUsed, resourceOps.GetLastResourceCommand(kube.GetResourceKey(tc.target)))
assert.Equal(t, tc.serverSideApply, resourceOps.GetLastServerSideApply())
assert.Equal(t, tc.manager, resourceOps.GetLastServerSideApplyManager())
assert.Equal(t, tc.serverSideApply, resourceOps.GetLastServerSideApply(kube.GetResourceKey(tc.target)))
assert.Equal(t, tc.manager, resourceOps.GetLastServerSideApplyManager(kube.GetResourceKey(tc.target)))
})
}
}
Expand Down Expand Up @@ -880,8 +881,17 @@ func TestSync_Force(t *testing.T) {
syncCtx.Sync()

resourceOps, _ := syncCtx.resourceOps.(*kubetest.MockResourceOps)
registeredCommands := resourceOps.RegisteredCommands(kube.GetResourceKey(tc.target))
assert.Len(t, registeredCommands, 2)
dryRunCommand := registeredCommands[0]
command := registeredCommands[1]
assert.Equal(t, tc.commandUsed, resourceOps.GetLastResourceCommand(kube.GetResourceKey(tc.target)))
assert.Equal(t, tc.force, resourceOps.GetLastForce())

assert.Equal(t, false, dryRunCommand.Force)
assert.Equal(t, cmdutil.DryRunClient, dryRunCommand.DryRunStrategy)

assert.Equal(t, tc.force, command.Force)
assert.Equal(t, cmdutil.DryRunNone, command.DryRunStrategy)
})
}
}
Expand Down
120 changes: 62 additions & 58 deletions pkg/utils/kube/kubetest/mock_resource_operations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,21 @@ import (
cmdutil "k8s.io/kubectl/pkg/cmd/util"
)

type RegisteredCommand struct {
Command string
Validate bool
ServerSideApply bool
ServerSideApplyManager string
Force bool
DryRunStrategy cmdutil.DryRunStrategy
}

type MockResourceOps struct {
Commands map[string]KubectlOutput
Events chan watch.Event
DynamicClient dynamic.Interface

lastCommandPerResource map[kube.ResourceKey]string
lastValidate bool
serverSideApply bool
serverSideApplyManager string
lastForce bool
commandsPerResource map[kube.ResourceKey][]RegisteredCommand

recordLock sync.RWMutex

Expand All @@ -35,82 +40,59 @@ func (r *MockResourceOps) WithGetResourceFunc(getResourcefunc func(context.Conte
return r
}

func (r *MockResourceOps) SetLastValidate(validate bool) {
r.recordLock.Lock()
r.lastValidate = validate
r.recordLock.Unlock()
}

func (r *MockResourceOps) GetLastValidate() bool {
func (r *MockResourceOps) GetLastValidate(key kube.ResourceKey) bool {
r.recordLock.RLock()
validate := r.lastValidate
validate := r.lastCommand(key).Validate
r.recordLock.RUnlock()
return validate
}

func (r *MockResourceOps) SetLastServerSideApply(serverSideApply bool) {
r.recordLock.Lock()
r.serverSideApply = serverSideApply
r.recordLock.Unlock()
}

func (r *MockResourceOps) GetLastServerSideApplyManager() string {
func (r *MockResourceOps) GetLastServerSideApplyManager(key kube.ResourceKey) string {
r.recordLock.Lock()
manager := r.serverSideApplyManager
manager := r.lastCommand(key).ServerSideApplyManager
r.recordLock.Unlock()
return manager
}

func (r *MockResourceOps) GetLastServerSideApply() bool {
func (r *MockResourceOps) GetLastServerSideApply(key kube.ResourceKey) bool {
r.recordLock.RLock()
serverSideApply := r.serverSideApply
serverSideApply := r.lastCommand(key).ServerSideApply
r.recordLock.RUnlock()
return serverSideApply
}

func (r *MockResourceOps) SetLastServerSideApplyManager(manager string) {
r.recordLock.Lock()
r.serverSideApplyManager = manager
r.recordLock.Unlock()
}

func (r *MockResourceOps) SetLastForce(force bool) {
r.recordLock.Lock()
r.lastForce = force
r.recordLock.Unlock()
}

func (r *MockResourceOps) GetLastForce() bool {
func (r *MockResourceOps) GetLastForce(key kube.ResourceKey) bool {
r.recordLock.RLock()
force := r.lastForce
force := r.lastCommand(key).Force
r.recordLock.RUnlock()
return force
}

func (r *MockResourceOps) SetLastResourceCommand(key kube.ResourceKey, cmd string) {
r.recordLock.Lock()
if r.lastCommandPerResource == nil {
r.lastCommandPerResource = map[kube.ResourceKey]string{}
}
r.lastCommandPerResource[key] = cmd
r.recordLock.Unlock()
}

func (r *MockResourceOps) GetLastResourceCommand(key kube.ResourceKey) string {
r.recordLock.Lock()
defer r.recordLock.Unlock()
if r.lastCommandPerResource == nil {
if r.commandsPerResource == nil {
return ""
}
return r.lastCommandPerResource[key]
return r.lastCommand(key).Command
}

func (r *MockResourceOps) RegisteredCommands(key kube.ResourceKey) []RegisteredCommand {
r.recordLock.RLock()
registeredCommands := r.commandsPerResource[key]
r.recordLock.RUnlock()
return registeredCommands
}

func (r *MockResourceOps) ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string, serverSideDiff bool) (string, error) {
r.SetLastValidate(validate)
r.SetLastServerSideApply(serverSideApply)
r.SetLastServerSideApplyManager(manager)
r.SetLastForce(force)
r.SetLastResourceCommand(kube.GetResourceKey(obj), "apply")
r.registerCommand(kube.GetResourceKey(obj), RegisteredCommand{
Command: "apply",
Validate: validate,
ServerSideApply: serverSideApply,
ServerSideApplyManager: manager,
Force: force,
DryRunStrategy: dryRunStrategy,
})
command, ok := r.Commands[obj.GetName()]
if !ok {
return "", nil
Expand All @@ -120,9 +102,12 @@ func (r *MockResourceOps) ApplyResource(ctx context.Context, obj *unstructured.U
}

func (r *MockResourceOps) ReplaceResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force bool) (string, error) {
r.SetLastForce(force)
r.registerCommand(kube.GetResourceKey(obj), RegisteredCommand{
Command: "replace",
Force: force,
DryRunStrategy: dryRunStrategy,
})
command, ok := r.Commands[obj.GetName()]
r.SetLastResourceCommand(kube.GetResourceKey(obj), "replace")
if !ok {
return "", nil
}
Expand All @@ -131,7 +116,10 @@ func (r *MockResourceOps) ReplaceResource(ctx context.Context, obj *unstructured
}

func (r *MockResourceOps) UpdateResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy) (*unstructured.Unstructured, error) {
r.SetLastResourceCommand(kube.GetResourceKey(obj), "update")
r.registerCommand(kube.GetResourceKey(obj), RegisteredCommand{
Command: "update",
DryRunStrategy: dryRunStrategy,
})
command, ok := r.Commands[obj.GetName()]
if !ok {
return obj, nil
Expand All @@ -141,15 +129,31 @@ func (r *MockResourceOps) UpdateResource(ctx context.Context, obj *unstructured.
}

func (r *MockResourceOps) CreateResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, validate bool) (string, error) {

r.SetLastResourceCommand(kube.GetResourceKey(obj), "create")
r.registerCommand(kube.GetResourceKey(obj), RegisteredCommand{
Command: "create",
Validate: validate,
DryRunStrategy: dryRunStrategy,
})
command, ok := r.Commands[obj.GetName()]
if !ok {
return "", nil
}
return command.Output, command.Err
}

func (r *MockResourceOps) registerCommand(key kube.ResourceKey, cmd RegisteredCommand) {
r.recordLock.Lock()
if r.commandsPerResource == nil {
r.commandsPerResource = map[kube.ResourceKey][]RegisteredCommand{}
}
r.commandsPerResource[key] = append(r.commandsPerResource[key], cmd)
r.recordLock.Unlock()
}

func (r *MockResourceOps) lastCommand(key kube.ResourceKey) RegisteredCommand {
return r.commandsPerResource[key][len(r.commandsPerResource[key])-1]
}

/*func (r *MockResourceOps) ConvertToVersion(obj *unstructured.Unstructured, group, version string) (*unstructured.Unstructured, error) {
if r.convertToVersionFunc != nil {
return (*r.convertToVersionFunc)(obj, group, version)
Expand Down

0 comments on commit b22663c

Please sign in to comment.