Merge pull request #3011 from saad-ali/2948

Issue 2948: fix "kubectl get events" result not sorted

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)
 	}
 }
 

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)
+}

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.

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)
+}

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)
+}

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)
+	}
+}