github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-23 19:56:01 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #40541 from fabianofranz/fix_sorting_printer_with_missing_fields

Browse Source 
Automatic merge from submit-queue (batch tested with PRs 40645, 40541, 40769)

Fix sorting printer when sorting by a missing field

**What this PR does / why we need it**:

When calling `kubectl get` with the `--sort-by` flag, the command will error out if the field used for sorting is not present in at least one of the objects returned in the list, *even if it is a field valid in the object's model*. 

For example, taking a list of `ReplicationController` where one of them has `status: { replicas: 0 }` (so nothing in `status.availableReplicas`, even that being a valid object in the model and present in every other object of the list) :

```
$ oc get rc --sort-by=status.availableReplicas
error: availableReplicas is not found
```

This PR now traverses the entire list of objects to be sorted and, if at least one has the field provided in `--sort-by`, we sort correctly and consider the field empty in every other object where the field is not present. If none of the objects has the field, we error out (that will catch really invalid fields, and valid ones but not present in any object in the list, which is acceptable). No swagger validation here.

**Release note**:
```release-note
Fixed an issue where 'kubectl get --sort-by=' would return an error when the specified field were not present in at least one of the returned objects, even that being a valid field in the object model.
```
This commit is contained in:
Kubernetes Submit Queue 2017-01-31 22:00:38 -08:00 committed by GitHub
commit 61d45f5900
3 changed files with 176 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pkg/kubectl/BUILD
View File

@ -174,6 +174,7 @@ go_test(
"//vendor:k8s.io/apimachinery/pkg/api/errors",
"//vendor:k8s.io/apimachinery/pkg/api/resource",
"//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
"//vendor:k8s.io/apimachinery/pkg/apis/meta/v1/unstructured",
"//vendor:k8s.io/apimachinery/pkg/runtime",
"//vendor:k8s.io/apimachinery/pkg/runtime/schema",
"//vendor:k8s.io/apimachinery/pkg/runtime/serializer/yaml",

59
pkg/kubectl/sorting_printer.go
View File

@ -88,17 +88,6 @@ func (s *SortingPrinter) sortObj(obj runtime.Object) error {
}
func SortObjects(decoder runtime.Decoder, objs []runtime.Object, fieldInput string) (*RuntimeSort, error) {
parser := jsonpath.New("sorting")
field, err := massageJSONPath(fieldInput)
if err != nil {
return nil, err
}
if err := parser.Parse(field); err != nil {
return nil, err
}
for ix := range objs {
item := objs[ix]
switch u := item.(type) {
@ -112,18 +101,38 @@ func SortObjects(decoder runtime.Decoder, objs []runtime.Object, fieldInput stri
}
}
var values [][]reflect.Value
if unstructured, ok := objs[0].(*unstructured.Unstructured); ok {
values, err = parser.FindResults(unstructured.Object)
} else {
values, err = parser.FindResults(reflect.ValueOf(objs[0]).Elem().Interface())
}
field, err := massageJSONPath(fieldInput)
if err != nil {
return nil, err
}
if len(values) == 0 {
return nil, fmt.Errorf("couldn't find any field with path: %s", field)
parser := jsonpath.New("sorting").AllowMissingKeys(true)
if err := parser.Parse(field); err != nil {
return nil, err
}
// We don't do any model validation here, so we traverse all objects to be sorted
// and, if the field is valid to at least one of them, we consider it to be a
// valid field; otherwise error out.
// Note that this requires empty fields to be considered later, when sorting.
var fieldFoundOnce bool
for _, obj := range objs {
var values [][]reflect.Value
if unstructured, ok := obj.(*unstructured.Unstructured); ok {
values, err = parser.FindResults(unstructured.Object)
} else {
values, err = parser.FindResults(reflect.ValueOf(obj).Elem().Interface())
}
if err != nil {
return nil, err
}
if len(values) > 0 && len(values[0]) > 0 {
fieldFoundOnce = true
break
}
}
if !fieldFoundOnce {
return nil, fmt.Errorf("couldn't find any field with path %q in the list of objects", field)
}
sorter := NewRuntimeSort(field, objs)
@ -260,7 +269,7 @@ func (r *RuntimeSort) Less(i, j int) bool {
iObj := r.objs[i]
jObj := r.objs[j]
parser := jsonpath.New("sorting")
parser := jsonpath.New("sorting").AllowMissingKeys(true)
parser.Parse(r.field)
var iValues [][]reflect.Value
@ -285,12 +294,18 @@ func (r *RuntimeSort) Less(i, j int) bool {
glog.Fatalf("Failed to get j values for %#v using %s (%v)", jObj, r.field, err)
}
if len(iValues) == 0 || len(iValues[0]) == 0 {
return true
}
if len(jValues) == 0 || len(jValues[0]) == 0 {
return false
}
iField := iValues[0][0]
jField := jValues[0][0]
less, err := isLess(iField, jField)
if err != nil {
glog.Fatalf("Field %s in %v is an unsortable type: %s, err: %v", r.field, iObj, iField.Kind().String(), err)
glog.Fatalf("Field %s in %T is an unsortable type: %s, err: %v", r.field, iObj, iField.Kind().String(), err)
}
return less
}

145
pkg/kubectl/sorting_printer_test.go
View File

@ -18,9 +18,11 @@ package kubectl
import (
"reflect"
"strings"
"testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
internal "k8s.io/kubernetes/pkg/api"
api "k8s.io/kubernetes/pkg/api/v1"
@ -56,10 +58,11 @@ func TestSortingPrinter(t *testing.T) {
}
tests := []struct {
obj runtime.Object
sort runtime.Object
field string
name string
obj runtime.Object
sort runtime.Object
field string
name string
expectedErr string
}{
{
name: "in-order-already",
@ -265,12 +268,140 @@ func TestSortingPrinter(t *testing.T) {
},
field: "{.metadata.name}",
},
{
name: "some-missing-fields",
obj: &unstructured.UnstructuredList{
Object: map[string]interface{}{
"kind": "List",
"apiVersion": "v1",
},
Items: []*unstructured.Unstructured{
{
Object: map[string]interface{}{
"kind": "ReplicationController",
"apiVersion": "v1",
"status": map[string]interface{}{
"availableReplicas": 2,
},
},
},
{
Object: map[string]interface{}{
"kind": "ReplicationController",
"apiVersion": "v1",
"status": map[string]interface{}{},
},
},
{
Object: map[string]interface{}{
"kind": "ReplicationController",
"apiVersion": "v1",
"status": map[string]interface{}{
"availableReplicas": 1,
},
},
},
},
},
sort: &unstructured.UnstructuredList{
Object: map[string]interface{}{
"kind": "List",
"apiVersion": "v1",
},
Items: []*unstructured.Unstructured{
{
Object: map[string]interface{}{
"kind": "ReplicationController",
"apiVersion": "v1",
"status": map[string]interface{}{},
},
},
{
Object: map[string]interface{}{
"kind": "ReplicationController",
"apiVersion": "v1",
"status": map[string]interface{}{
"availableReplicas": 1,
},
},
},
{
Object: map[string]interface{}{
"kind": "ReplicationController",
"apiVersion": "v1",
"status": map[string]interface{}{
"availableReplicas": 2,
},
},
},
},
},
field: "{.status.availableReplicas}",
},
{
name: "all-missing-fields",
obj: &unstructured.UnstructuredList{
Object: map[string]interface{}{
"kind": "List",
"apiVersion": "v1",
},
Items: []*unstructured.Unstructured{
{
Object: map[string]interface{}{
"kind": "ReplicationController",
"apiVersion": "v1",
"status": map[string]interface{}{
"replicas": 0,
},
},
},
{
Object: map[string]interface{}{
"kind": "ReplicationController",
"apiVersion": "v1",
"status": map[string]interface{}{
"replicas": 0,
},
},
},
},
},
field: "{.status.availableReplicas}",
expectedErr: "couldn't find any field with path \"{.status.availableReplicas}\" in the list of objects",
},
{
name: "model-invalid-fields",
obj: &api.ReplicationControllerList{
Items: []api.ReplicationController{
{
Status: api.ReplicationControllerStatus{},
},
{
Status: api.ReplicationControllerStatus{},
},
{
Status: api.ReplicationControllerStatus{},
},
},
},
field: "{.invalid}",
expectedErr: "couldn't find any field with path \"{.invalid}\" in the list of objects",
},
}
for _, test := range tests {
sort := &SortingPrinter{SortField: test.field, Decoder: internal.Codecs.UniversalDecoder()}
if err := sort.sortObj(test.obj); err != nil {
t.Errorf("unexpected error: %v (%s)", err, test.name)
continue
err := sort.sortObj(test.obj)
if err != nil {
if len(test.expectedErr) > 0 {
if strings.Contains(err.Error(), test.expectedErr) {
continue
}
t.Fatalf("%s: expected error containing: %q, got: \"%v\"", test.name, test.expectedErr, err)
}
t.Fatalf("%s: unexpected error: %v", test.name, err)
}
if len(test.expectedErr) > 0 {
t.Fatalf("%s: expected error containing: %q, got none", test.name, test.expectedErr)
}
if !reflect.DeepEqual(test.obj, test.sort) {
t.Errorf("[%s]\nexpected:\n%v\nsaw:\n%v", test.name, test.sort, test.obj)