Skip to content

Commit

Permalink
feat: New sync flags for replace and force
Browse files Browse the repository at this point in the history
Gitops-engine part of argoproj/argo-cd#21427

Add new way of specifying options and include options always and never. Also, include a way to specify options per group/kind.

Signed-off-by: Andrii Korotkov <[email protected]>
  • Loading branch information
andrii-korotkov-verkada committed Jan 11, 2025
1 parent 54992bf commit 548f0bb
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 33 deletions.
8 changes: 8 additions & 0 deletions pkg/sync/common/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,16 @@ const (
SyncOptionPruneLast = "PruneLast=true"
// Sync option that enables use of replace or create command instead of apply
SyncOptionReplace = "Replace=true"
// Sync option that enables use of replace or create command instead of apply
SyncOptionReplaceAlways = "Replace=always"
// Sync option that disables use of replace or create command instead of apply
SyncOptionReplaceNever = "Replace=never"
// Sync option that enables use of --force flag, delete and re-create
SyncOptionForce = "Force=true"
// Sync option that enables use of --force flag, delete and re-create
SyncOptionForceAlways = "Force=always"
// Sync option that disables use of --force flag, delete and re-create
SyncOptionForceNever = "Force=never"
// Sync option that enables use of --server-side flag instead of client-side
SyncOptionServerSideApply = "ServerSideApply=true"
// Sync option that disables use of --server-side flag instead of client-side
Expand Down
140 changes: 118 additions & 22 deletions pkg/sync/sync_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"k8s.io/client-go/rest"
"k8s.io/client-go/util/retry"
"k8s.io/klog/v2/textlogger"
"k8s.io/kubectl/pkg/cmd/util"
cmdutil "k8s.io/kubectl/pkg/cmd/util"
"k8s.io/kubectl/pkg/util/openapi"

Expand Down Expand Up @@ -123,7 +124,15 @@ func WithPruneConfirmed(confirmed bool) SyncOpt {
}
}

// WithDryRun sets dry run setting
func WithDryRun(dryRun bool) SyncOpt {
return func(ctx *syncContext) {
ctx.dryRun = dryRun
}
}

// WithOperationSettings allows to set sync operation settings
// Deprecated, use individual setters
func WithOperationSettings(dryRun bool, prune bool, force bool, skipHooks bool) SyncOpt {
return func(ctx *syncContext) {
ctx.prune = prune
Expand Down Expand Up @@ -183,12 +192,42 @@ func WithSyncWaveHook(syncWaveHook common.SyncWaveHook) SyncOpt {
}
}

// WithReplace sets a replace to a given value
// Deprecated, prefer using WithReplaceOption
func WithReplace(replace bool) SyncOpt {
return func(ctx *syncContext) {
ctx.replace = replace
}
}

// WithReplaceOption sets replace option
func WithReplaceOption(replaceOption string) SyncOpt {
return func(ctx *syncContext) {
ctx.replaceOption = replaceOption
}
}

// WithReplaceForGK sets a replace option for a given group/kind
func WithReplaceForGK(gk schema.GroupKind, replaceOption string) SyncOpt {
return func(ctx *syncContext) {
ctx.replaceForGKs[gk] = replaceOption
}
}

// WithForceOption sets force option
func WithForceOption(forceOption string) SyncOpt {
return func(ctx *syncContext) {
ctx.forceOption = forceOption
}
}

// WithForceForGK sets a force option for a given group/kind
func WithForceForGK(gk schema.GroupKind, forceOption string) SyncOpt {
return func(ctx *syncContext) {
ctx.forceForGKs[gk] = forceOption
}
}

func WithServerSideApply(serverSideApply bool) SyncOpt {
return func(ctx *syncContext) {
ctx.serverSideApply = serverSideApply
Expand Down Expand Up @@ -337,11 +376,15 @@ type syncContext struct {

dryRun bool
force bool
forceOption string
forceForGKs map[schema.GroupKind]string
validate bool
skipHooks bool
resourcesFilter func(key kube.ResourceKey, target *unstructured.Unstructured, live *unstructured.Unstructured) bool
prune bool
replace bool
replaceOption string
replaceForGKs map[schema.GroupKind]string
serverSideApply bool
serverSideApplyManager string
pruneLast bool
Expand Down Expand Up @@ -965,6 +1008,77 @@ func (sc *syncContext) shouldUseServerSideApply(targetObj *unstructured.Unstruct
return sc.serverSideApply || resourceutil.HasAnnotationOption(targetObj, common.AnnotationSyncOptions, common.SyncOptionServerSideApply)
}

func (sc *syncContext) replaceObject(t *syncTask, dryRunStrategy util.DryRunStrategy, force bool, validate bool) (message string, err error) {
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)
}
} else {
message, err = sc.resourceOps.CreateResource(context.TODO(), t.targetObj, dryRunStrategy, validate)
}
return message, err
}

func (sc *syncContext) shouldReplace(targetObj *unstructured.Unstructured) bool {
if resourceutil.HasAnnotationOption(targetObj, common.AnnotationSyncOptions, common.SyncOptionReplace) ||
resourceutil.HasAnnotationOption(targetObj, common.AnnotationSyncOptions, common.SyncOptionReplaceAlways) {
return true
}
if resourceutil.HasAnnotationOption(targetObj, common.AnnotationSyncOptions, common.SyncOptionReplaceNever) {
return false
}
targetObjGK := targetObj.GroupVersionKind().GroupKind()
if sc.replaceForGKs[targetObjGK] == common.SyncOptionReplace ||
sc.replaceForGKs[targetObjGK] == common.SyncOptionReplaceAlways {
return true
}
if sc.replaceForGKs[targetObjGK] == common.SyncOptionReplaceNever {
return false
}
if sc.replace ||
sc.replaceOption == common.SyncOptionReplace ||
sc.replaceOption == common.SyncOptionReplaceAlways {
return true
}
return false
}

func (sc *syncContext) shouldForce(targetObj *unstructured.Unstructured) bool {
if resourceutil.HasAnnotationOption(targetObj, common.AnnotationSyncOptions, common.SyncOptionForce) ||
resourceutil.HasAnnotationOption(targetObj, common.AnnotationSyncOptions, common.SyncOptionForceAlways) {
return true
}
if resourceutil.HasAnnotationOption(targetObj, common.AnnotationSyncOptions, common.SyncOptionForceNever) {
return false
}
targetObjGK := targetObj.GroupVersionKind().GroupKind()
if sc.forceForGKs[targetObjGK] == common.SyncOptionForce ||
sc.forceForGKs[targetObjGK] == common.SyncOptionForceAlways {
return true
}
if sc.forceForGKs[targetObjGK] == common.SyncOptionForceNever {
return false
}
if sc.force ||
sc.forceOption == common.SyncOptionForce ||
sc.forceOption == common.SyncOptionForceAlways {
return true
}
return false
}

func (sc *syncContext) applyObject(t *syncTask, dryRun, validate bool) (common.ResultCode, string) {
dryRunStrategy := cmdutil.DryRunNone
if dryRun {
Expand All @@ -978,31 +1092,13 @@ 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)
shouldReplace := sc.shouldReplace(t.targetObj)
shouldForce := sc.shouldForce(t.targetObj)
serverSideApply := sc.shouldUseServerSideApply(t.targetObj)
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)
}
} else {
message, err = sc.resourceOps.CreateResource(context.TODO(), t.targetObj, dryRunStrategy, validate)
}
message, err = sc.replaceObject(t, dryRunStrategy, shouldForce, 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
48 changes: 37 additions & 11 deletions pkg/sync/sync_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -753,16 +753,28 @@ func withReplaceAnnotation(un *unstructured.Unstructured) *unstructured.Unstruct
return un
}

func withReplaceOptionAnnotation(un *unstructured.Unstructured, option string) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: option})
return un
}

func TestSync_Replace(t *testing.T) {
testCases := []struct {
name string
target *unstructured.Unstructured
live *unstructured.Unstructured
commandUsed string
name string
target *unstructured.Unstructured
live *unstructured.Unstructured
replaceForGKs map[schema.GroupKind]string
commandUsed string
}{
{"NoAnnotation", NewPod(), NewPod(), "apply"},
{"AnnotationIsSet", withReplaceAnnotation(NewPod()), NewPod(), "replace"},
{"LiveObjectMissing", withReplaceAnnotation(NewPod()), nil, "create"},
{"NoAnnotation", NewPod(), NewPod(), nil, "apply"},
{"AnnotationIsSet", withReplaceAnnotation(NewPod()), NewPod(), nil, "replace"},
{"LiveObjectMissing", withReplaceAnnotation(NewPod()), nil, nil, "create"},
{"AnnotationAlways", withReplaceOptionAnnotation(NewPod(), synccommon.SyncOptionReplaceAlways), NewPod(), nil, "replace"},
{"AnnotationNever", withReplaceOptionAnnotation(NewPod(), synccommon.SyncOptionReplaceNever), NewPod(), nil, "apply"},
{"SettingForPods", NewPod(), NewPod(), map[schema.GroupKind]string{{Group: "", Kind: "Pod"}: synccommon.SyncOptionReplaceAlways}, "replace"},
{"SettingNotForPods", NewPod(), NewPod(), map[schema.GroupKind]string{{Group: "", Kind: "ConfigMap"}: synccommon.SyncOptionReplaceAlways}, "apply"},
{"AnnotationAlwaysSettingForPodsNever", withReplaceOptionAnnotation(NewPod(), synccommon.SyncOptionReplaceAlways), NewPod(), map[schema.GroupKind]string{{Group: "", Kind: "Pod"}: synccommon.SyncOptionReplaceNever}, "replace"},
{"AnnotationNeverSettingForPodsAlways", withReplaceOptionAnnotation(NewPod(), synccommon.SyncOptionReplaceNever), NewPod(), map[schema.GroupKind]string{{Group: "", Kind: "Pod"}: synccommon.SyncOptionReplaceAlways}, "apply"},
}

for _, tc := range testCases {
Expand All @@ -777,6 +789,7 @@ func TestSync_Replace(t *testing.T) {
Live: []*unstructured.Unstructured{tc.live},
Target: []*unstructured.Unstructured{tc.target},
})
syncCtx.replaceForGKs = tc.replaceForGKs

syncCtx.Sync()

Expand Down Expand Up @@ -851,6 +864,11 @@ func withForceAnnotation(un *unstructured.Unstructured) *unstructured.Unstructur
return un
}

func withForceOptionAnnotation(un *unstructured.Unstructured, option string) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: option})
return un
}

func withForceAndReplaceAnnotations(un *unstructured.Unstructured) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: "Force=true,Replace=true"})
return un
Expand All @@ -861,13 +879,20 @@ func TestSync_Force(t *testing.T) {
name string
target *unstructured.Unstructured
live *unstructured.Unstructured
forceForGKs map[schema.GroupKind]string
commandUsed string
force bool
}{
{"NoAnnotation", NewPod(), NewPod(), "apply", false},
{"ForceApplyAnnotationIsSet", withForceAnnotation(NewPod()), NewPod(), "apply", true},
{"ForceReplaceAnnotationIsSet", withForceAndReplaceAnnotations(NewPod()), NewPod(), "replace", true},
{"LiveObjectMissing", withReplaceAnnotation(NewPod()), nil, "create", false},
{"NoAnnotation", NewPod(), NewPod(), nil, "apply", false},
{"ForceApplyAnnotationIsSet", withForceAnnotation(NewPod()), NewPod(), nil, "apply", true},
{"ForceReplaceAnnotationIsSet", withForceAndReplaceAnnotations(NewPod()), NewPod(), nil, "replace", true},
{"LiveObjectMissing", withReplaceAnnotation(NewPod()), nil, nil, "create", false},
{"AnnotationAlways", withForceOptionAnnotation(NewPod(), synccommon.SyncOptionForceAlways), NewPod(), nil, "apply", true},
{"AnnotationNever", withForceOptionAnnotation(NewPod(), synccommon.SyncOptionForceNever), NewPod(), nil, "apply", false},
{"SettingForPods", NewPod(), NewPod(), map[schema.GroupKind]string{{Group: "", Kind: "Pod"}: synccommon.SyncOptionForceAlways}, "apply", true},
{"SettingNotForPods", NewPod(), NewPod(), map[schema.GroupKind]string{{Group: "", Kind: "ConfigMap"}: synccommon.SyncOptionForceAlways}, "apply", false},
{"AnnotationAlwaysSettingForPodsNever", withForceOptionAnnotation(NewPod(), synccommon.SyncOptionForceAlways), NewPod(), map[schema.GroupKind]string{{Group: "", Kind: "Pod"}: synccommon.SyncOptionReplaceNever}, "apply", true},
{"AnnotationNeverSettingForPodsAlways", withForceOptionAnnotation(NewPod(), synccommon.SyncOptionForceNever), NewPod(), map[schema.GroupKind]string{{Group: "", Kind: "Pod"}: synccommon.SyncOptionReplaceAlways}, "apply", false},
}

for _, tc := range testCases {
Expand All @@ -882,6 +907,7 @@ func TestSync_Force(t *testing.T) {
Live: []*unstructured.Unstructured{tc.live},
Target: []*unstructured.Unstructured{tc.target},
})
syncCtx.forceForGKs = tc.forceForGKs

syncCtx.Sync()

Expand Down

0 comments on commit 548f0bb

Please sign in to comment.