From f1afc3d09d58fc9649b4dc9d8d3a5f46a9480cbd Mon Sep 17 00:00:00 2001
From: Shiyang Wang <shiywang@redhat.com>
Date: Sat, 8 Jul 2017 13:56:02 +0800
Subject: [PATCH] fix sort-by output problem

---
 hack/make-rules/test-cmd-util.sh        | 20 ++++++++++++++++++++
 pkg/kubectl/cmd/util/factory_builder.go | 11 -----------
 pkg/kubectl/cmd/util/printing.go        |  8 ++++++++
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh
index 639ceffe5cd..33befe35c28 100644
--- a/hack/make-rules/test-cmd-util.sh
+++ b/hack/make-rules/test-cmd-util.sh
@@ -3913,6 +3913,26 @@ run_kubectl_sort_by_tests() {
   kubectl get pods --sort-by="{metadata.name}"
   kubectl get pods --sort-by="{metadata.creationTimestamp}"
 
+  ### sort-by should works if pod exists
+  # Create POD
+  # Pre-condition: no POD exists
+  kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
+  # Command
+  kubectl create "${kube_flags[@]}" -f test/fixtures/doc-yaml/admin/limitrange/valid-pod.yaml
+  # Post-condition: valid-pod is created
+  kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:'
+  # Check output of sort-by
+  output_message=$(kubectl get pods --sort-by="{metadata.name}")
+  kube::test::if_has_string "${output_message}" "valid-pod"
+  ### Clean up
+  # Pre-condition: valid-pod exists
+  kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:'
+  # Command
+  kubectl delete "${kube_flags[@]}" pod valid-pod --grace-period=0 --force
+  # Post-condition: valid-pod doesn't exist
+  kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
+
+
   set +o nounset
   set +o errexit
 }
diff --git a/pkg/kubectl/cmd/util/factory_builder.go b/pkg/kubectl/cmd/util/factory_builder.go
index 52967300ca2..e3a270fee30 100644
--- a/pkg/kubectl/cmd/util/factory_builder.go
+++ b/pkg/kubectl/cmd/util/factory_builder.go
@@ -32,7 +32,6 @@ import (
 	"k8s.io/kubernetes/pkg/kubectl/plugins"
 	"k8s.io/kubernetes/pkg/kubectl/resource"
 	"k8s.io/kubernetes/pkg/printers"
-	printersinternal "k8s.io/kubernetes/pkg/printers/internalversion"
 )
 
 type ring2Factory struct {
@@ -103,16 +102,6 @@ func (f *ring2Factory) PrinterForMapping(cmd *cobra.Command, isLocal bool, outpu
 
 		printer = printers.NewVersionedPrinter(printer, mapping.ObjectConvertor, version, mapping.GroupVersionKind.GroupVersion())
 
-	} else {
-		// We add handlers to the printer in case it is printers.HumanReadablePrinter.
-		// printers.AddHandlers expects concrete type of printers.HumanReadablePrinter
-		// as its parameter because of this we have to do a type check on printer and
-		// extract out concrete HumanReadablePrinter from it. We are then able to attach
-		// handlers on it.
-		if humanReadablePrinter, ok := printer.(*printers.HumanReadablePrinter); ok {
-			printersinternal.AddHandlers(humanReadablePrinter)
-			printer = humanReadablePrinter
-		}
 	}
 
 	return printer, nil
diff --git a/pkg/kubectl/cmd/util/printing.go b/pkg/kubectl/cmd/util/printing.go
index b1e9b3527d5..bdff16d66ec 100644
--- a/pkg/kubectl/cmd/util/printing.go
+++ b/pkg/kubectl/cmd/util/printing.go
@@ -26,6 +26,7 @@ import (
 	"k8s.io/kubernetes/pkg/kubectl"
 	"k8s.io/kubernetes/pkg/kubectl/resource"
 	"k8s.io/kubernetes/pkg/printers"
+	printersinternal "k8s.io/kubernetes/pkg/printers/internalversion"
 
 	"github.com/spf13/cobra"
 )
@@ -126,6 +127,13 @@ func PrinterForCommand(cmd *cobra.Command, outputOpts *printers.OutputOptions, m
 		return nil, err
 	}
 
+	// we try to convert to HumanReadablePrinter, if return ok, it must be no generic
+	// we execute AddHandlers() here before maybeWrapSortingPrinter so that we don't
+	// need to convert to delegatePrinter again then invoke AddHandlers()
+	if humanReadablePrinter, ok := printer.(*printers.HumanReadablePrinter); ok {
+		printersinternal.AddHandlers(humanReadablePrinter)
+	}
+
 	return maybeWrapSortingPrinter(cmd, printer), nil
 }