From ae1db31a0f433f670e601e297b9e4a14826f9e06 Mon Sep 17 00:00:00 2001 From: saadali Date: Tue, 16 Dec 2014 14:20:51 -0800 Subject: [PATCH] Issue 2948: fix "kubectl get events" result not sorted --- pkg/kubectl/describe.go | 20 +++---- pkg/kubectl/describe_test.go | 43 ++++++++++++++ pkg/kubectl/resource_printer.go | 15 +++-- pkg/kubectl/resource_printer_test.go | 37 ++++++++++++ pkg/kubectl/sorted_event_list.go | 36 ++++++++++++ pkg/kubectl/sorted_event_list_test.go | 82 +++++++++++++++++++++++++++ 6 files changed, 217 insertions(+), 16 deletions(-) create mode 100644 pkg/kubectl/sorted_event_list.go create mode 100644 pkg/kubectl/sorted_event_list_test.go diff --git a/pkg/kubectl/describe.go b/pkg/kubectl/describe.go index 028f6503dfe..f1a69ed519b 100644 --- a/pkg/kubectl/describe.go +++ b/pkg/kubectl/describe.go @@ -21,6 +21,7 @@ import ( "io" "sort" "strings" + "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" @@ -193,22 +194,21 @@ func (d *MinionDescriber) Describe(namespace, name string) (string, error) { }) } -type sortableEvents []api.Event - -func (s sortableEvents) Len() int { return len(s) } -func (s sortableEvents) Less(i, j int) bool { return s[i].Timestamp.Before(s[j].Timestamp.Time) } -func (s sortableEvents) Swap(i, j int) { s[i], s[j] = s[j], s[i] } - func describeEvents(el *api.EventList, w io.Writer) { if len(el.Items) == 0 { fmt.Fprint(w, "No events.") return } - sort.Sort(sortableEvents(el.Items)) - fmt.Fprint(w, "Events:\nFrom\tSubobjectPath\tCondition\tReason\tMessage\n") + sort.Sort(SortableEvents(el.Items)) + fmt.Fprint(w, "Events:\nTime\tFrom\tSubobjectPath\tCondition\tReason\tMessage\n") for _, e := range el.Items { - fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%v\n", - e.Source, e.InvolvedObject.FieldPath, e.Condition, e.Reason, e.Message) + fmt.Fprintf(w, "%s\t%v\t%v\t%v\t%v\t%v\n", + e.Timestamp.Time.Format(time.RFC1123Z), + e.Source, + e.InvolvedObject.FieldPath, + e.Condition, + e.Reason, + e.Message) } } diff --git a/pkg/kubectl/describe_test.go b/pkg/kubectl/describe_test.go index d1f54a67ccc..178083919d0 100644 --- a/pkg/kubectl/describe_test.go +++ b/pkg/kubectl/describe_test.go @@ -19,8 +19,11 @@ package kubectl import ( "strings" "testing" + "time" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) type describeClient struct { @@ -30,6 +33,10 @@ type describeClient struct { *client.Fake } +func init() { + api.ForTesting_ReferencesAllowBlankSelfLinks = true +} + func TestDescribePod(t *testing.T) { fake := &client.Fake{} c := &describeClient{T: t, Namespace: "foo", Fake: fake} @@ -55,3 +62,39 @@ func TestDescribeService(t *testing.T) { t.Errorf("unexpected out: %s", out) } } + +func TestPodDescribeResultsSorted(t *testing.T) { + // Arrange + fake := &client.Fake{ + EventsList: api.EventList{ + Items: []api.Event{ + { + Source: "kubelet", + Message: "Item 1", + Timestamp: util.NewTime(time.Date(2014, time.January, 15, 0, 0, 0, 0, time.UTC)), + }, + { + Source: "scheduler", + Message: "Item 2", + Timestamp: util.NewTime(time.Date(1987, time.June, 17, 0, 0, 0, 0, time.UTC)), + }, + { + Source: "kubelet", + Message: "Item 3", + Timestamp: util.NewTime(time.Date(2002, time.December, 25, 0, 0, 0, 0, time.UTC)), + }, + }, + }, + } + c := &describeClient{T: t, Namespace: "foo", Fake: fake} + d := PodDescriber{c} + + // Act + out, err := d.Describe("foo", "bar") + + // Assert + if err != nil { + t.Errorf("unexpected error: %v", err) + } + VerifyDatesInOrder(out, "\n" /* rowDelimiter */, "\t" /* columnDelimiter */, t) +} diff --git a/pkg/kubectl/resource_printer.go b/pkg/kubectl/resource_printer.go index acd1ea237f8..dbe53cbc8eb 100644 --- a/pkg/kubectl/resource_printer.go +++ b/pkg/kubectl/resource_printer.go @@ -23,9 +23,11 @@ import ( "io" "io/ioutil" "reflect" + "sort" "strings" "text/tabwriter" "text/template" + "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" @@ -191,7 +193,7 @@ var replicationControllerColumns = []string{"NAME", "IMAGE(S)", "SELECTOR", "REP var serviceColumns = []string{"NAME", "LABELS", "SELECTOR", "IP", "PORT"} var minionColumns = []string{"NAME", "LABELS"} var statusColumns = []string{"STATUS"} -var eventColumns = []string{"NAME", "KIND", "CONDITION", "REASON", "MESSAGE"} +var eventColumns = []string{"TIME", "NAME", "KIND", "CONDITION", "REASON", "MESSAGE"} // addDefaultHandlers adds print handlers for default Kubernetes types. func (h *HumanReadablePrinter) addDefaultHandlers() { @@ -317,7 +319,8 @@ func printStatus(status *api.Status, w io.Writer) error { func printEvent(event *api.Event, w io.Writer) error { _, err := fmt.Fprintf( - w, "%s\t%s\t%s\t%s\t%s\n", + w, "%s\t%s\t%s\t%s\t%s\t%s\n", + event.Timestamp.Time.Format(time.RFC1123Z), event.InvolvedObject.Name, event.InvolvedObject.Kind, event.Condition, @@ -327,7 +330,9 @@ func printEvent(event *api.Event, w io.Writer) error { return err } +// Sorts and prints the EventList in a human-friendly format. func printEventList(list *api.EventList, w io.Writer) error { + sort.Sort(SortableEvents(list.Items)) for i := range list.Items { if err := printEvent(&list.Items[i], w); err != nil { return err @@ -350,12 +355,10 @@ func (h *HumanReadablePrinter) PrintObj(obj runtime.Object, output io.Writer) er resultValue := handler.printFunc.Call(args)[0] if resultValue.IsNil() { return nil - } else { - return resultValue.Interface().(error) } - } else { - return fmt.Errorf("error: unknown type %#v", obj) + return resultValue.Interface().(error) } + return fmt.Errorf("error: unknown type %#v", obj) } // TemplatePrinter is an implementation of ResourcePrinter which formats data with a Go Template. diff --git a/pkg/kubectl/resource_printer_test.go b/pkg/kubectl/resource_printer_test.go index 7285567694f..a79852775ca 100644 --- a/pkg/kubectl/resource_printer_test.go +++ b/pkg/kubectl/resource_printer_test.go @@ -23,6 +23,7 @@ import ( "io" "reflect" "testing" + "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/testapi" @@ -330,3 +331,39 @@ func TestPrinters(t *testing.T) { } } } + +func TestPrintEventsResultSorted(t *testing.T) { + // Arrange + printer := NewHumanReadablePrinter(false /* noHeaders */) + + obj := api.EventList{ + Items: []api.Event{ + { + Source: "kubelet", + Message: "Item 1", + Timestamp: util.NewTime(time.Date(2014, time.January, 15, 0, 0, 0, 0, time.UTC)), + }, + { + Source: "scheduler", + Message: "Item 2", + Timestamp: util.NewTime(time.Date(1987, time.June, 17, 0, 0, 0, 0, time.UTC)), + }, + { + Source: "kubelet", + Message: "Item 3", + Timestamp: util.NewTime(time.Date(2002, time.December, 25, 0, 0, 0, 0, time.UTC)), + }, + }, + } + buffer := &bytes.Buffer{} + + // Act + err := printer.PrintObj(&obj, buffer) + + // Assert + if err != nil { + t.Fatalf("An error occurred printing the EventList: %#v", err) + } + out := buffer.String() + VerifyDatesInOrder(out, "\n" /* rowDelimiter */, " " /* columnDelimiter */, t) +} diff --git a/pkg/kubectl/sorted_event_list.go b/pkg/kubectl/sorted_event_list.go new file mode 100644 index 00000000000..4e6a5ffb3d8 --- /dev/null +++ b/pkg/kubectl/sorted_event_list.go @@ -0,0 +1,36 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kubectl + +import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" +) + +// SortableEvents implements sort.Interface for []api.Event based on the Timestamp field +type SortableEvents []api.Event + +func (list SortableEvents) Len() int { + return len(list) +} + +func (list SortableEvents) Swap(i, j int) { + list[i], list[j] = list[j], list[i] +} + +func (list SortableEvents) Less(i, j int) bool { + return list[i].Timestamp.Time.Before(list[j].Timestamp.Time) +} diff --git a/pkg/kubectl/sorted_event_list_test.go b/pkg/kubectl/sorted_event_list_test.go new file mode 100644 index 00000000000..62716183924 --- /dev/null +++ b/pkg/kubectl/sorted_event_list_test.go @@ -0,0 +1,82 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kubectl + +import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "sort" + "strings" + "testing" + "time" +) + +// VerifyDatesInOrder checks the start of each line for a RFC1123Z date +// and posts error if all subsequent dates are not equal or increasing +func VerifyDatesInOrder( + resultToTest, rowDelimiter, columnDelimiter string, t *testing.T) { + lines := strings.Split(resultToTest, rowDelimiter) + var previousTime time.Time + for _, str := range lines { + columns := strings.Split(str, columnDelimiter) + if len(columns) > 0 { + currentTime, err := time.Parse(time.RFC1123Z, columns[0]) + if err == nil { + if previousTime.After(currentTime) { + t.Errorf( + "Output is not sorted by time. %s should be listed after %s. Complete output: %s", + previousTime.Format(time.RFC1123Z), + currentTime.Format(time.RFC1123Z), + resultToTest) + } + previousTime = currentTime + } + } + } + +} + +func TestSortableEvents(t *testing.T) { + // Arrange + list := SortableEvents([]api.Event{ + { + Source: "kubelet", + Message: "Item 1", + Timestamp: util.NewTime(time.Date(2014, time.January, 15, 0, 0, 0, 0, time.UTC)), + }, + { + Source: "scheduler", + Message: "Item 2", + Timestamp: util.NewTime(time.Date(1987, time.June, 17, 0, 0, 0, 0, time.UTC)), + }, + { + Source: "kubelet", + Message: "Item 3", + Timestamp: util.NewTime(time.Date(2002, time.December, 25, 0, 0, 0, 0, time.UTC)), + }, + }) + + // Act + sort.Sort(list) + + // Assert + if list[0].Message != "Item 2" || + list[1].Message != "Item 3" || + list[2].Message != "Item 1" { + t.Fatal("List is not sorted by time. List: ", list) + } +}