Fix sinceTime pod log options

pkg/api/conversion.go

@@ -41,6 +41,7 @@ func init() {
 		Convert_unversioned_ListMeta_To_unversioned_ListMeta,
 		Convert_intstr_IntOrString_To_intstr_IntOrString,
 		Convert_unversioned_Time_To_unversioned_Time,
+		Convert_string_slice_To_unversioned_Time,
 		Convert_string_To_labels_Selector,
 		Convert_string_To_fields_Selector,
 		Convert_bool_ref_To_bool,
@@ -116,6 +117,16 @@ func Convert_unversioned_Time_To_unversioned_Time(in *unversioned.Time, out *unv
 	*out = *in
 	return nil
 }
+
+// Convert_string_slice_To_unversioned_Time allows converting a URL query parameter value
+func Convert_string_slice_To_unversioned_Time(input *[]string, out *unversioned.Time, s conversion.Scope) error {
+	str := ""
+	if len(*input) > 0 {
+		str = (*input)[0]
+	}
+	return out.UnmarshalQueryParameter(str)
+}
+
 func Convert_string_To_labels_Selector(in *string, out *labels.Selector, s conversion.Scope) error {
 	selector, err := labels.Parse(*in)
 	if err != nil {

pkg/api/unversioned/time.go

@@ -97,6 +97,27 @@ func (t *Time) UnmarshalJSON(b []byte) error {
 	return nil
 }
 
+// UnmarshalQueryParameter converts from a URL query parameter value to an object
+func (t *Time) UnmarshalQueryParameter(str string) error {
+	if len(str) == 0 {
+		t.Time = time.Time{}
+		return nil
+	}
+	// Tolerate requests from older clients that used JSON serialization to build query params
+	if len(str) == 4 && str == "null" {
+		t.Time = time.Time{}
+		return nil
+	}
+
+	pt, err := time.Parse(time.RFC3339, str)
+	if err != nil {
+		return err
+	}
+
+	t.Time = pt.Local()
+	return nil
+}
+
 // MarshalJSON implements the json.Marshaler interface.
 func (t Time) MarshalJSON() ([]byte, error) {
 	if t.IsZero() {
@@ -107,6 +128,16 @@ func (t Time) MarshalJSON() ([]byte, error) {
 	return json.Marshal(t.UTC().Format(time.RFC3339))
 }
 
+// MarshalQueryParameter converts to a URL query parameter value
+func (t Time) MarshalQueryParameter() (string, error) {
+	if t.IsZero() {
+		// Encode unset/nil objects as an empty string
+		return "", nil
+	}
+
+	return t.UTC().Format(time.RFC3339), nil
+}
+
 // Fuzz satisfies fuzz.Interface.
 func (t *Time) Fuzz(c fuzz.Continue) {
 	if t == nil {

pkg/api/v1/conversion_test.go

@@ -17,13 +17,105 @@ limitations under the License.
 package v1_test
 
 import (
+	"net/url"
+	"reflect"
 	"testing"
+	"time"
 
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/resource"
+	"k8s.io/kubernetes/pkg/api/unversioned"
 	versioned "k8s.io/kubernetes/pkg/api/v1"
+	"k8s.io/kubernetes/pkg/runtime"
+	"k8s.io/kubernetes/pkg/util"
 )
 
+func TestPodLogOptions(t *testing.T) {
+	sinceSeconds := int64(1)
+	sinceTime := unversioned.NewTime(time.Date(2000, 1, 1, 12, 34, 56, 0, time.UTC).Local())
+	tailLines := int64(2)
+	limitBytes := int64(3)
+
+	versionedLogOptions := &versioned.PodLogOptions{
+		Container:    "mycontainer",
+		Follow:       true,
+		Previous:     true,
+		SinceSeconds: &sinceSeconds,
+		SinceTime:    &sinceTime,
+		Timestamps:   true,
+		TailLines:    &tailLines,
+		LimitBytes:   &limitBytes,
+	}
+	unversionedLogOptions := &api.PodLogOptions{
+		Container:    "mycontainer",
+		Follow:       true,
+		Previous:     true,
+		SinceSeconds: &sinceSeconds,
+		SinceTime:    &sinceTime,
+		Timestamps:   true,
+		TailLines:    &tailLines,
+		LimitBytes:   &limitBytes,
+	}
+	expectedParameters := url.Values{
+		"container":    {"mycontainer"},
+		"follow":       {"true"},
+		"previous":     {"true"},
+		"sinceSeconds": {"1"},
+		"sinceTime":    {"2000-01-01T12:34:56Z"},
+		"timestamps":   {"true"},
+		"tailLines":    {"2"},
+		"limitBytes":   {"3"},
+	}
+
+	codec := runtime.NewParameterCodec(api.Scheme)
+
+	// unversioned -> query params
+	{
+		actualParameters, err := codec.EncodeParameters(unversionedLogOptions, versioned.SchemeGroupVersion)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !reflect.DeepEqual(actualParameters, expectedParameters) {
+			t.Fatalf("Expected\n%#v\ngot\n%#v", expectedParameters, actualParameters)
+		}
+	}
+
+	// versioned -> query params
+	{
+		actualParameters, err := codec.EncodeParameters(versionedLogOptions, versioned.SchemeGroupVersion)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !reflect.DeepEqual(actualParameters, expectedParameters) {
+			t.Fatalf("Expected\n%#v\ngot\n%#v", expectedParameters, actualParameters)
+		}
+	}
+
+	// query params -> versioned
+	{
+		convertedLogOptions := &versioned.PodLogOptions{}
+		err := codec.DecodeParameters(expectedParameters, versioned.SchemeGroupVersion, convertedLogOptions)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !reflect.DeepEqual(convertedLogOptions, versionedLogOptions) {
+			t.Fatalf("Unexpected deserialization:\n%s", util.ObjectGoPrintSideBySide(versionedLogOptions, convertedLogOptions))
+		}
+	}
+
+	// query params -> unversioned
+	{
+		convertedLogOptions := &api.PodLogOptions{}
+		err := codec.DecodeParameters(expectedParameters, versioned.SchemeGroupVersion, convertedLogOptions)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !reflect.DeepEqual(convertedLogOptions, unversionedLogOptions) {
+			t.Fatalf("Unexpected deserialization:\n%s", util.ObjectGoPrintSideBySide(unversionedLogOptions, convertedLogOptions))
+		}
+	}
+}
+
 // TestPodSpecConversion tests that ServiceAccount is an alias for
 // ServiceAccountName.
 func TestPodSpecConversion(t *testing.T) {

pkg/conversion/queryparams/convert.go

@@ -23,6 +23,16 @@ import (
 	"strings"
 )
 
+// Marshaler converts an object to a query parameter string representation
+type Marshaler interface {
+	MarshalQueryParameter() (string, error)
+}
+
+// Unmarshaler converts a string representation to an object
+type Unmarshaler interface {
+	UnmarshalQueryParameter(string) error
+}
+
 func jsonTag(field reflect.StructField) (string, bool) {
 	structTag := field.Tag.Get("json")
 	if len(structTag) == 0 {
@@ -72,6 +82,31 @@ func zeroValue(value reflect.Value) bool {
 	return reflect.DeepEqual(reflect.Zero(value.Type()).Interface(), value.Interface())
 }
 
+func customMarshalValue(value reflect.Value) (reflect.Value, bool) {
+	// Return unless we implement a custom query marshaler
+	if !value.CanInterface() {
+		return reflect.Value{}, false
+	}
+
+	marshaler, ok := value.Interface().(Marshaler)
+	if !ok {
+		return reflect.Value{}, false
+	}
+
+	// Don't invoke functions on nil pointers
+	// If the type implements MarshalQueryParameter, AND the tag is not omitempty, AND the value is a nil pointer, "" seems like a reasonable response
+	if isPointerKind(value.Kind()) && zeroValue(value) {
+		return reflect.ValueOf(""), true
+	}
+
+	// Get the custom marshalled value
+	v, err := marshaler.MarshalQueryParameter()
+	if err != nil {
+		return reflect.Value{}, false
+	}
+	return reflect.ValueOf(v), true
+}
+
 func addParam(values url.Values, tag string, omitempty bool, value reflect.Value) {
 	if omitempty && zeroValue(value) {
 		return
@@ -128,7 +163,8 @@ func convertStruct(result url.Values, st reflect.Type, sv reflect.Value) {
 
 		kind := ft.Kind()
 		if isPointerKind(kind) {
-			kind = ft.Elem().Kind()
+			ft = ft.Elem()
+			kind = ft.Kind()
 			if !field.IsNil() {
 				field = reflect.Indirect(field)
 			}
@@ -142,7 +178,11 @@ func convertStruct(result url.Values, st reflect.Type, sv reflect.Value) {
 				addListOfParams(result, tag, omitempty, field)
 			}
 		case isStructKind(kind) && !(zeroValue(field) && omitempty):
-			convertStruct(result, ft, field)
+			if marshalValue, ok := customMarshalValue(field); ok {
+				addParam(result, tag, omitempty, marshalValue)
+			} else {
+				convertStruct(result, ft, field)
+			}
 		}
 	}
 }

pkg/conversion/queryparams/convert_test.go

@@ -20,6 +20,7 @@ import (
 	"net/url"
 	"reflect"
 	"testing"
+	"time"
 
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/conversion/queryparams"
@@ -61,6 +62,19 @@ type baz struct {
 
 func (obj *baz) GetObjectKind() unversioned.ObjectKind { return unversioned.EmptyObjectKind }
 
+// childStructs tests some of the types we serialize to query params for log API calls
+// notably, the nested time struct
+type childStructs struct {
+	Container    string            `json:"container,omitempty"`
+	Follow       bool              `json:"follow,omitempty"`
+	Previous     bool              `json:"previous,omitempty"`
+	SinceSeconds *int64            `json:"sinceSeconds,omitempty"`
+	SinceTime    *unversioned.Time `json:"sinceTime,omitempty"`
+	EmptyTime    *unversioned.Time `json:"emptyTime"`
+}
+
+func (obj *childStructs) GetObjectKind() unversioned.ObjectKind { return unversioned.EmptyObjectKind }
+
 func validateResult(t *testing.T, input interface{}, actual, expected url.Values) {
 	local := url.Values{}
 	for k, v := range expected {
@@ -73,7 +87,6 @@ func validateResult(t *testing.T, input interface{}, actual, expected url.Values
 			} else {
 				t.Errorf("%#v: values don't match: actual: %#v, expected: %#v", input, v, ev)
 			}
-			break
 		}
 		delete(local, k)
 	}
@@ -83,6 +96,9 @@ func validateResult(t *testing.T, input interface{}, actual, expected url.Values
 }
 
 func TestConvert(t *testing.T) {
+	sinceSeconds := int64(123)
+	sinceTime := unversioned.Date(2000, 1, 1, 12, 34, 56, 0, time.UTC)
+
 	tests := []struct {
 		input    interface{}
 		expected url.Values
@@ -158,6 +174,27 @@ func TestConvert(t *testing.T) {
 			},
 			expected: url.Values{"ptr": {"5"}},
 		},
+		{
+			input: &childStructs{
+				Container:    "mycontainer",
+				Follow:       true,
+				Previous:     true,
+				SinceSeconds: &sinceSeconds,
+				SinceTime:    &sinceTime, // test a custom marshaller
+				EmptyTime:    nil,        // test a nil custom marshaller without omitempty
+			},
+			expected: url.Values{"container": {"mycontainer"}, "follow": {"true"}, "previous": {"true"}, "sinceSeconds": {"123"}, "sinceTime": {"2000-01-01T12:34:56Z"}, "emptyTime": {""}},
+		},
+		{
+			input: &childStructs{
+				Container:    "mycontainer",
+				Follow:       true,
+				Previous:     true,
+				SinceSeconds: &sinceSeconds,
+				SinceTime:    nil, // test a nil custom marshaller with omitempty
+			},
+			expected: url.Values{"container": {"mycontainer"}, "follow": {"true"}, "previous": {"true"}, "sinceSeconds": {"123"}, "emptyTime": {""}},
+		},
 	}
 
 	for _, test := range tests {

test/e2e_node/kubelet_test.go

@@ -26,6 +26,7 @@ import (
 	"time"
 
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/unversioned"
 	client "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/stats"
 
@@ -69,7 +70,8 @@ var _ = Describe("Kubelet", func() {
 
 			It("it should print the output to logs", func() {
 				Eventually(func() string {
-					rc, err := cl.Pods(api.NamespaceDefault).GetLogs("busybox", &api.PodLogOptions{}).Stream()
+					sinceTime := unversioned.NewTime(time.Now().Add(time.Duration(-1 * time.Hour)))
+					rc, err := cl.Pods(api.NamespaceDefault).GetLogs("busybox", &api.PodLogOptions{SinceTime: &sinceTime}).Stream()
 					if err != nil {
 						return ""
 					}