Merge pull request #60307 from deads2k/cli-09-rebreak-filter

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. rebreak the filter Pull https://github.com/kubernetes/kubernetes/pull/60117/commits fixed a bug in the filtering code which was actually being exploited to get inconsistent printing behavior. This reverts the commit that "fixed" the inconsistency and adjusts the test back to the equivalent, pre-printing fixes. /assign @soltysh ```release-note NONE ```

Merge pull request #60307 from deads2k/cli-09-rebreak-filter
a6797824 · Kubernetes Submit Queue · GitHub · 58997133 · f9a884d3 · a6797824
Unverified Commit a6797824 authored Feb 26, 2018 by Kubernetes Submit Queue Committed by GitHub Feb 26, 2018
Hide whitespace changes
Inline Side-by-side

Showing with 25 additions and 33 deletions

get_test.go pkg/kubectl/cmd/resource/get_test.go +25 -10

resource_filter.go pkg/kubectl/resource_filter.go +0 -23

No files found.
--- a/pkg/kubectl/cmd/resource/get_test.go
+++ b/pkg/kubectl/cmd/resource/get_test.go
@@ -386,16 +386,32 @@ func TestGetObjectsFiltered(t *testing.T) {
 		flags  map[string]string
 		expect string
 	}{
-		{args: []string{"pods", "foo"}, flags: map[string]string{"show-all": "true"}, resp: first, expect: "pod/foo\n"},
-		{args: []string{"pods", "foo"}, resp: first, expect: "pod/foo\n"},
-		{args: []string{"pods"}, resp: pods, expect: "pod/foo\npod/bar\n"},
-		{args: []string{"pods/foo"}, flags: map[string]string{"show-all": "false"}, resp: first, expect: "pod/foo\n"},
-		{args: []string{"pods"}, flags: map[string]string{"show-all": "false", "output": "yaml"}, resp: pods, expect: "pod/bar\n"},
-		{args: []string{}, flags: map[string]string{"show-all": "false", "filename": "../../../../examples/storage/cassandra/cassandra-controller.yaml"}, resp: pods, expect: "pod/foo\npod/bar\n"},
+		{args: []string{"pods", "foo"}, resp: first, flags: map[string]string{"show-all": "true"},
+			expect: "NAME      READY     STATUS    RESTARTS   AGE\nfoo       0/0       Failed    0          <unknown>\n"},

-		{args: []string{"pods"}, flags: map[string]string{"show-all": "false"}, resp: pods, expect: "pod/bar\n"},
-		{args: []string{"pods"}, flags: map[string]string{"show-all": "true", "output": "yaml"}, resp: pods, expect: "pod/foo\npod/bar\n"},
-		{args: []string{"pods"}, flags: map[string]string{"show-all": "false"}, resp: pods, expect: "pod/bar\n"},
+		{args: []string{"pods", "foo"}, flags: map[string]string{"show-all": "false"}, resp: first,
+			expect: "NAME      READY     STATUS    RESTARTS   AGE\nfoo       0/0       Failed    0          <unknown>\n"},
+
+		{args: []string{"pods"}, flags: map[string]string{"show-all": "true"}, resp: pods,
+			expect: "NAME      READY     STATUS    RESTARTS   AGE\nfoo       0/0       Failed    0          <unknown>\nbar       0/0                 0          <unknown>\n"},
+
+		{args: []string{"pods/foo"}, resp: first, flags: map[string]string{"show-all": "false"},
+			expect: "NAME      READY     STATUS    RESTARTS   AGE\nfoo       0/0       Failed    0          <unknown>\n"},
+
+		{args: []string{"pods"}, flags: map[string]string{"show-all": "false", "output": "name"}, resp: pods,
+			expect: "pod/foo\npod/bar\n"},
+
+		{args: []string{}, flags: map[string]string{"show-all": "false", "filename": "../../../../examples/storage/cassandra/cassandra-controller.yaml"}, resp: pods,
+			expect: "NAME      READY     STATUS    RESTARTS   AGE\nfoo       0/0       Failed    0          <unknown>\nbar       0/0                 0          <unknown>\n"},
+
+		{args: []string{"pods"}, resp: pods, flags: map[string]string{"show-all": "false"},
+			expect: "NAME      READY     STATUS    RESTARTS   AGE\nbar       0/0                 0          <unknown>\n"},
+
+		{args: []string{"pods"}, flags: map[string]string{"show-all": "true", "output": "name"}, resp: pods,
+			expect: "pod/foo\npod/bar\n"},
+
+		{args: []string{"pods"}, flags: map[string]string{"show-all": "false"}, resp: pods,
+			expect: "NAME      READY     STATUS    RESTARTS   AGE\nbar       0/0                 0          <unknown>\n"},
 	}

 	for i, test := range testCases {
@@ -417,7 +433,6 @@ func TestGetObjectsFiltered(t *testing.T) {
 			for k, v := range test.flags {
 				cmd.Flags().Lookup(k).Value.Set(v)
 			}
-			cmd.Flags().Set("output", "name")
 			cmd.Run(cmd, test.args)

 			if e, a := test.expect, buf.String(); e != a {

--- a/pkg/kubectl/resource_filter.go
+++ b/pkg/kubectl/resource_filter.go
@@ -18,9 +18,7 @@ package kubectl

 import (
 	"k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/printers"
 )
@@ -52,27 +50,6 @@ func filterPods(obj runtime.Object, options printers.PrintOptions) bool {
 		return p.Status.Phase == v1.PodSucceeded || p.Status.Phase == v1.PodFailed
 	case *api.Pod:
 		return p.Status.Phase == api.PodSucceeded || p.Status.Phase == api.PodFailed
-	case *unstructured.Unstructured:
-		if (p.GroupVersionKind().GroupKind() != schema.GroupKind{Kind: "Pod"}) {
-			return false
-		}
-		status, ok := p.Object["status"]
-		if !ok {
-			return false
-		}
-		statusMap, ok := status.(map[string]interface{})
-		if !ok {
-			return false
-		}
-		phase, ok := statusMap["phase"]
-		if !ok {
-			return false
-		}
-		phaseString, ok := phase.(string)
-		if !ok {
-			return false
-		}
-		return phaseString == string(api.PodSucceeded) || phaseString == string(api.PodFailed)
 	}
 	return false
 }