Merge pull request #71820 from liggitt/automated-cherry-pick-of-#71805-upstream-release-1.13

Automated cherry pick of #71805: Fix sort-by regression

pkg/kubectl/cmd/get/get.go

@@ -334,8 +334,10 @@ func (r *RuntimeSorter) Sort() error {
 		case *metav1beta1.Table:
 			includesTable = true
 
-			if err := NewTableSorter(t, r.field).Sort(); err != nil {
-				continue
+			if sorter, err := NewTableSorter(t, r.field); err != nil {
+				return err
+			} else if err := sorter.Sort(); err != nil {
+				return err
 			}
 		default:
 			includesRuntimeObjs = true

pkg/kubectl/cmd/get/sorter.go

@@ -318,13 +318,18 @@ func (t *TableSorter) Len() int {
 
 func (t *TableSorter) Swap(i, j int) {
 	t.obj.Rows[i], t.obj.Rows[j] = t.obj.Rows[j], t.obj.Rows[i]
+	t.parsedRows[i], t.parsedRows[j] = t.parsedRows[j], t.parsedRows[i]
 }
 
 func (t *TableSorter) Less(i, j int) bool {
 	iValues := t.parsedRows[i]
 	jValues := t.parsedRows[j]
-	if len(iValues) == 0 || len(iValues[0]) == 0 || len(jValues) == 0 || len(jValues[0]) == 0 {
-		klog.Fatalf("couldn't find any field with path %q in the list of objects", t.field)
+
+	if len(iValues) == 0 || len(iValues[0]) == 0 {
+		return true
+	}
+	if len(jValues) == 0 || len(jValues[0]) == 0 {
+		return false
 	}
 
 	iField := iValues[0][0]
@@ -342,28 +347,36 @@ func (t *TableSorter) Sort() error {
 	return nil
 }
 
-func NewTableSorter(table *metav1beta1.Table, field string) *TableSorter {
+func NewTableSorter(table *metav1beta1.Table, field string) (*TableSorter, error) {
 	var parsedRows [][][]reflect.Value
 
 	parser := jsonpath.New("sorting").AllowMissingKeys(true)
 	err := parser.Parse(field)
 	if err != nil {
-		klog.Fatalf("sorting error: %v\n", err)
+		return nil, fmt.Errorf("sorting error: %v", err)
 	}
 
+	fieldFoundOnce := false
 	for i := range table.Rows {
 		parsedRow, err := findJSONPathResults(parser, table.Rows[i].Object.Object)
 		if err != nil {
-			klog.Fatalf("Failed to get values for %#v using %s (%#v)", parsedRow, field, err)
+			return nil, fmt.Errorf("Failed to get values for %#v using %s (%#v)", parsedRow, field, err)
 		}
 		parsedRows = append(parsedRows, parsedRow)
+		if len(parsedRow) > 0 && len(parsedRow[0]) > 0 {
+			fieldFoundOnce = true
+		}
+	}
+
+	if len(table.Rows) > 0 && !fieldFoundOnce {
+		return nil, fmt.Errorf("couldn't find any field with path %q in the list of objects", field)
 	}
 
 	return &TableSorter{
 		obj:        table,
 		field:      field,
 		parsedRows: parsedRows,
-	}
+	}, nil
 }
 func findJSONPathResults(parser *jsonpath.JSONPath, from runtime.Object) ([][]reflect.Value, error) {
 	if unstructuredObj, ok := from.(*unstructured.Unstructured); ok {

pkg/kubectl/cmd/get/sorter_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package get
 
 import (
+	"encoding/json"
 	"reflect"
 	"strings"
 	"testing"
@@ -25,10 +26,20 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/kubernetes/pkg/kubectl/scheme"
 )
 
+func toUnstructuredOrDie(data []byte) *unstructured.Unstructured {
+	unstrBody := map[string]interface{}{}
+	err := json.Unmarshal(data, &unstrBody)
+	if err != nil {
+		panic(err)
+	}
+	return &unstructured.Unstructured{Object: unstrBody}
+}
 func encodeOrDie(obj runtime.Object) []byte {
 	data, err := runtime.Encode(scheme.Codecs.LegacyCodec(corev1.SchemeGroupVersion), obj)
 	if err != nil {
@@ -65,6 +76,16 @@ func TestSortingPrinter(t *testing.T) {
 		name        string
 		expectedErr string
 	}{
+		{
+			name: "empty",
+			obj: &corev1.PodList{
+				Items: []corev1.Pod{},
+			},
+			sort: &corev1.PodList{
+				Items: []corev1.Pod{},
+			},
+			field: "{.metadata.name}",
+		},
 		{
 			name: "in-order-already",
 			obj: &corev1.PodList{
@@ -237,16 +258,16 @@ func TestSortingPrinter(t *testing.T) {
 			name: "v1.List in order",
 			obj: &corev1.List{
 				Items: []runtime.RawExtension{
-					{Raw: encodeOrDie(a)},
-					{Raw: encodeOrDie(b)},
-					{Raw: encodeOrDie(c)},
+					{Object: a, Raw: encodeOrDie(a)},
+					{Object: b, Raw: encodeOrDie(b)},
+					{Object: c, Raw: encodeOrDie(c)},
 				},
 			},
 			sort: &corev1.List{
 				Items: []runtime.RawExtension{
-					{Raw: encodeOrDie(a)},
-					{Raw: encodeOrDie(b)},
-					{Raw: encodeOrDie(c)},
+					{Object: a, Raw: encodeOrDie(a)},
+					{Object: b, Raw: encodeOrDie(b)},
+					{Object: c, Raw: encodeOrDie(c)},
 				},
 			},
 			field: "{.metadata.name}",
@@ -255,16 +276,16 @@ func TestSortingPrinter(t *testing.T) {
 			name: "v1.List in reverse",
 			obj: &corev1.List{
 				Items: []runtime.RawExtension{
-					{Raw: encodeOrDie(c)},
-					{Raw: encodeOrDie(b)},
-					{Raw: encodeOrDie(a)},
+					{Object: c, Raw: encodeOrDie(c)},
+					{Object: b, Raw: encodeOrDie(b)},
+					{Object: a, Raw: encodeOrDie(a)},
 				},
 			},
 			sort: &corev1.List{
 				Items: []runtime.RawExtension{
-					{Raw: encodeOrDie(a)},
-					{Raw: encodeOrDie(b)},
-					{Raw: encodeOrDie(c)},
+					{Object: a, Raw: encodeOrDie(a)},
+					{Object: b, Raw: encodeOrDie(b)},
+					{Object: c, Raw: encodeOrDie(c)},
 				},
 			},
 			field: "{.metadata.name}",
@@ -390,6 +411,43 @@ func TestSortingPrinter(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
+		t.Run(tt.name+" table", func(t *testing.T) {
+			table := &metav1beta1.Table{}
+			meta.EachListItem(tt.obj, func(item runtime.Object) error {
+				table.Rows = append(table.Rows, metav1beta1.TableRow{
+					Object: runtime.RawExtension{Object: toUnstructuredOrDie(encodeOrDie(item))},
+				})
+				return nil
+			})
+
+			expectedTable := &metav1beta1.Table{}
+			meta.EachListItem(tt.sort, func(item runtime.Object) error {
+				expectedTable.Rows = append(expectedTable.Rows, metav1beta1.TableRow{
+					Object: runtime.RawExtension{Object: toUnstructuredOrDie(encodeOrDie(item))},
+				})
+				return nil
+			})
+
+			sorter, err := NewTableSorter(table, tt.field)
+			if err == nil {
+				err = sorter.Sort()
+			}
+			if err != nil {
+				if len(tt.expectedErr) > 0 {
+					if strings.Contains(err.Error(), tt.expectedErr) {
+						return
+					}
+					t.Fatalf("%s: expected error containing: %q, got: \"%v\"", tt.name, tt.expectedErr, err)
+				}
+				t.Fatalf("%s: unexpected error: %v", tt.name, err)
+			}
+			if len(tt.expectedErr) > 0 {
+				t.Fatalf("%s: expected error containing: %q, got none", tt.name, tt.expectedErr)
+			}
+			if !reflect.DeepEqual(table, expectedTable) {
+				t.Errorf("[%s]\nexpected/saw:\n%s", tt.name, diff.ObjectReflectDiff(expectedTable, table))
+			}
+		})
 		t.Run(tt.name, func(t *testing.T) {
 			sort := &SortingPrinter{SortField: tt.field, Decoder: scheme.Codecs.UniversalDecoder()}
 			err := sort.sortObj(tt.obj)