Merge pull request #65434 from yue9944882/bugfix-show-kind-for-crd

Automatic merge from submit-queue (batch tested with PRs 64575, 65120, 65463, 65434, 65522). 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>.

Set flag show-kind when getting multiple types

**What this PR does / why we need it**:

Set "--show-kind" flag if requesting multiple resource types.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #65375 

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

pkg/kubectl/cmd/get/BUILD

@@ -77,7 +77,12 @@ go_test(
         "//pkg/kubectl/genericclioptions:go_default_library",
         "//pkg/kubectl/genericclioptions/resource:go_default_library",
         "//pkg/kubectl/scheme:go_default_library",
+        "//staging/src/k8s.io/api/apps/v1:go_default_library",
+        "//staging/src/k8s.io/api/autoscaling/v1:go_default_library",
+        "//staging/src/k8s.io/api/batch/v1:go_default_library",
+        "//staging/src/k8s.io/api/batch/v1beta1:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
+        "//staging/src/k8s.io/api/extensions/v1beta1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",

pkg/kubectl/cmd/get/get.go

@@ -51,7 +51,7 @@ import (
 // GetOptions contains the input to the get command.
 type GetOptions struct {
 	PrintFlags             *PrintFlags
-	ToPrinter              func(*meta.RESTMapping, bool) (printers.ResourcePrinterFunc, error)
+	ToPrinter              func(*meta.RESTMapping, bool, bool) (printers.ResourcePrinterFunc, error)
 	IsHumanReadablePrinter bool
 	PrintWithOpenAPICols   bool
 
@@ -224,10 +224,7 @@ func (o *GetOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []stri
 
 	o.IncludeUninitialized = cmdutil.ShouldIncludeUninitialized(cmd, false)
 
-	if resource.MultipleTypesRequested(args) {
+	o.ToPrinter = func(mapping *meta.RESTMapping, withNamespace bool, withKind bool) (printers.ResourcePrinterFunc, error) {
-		o.PrintFlags.EnsureWithKind()
-	}
-	o.ToPrinter = func(mapping *meta.RESTMapping, withNamespace bool) (printers.ResourcePrinterFunc, error) {
 		// make a new copy of current flags / opts before mutating
 		printFlags := o.PrintFlags.Copy()
 
@@ -242,6 +239,9 @@ func (o *GetOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []stri
 		if withNamespace {
 			printFlags.EnsureWithNamespace()
 		}
+		if withKind {
+			printFlags.EnsureWithKind()
+		}
 
 		printer, err := printFlags.ToPrinter()
 		if err != nil {
@@ -345,6 +345,7 @@ func (o *GetOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e
 	if err != nil {
 		allErrs = append(allErrs, err)
 	}
+	printWithKind := multipleGVKsRequested(infos)
 
 	objs := make([]runtime.Object, len(infos))
 	for ix := range infos {
@@ -400,6 +401,7 @@ func (o *GetOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e
 		nonEmptyObjCount++
 
 		printWithNamespace := o.AllNamespaces
+
 		if mapping != nil && mapping.Scope.Name() == meta.RESTScopeNameRoot {
 			printWithNamespace = false
 		}
@@ -414,7 +416,7 @@ func (o *GetOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e
 				fmt.Fprintln(o.ErrOut)
 			}
 
-			printer, err = o.ToPrinter(mapping, printWithNamespace)
+			printer, err = o.ToPrinter(mapping, printWithNamespace, printWithKind)
 			if err != nil {
 				if !errs.Has(err.Error()) {
 					errs.Insert(err.Error())
@@ -493,30 +495,13 @@ func (o *GetOptions) watch(f cmdutil.Factory, cmd *cobra.Command, args []string)
 	if err != nil {
 		return err
 	}
-	if len(infos) > 1 {
+	if multipleGVKsRequested(infos) {
-		gvk := infos[0].Mapping.GroupVersionKind
+		return i18n.Errorf("watch is only supported on individual resources and resource collections - more than 1 resources were found")
-		uniqueGVKs := 1
-
-		// If requesting a resource count greater than a request's --chunk-size,
-		// we will end up making multiple requests to the server, with each
-		// request producing its own "Info" object. Although overall we are
-		// dealing with a single resource type, we will end up with multiple
-		// infos returned by the builder. To handle this case, only fail if we
-		// have at least one info with a different GVK than the others.
-		for _, info := range infos {
-			if info.Mapping.GroupVersionKind != gvk {
-				uniqueGVKs++
-			}
-		}
-
-		if uniqueGVKs > 1 {
-			return i18n.Errorf("watch is only supported on individual resources and resource collections - %d resources were found", uniqueGVKs)
-		}
 	}
 
 	info := infos[0]
 	mapping := info.ResourceMapping()
-	printer, err := o.ToPrinter(mapping, o.AllNamespaces)
+	printer, err := o.ToPrinter(mapping, o.AllNamespaces, false)
 	if err != nil {
 		return err
 	}
@@ -650,7 +635,7 @@ func (o *GetOptions) printGeneric(r *resource.Result) error {
 		return utilerrors.Reduce(utilerrors.Flatten(utilerrors.NewAggregate(errs)))
 	}
 
-	printer, err := o.ToPrinter(nil, false)
+	printer, err := o.ToPrinter(nil, false, false)
 	if err != nil {
 		return err
 	}
@@ -750,3 +735,16 @@ func maybeWrapSortingPrinter(printer printers.ResourcePrinter, sortBy string) pr
 	}
 	return printer
 }
+
+func multipleGVKsRequested(infos []*resource.Info) bool {
+	if len(infos) < 2 {
+		return false
+	}
+	gvk := infos[0].Mapping.GroupVersionKind
+	for _, info := range infos {
+		if info.Mapping.GroupVersionKind != gvk {
+			return true
+		}
+	}
+	return false
+}

pkg/kubectl/cmd/get/get_test.go

@@ -27,7 +27,12 @@ import (
 	"strings"
 	"testing"
 
+	apiapps "k8s.io/api/apps/v1"
+	apiautoscaling "k8s.io/api/autoscaling/v1"
+	apibatchv1 "k8s.io/api/batch/v1"
+	apibatchv1beta1 "k8s.io/api/batch/v1beta1"
 	api "k8s.io/api/core/v1"
+	apiextensionsv1beta1 "k8s.io/api/extensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -351,6 +356,65 @@ pod/foo   0/0                 0          <unknown>
 	}
 }
 
+func TestGetMultipleResourceTypesShowKinds(t *testing.T) {
+	pods, svcs, _ := testData()
+
+	tf := cmdtesting.NewTestFactory().WithNamespace("test")
+	defer tf.Cleanup()
+	codec := legacyscheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...)
+
+	tf.UnstructuredClient = &fake.RESTClient{
+		NegotiatedSerializer: unstructuredSerializer,
+		Resp:                 &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &pods.Items[0])},
+	}
+	tf.UnstructuredClient = &fake.RESTClient{
+		NegotiatedSerializer: unstructuredSerializer,
+		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
+			switch p, m := req.URL.Path, req.Method; {
+			case p == "/namespaces/test/pods" && m == "GET":
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, pods)}, nil
+			case p == "/namespaces/test/replicationcontrollers" && m == "GET":
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &api.ReplicationControllerList{})}, nil
+			case p == "/namespaces/test/services" && m == "GET":
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, svcs)}, nil
+			case p == "/namespaces/test/statefulsets" && m == "GET":
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &apiapps.StatefulSetList{})}, nil
+			case p == "/namespaces/test/horizontalpodautoscalers" && m == "GET":
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &apiautoscaling.HorizontalPodAutoscalerList{})}, nil
+			case p == "/namespaces/test/jobs" && m == "GET":
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &apibatchv1.JobList{})}, nil
+			case p == "/namespaces/test/cronjobs" && m == "GET":
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &apibatchv1beta1.CronJobList{})}, nil
+			case p == "/namespaces/test/daemonsets" && m == "GET":
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &apiapps.DaemonSetList{})}, nil
+			case p == "/namespaces/test/deployments" && m == "GET":
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &apiextensionsv1beta1.DeploymentList{})}, nil
+			case p == "/namespaces/test/replicasets" && m == "GET":
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &apiextensionsv1beta1.ReplicaSetList{})}, nil
+
+			default:
+				t.Fatalf("request url: %#v,and request: %#v", req.URL, req)
+				return nil, nil
+			}
+		}),
+	}
+
+	streams, _, buf, _ := genericclioptions.NewTestIOStreams()
+	cmd := NewCmdGet("kubectl", tf, streams)
+	cmd.SetOutput(buf)
+	cmd.Run(cmd, []string{"all"})
+
+	expected := `NAME      READY     STATUS    RESTARTS   AGE
+pod/foo   0/0                 0          <unknown>
+pod/bar   0/0                 0          <unknown>
+NAME          TYPE        CLUSTER-IP   EXTERNAL-IP   PORT(S)   AGE
+service/baz   ClusterIP   <none>       <none>        <none>    <unknown>
+`
+	if e, a := expected, buf.String(); e != a {
+		t.Errorf("expected %v, got %v", e, a)
+	}
+}
+
 func TestGetObjectsShowLabels(t *testing.T) {
 	pods, _, _ := testData()
 

pkg/kubectl/cmd/testing/fake.go

@@ -501,6 +501,24 @@ func testDynamicResources() []*restmapper.APIGroupResources {
 				},
 			},
 		},
+		{
+			Group: metav1.APIGroup{
+				Name: "batch",
+				Versions: []metav1.GroupVersionForDiscovery{
+					{Version: "v1beta1"},
+					{Version: "v1"},
+				},
+				PreferredVersion: metav1.GroupVersionForDiscovery{Version: "v1"},
+			},
+			VersionedResources: map[string][]metav1.APIResource{
+				"v1beta1": {
+					{Name: "cronjobs", Namespaced: true, Kind: "CronJob"},
+				},
+				"v1": {
+					{Name: "jobs", Namespaced: true, Kind: "Job"},
+				},
+			},
+		},
 		{
 			Group: metav1.APIGroup{
 				Name: "autoscaling",

pkg/kubectl/genericclioptions/resource/builder.go

@@ -156,6 +156,8 @@ func newBuilder(clientConfigFn ClientConfigFunc, restMapper meta.RESTMapper, cat
 		restMapper:       restMapper,
 		categoryExpander: categoryExpander,
 		requireObject:    true,
+		labelSelector:    nil,
+		fieldSelector:    nil,
 	}
 }
 
@@ -1107,37 +1109,3 @@ func HasNames(args []string) (bool, error) {
 	}
 	return hasCombinedTypes || len(args) > 1, nil
 }
-
-// MultipleTypesRequested returns true if the provided args contain multiple resource kinds
-func MultipleTypesRequested(args []string) bool {
-	if len(args) == 1 && args[0] == "all" {
-		return true
-	}
-
-	args = normalizeMultipleResourcesArgs(args)
-	rKinds := sets.NewString()
-	for _, arg := range args {
-		rTuple, found, err := splitResourceTypeName(arg)
-		if err != nil {
-			continue
-		}
-
-		// if tuple not found, assume arg is of the form "type1,type2,...".
-		// Since SplitResourceArgument returns a unique list of kinds,
-		// return true here if len(uniqueList) > 1
-		if !found {
-			if strings.Contains(arg, ",") {
-				splitArgs := SplitResourceArgument(arg)
-				if len(splitArgs) > 1 {
-					return true
-				}
-			}
-			continue
-		}
-		if rKinds.Has(rTuple.Resource) {
-			continue
-		}
-		rKinds.Insert(rTuple.Resource)
-	}
-	return rKinds.Len() > 1
-}

pkg/kubectl/genericclioptions/resource/builder_test.go

@@ -1396,80 +1396,3 @@ func TestHasNames(t *testing.T) {
 		})
 	}
 }
-
-func TestMultipleTypesRequested(t *testing.T) {
-	tests := []struct {
-		name                  string
-		args                  []string
-		expectedMultipleTypes bool
-	}{
-		{
-			name: "test1",
-			args: []string{""},
-			expectedMultipleTypes: false,
-		},
-		{
-			name: "test2",
-			args: []string{"all"},
-			expectedMultipleTypes: true,
-		},
-		{
-			name: "test3",
-			args: []string{"rc"},
-			expectedMultipleTypes: false,
-		},
-		{
-			name: "test4",
-			args: []string{"pod,all"},
-			expectedMultipleTypes: true,
-		},
-		{
-			name: "test5",
-			args: []string{"all,rc,pod"},
-			expectedMultipleTypes: true,
-		},
-		{
-			name: "test6",
-			args: []string{"rc,pod,svc"},
-			expectedMultipleTypes: true,
-		},
-		{
-			name: "test7",
-			args: []string{"rc/foo"},
-			expectedMultipleTypes: false,
-		},
-		{
-			name: "test8",
-			args: []string{"rc/foo", "rc/bar"},
-			expectedMultipleTypes: false,
-		},
-		{
-			name: "test9",
-			args: []string{"rc", "foo"},
-			expectedMultipleTypes: false,
-		},
-		{
-			name: "test10",
-			args: []string{"rc,pod,svc", "foo"},
-			expectedMultipleTypes: true,
-		},
-		{
-			name: "test11",
-			args: []string{"rc,secrets"},
-			expectedMultipleTypes: true,
-		},
-		{
-			name: "test12",
-			args: []string{"rc/foo", "rc/bar", "svc/svc"},
-			expectedMultipleTypes: true,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			hasMultipleTypes := MultipleTypesRequested(tt.args)
-			if hasMultipleTypes != tt.expectedMultipleTypes {
-				t.Errorf("expected MultipleTypesRequested to return %v for %s", tt.expectedMultipleTypes, tt.args)
-			}
-		})
-	}
-}