diff --git a/hack/testdata/sorted-pods/sorted-pod1.yaml b/hack/testdata/sorted-pods/sorted-pod1.yaml index 444ff9be229..9c6372f7dcd 100644 --- a/hack/testdata/sorted-pods/sorted-pod1.yaml +++ b/hack/testdata/sorted-pods/sorted-pod1.yaml @@ -9,3 +9,10 @@ spec: containers: - name: kubernetes-pause2 image: k8s.gcr.io/pause:3.5 + resources: + requests: + memory: "64Mi" + cpu: "250m" + limits: + memory: "128Mi" + cpu: "500m" diff --git a/hack/testdata/sorted-pods/sorted-pod2.yaml b/hack/testdata/sorted-pods/sorted-pod2.yaml index bd505a3415d..c7406bec2bb 100644 --- a/hack/testdata/sorted-pods/sorted-pod2.yaml +++ b/hack/testdata/sorted-pods/sorted-pod2.yaml @@ -9,3 +9,11 @@ spec: containers: - name: kubernetes-pause1 image: k8s.gcr.io/pause:3.5 + resources: + requests: + memory: "1G" + cpu: "0.5" + limits: + memory: "2G" + cpu: "1" + diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/get/get_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/get/get_test.go index 004b292a4cb..ee0e39a7b66 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/get/get_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/get/get_test.go @@ -860,7 +860,7 @@ func TestGetSortedObjects(t *testing.T) { cmd := NewCmdGet("kubectl", tf, streams) cmd.SetOutput(buf) - // sorting with metedata.name + // sorting with metadata.name cmd.Flags().Set("sort-by", ".metadata.name") cmd.Run(cmd, []string{"pods"}) @@ -899,7 +899,7 @@ func TestGetSortedObjectsUnstructuredTable(t *testing.T) { cmd := NewCmdGet("kubectl", tf, streams) cmd.SetOutput(buf) - // sorting with metedata.name + // sorting with metadata.name cmd.Flags().Set("sort-by", ".metadata.name") cmd.Run(cmd, []string{"pods"}) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/get/sorter.go b/staging/src/k8s.io/kubectl/pkg/cmd/get/sorter.go index 3374480fd51..d24fbe11e11 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/get/sorter.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/get/sorter.go @@ -26,6 +26,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -95,6 +96,8 @@ func (s *SortingPrinter) sortObj(obj runtime.Object) error { return meta.SetList(obj, objs) } +// SortObjects sorts the runtime.Object based on fieldInput and returns RuntimeSort that implements +// the golang sort interface func SortObjects(decoder runtime.Decoder, objs []runtime.Object, fieldInput string) (*RuntimeSort, error) { for ix := range objs { item := objs[ix] @@ -151,6 +154,7 @@ type RuntimeSort struct { origPosition []int } +// NewRuntimeSort creates a new RuntimeSort struct that implements golang sort interface func NewRuntimeSort(field string, objs []runtime.Object) *RuntimeSort { sorter := &RuntimeSort{field: field, objs: objs, origPosition: make([]int, len(objs))} for ix := range objs { @@ -187,6 +191,11 @@ func isLess(i, j reflect.Value) (bool, error) { time := j.Interface().(metav1.Time) return t.Before(&time), nil } + // sort resource.Quantity + if iQuantity, ok := in.(resource.Quantity); ok { + jQuantity := j.Interface().(resource.Quantity) + return iQuantity.Cmp(jQuantity) < 0, nil + } // fallback to the fields comparison for idx := 0; idx < i.NumField(); idx++ { less, err := isLess(i.Field(idx), j.Field(idx)) @@ -204,7 +213,6 @@ func isLess(i, j reflect.Value) (bool, error) { } } return true, nil - case reflect.Interface: if i.IsNil() && j.IsNil() { return false, nil @@ -264,7 +272,17 @@ func isLess(i, j reflect.Value) (bool, error) { } case string: if jtype, ok := j.Interface().(string); ok { - return sortorder.NaturalLess(itype, jtype), nil + // check if it's a Quantity + itypeQuantity, err := resource.ParseQuantity(itype) + if err != nil { + return sortorder.NaturalLess(itype, jtype), nil + } + jtypeQuantity, _ := resource.ParseQuantity(jtype) + if err != nil { + return sortorder.NaturalLess(itype, jtype), nil + } + // Both strings are quantity + return itypeQuantity.Cmp(jtypeQuantity) < 0, nil } default: return false, fmt.Errorf("unsortable type: %T", itype) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/get/sorter_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/get/sorter_test.go index 9f90c38705a..f0ddace35f7 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/get/sorter_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/get/sorter_test.go @@ -24,6 +24,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" @@ -47,6 +48,59 @@ func encodeOrDie(obj runtime.Object) []byte { } return data } +func createPodSpecResource(t *testing.T, memReq, memLimit, cpuReq, cpuLimit string) corev1.PodSpec { + t.Helper() + podSpec := corev1.PodSpec{ + Containers: []corev1.Container{ + { + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{}, + Limits: corev1.ResourceList{}, + }, + }, + }, + } + + req := podSpec.Containers[0].Resources.Requests + if memReq != "" { + memReq, err := resource.ParseQuantity(memReq) + if err != nil { + t.Errorf("memory request string is not a valid quantity") + } + req["memory"] = memReq + } + if cpuReq != "" { + cpuReq, err := resource.ParseQuantity(cpuReq) + if err != nil { + t.Errorf("cpu request string is not a valid quantity") + } + req["cpu"] = cpuReq + } + limit := podSpec.Containers[0].Resources.Limits + if memLimit != "" { + memLimit, err := resource.ParseQuantity(memLimit) + if err != nil { + t.Errorf("memory limit string is not a valid quantity") + } + limit["memory"] = memLimit + } + if cpuLimit != "" { + cpuLimit, err := resource.ParseQuantity(cpuLimit) + if err != nil { + t.Errorf("cpu limit string is not a valid quantity") + } + limit["cpu"] = cpuLimit + } + + return podSpec +} +func createUnstructuredPodResource(t *testing.T, memReq, memLimit, cpuReq, cpuLimit string) unstructured.Unstructured { + t.Helper() + pod := &corev1.Pod{ + Spec: createPodSpecResource(t, memReq, memLimit, cpuReq, cpuLimit), + } + return *toUnstructuredOrDie(encodeOrDie(pod)) +} func TestSortingPrinter(t *testing.T) { intPtr := func(val int32) *int32 { return &val } @@ -435,6 +489,158 @@ func TestSortingPrinter(t *testing.T) { }, field: "{.lastTimestamp}", }, + { + name: "pod-resources-cpu-random-order-with-missing-fields", + obj: &corev1.PodList{ + Items: []corev1.Pod{ + { + Spec: createPodSpecResource(t, "", "", "0.5", ""), + }, + { + Spec: createPodSpecResource(t, "", "", "10", ""), + }, + { + Spec: createPodSpecResource(t, "", "", "100m", ""), + }, + { + Spec: createPodSpecResource(t, "", "", "", ""), + }, + }, + }, + sort: &corev1.PodList{ + Items: []corev1.Pod{ + { + Spec: createPodSpecResource(t, "", "", "", ""), + }, + { + Spec: createPodSpecResource(t, "", "", "100m", ""), + }, + { + Spec: createPodSpecResource(t, "", "", "0.5", ""), + }, + { + Spec: createPodSpecResource(t, "", "", "10", ""), + }, + }, + }, + field: "{.spec.containers[].resources.requests.cpu}", + }, + { + name: "pod-resources-memory-random-order-with-missing-fields", + obj: &corev1.PodList{ + Items: []corev1.Pod{ + { + Spec: createPodSpecResource(t, "128Mi", "", "", ""), + }, + { + Spec: createPodSpecResource(t, "10Ei", "", "", ""), + }, + { + Spec: createPodSpecResource(t, "8Ti", "", "", ""), + }, + { + Spec: createPodSpecResource(t, "64Gi", "", "", ""), + }, + { + Spec: createPodSpecResource(t, "55Pi", "", "", ""), + }, + { + Spec: createPodSpecResource(t, "2Ki", "", "", ""), + }, + { + Spec: createPodSpecResource(t, "", "", "", ""), + }, + }, + }, + sort: &corev1.PodList{ + Items: []corev1.Pod{ + { + Spec: createPodSpecResource(t, "", "", "", ""), + }, + { + Spec: createPodSpecResource(t, "2Ki", "", "", ""), + }, + { + Spec: createPodSpecResource(t, "128Mi", "", "", ""), + }, + { + Spec: createPodSpecResource(t, "64Gi", "", "", ""), + }, + { + Spec: createPodSpecResource(t, "8Ti", "", "", ""), + }, + { + Spec: createPodSpecResource(t, "55Pi", "", "", ""), + }, + { + Spec: createPodSpecResource(t, "10Ei", "", "", ""), + }, + }, + }, + field: "{.spec.containers[].resources.requests.memory}", + }, + { + name: "pod-unstructured-resources-cpu-random-order-with-missing-fields", + obj: &unstructured.UnstructuredList{ + Object: map[string]interface{}{ + "kind": "List", + "apiVersion": "v1", + }, + Items: []unstructured.Unstructured{ + createUnstructuredPodResource(t, "", "", "0.5", ""), + createUnstructuredPodResource(t, "", "", "10", ""), + createUnstructuredPodResource(t, "", "", "100m", ""), + createUnstructuredPodResource(t, "", "", "", ""), + }, + }, + sort: &unstructured.UnstructuredList{ + Object: map[string]interface{}{ + "kind": "List", + "apiVersion": "v1", + }, + Items: []unstructured.Unstructured{ + createUnstructuredPodResource(t, "", "", "", ""), + createUnstructuredPodResource(t, "", "", "100m", ""), + createUnstructuredPodResource(t, "", "", "0.5", ""), + createUnstructuredPodResource(t, "", "", "10", ""), + }, + }, + field: "{.spec.containers[].resources.requests.cpu}", + }, + { + name: "pod-unstructured-resources-memory-random-order-with-missing-fields", + obj: &unstructured.UnstructuredList{ + Object: map[string]interface{}{ + "kind": "List", + "apiVersion": "v1", + }, + Items: []unstructured.Unstructured{ + createUnstructuredPodResource(t, "128Mi", "", "", ""), + createUnstructuredPodResource(t, "10Ei", "", "", ""), + createUnstructuredPodResource(t, "8Ti", "", "", ""), + createUnstructuredPodResource(t, "64Gi", "", "", ""), + createUnstructuredPodResource(t, "55Pi", "", "", ""), + createUnstructuredPodResource(t, "2Ki", "", "", ""), + createUnstructuredPodResource(t, "", "", "", ""), + }, + }, + sort: &unstructured.UnstructuredList{ + Object: map[string]interface{}{ + "kind": "List", + "apiVersion": "v1", + }, + Items: []unstructured.Unstructured{ + createUnstructuredPodResource(t, "", "", "", ""), + createUnstructuredPodResource(t, "2Ki", "", "", ""), + createUnstructuredPodResource(t, "128Mi", "", "", ""), + createUnstructuredPodResource(t, "64Gi", "", "", ""), + createUnstructuredPodResource(t, "8Ti", "", "", ""), + createUnstructuredPodResource(t, "55Pi", "", "", ""), + createUnstructuredPodResource(t, "10Ei", "", "", ""), + }, + }, + field: "{.spec.containers[].resources.requests.memory}", + }, } for _, tt := range tests { t.Run(tt.name+" table", func(t *testing.T) { @@ -505,16 +711,19 @@ func TestRuntimeSortLess(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "b", }, + Spec: createPodSpecResource(t, "0.5", "", "1Gi", ""), }, { ObjectMeta: metav1.ObjectMeta{ Name: "c", }, + Spec: createPodSpecResource(t, "2", "", "1Ti", ""), }, { ObjectMeta: metav1.ObjectMeta{ Name: "a", }, + Spec: createPodSpecResource(t, "10m", "", "1Ki", ""), }, }, } @@ -524,8 +733,15 @@ func TestRuntimeSortLess(t *testing.T) { t.Fatalf("ExtractList testobj got unexpected error: %v", err) } - testfield := "{.metadata.name}" - testruntimeSort := NewRuntimeSort(testfield, testobjs) + testfieldName := "{.metadata.name}" + testruntimeSortName := NewRuntimeSort(testfieldName, testobjs) + + testfieldCPU := "{.spec.containers[].resources.requests.cpu}" + testruntimeSortCPU := NewRuntimeSort(testfieldCPU, testobjs) + + testfieldMemory := "{.spec.containers[].resources.requests.memory}" + testruntimeSortMemory := NewRuntimeSort(testfieldMemory, testobjs) + tests := []struct { name string runtimeSort *RuntimeSort @@ -535,19 +751,68 @@ func TestRuntimeSortLess(t *testing.T) { expectErr bool }{ { - name: "test less true", - runtimeSort: testruntimeSort, + name: "test name b c less true", + runtimeSort: testruntimeSortName, i: 0, j: 1, expectResult: true, }, { - name: "test less false", - runtimeSort: testruntimeSort, + name: "test name c a less false", + runtimeSort: testruntimeSortName, i: 1, j: 2, expectResult: false, }, + { + name: "test name b a less false", + runtimeSort: testruntimeSortName, + i: 0, + j: 2, + expectResult: false, + }, + { + name: "test cpu 0.5 2 less true", + runtimeSort: testruntimeSortCPU, + i: 0, + j: 1, + expectResult: true, + }, + { + name: "test cpu 2 10mi less false", + runtimeSort: testruntimeSortCPU, + i: 1, + j: 2, + expectResult: false, + }, + { + name: "test cpu 0.5 10mi less false", + runtimeSort: testruntimeSortCPU, + i: 0, + j: 2, + expectResult: false, + }, + { + name: "test memory 1Gi 1Ti less true", + runtimeSort: testruntimeSortMemory, + i: 0, + j: 1, + expectResult: true, + }, + { + name: "test memory 1Ti 1Ki less false", + runtimeSort: testruntimeSortMemory, + i: 1, + j: 2, + expectResult: false, + }, + { + name: "test memory 1Gi 1Ki less false", + runtimeSort: testruntimeSortMemory, + i: 0, + j: 2, + expectResult: false, + }, } for i, test := range tests { diff --git a/test/cmd/get.sh b/test/cmd/get.sh index 4469d9187a5..d9b9107c80f 100755 --- a/test/cmd/get.sh +++ b/test/cmd/get.sh @@ -315,6 +315,22 @@ run_kubectl_sort_by_tests() { output_message=$(kubectl get pods --sort-by="{metadata.creationTimestamp}") kube::test::if_sort_by_has_correct_order "${output_message}" "sorted-pod1:sorted-pod2:sorted-pod3:" + # ensure sorting by resource memory request works + output_message=$(kubectl get pods --sort-by="{.spec.containers[0].resources.requests.memory}") + kube::test::if_sort_by_has_correct_order "${output_message}" "sorted-pod3:sorted-pod1:sorted-pod2:" + + # ensure sorting by resource cpu request works + output_message=$(kubectl get pods --sort-by="{.spec.containers[0].resources.requests.cpu}") + kube::test::if_sort_by_has_correct_order "${output_message}" "sorted-pod3:sorted-pod1:sorted-pod2:" + + # ensure sorting by resource memory limit works + output_message=$(kubectl get pods --sort-by="{.spec.containers[0].resources.limits.memory}") + kube::test::if_sort_by_has_correct_order "${output_message}" "sorted-pod3:sorted-pod1:sorted-pod2:" + + # ensure sorting by resource cpu limit works + output_message=$(kubectl get pods --sort-by="{.spec.containers[0].resources.limits.cpu}") + kube::test::if_sort_by_has_correct_order "${output_message}" "sorted-pod3:sorted-pod1:sorted-pod2:" + # ensure sorting using fallback codepath still works output_message=$(kubectl get pods --sort-by="{spec.containers[0].name}" --server-print=false --v=8 2>&1) kube::test::if_sort_by_has_correct_order "${output_message}" "sorted-pod2:sorted-pod1:sorted-pod3:"