Merge pull request #25022 from zhouhaibing089/sort-fix

Automatic merge from submit-queue

Support struct,array,slice types when sorting kubectl output

Fixes https://github.com/kubernetes/kubernetes/issues/24328.

Briefly, `sorting_printer` only take cares of the following type kinds:

*   `reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64`
*   `reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64`
*   `reflect.Float32, reflect.Float64`
*   `reflect.String`
*   `reflect.Ptr`

This commit aims to add `reflect.Struct, reflect.Slice, reflect.Array`.

/cc @bgrant0607

hack/test-cmd.sh

@@ -2049,6 +2049,7 @@ __EOF__
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
   # Command
   kubectl get pods --sort-by="{metadata.name}"
+  kubectl get pods --sort-by="{metadata.creationTimestamp}"
 
   ############################
   # Kubectl --all-namespaces #

pkg/kubectl/sorting_printer.go

@@ -23,8 +23,10 @@ import (
 	"sort"
 
 	"k8s.io/kubernetes/pkg/api/meta"
+	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/runtime"
+	"k8s.io/kubernetes/pkg/util/integer"
 	"k8s.io/kubernetes/pkg/util/jsonpath"
 
 	"github.com/golang/glog"
@@ -153,11 +155,56 @@ func isLess(i, j reflect.Value) (bool, error) {
 		return i.String() < j.String(), nil
 	case reflect.Ptr:
 		return isLess(i.Elem(), j.Elem())
+	case reflect.Struct:
+		// special case handling
+		lessFuncList := []structLessFunc{timeLess}
+		if ok, less := structLess(i, j, lessFuncList); ok {
+			return less, nil
+		}
+		// fallback to the fields comparision
+		for idx := 0; idx < i.NumField(); idx++ {
+			less, err := isLess(i.Field(idx), j.Field(idx))
+			if err != nil || !less {
+				return less, err
+			}
+		}
+		return true, nil
+	case reflect.Array, reflect.Slice:
+		// note: the length of i and j may be different
+		for idx := 0; idx < integer.IntMin(i.Len(), j.Len()); idx++ {
+			less, err := isLess(i.Index(idx), j.Index(idx))
+			if err != nil || !less {
+				return less, err
+			}
+		}
+		return true, nil
 	default:
 		return false, fmt.Errorf("unsortable type: %v", i.Kind())
 	}
 }
 
+// structLessFunc checks whether i and j could be compared(the first return value),
+// and if it could, return whether i is less than j(the second return value)
+type structLessFunc func(i, j reflect.Value) (bool, bool)
+
+// structLess returns whether i and j could be compared with the given function list
+func structLess(i, j reflect.Value, lessFuncList []structLessFunc) (bool, bool) {
+	for _, lessFunc := range lessFuncList {
+		if ok, less := lessFunc(i, j); ok {
+			return ok, less
+		}
+	}
+	return false, false
+}
+
+// compare two unversioned.Time values.
+func timeLess(i, j reflect.Value) (bool, bool) {
+	if i.Type() != reflect.TypeOf(unversioned.Unix(0, 0)) {
+		return false, false
+	}
+	return true, i.MethodByName("Before").Call([]reflect.Value{j})[0].Bool()
+}
+
 func (r *RuntimeSort) Less(i, j int) bool {
 	iObj := r.objs[i]
 	jObj := r.objs[j]

pkg/kubectl/sorting_printer_test.go

@@ -21,6 +21,7 @@ import (
 	"testing"
 
 	internal "k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/unversioned"
 	api "k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/runtime"
 )
@@ -144,6 +145,48 @@ func TestSortingPrinter(t *testing.T) {
 			},
 			field: "{.metadata.name}",
 		},
+		{
+			name: "random-order-timestamp",
+			obj: &api.PodList{
+				Items: []api.Pod{
+					{
+						ObjectMeta: api.ObjectMeta{
+							CreationTimestamp: unversioned.Unix(300, 0),
+						},
+					},
+					{
+						ObjectMeta: api.ObjectMeta{
+							CreationTimestamp: unversioned.Unix(100, 0),
+						},
+					},
+					{
+						ObjectMeta: api.ObjectMeta{
+							CreationTimestamp: unversioned.Unix(200, 0),
+						},
+					},
+				},
+			},
+			sort: &api.PodList{
+				Items: []api.Pod{
+					{
+						ObjectMeta: api.ObjectMeta{
+							CreationTimestamp: unversioned.Unix(100, 0),
+						},
+					},
+					{
+						ObjectMeta: api.ObjectMeta{
+							CreationTimestamp: unversioned.Unix(200, 0),
+						},
+					},
+					{
+						ObjectMeta: api.ObjectMeta{
+							CreationTimestamp: unversioned.Unix(300, 0),
+						},
+					},
+				},
+			},
+			field: "{.metadata.creationTimestamp}",
+		},
 		{
 			name: "random-order-numbers",
 			obj: &api.ReplicationControllerList{