From 44aff9917566885ab5d8cc712a3330bac1923214 Mon Sep 17 00:00:00 2001 From: Nick Maliwacki Date: Sun, 20 Dec 2020 00:35:31 -0800 Subject: [PATCH 1/2] wait ensures observedGeneration >= generation --- .../src/k8s.io/kubectl/pkg/cmd/wait/wait.go | 16 +++ .../k8s.io/kubectl/pkg/cmd/wait/wait_test.go | 102 ++++++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait.go b/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait.go index d781a76f7f5..fc5de3dd24b 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait.go @@ -442,6 +442,13 @@ func (w ConditionalWait) checkCondition(obj *unstructured.Unstructured) (bool, e if !found || err != nil { continue } + generation, found, _ := unstructured.NestedInt64(obj.Object, "metadata", "generation") + if found { + observedGeneration, found := getObservedGeneration(obj, condition) + if found && observedGeneration < generation { + return false, nil + } + } return strings.EqualFold(status, w.conditionStatus), nil } @@ -467,3 +474,12 @@ func (w ConditionalWait) isConditionMet(event watch.Event) (bool, error) { func extendErrWaitTimeout(err error, info *resource.Info) error { return fmt.Errorf("%s on %s/%s", err.Error(), info.Mapping.Resource.Resource, info.Name) } + +func getObservedGeneration(obj *unstructured.Unstructured, condition map[string]interface{}) (int64, bool) { + conditionObservedGeneration, found, _ := unstructured.NestedInt64(condition, "observedGeneration") + if found { + return conditionObservedGeneration, true + } + statusObservedGeneration, found, _ := unstructured.NestedInt64(obj.Object, "status", "observedGeneration") + return statusObservedGeneration, found +} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go index f347ea25737..5b5f67f03ac 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go @@ -60,6 +60,21 @@ func newUnstructured(apiVersion, kind, namespace, name string) *unstructured.Uns } } +func newUnstructuredWithGeneration(apiVersion, kind, namespace, name string, generation int64) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": apiVersion, + "kind": kind, + "metadata": map[string]interface{}{ + "namespace": namespace, + "name": name, + "uid": "some-UID-value", + "generation": generation, + }, + }, + } +} + func newUnstructuredStatus(status *metav1.Status) runtime.Unstructured { obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(status) if err != nil { @@ -80,6 +95,17 @@ func addCondition(in *unstructured.Unstructured, name, status string) *unstructu return in } +func addConditionWithObservedGeneration(in *unstructured.Unstructured, name, status string, observedGeneration int64) *unstructured.Unstructured { + conditions, _, _ := unstructured.NestedSlice(in.Object, "status", "conditions") + conditions = append(conditions, map[string]interface{}{ + "type": name, + "status": status, + "observedGeneration": observedGeneration, + }) + unstructured.SetNestedSlice(in.Object, conditions, "status", "conditions") + return in +} + func TestWaitForDeletion(t *testing.T) { scheme := runtime.NewScheme() listMapping := map[schema.GroupVersionResource]string{ @@ -764,6 +790,82 @@ func TestWaitForCondition(t *testing.T) { } }, }, + { + name: "times out due to observedGeneration", + infos: []*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.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + return true, addConditionWithObservedGeneration( + newUnstructuredWithGeneration("group/version", "TheKind", "ns-foo", "name-foo", 2), + "the-condition", "status-value", 1, + ), nil + }) + return fakeClient + }, + timeout: 1 * time.Second, + + expectedErr: "timed out waiting for the condition on theresource/name-foo", + validateActions: func(t *testing.T, actions []clienttesting.Action) { + if len(actions) != 2 { + t.Fatal(spew.Sdump(actions)) + } + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { + t.Error(spew.Sdump(actions)) + } + if !actions[1].Matches("watch", "theresource") { + t.Error(spew.Sdump(actions)) + } + }, + }, + { + name: "handles watch observedGeneration change", + infos: []*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.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + return true, newUnstructuredList(addConditionWithObservedGeneration(newUnstructuredWithGeneration("group/version", "TheKind", "ns-foo", "name-foo", 2), "the-condition", "status-value", 1)), nil + }) + fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { + fakeWatch := watch.NewRaceFreeFake() + fakeWatch.Action(watch.Modified, addConditionWithObservedGeneration( + newUnstructuredWithGeneration("group/version", "TheKind", "ns-foo", "name-foo", 2), + "the-condition", "status-value", 2, + )) + return true, fakeWatch, nil + }) + return fakeClient + }, + timeout: 10 * time.Second, + + validateActions: func(t *testing.T, actions []clienttesting.Action) { + if len(actions) != 2 { + t.Fatal(spew.Sdump(actions)) + } + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { + t.Error(spew.Sdump(actions)) + } + if !actions[1].Matches("watch", "theresource") { + t.Error(spew.Sdump(actions)) + } + }, + }, } for _, test := range tests { From bca0ddeba0d669093d04a148cef4d4179e2fa271 Mon Sep 17 00:00:00 2001 From: Nick Maliwacki Date: Wed, 13 Jan 2021 22:35:58 -0800 Subject: [PATCH 2/2] add .status.observedGeneration tests & update names --- .../k8s.io/kubectl/pkg/cmd/wait/wait_test.go | 86 ++++++++++++++++++- 1 file changed, 84 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go index 5b5f67f03ac..5eee26beaf0 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go @@ -791,7 +791,7 @@ func TestWaitForCondition(t *testing.T) { }, }, { - name: "times out due to observedGeneration", + name: "times out due to stale .status.conditions[0].observedGeneration", infos: []*resource.Info{ { Mapping: &meta.RESTMapping{ @@ -827,7 +827,7 @@ func TestWaitForCondition(t *testing.T) { }, }, { - name: "handles watch observedGeneration change", + name: "handles watch .status.conditions[0].observedGeneration change", infos: []*resource.Info{ { Mapping: &meta.RESTMapping{ @@ -854,6 +854,88 @@ func TestWaitForCondition(t *testing.T) { }, timeout: 10 * time.Second, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + if len(actions) != 2 { + t.Fatal(spew.Sdump(actions)) + } + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { + t.Error(spew.Sdump(actions)) + } + if !actions[1].Matches("watch", "theresource") { + t.Error(spew.Sdump(actions)) + } + }, + }, + { + name: "times out due to stale .status.observedGeneration", + infos: []*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.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + instance := addCondition( + newUnstructuredWithGeneration("group/version", "TheKind", "ns-foo", "name-foo", 2), + "the-condition", "status-value") + unstructured.SetNestedField(instance.Object, int64(1), "status", "observedGeneration") + return true, instance, nil + }) + return fakeClient + }, + timeout: 1 * time.Second, + + expectedErr: "timed out waiting for the condition on theresource/name-foo", + validateActions: func(t *testing.T, actions []clienttesting.Action) { + if len(actions) != 2 { + t.Fatal(spew.Sdump(actions)) + } + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { + t.Error(spew.Sdump(actions)) + } + if !actions[1].Matches("watch", "theresource") { + t.Error(spew.Sdump(actions)) + } + }, + }, + { + name: "handles watch .status.observedGeneration change", + infos: []*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.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + instance := addCondition( + newUnstructuredWithGeneration("group/version", "TheKind", "ns-foo", "name-foo", 2), + "the-condition", "status-value") + unstructured.SetNestedField(instance.Object, int64(1), "status", "observedGeneration") + return true, newUnstructuredList(instance), nil + }) + fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { + instance := addCondition( + newUnstructuredWithGeneration("group/version", "TheKind", "ns-foo", "name-foo", 2), + "the-condition", "status-value") + unstructured.SetNestedField(instance.Object, int64(2), "status", "observedGeneration") + fakeWatch := watch.NewRaceFreeFake() + fakeWatch.Action(watch.Modified, instance) + return true, fakeWatch, nil + }) + return fakeClient + }, + timeout: 10 * time.Second, + validateActions: func(t *testing.T, actions []clienttesting.Action) { if len(actions) != 2 { t.Fatal(spew.Sdump(actions))