From d2a62fd42234a96cbab2dbcf402c168c59b41784 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 15 Nov 2017 21:01:49 -0500 Subject: [PATCH 1/2] Table printers and server generation should always copy ListMeta Tables should be a mapping from lists, so if the incoming object has these add them to the table. Allows paging over server side tables. Add tests on the generic creater and on the resttest compatibility. --- pkg/printers/humanreadable.go | 10 ++ .../k8s.io/apimachinery/pkg/api/meta/meta.go | 2 +- .../apiserver/pkg/endpoints/apiserver_test.go | 132 +++++++++++------- .../pkg/registry/rest/resttest/resttest.go | 11 ++ .../apiserver/pkg/registry/rest/table.go | 10 ++ test/e2e/apimachinery/table_conversion.go | 53 +++++++ 6 files changed, 168 insertions(+), 50 deletions(-) diff --git a/pkg/printers/humanreadable.go b/pkg/printers/humanreadable.go index 1d091786e42..73ef967b150 100644 --- a/pkg/printers/humanreadable.go +++ b/pkg/printers/humanreadable.go @@ -525,6 +525,16 @@ func (h *HumanReadablePrinter) PrintTable(obj runtime.Object, options PrintOptio ColumnDefinitions: columns, Rows: results[0].Interface().([]metav1alpha1.TableRow), } + if m, err := meta.ListAccessor(obj); err == nil { + table.ResourceVersion = m.GetResourceVersion() + table.SelfLink = m.GetSelfLink() + table.Continue = m.GetContinue() + } else { + if m, err := meta.CommonAccessor(obj); err == nil { + table.ResourceVersion = m.GetResourceVersion() + table.SelfLink = m.GetSelfLink() + } + } if err := DecorateTable(table, options); err != nil { return nil, err } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/meta.go b/staging/src/k8s.io/apimachinery/pkg/api/meta/meta.go index cec4496e4dc..c2d51b43c73 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/meta.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/meta.go @@ -89,7 +89,7 @@ func ListAccessor(obj interface{}) (List, error) { } return nil, errNotList default: - panic(fmt.Errorf("%T does not implement the List interface", obj)) + return nil, errNotList } } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index 11024bf5f76..eda01358252 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go @@ -441,6 +441,10 @@ func (storage *SimpleRESTStorage) ConvertToTable(ctx request.Context, obj runtim func (storage *SimpleRESTStorage) List(ctx request.Context, options *metainternalversion.ListOptions) (runtime.Object, error) { storage.checkContext(ctx) result := &genericapitesting.SimpleList{ + ListMeta: metav1.ListMeta{ + ResourceVersion: "10", + SelfLink: "/test/link", + }, Items: storage.list, } storage.requestedLabelSelector = labels.Everything() @@ -1832,24 +1836,10 @@ func TestGetPretty(t *testing.T) { func TestGetTable(t *testing.T) { now := metav1.Now() - storage := map[string]rest.Storage{} obj := genericapitesting.Simple{ - ObjectMeta: metav1.ObjectMeta{Name: "foo1", Namespace: "ns1", CreationTimestamp: now, UID: types.UID("abcdef0123")}, + ObjectMeta: metav1.ObjectMeta{Name: "foo1", Namespace: "ns1", ResourceVersion: "10", SelfLink: "/blah", CreationTimestamp: now, UID: types.UID("abcdef0123")}, Other: "foo", } - simpleStorage := SimpleRESTStorage{ - item: obj, - } - selfLinker := &setTestSelfLinker{ - t: t, - expectedSet: "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple/id", - name: "id", - namespace: "default", - } - storage["simple"] = &simpleStorage - handler := handleLinker(storage, selfLinker) - server := httptest.NewServer(handler) - defer server.Close() m, err := meta.Accessor(&obj) if err != nil { @@ -1872,15 +1862,34 @@ func TestGetTable(t *testing.T) { pretty bool expected *metav1alpha1.Table statusCode int + item bool }{ { accept: runtime.ContentTypeJSON + ";as=Table;v=v1;g=meta.k8s.io", statusCode: http.StatusNotAcceptable, }, { + item: true, accept: runtime.ContentTypeJSON + ";as=Table;v=v1alpha1;g=meta.k8s.io", expected: &metav1alpha1.Table{ TypeMeta: metav1.TypeMeta{Kind: "Table", APIVersion: "meta.k8s.io/v1alpha1"}, + ListMeta: metav1.ListMeta{ResourceVersion: "10", SelfLink: "/blah"}, + ColumnDefinitions: []metav1alpha1.TableColumnDefinition{ + {Name: "Name", Type: "string", Format: "name", Description: metaDoc["name"]}, + {Name: "Created At", Type: "date", Description: metaDoc["creationTimestamp"]}, + }, + Rows: []metav1alpha1.TableRow{ + {Cells: []interface{}{"foo1", now.Time.UTC().Format(time.RFC3339)}, Object: runtime.RawExtension{Raw: encodedBody}}, + }, + }, + }, + { + item: true, + accept: runtime.ContentTypeJSON + ";as=Table;v=v1alpha1;g=meta.k8s.io", + params: url.Values{"includeObject": []string{"Metadata"}}, + expected: &metav1alpha1.Table{ + TypeMeta: metav1.TypeMeta{Kind: "Table", APIVersion: "meta.k8s.io/v1alpha1"}, + ListMeta: metav1.ListMeta{ResourceVersion: "10", SelfLink: "/blah"}, ColumnDefinitions: []metav1alpha1.TableColumnDefinition{ {Name: "Name", Type: "string", Format: "name", Description: metaDoc["name"]}, {Name: "Created At", Type: "date", Description: metaDoc["creationTimestamp"]}, @@ -1895,6 +1904,7 @@ func TestGetTable(t *testing.T) { params: url.Values{"includeObject": []string{"Metadata"}}, expected: &metav1alpha1.Table{ TypeMeta: metav1.TypeMeta{Kind: "Table", APIVersion: "meta.k8s.io/v1alpha1"}, + ListMeta: metav1.ListMeta{ResourceVersion: "10", SelfLink: "/test/link"}, ColumnDefinitions: []metav1alpha1.TableColumnDefinition{ {Name: "Name", Type: "string", Format: "name", Description: metaDoc["name"]}, {Name: "Created At", Type: "date", Description: metaDoc["creationTimestamp"]}, @@ -1906,45 +1916,69 @@ func TestGetTable(t *testing.T) { }, } for i, test := range tests { - u, err := url.Parse(server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple/id") - if err != nil { - t.Fatal(err) - } - u.RawQuery = test.params.Encode() - req := &http.Request{Method: "GET", URL: u} - req.Header = http.Header{} - req.Header.Set("Accept", test.accept) - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatal(err) - } - if test.statusCode != 0 { - if resp.StatusCode != test.statusCode { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + storage := map[string]rest.Storage{} + simpleStorage := SimpleRESTStorage{ + item: obj, + list: []genericapitesting.Simple{obj}, + } + selfLinker := &setTestSelfLinker{ + t: t, + expectedSet: "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple", + namespace: "default", + } + if test.item { + selfLinker.expectedSet += "/id" + selfLinker.name = "id" + } + storage["simple"] = &simpleStorage + handler := handleLinker(storage, selfLinker) + server := httptest.NewServer(handler) + defer server.Close() + + var id string + if test.item { + id = "/id" + } + u, err := url.Parse(server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple" + id) + if err != nil { + t.Fatal(err) + } + u.RawQuery = test.params.Encode() + req := &http.Request{Method: "GET", URL: u} + req.Header = http.Header{} + req.Header.Set("Accept", test.accept) + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + if test.statusCode != 0 { + if resp.StatusCode != test.statusCode { + t.Errorf("%d: unexpected response: %#v", i, resp) + } + obj, _, err := extractBodyObject(resp, unstructured.UnstructuredJSONScheme) + if err != nil { + t.Fatalf("%d: unexpected body read error: %v", err) + } + gvk := schema.GroupVersionKind{Version: "v1", Kind: "Status"} + if obj.GetObjectKind().GroupVersionKind() != gvk { + t.Fatalf("%d: unexpected error body: %#v", obj) + } + return + } + if resp.StatusCode != http.StatusOK { t.Errorf("%d: unexpected response: %#v", i, resp) } - obj, _, err := extractBodyObject(resp, unstructured.UnstructuredJSONScheme) + var itemOut metav1alpha1.Table + body, err := extractBody(resp, &itemOut) if err != nil { - t.Errorf("%d: unexpected body read error: %v", err) - continue + t.Fatal(err) } - gvk := schema.GroupVersionKind{Version: "v1", Kind: "Status"} - if obj.GetObjectKind().GroupVersionKind() != gvk { - t.Errorf("%d: unexpected error body: %#v", obj) + if !reflect.DeepEqual(test.expected, &itemOut) { + t.Log(body) + t.Errorf("%d: did not match: %s", i, diff.ObjectReflectDiff(test.expected, &itemOut)) } - continue - } - if resp.StatusCode != http.StatusOK { - t.Errorf("%d: unexpected response: %#v", i, resp) - } - var itemOut metav1alpha1.Table - body, err := extractBody(resp, &itemOut) - if err != nil { - t.Fatal(err) - } - if !reflect.DeepEqual(test.expected, &itemOut) { - t.Log(body) - t.Errorf("%d: did not match: %s", i, diff.ObjectReflectDiff(test.expected, &itemOut)) - } + }) } } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go index 11033454682..8c590e34d1f 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go @@ -1291,10 +1291,21 @@ func (t *Tester) testListTableConversion(obj runtime.Object, assignFn AssignFunc t.Errorf("expected: %#v, got: %#v", objs, items) } + m, err := meta.ListAccessor(listObj) + if err != nil { + t.Fatalf("list should support ListMeta %T: %v", listObj, err) + } + m.SetContinue("continuetoken") + m.SetResourceVersion("11") + m.SetSelfLink("/list/link") + table, err := t.storage.(rest.TableConvertor).ConvertToTable(ctx, listObj, nil) if err != nil { t.Errorf("unexpected error: %v", err) } + if table.ResourceVersion != "11" || table.SelfLink != "/list/link" || table.Continue != "continuetoken" { + t.Errorf("printer lost list meta: %#v", table.ListMeta) + } if len(table.Rows) != len(items) { t.Errorf("unexpected number of rows: %v", len(table.Rows)) } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/table.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/table.go index 087421e9f3d..4bc1b36685e 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/table.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/table.go @@ -63,6 +63,16 @@ func (c defaultTableConvertor) ConvertToTable(ctx genericapirequest.Context, obj return nil, err } } + if m, err := meta.ListAccessor(object); err == nil { + table.ResourceVersion = m.GetResourceVersion() + table.SelfLink = m.GetSelfLink() + table.Continue = m.GetContinue() + } else { + if m, err := meta.CommonAccessor(object); err == nil { + table.ResourceVersion = m.GetResourceVersion() + table.SelfLink = m.GetSelfLink() + } + } table.ColumnDefinitions = []metav1alpha1.TableColumnDefinition{ {Name: "Name", Type: "string", Format: "name", Description: swaggerMetadataDescriptions["name"]}, {Name: "Created At", Type: "date", Description: swaggerMetadataDescriptions["creationTimestamp"]}, diff --git a/test/e2e/apimachinery/table_conversion.go b/test/e2e/apimachinery/table_conversion.go index ffa1a6b77aa..0068bb60991 100644 --- a/test/e2e/apimachinery/table_conversion.go +++ b/test/e2e/apimachinery/table_conversion.go @@ -28,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1alpha1 "k8s.io/apimachinery/pkg/apis/meta/v1alpha1" + "k8s.io/client-go/util/workqueue" "k8s.io/kubernetes/pkg/printers" "k8s.io/kubernetes/test/e2e/framework" imageutils "k8s.io/kubernetes/test/utils/image" @@ -63,6 +64,56 @@ var _ = SIGDescribe("Servers with support for Table transformation", func() { framework.Logf("Table:\n%s", out) }) + It("should return chunks of table results for list calls", func() { + ns := f.Namespace.Name + c := f.ClientSet + client := c.CoreV1().PodTemplates(ns) + + By("creating a large number of resources") + workqueue.Parallelize(5, 20, func(i int) { + for tries := 3; tries >= 0; tries-- { + _, err := client.Create(&v1.PodTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("template-%04d", i), + }, + Template: v1.PodTemplateSpec{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + {Name: "test", Image: "test2"}, + }, + }, + }, + }) + if err == nil { + return + } + framework.Logf("Got an error creating template %d: %v", i, err) + } + Fail("Unable to create template %d, exiting", i) + }) + + pagedTable := &metav1alpha1.Table{} + err := c.CoreV1().RESTClient().Get().Namespace(ns).Resource("podtemplates"). + VersionedParams(&metav1.ListOptions{Limit: 2}, metav1.ParameterCodec). + SetHeader("Accept", "application/json;as=Table;v=v1alpha1;g=meta.k8s.io"). + Do().Into(pagedTable) + Expect(err).NotTo(HaveOccurred()) + Expect(len(pagedTable.Rows)).To(Equal(2)) + Expect(pagedTable.ResourceVersion).ToNot(Equal("")) + Expect(pagedTable.SelfLink).ToNot(Equal("")) + Expect(pagedTable.Continue).ToNot(Equal("")) + Expect(pagedTable.Rows[0].Cells[0]).To(Equal("template-0000")) + Expect(pagedTable.Rows[1].Cells[0]).To(Equal("template-0001")) + + err = c.CoreV1().RESTClient().Get().Namespace(ns).Resource("podtemplates"). + VersionedParams(&metav1.ListOptions{Continue: pagedTable.Continue}, metav1.ParameterCodec). + SetHeader("Accept", "application/json;as=Table;v=v1alpha1;g=meta.k8s.io"). + Do().Into(pagedTable) + Expect(err).NotTo(HaveOccurred()) + Expect(len(pagedTable.Rows)).To(BeNumerically(">", 0)) + Expect(pagedTable.Rows[0].Cells[0]).To(Equal("template-0002")) + }) + It("should return generic metadata details across all namespaces for nodes", func() { c := f.ClientSet @@ -75,6 +126,8 @@ var _ = SIGDescribe("Servers with support for Table transformation", func() { Expect(len(table.Rows)).To(BeNumerically(">=", 1)) Expect(len(table.Rows[0].Cells)).To(Equal(len(table.ColumnDefinitions))) Expect(table.ColumnDefinitions[0].Name).To(Equal("Name")) + Expect(table.ResourceVersion).ToNot(Equal("")) + Expect(table.SelfLink).ToNot(Equal("")) out := printTable(table) Expect(out).To(MatchRegexp("^NAME\\s")) From 8db90f1ee653d090abdf2a3e77c97dbb14a1a346 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 16 Nov 2017 18:04:59 -0500 Subject: [PATCH 2/2] API chunking tests should fail if limit is breached Chunking is now beta and on by default. The kops job is still using etcd2 which does not support chunking, so flag the test as skipped until kops is updated to a supported etcd version. --- test/e2e/apimachinery/chunking.go | 5 +++++ test/e2e/apimachinery/table_conversion.go | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/test/e2e/apimachinery/chunking.go b/test/e2e/apimachinery/chunking.go index 063a7bd4dac..c8b705bac0a 100644 --- a/test/e2e/apimachinery/chunking.go +++ b/test/e2e/apimachinery/chunking.go @@ -72,6 +72,11 @@ var _ = SIGDescribe("Servers with support for API chunking", func() { list, err := client.List(opts) Expect(err).ToNot(HaveOccurred()) framework.Logf("Retrieved %d/%d results with rv %s and continue %s", len(list.Items), opts.Limit, list.ResourceVersion, list.Continue) + // TODO: kops PR job is still using etcd2, which prevents this feature from working. Remove this check when kops is upgraded to etcd3 + if len(list.Items) > int(opts.Limit) { + framework.Skipf("ERROR: This cluster does not support chunking, which means it is running etcd2 and not supported.") + } + Expect(len(list.Items)).To(BeNumerically("<=", opts.Limit)) if len(lastRV) == 0 { lastRV = list.ResourceVersion diff --git a/test/e2e/apimachinery/table_conversion.go b/test/e2e/apimachinery/table_conversion.go index 0068bb60991..e3811402684 100644 --- a/test/e2e/apimachinery/table_conversion.go +++ b/test/e2e/apimachinery/table_conversion.go @@ -98,6 +98,10 @@ var _ = SIGDescribe("Servers with support for Table transformation", func() { SetHeader("Accept", "application/json;as=Table;v=v1alpha1;g=meta.k8s.io"). Do().Into(pagedTable) Expect(err).NotTo(HaveOccurred()) + // TODO: kops PR job is still using etcd2, which prevents this feature from working. Remove this check when kops is upgraded to etcd3 + if len(pagedTable.Rows) > 2 { + framework.Skipf("ERROR: This cluster does not support chunking, which means it is running etcd2 and not supported.") + } Expect(len(pagedTable.Rows)).To(Equal(2)) Expect(pagedTable.ResourceVersion).ToNot(Equal("")) Expect(pagedTable.SelfLink).ToNot(Equal(""))