From 493cd340091e441ab6a84c3f0be220270c755467 Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Wed, 15 Apr 2020 12:56:28 -0400 Subject: [PATCH] Fix issue with sorting pod lists The sorting previously used the selfref which contained the name on the end for all cases except pods. In that case the selfref was to pods and not the specific pod. This caused an issue where multiple pods had the same selfref used as the key for softing. The objects being sorted are tables that each have one row. In the new setup the key is the first cells value from the first and only row. This is the name of the resource. Note, the Get function now requests a table. The tests have been updated to return a Table type for the objects. Closes #7924 Signed-off-by: Matt Farina --- pkg/kube/client.go | 20 ++++++++++++++++-- pkg/kube/client_test.go | 45 ++++++++++++++++++++++++++++++++--------- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 8d3a039d3..20edb3ba0 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -270,6 +270,7 @@ func sortTableSlice(objs []runtime.Object) []runtime.Object { var newObjs []runtime.Object namesCache := make(map[string]runtime.Object, len(objs)) var names []string + var key string for _, obj := range objs { unstr, ok := obj.(*unstructured.Unstructured) if !ok { @@ -278,8 +279,23 @@ func sortTableSlice(objs []runtime.Object) []runtime.Object { if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstr.Object, ntbl); err != nil { return objs } - namesCache[ntbl.GetSelfLink()] = obj - names = append(names, ntbl.GetSelfLink()) + + // At this point we have a table. Each table has just one row. We are + // sorting the tables by the first cell (name) in the first and only + // row. If the first cell of the first row cannot be gotten as a string + // we return the original unsorted list. + if len(ntbl.Rows) == 0 { // Make sure there are rows to read from + return objs + } + if len(ntbl.Rows[0].Cells) == 0 { // Make sure there are cells to read + return objs + } + key, ok = ntbl.Rows[0].Cells[0].(string) + if !ok { + return objs + } + namesCache[key] = obj + names = append(names, key) } sort.Strings(names) diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index fc585f3b6..8a039eccd 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -59,6 +59,7 @@ var ( ) func getCodec() runtime.Codec { + metav1.AddMetaToScheme(scheme.Scheme) return scheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...) } @@ -112,6 +113,32 @@ func newService(name string) v1.Service { } } +func newTable(name string) metav1.Table { + + return metav1.Table{ + ColumnDefinitions: []metav1.TableColumnDefinition{{ + Description: "Name must be unique within a namespace. Is required when creating resources, although some resources may allow a client to request the generation of an appropriate name automatically. Name is primarily intended for creation idempotence and configuration definition. Cannot be updated. More info: http://kubernetes.io/docs/user-guide/identifiers#names", + Format: "name", + Name: "Name", + Priority: 0, + Type: "string", + }}, + Rows: []metav1.TableRow{{ + Cells: []interface{}{ + name, + }, + }}, + } +} + +func newTableList(names ...string) []metav1.Table { + var list []metav1.Table + for _, name := range names { + list = append(list, newTable(name)) + } + return list +} + func notFoundBody() *metav1.Status { return &metav1.Status{ Code: http.StatusNotFound, @@ -373,7 +400,7 @@ func TestBuild(t *testing.T) { } func TestGet(t *testing.T) { - list := newPodList("starfish", "otter") + list := newTableList("starfish", "otter") c := newTestClient() defer c.Cleanup() c.TestFactory.UnstructuredClient = &fake.RESTClient{ @@ -386,7 +413,7 @@ func TestGet(t *testing.T) { case p == "/namespaces/default/pods/starfish" && m == "GET": return newResponse(404, notFoundBody()) case p == "/namespaces/default/pods/otter" && m == "GET": - return newResponse(200, &list.Items[1]) + return newResponse(200, &list[1]) default: t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) return nil, nil @@ -416,8 +443,8 @@ func TestGet(t *testing.T) { } func TestResourceTypeSortOrder(t *testing.T) { - pod := newPod("my-pod") - service := newService("my-service") + pod := newTable("my-pod") + service := newTable("my-service") c := newTestClient() defer c.Cleanup() c.TestFactory.UnstructuredClient = &fake.RESTClient{ @@ -458,22 +485,22 @@ func TestResourceTypeSortOrder(t *testing.T) { } func TestResourceSortOrder(t *testing.T) { - list := newPodList("albacore", "coral", "beluga") + list := newTableList("albacore", "coral", "beluga") c := newTestClient() defer c.Cleanup() c.TestFactory.UnstructuredClient = &fake.RESTClient{ - GroupVersion: schema.GroupVersion{Version: "v1"}, + GroupVersion: schema.GroupVersion{Version: "v1", Group: "meta.k8s.io"}, NegotiatedSerializer: unstructuredSerializer, Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { p, m := req.URL.Path, req.Method t.Logf("got request %s %s", p, m) switch { case p == "/namespaces/default/pods/albacore" && m == "GET": - return newResponse(200, &list.Items[0]) + return newResponse(200, &list[0]) case p == "/namespaces/default/pods/coral" && m == "GET": - return newResponse(200, &list.Items[1]) + return newResponse(200, &list[1]) case p == "/namespaces/default/pods/beluga" && m == "GET": - return newResponse(200, &list.Items[2]) + return newResponse(200, &list[2]) default: t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) return nil, nil