make delete waits match on UID

pull/8/head
David Eads 2018-07-12 14:35:59 -04:00
parent d2696d5612
commit 9fe20cfd46
6 changed files with 112 additions and 11 deletions

View File

@ -27,6 +27,7 @@ import (
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/dynamic"
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
@ -229,6 +230,7 @@ func (o *DeleteOptions) DeleteResult(r *resource.Result) error {
r = r.IgnoreErrors(errors.IsNotFound)
}
deletedInfos := []*resource.Info{}
uidMap := kubectlwait.UIDMap{}
err := r.Visit(func(info *resource.Info, err error) error {
if err != nil {
return err
@ -246,7 +248,28 @@ func (o *DeleteOptions) DeleteResult(r *resource.Result) error {
}
options.PropagationPolicy = &policy
return o.deleteResource(info, options)
response, err := o.deleteResource(info, options)
if err != nil {
return err
}
resourceLocation := kubectlwait.ResourceLocation{
GroupResource: info.Mapping.Resource.GroupResource(),
Namespace: info.Namespace,
Name: info.Name,
}
if status, ok := response.(*metav1.Status); ok && status.Details != nil {
uidMap[resourceLocation] = status.Details.UID
return nil
}
responseMetadata, err := meta.Accessor(response)
if err != nil {
// we don't have UID, but we didn't fail the delete, next best thing is just skipping the UID
glog.V(1).Info(err)
return nil
}
uidMap[resourceLocation] = responseMetadata.GetUID()
return nil
})
if err != nil {
return err
@ -271,6 +294,7 @@ func (o *DeleteOptions) DeleteResult(r *resource.Result) error {
}
waitOptions := kubectlwait.WaitOptions{
ResourceFinder: genericclioptions.ResourceFinderForResult(resource.InfoListVisitor(deletedInfos)),
UIDMap: uidMap,
DynamicClient: o.DynamicClient,
Timeout: effectiveTimeout,
@ -281,19 +305,21 @@ func (o *DeleteOptions) DeleteResult(r *resource.Result) error {
err = waitOptions.RunWait()
if errors.IsForbidden(err) || errors.IsMethodNotSupported(err) {
// if we're forbidden from waiting, we shouldn't fail.
// if the resource doesn't support a verb we need, we shouldn't fail.
glog.V(1).Info(err)
return nil
}
return err
}
func (o *DeleteOptions) deleteResource(info *resource.Info, deleteOptions *metav1.DeleteOptions) error {
if err := resource.NewHelper(info.Client, info.Mapping).DeleteWithOptions(info.Namespace, info.Name, deleteOptions); err != nil {
return cmdutil.AddSourceToErr("deleting", info.Source, err)
func (o *DeleteOptions) deleteResource(info *resource.Info, deleteOptions *metav1.DeleteOptions) (runtime.Object, error) {
deleteResponse, err := resource.NewHelper(info.Client, info.Mapping).DeleteWithOptions(info.Namespace, info.Name, deleteOptions)
if err != nil {
return nil, cmdutil.AddSourceToErr("deleting", info.Source, err)
}
o.PrintObj(info)
return nil
return deleteResponse, nil
}
// deletion printing is special because we do not have an object to print.

View File

@ -15,6 +15,8 @@ go_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",
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library",
"//staging/src/k8s.io/client-go/dynamic:go_default_library",
@ -48,6 +50,7 @@ go_test(
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library",
"//staging/src/k8s.io/client-go/dynamic/fake:go_default_library",

View File

@ -27,6 +27,8 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/client-go/dynamic"
@ -176,12 +178,23 @@ func conditionFuncFor(condition string) (ConditionFunc, error) {
return nil, fmt.Errorf("unrecognized condition: %q", condition)
}
type ResourceLocation struct {
GroupResource schema.GroupResource
Namespace string
Name string
}
type UIDMap map[ResourceLocation]types.UID
// WaitOptions is a set of options that allows you to wait. This is the object reflects the runtime needs of a wait
// command, making the logic itself easy to unit test with our existing mocks.
type WaitOptions struct {
ResourceFinder genericclioptions.ResourceFinder
DynamicClient dynamic.Interface
Timeout time.Duration
// UIDMap maps a resource location to a UID. It is optional, but ConditionFuncs may choose to use it to make the result
// more reliable. For instance, delete can look for UID consistency during delegated calls.
UIDMap UIDMap
DynamicClient dynamic.Interface
Timeout time.Duration
Printer printers.ResourcePrinter
ConditionFn ConditionFunc
@ -222,6 +235,16 @@ func IsDeleted(info *resource.Info, o *WaitOptions) (runtime.Object, bool, error
// TODO this could do something slightly fancier if we wish
return info.Object, false, err
}
resourceLocation := ResourceLocation{
GroupResource: info.Mapping.Resource.GroupResource(),
Namespace: gottenObj.GetNamespace(),
Name: gottenObj.GetName(),
}
if uid, ok := o.UIDMap[resourceLocation]; ok {
if gottenObj.GetUID() != uid {
return gottenObj, true, nil
}
}
watchOptions := metav1.ListOptions{}
watchOptions.FieldSelector = "metadata.name=" + info.Name

View File

@ -29,6 +29,7 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apimachinery/pkg/watch"
dynamicfakeclient "k8s.io/client-go/dynamic/fake"
@ -46,6 +47,7 @@ func newUnstructured(apiVersion, kind, namespace, name string) *unstructured.Uns
"metadata": map[string]interface{}{
"namespace": namespace,
"name": name,
"uid": "some-UID-value",
},
},
}
@ -69,6 +71,7 @@ func TestWaitForDeletion(t *testing.T) {
info *resource.Info
fakeClient func() *dynamicfakeclient.FakeDynamicClient
timeout time.Duration
uidMap UIDMap
expectedErr string
validateActions func(t *testing.T, actions []clienttesting.Action)
@ -96,6 +99,51 @@ func TestWaitForDeletion(t *testing.T) {
}
},
},
{
name: "uid conflict on get",
info: &resource.Info{
Mapping: &meta.RESTMapping{
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
},
Name: "name-foo",
Namespace: "ns-foo",
},
fakeClient: func() *dynamicfakeclient.FakeDynamicClient {
fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme)
fakeClient.PrependReactor("get", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) {
return true, newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"), nil
})
count := 0
fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) {
if count == 0 {
count++
fakeWatch := watch.NewRaceFreeFake()
go func() {
time.Sleep(100 * time.Millisecond)
fakeWatch.Stop()
}()
return true, fakeWatch, nil
}
fakeWatch := watch.NewRaceFreeFake()
return true, fakeWatch, nil
})
return fakeClient
},
timeout: 10 * time.Second,
uidMap: UIDMap{
ResourceLocation{Namespace: "ns-foo", Name: "name-foo"}: types.UID("some-UID-value"),
ResourceLocation{GroupResource: schema.GroupResource{Group: "group", Resource: "theresource"}, Namespace: "ns-foo", Name: "name-foo"}: types.UID("some-nonmatching-UID-value"),
},
validateActions: func(t *testing.T, actions []clienttesting.Action) {
if len(actions) != 1 {
t.Fatal(spew.Sdump(actions))
}
if !actions[0].Matches("get", "theresource") || actions[0].(clienttesting.GetAction).GetName() != "name-foo" {
t.Error(spew.Sdump(actions))
}
},
},
{
name: "times out",
info: &resource.Info{
@ -220,6 +268,7 @@ func TestWaitForDeletion(t *testing.T) {
fakeClient := test.fakeClient()
o := &WaitOptions{
ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(test.info),
UIDMap: test.uidMap,
DynamicClient: fakeClient,
Timeout: test.timeout,

View File

@ -94,18 +94,18 @@ func (m *Helper) WatchSingle(namespace, name, resourceVersion string) (watch.Int
Watch()
}
func (m *Helper) Delete(namespace, name string) error {
func (m *Helper) Delete(namespace, name string) (runtime.Object, error) {
return m.DeleteWithOptions(namespace, name, nil)
}
func (m *Helper) DeleteWithOptions(namespace, name string, options *metav1.DeleteOptions) error {
func (m *Helper) DeleteWithOptions(namespace, name string, options *metav1.DeleteOptions) (runtime.Object, error) {
return m.RESTClient.Delete().
NamespaceIfScoped(namespace, m.NamespaceScoped).
Resource(m.Resource).
Name(name).
Body(options).
Do().
Error()
Get()
}
func (m *Helper) Create(namespace string, modify bool, obj runtime.Object) (runtime.Object, error) {

View File

@ -129,7 +129,7 @@ func TestHelperDelete(t *testing.T) {
RESTClient: client,
NamespaceScoped: true,
}
err := modifier.Delete("bar", "foo")
_, err := modifier.Delete("bar", "foo")
if (err != nil) != tt.Err {
t.Errorf("unexpected error: %t %v", tt.Err, err)
}