Optimistic-locking on diff

There is currently a race-condition when diffing, where we get the
object and then run a server-side dry-run patch and compare the two
results. If something changes the object on the server between the get
and the patch, the diff is going to show unrelated changes. We can now
specify the exact revisionversion that we want to patch, and that will
return a conflict, and we can retry multiple times to get a
non-conflicting diff. Eventually (after 3 times), we diff without
checking the version and throw a warning that the diff might be
partially wrong.
This commit is contained in:
Antoine Pelisse 2018-11-16 13:16:09 -08:00
parent 89daa462ff
commit a889f37505
2 changed files with 56 additions and 18 deletions

View File

@ -14,6 +14,7 @@ go_library(
"//pkg/kubectl/util/i18n:go_default_library",
"//pkg/kubectl/util/templates:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
@ -21,6 +22,7 @@ go_library(
"//staging/src/k8s.io/cli-runtime/pkg/genericclioptions/resource:go_default_library",
"//vendor/github.com/jonboulle/clockwork:go_default_library",
"//vendor/github.com/spf13/cobra:go_default_library",
"//vendor/k8s.io/klog:go_default_library",
"//vendor/k8s.io/utils/exec:go_default_library",
"//vendor/sigs.k8s.io/yaml:go_default_library",
],

View File

@ -26,11 +26,13 @@ import (
"github.com/jonboulle/clockwork"
"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/cli-runtime/pkg/genericclioptions"
"k8s.io/cli-runtime/pkg/genericclioptions/resource"
"k8s.io/klog"
"k8s.io/kubernetes/pkg/kubectl"
"k8s.io/kubernetes/pkg/kubectl/cmd/apply"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
@ -60,6 +62,9 @@ var (
cat service.yaml | kubectl diff -f -`))
)
// Number of times we try to diff before giving-up
const maxRetries = 4
type DiffOptions struct {
FilenameOptions resource.FilenameOptions
}
@ -228,6 +233,7 @@ type InfoObject struct {
Info *resource.Info
Encoder runtime.Encoder
OpenAPI openapi.Resources
Force bool
}
var _ Object = &InfoObject{}
@ -251,6 +257,16 @@ func (obj InfoObject) Merged() (runtime.Object, error) {
)
}
var resourceVersion *string
if !obj.Force {
accessor, err := meta.Accessor(obj.Info.Object)
if err != nil {
return nil, err
}
str := accessor.GetResourceVersion()
resourceVersion = &str
}
modified, err := kubectl.GetModifiedConfiguration(obj.LocalObj, false, unstructured.UnstructuredJSONScheme)
if err != nil {
return nil, err
@ -259,12 +275,13 @@ func (obj InfoObject) Merged() (runtime.Object, error) {
// This is using the patcher from apply, to keep the same behavior.
// We plan on replacing this with server-side apply when it becomes available.
patcher := &apply.Patcher{
Mapping: obj.Info.Mapping,
Helper: resource.NewHelper(obj.Info.Client, obj.Info.Mapping),
Overwrite: true,
BackOff: clockwork.NewRealClock(),
ServerDryRun: true,
OpenapiSchema: obj.OpenAPI,
Mapping: obj.Info.Mapping,
Helper: resource.NewHelper(obj.Info.Client, obj.Info.Mapping),
Overwrite: true,
BackOff: clockwork.NewRealClock(),
ServerDryRun: true,
OpenapiSchema: obj.OpenAPI,
ResourceVersion: resourceVersion,
}
_, result, err := patcher.Patch(obj.Info.Object, modified, obj.Info.Source, obj.Info.Namespace, obj.Info.Name, nil)
@ -319,6 +336,10 @@ func (d *Differ) TearDown() {
d.To.Dir.Delete() // Ignore error
}
func isConflict(err error) bool {
return err != nil && errors.IsConflict(err)
}
// RunDiff uses the factory to parse file arguments, find the version to
// diff, and find each Info object for each files, and runs against the
// differ.
@ -376,21 +397,36 @@ func RunDiff(f cmdutil.Factory, diff *DiffProgram, options *DiffOptions) error {
}
local := info.Object.DeepCopyObject()
if err := info.Get(); err != nil {
if !errors.IsNotFound(err) {
return err
for i := 1; i <= maxRetries; i++ {
if err = info.Get(); err != nil {
if !errors.IsNotFound(err) {
return err
}
info.Object = nil
}
info.Object = nil
}
obj := InfoObject{
LocalObj: local,
Info: info,
Encoder: scheme.DefaultJSONEncoder(),
OpenAPI: schema,
}
force := i == maxRetries
if force {
klog.Warningf(
"Object (%v: %v) keeps changing, diffing without lock",
info.Object.GetObjectKind().GroupVersionKind(),
info.Name,
)
}
obj := InfoObject{
LocalObj: local,
Info: info,
Encoder: scheme.DefaultJSONEncoder(),
OpenAPI: schema,
Force: force,
}
return differ.Diff(obj, printer)
err = differ.Diff(obj, printer)
if !isConflict(err) {
break
}
}
return err
})
if err != nil {
return err