Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: New sync flags for replace and force #653

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading