Merge pull request #46881 from smarterclayton/fixes_to_table_print

Automatic merge from submit-queue (batch tested with PRs 47083, 44115, 46881, 47082, 46577)

Add an e2e test for server side get

Print a better error from the response. Performs validation to ensure it
does not regress in alpha state.

This is tests and bug fixes for https://github.com/kubernetes/community/pull/363

@kubernetes/sig-api-machinery-pr-reviews

staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go

@@ -438,6 +438,10 @@ func (storage *SimpleRESTStorage) Export(ctx request.Context, name string, opts
 	return obj, storage.errors["export"]
 }
 
+func (storage *SimpleRESTStorage) ConvertToTable(ctx request.Context, obj runtime.Object, tableOptions runtime.Object) (*metav1alpha1.Table, error) {
+	return rest.NewDefaultTableConvertor(schema.GroupResource{Resource: "simple"}).ConvertToTable(ctx, obj, tableOptions)
+}
+
 func (storage *SimpleRESTStorage) List(ctx request.Context, options *metainternalversion.ListOptions) (runtime.Object, error) {
 	storage.checkContext(ctx)
 	result := &genericapitesting.SimpleList{
@@ -1867,12 +1871,11 @@ func TestGetTable(t *testing.T) {
 			expected: &metav1alpha1.Table{
 				TypeMeta: metav1.TypeMeta{Kind: "Table", APIVersion: "meta.k8s.io/v1alpha1"},
 				ColumnDefinitions: []metav1alpha1.TableColumnDefinition{
-					{Name: "Namespace", Type: "string", Description: metaDoc["namespace"]},
 					{Name: "Name", Type: "string", Description: metaDoc["name"]},
 					{Name: "Created At", Type: "date", Description: metaDoc["creationTimestamp"]},
 				},
 				Rows: []metav1alpha1.TableRow{
-					{Cells: []interface{}{"ns1", "foo1", now.Time.UTC().Format(time.RFC3339)}, Object: runtime.RawExtension{Raw: encodedBody}},
+					{Cells: []interface{}{"foo1", now.Time.UTC().Format(time.RFC3339)}, Object: runtime.RawExtension{Raw: encodedBody}},
 				},
 			},
 		},
@@ -1897,7 +1900,7 @@ func TestGetTable(t *testing.T) {
 			continue
 		}
 		if resp.StatusCode != http.StatusOK {
-			t.Fatal(err)
+			t.Errorf("%d: unexpected response: %#v", resp)
 		}
 		var itemOut metav1alpha1.Table
 		if _, err = extractBody(resp, &itemOut); err != nil {

staging/src/k8s.io/apiserver/pkg/endpoints/installer.go

@@ -375,10 +375,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		shortNames = shortNamesProvider.ShortNames()
 	}
 
-	tableProvider, ok := storage.(rest.TableConvertor)
-	if !ok {
-		tableProvider = rest.DefaultTableConvertor
-	}
+	tableProvider, _ := storage.(rest.TableConvertor)
 
 	var categories []string
 	categoriesProvider, ok := storage.(rest.CategoriesProvider)

staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go

@@ -1351,5 +1351,5 @@ func (e *Store) ConvertToTable(ctx genericapirequest.Context, object runtime.Obj
 	if e.TableConvertor != nil {
 		return e.TableConvertor.ConvertToTable(ctx, object, tableOptions)
 	}
-	return rest.DefaultTableConvertor.ConvertToTable(ctx, object, tableOptions)
+	return rest.NewDefaultTableConvertor(e.QualifiedResource).ConvertToTable(ctx, object, tableOptions)
 }

staging/src/k8s.io/apiserver/pkg/registry/rest/table.go

@@ -17,31 +17,38 @@ limitations under the License.
 package rest
 
 import (
+	"fmt"
+	"net/http"
 	"time"
 
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	metav1alpha1 "k8s.io/apimachinery/pkg/apis/meta/v1alpha1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 )
 
-var DefaultTableConvertor TableConvertor = defaultTableConvertor{}
+type defaultTableConvertor struct {
+	qualifiedResource schema.GroupResource
+}
 
-type defaultTableConvertor struct{}
+// NewDefaultTableConvertor creates a default convertor for the provided resource.
+func NewDefaultTableConvertor(resource schema.GroupResource) TableConvertor {
+	return defaultTableConvertor{qualifiedResource: resource}
+}
 
 var swaggerMetadataDescriptions = metav1.ObjectMeta{}.SwaggerDoc()
 
-func (defaultTableConvertor) ConvertToTable(ctx genericapirequest.Context, object runtime.Object, tableOptions runtime.Object) (*metav1alpha1.Table, error) {
+func (c defaultTableConvertor) ConvertToTable(ctx genericapirequest.Context, object runtime.Object, tableOptions runtime.Object) (*metav1alpha1.Table, error) {
 	var table metav1alpha1.Table
 	fn := func(obj runtime.Object) error {
 		m, err := meta.Accessor(obj)
 		if err != nil {
-			// TODO: skip objects we don't recognize
-			return nil
+			return errNotAcceptable{resource: c.qualifiedResource}
 		}
 		table.Rows = append(table.Rows, metav1alpha1.TableRow{
-			Cells:  []interface{}{m.GetClusterName(), m.GetNamespace(), m.GetName(), m.GetCreationTimestamp().Time.UTC().Format(time.RFC3339)},
+			Cells:  []interface{}{m.GetName(), m.GetCreationTimestamp().Time.UTC().Format(time.RFC3339)},
 			Object: runtime.RawExtension{Object: obj},
 		})
 		return nil
@@ -57,50 +64,26 @@ func (defaultTableConvertor) ConvertToTable(ctx genericapirequest.Context, objec
 		}
 	}
 	table.ColumnDefinitions = []metav1alpha1.TableColumnDefinition{
-		{Name: "Cluster Name", Type: "string", Description: swaggerMetadataDescriptions["clusterName"]},
-		{Name: "Namespace", Type: "string", Description: swaggerMetadataDescriptions["namespace"]},
 		{Name: "Name", Type: "string", Description: swaggerMetadataDescriptions["name"]},
 		{Name: "Created At", Type: "date", Description: swaggerMetadataDescriptions["creationTimestamp"]},
 	}
-	// trim the left two columns if completely empty
-	if trimColumn(0, &table) {
-		trimColumn(0, &table)
-	} else {
-		trimColumn(1, &table)
-	}
 	return &table, nil
 }
 
-func trimColumn(column int, table *metav1alpha1.Table) bool {
-	for _, item := range table.Rows {
-		switch t := item.Cells[column].(type) {
-		case string:
-			if len(t) > 0 {
-				return false
-			}
-		case interface{}:
-			if t == nil {
-				return false
-			}
-		}
-	}
-	if column == 0 {
-		table.ColumnDefinitions = table.ColumnDefinitions[1:]
-	} else {
-		for j := column; j < len(table.ColumnDefinitions); j++ {
-			table.ColumnDefinitions[j] = table.ColumnDefinitions[j+1]
-		}
-	}
-	for i := range table.Rows {
-		cells := table.Rows[i].Cells
-		if column == 0 {
-			table.Rows[i].Cells = cells[1:]
-			continue
-		}
-		for j := column; j < len(cells); j++ {
-			cells[j] = cells[j+1]
-		}
-		table.Rows[i].Cells = cells[:len(cells)-1]
-	}
-	return true
+// errNotAcceptable indicates the resource doesn't support Table conversion
+type errNotAcceptable struct {
+	resource schema.GroupResource
+}
+
+func (e errNotAcceptable) Error() string {
+	return fmt.Sprintf("the resource %s does not support being converted to a Table", e.resource)
+}
+
+func (e errNotAcceptable) Status() metav1.Status {
+	return metav1.Status{
+		Status:  metav1.StatusFailure,
+		Code:    http.StatusNotAcceptable,
+		Reason:  metav1.StatusReason("NotAcceptable"),
+		Message: e.Error(),
+	}
 }

test/e2e/BUILD

@@ -21,6 +21,7 @@ go_test(
         "//pkg/api/v1:go_default_library",
         "//pkg/client/clientset_generated/clientset:go_default_library",
         "//pkg/metrics:go_default_library",
+        "//test/e2e/api:go_default_library",
         "//test/e2e/autoscaling:go_default_library",
         "//test/e2e/cluster-logging:go_default_library",
         "//test/e2e/extension:go_default_library",
@@ -232,6 +233,7 @@ filegroup(
     name = "all-srcs",
     srcs = [
         ":package-srcs",
+        "//test/e2e/api:all-srcs",
         "//test/e2e/autoscaling:all-srcs",
         "//test/e2e/chaosmonkey:all-srcs",
         "//test/e2e/cluster-logging:all-srcs",

test/e2e/api/BUILD

@@ -0,0 +1,37 @@
+package(default_visibility = ["//visibility:public"])
+
+licenses(["notice"])
+
+load(
+    "@io_bazel_rules_go//go:def.bzl",
+    "go_library",
+)
+
+go_library(
+    name = "go_default_library",
+    srcs = ["table_conversion.go"],
+    tags = ["automanaged"],
+    deps = [
+        "//pkg/api/v1:go_default_library",
+        "//pkg/printers:go_default_library",
+        "//test/e2e/framework:go_default_library",
+        "//vendor/github.com/onsi/ginkgo:go_default_library",
+        "//vendor/github.com/onsi/gomega:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1alpha1:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+)

test/e2e/api/table_conversion.go

@@ -0,0 +1,122 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package extension
+
+import (
+	"bytes"
+	"fmt"
+	"text/tabwriter"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	metav1alpha1 "k8s.io/apimachinery/pkg/apis/meta/v1alpha1"
+	"k8s.io/kubernetes/pkg/api/v1"
+	"k8s.io/kubernetes/pkg/printers"
+	"k8s.io/kubernetes/test/e2e/framework"
+)
+
+var _ = framework.KubeDescribe("Servers with support for Table transformation", func() {
+	f := framework.NewDefaultFramework("tables")
+
+	It("should return pod details", func() {
+		ns := f.Namespace.Name
+		c := f.ClientSet
+
+		podName := "pod-1"
+		framework.Logf("Creating pod %s", podName)
+
+		_, err := c.Core().Pods(ns).Create(newPod(podName))
+		Expect(err).NotTo(HaveOccurred())
+
+		table := &metav1alpha1.Table{}
+		err = c.Core().RESTClient().Get().Resource("pods").Namespace(ns).Name(podName).SetHeader("Accept", "application/json;as=Table;v=v1alpha1;g=meta.k8s.io").Do().Into(table)
+		Expect(err).NotTo(HaveOccurred())
+		framework.Logf("Table: %#v", table)
+
+		Expect(len(table.ColumnDefinitions)).To(BeNumerically(">", 2))
+		Expect(len(table.Rows)).To(Equal(1))
+		Expect(len(table.Rows[0].Cells)).To(Equal(len(table.ColumnDefinitions)))
+		Expect(table.ColumnDefinitions[0].Name).To(Equal("Name"))
+		Expect(table.Rows[0].Cells[0]).To(Equal(podName))
+
+		out := printTable(table)
+		Expect(out).To(MatchRegexp("^NAME\\s"))
+		Expect(out).To(MatchRegexp("\npod-1\\s"))
+		framework.Logf("Table:\n%s", out)
+	})
+
+	It("should return generic metadata details across all namespaces for nodes", func() {
+		c := f.ClientSet
+
+		table := &metav1alpha1.Table{}
+		err := c.Core().RESTClient().Get().Resource("nodes").SetHeader("Accept", "application/json;as=Table;v=v1alpha1;g=meta.k8s.io").Do().Into(table)
+		Expect(err).NotTo(HaveOccurred())
+		framework.Logf("Table: %#v", table)
+
+		Expect(len(table.ColumnDefinitions)).To(BeNumerically(">=", 2))
+		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"))
+
+		out := printTable(table)
+		Expect(out).To(MatchRegexp("^NAME\\s"))
+		framework.Logf("Table:\n%s", out)
+	})
+
+	It("should return a 406 for a backend which does not implement metadata", func() {
+		c := f.ClientSet
+
+		table := &metav1alpha1.Table{}
+		err := c.Core().RESTClient().Get().Resource("services").SetHeader("Accept", "application/json;as=Table;v=v1alpha1;g=meta.k8s.io").Do().Into(table)
+		Expect(err).To(HaveOccurred())
+		Expect(err.(errors.APIStatus).Status().Code).To(Equal(int32(406)))
+	})
+})
+
+func printTable(table *metav1alpha1.Table) string {
+	buf := &bytes.Buffer{}
+	tw := tabwriter.NewWriter(buf, 5, 8, 1, ' ', 0)
+	err := printers.PrintTable(table, tw, printers.PrintOptions{})
+	Expect(err).NotTo(HaveOccurred())
+	tw.Flush()
+	return buf.String()
+}
+
+func newPod(podName string) *v1.Pod {
+	containerName := fmt.Sprintf("%s-container", podName)
+	port := 8080
+	pod := &v1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: podName,
+		},
+		Spec: v1.PodSpec{
+			Containers: []v1.Container{
+				{
+					Name:  containerName,
+					Image: "gcr.io/google_containers/porter:4524579c0eb935c056c8e75563b4e1eda31587e0",
+					Env:   []v1.EnvVar{{Name: fmt.Sprintf("SERVE_PORT_%d", port), Value: "foo"}},
+					Ports: []v1.ContainerPort{{ContainerPort: int32(port)}},
+				},
+			},
+			RestartPolicy: v1.RestartPolicyNever,
+		},
+	}
+	return pod
+}

test/e2e/e2e_test.go

@@ -19,6 +19,7 @@ package e2e
 import (
 	"testing"
 
+	_ "k8s.io/kubernetes/test/e2e/api"
 	_ "k8s.io/kubernetes/test/e2e/autoscaling"
 	_ "k8s.io/kubernetes/test/e2e/cluster-logging"
 	_ "k8s.io/kubernetes/test/e2e/extension"