Merge pull request #26686 from metral/fix-get-output

Automatic merge from submit-queue

fix recursive & non-recursive kubectl get of generic output format

This PR fixes the issues with `kubectl get` in https://github.com/kubernetes/kubernetes/issues/26466

Changes made:
- fix printing when using the generic output format in both non-recursive & recurvise settings to ensure that errors are being shown
- add tests to check printing generic output in a **non-recursive** setting with non-existent pods
- clean up the **recursive** `kubectl get` tests

/cc @janetkuo

hack/test-cmd.sh

@@ -932,6 +932,25 @@ __EOF__
   # Clean up
   kubectl delete deployment nginx "${kube_flags[@]}"
 
+  ###############
+  # Kubectl get #
+  ###############
+
+  ### Test retrieval of non-existing pods
+  # Pre-condition: no POD exists
+  kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
+  # Command
+  output_message=$(! kubectl get pods abc 2>&1 "${kube_flags[@]}")
+  # Post-condition: POD abc should error since it doesn't exist
+  kube::test::if_has_string "${output_message}" 'pods "abc" not found'
+
+  ### Test retrieval of non-existing POD with output flag specified
+  # Pre-condition: no POD exists
+  kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
+  # Command
+  output_message=$(! kubectl get pods abc 2>&1 "${kube_flags[@]}" -o name)
+  # Post-condition: POD abc should error since it doesn't exist
+  kube::test::if_has_string "${output_message}" 'pods "abc" not found'
 
   #####################################
   # Recursive Resources via directory #
@@ -997,11 +1016,10 @@ __EOF__
   # Pre-condition: busybox0 & busybox1 PODs exist
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'busybox0:busybox1:'
   # Command
-  output_message1=$(kubectl get -f hack/testdata/recursive/pod --recursive 2>&1 "${kube_flags[@]}" -o go-template="{{range.items}}{{$id_field}}:{{end}}")
+  output_message=$(! kubectl get -f hack/testdata/recursive/pod --recursive 2>&1 "${kube_flags[@]}" -o go-template="{{range.items}}{{$id_field}}:{{end}}")
-  output_message2=$(! kubectl get -f hack/testdata/recursive/pod --recursive 2>&1 "${kube_flags[@]}")
   # Post-condition: busybox0 & busybox1 PODs are retrieved, but because busybox2 is malformed, it should not show up
-  kube::test::if_has_string "${output_message1}" "busybox0:busybox1:"
+  kube::test::if_has_string "${output_message}" "busybox0:busybox1:"
-  kube::test::if_has_string "${output_message2}" "Object 'Kind' is missing"
+  kube::test::if_has_string "${output_message}" "Object 'Kind' is missing"
 
   ## Label multiple busybox PODs recursively from directory of YAML files
   # Pre-condition: busybox0 & busybox1 PODs exist

pkg/kubectl/cmd/get.go

@@ -219,20 +219,18 @@ func RunGet(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string
 		return err
 	}
 
-	allErrs := []error{}
-	infos, err := r.Infos()
-	if err != nil {
-		allErrs = append(allErrs, err)
-	}
-
 	if generic {
 		clientConfig, err := f.ClientConfig()
 		if err != nil {
 			return err
 		}
 
+		allErrs := []error{}
 		singular := false
-		r.IntoSingular(&singular)
+		infos, err := r.IntoSingular(&singular).Infos()
+		if err != nil {
+			allErrs = append(allErrs, err)
+		}
 
 		// the outermost object will be converted to the output-version, but inner
 		// objects can use their mappings
@@ -240,12 +238,22 @@ func RunGet(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string
 		if err != nil {
 			return err
 		}
+
 		obj, err := resource.AsVersionedObject(infos, !singular, version, f.JSONEncoder())
 		if err != nil {
 			return err
 		}
 
-		return printer.PrintObj(obj, out)
+		if err := printer.PrintObj(obj, out); err != nil {
+			allErrs = append(allErrs, err)
+		}
+		return utilerrors.NewAggregate(allErrs)
+	}
+
+	allErrs := []error{}
+	infos, err := r.Infos()
+	if err != nil {
+		allErrs = append(allErrs, err)
 	}
 
 	objs := make([]runtime.Object, len(infos))
@@ -315,5 +323,5 @@ func RunGet(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string
 			continue
 		}
 	}
-	return utilerrors.Flatten(utilerrors.NewAggregate(allErrs))
+	return utilerrors.NewAggregate(allErrs)
 }

pkg/kubectl/resource/builder.go

@@ -71,6 +71,8 @@ type Builder struct {
 	singleResourceType bool
 	continueOnError    bool
 
+	singular bool
+
 	export bool
 
 	schema validation.Schema
@@ -113,6 +115,9 @@ func (b *Builder) FilenameParam(enforceNamespace, recursive bool, paths ...strin
 			}
 			b.URL(defaultHttpGetAttempts, url)
 		default:
+			if !recursive {
+				b.singular = true
+			}
 			b.Path(recursive, s)
 		}
 	}
@@ -546,7 +551,12 @@ func (b *Builder) visitorResult() *Result {
 
 	// visit items specified by resource and name
 	if len(b.resourceTuples) != 0 {
-		isSingular := len(b.resourceTuples) == 1
+		// if b.singular is false, this could be by default, so double-check length
+		// of resourceTuples to determine if in fact it is singular or not
+		isSingular := b.singular
+		if !isSingular {
+			isSingular = len(b.resourceTuples) == 1
+		}
 
 		if len(b.paths) != 0 {
 			return &Result{singular: isSingular, err: fmt.Errorf("when paths, URLs, or stdin is provided as input, you may not specify a resource by arguments as well")}