From 8308033effc94b1fe92c6b675bd345997f9c2a49 Mon Sep 17 00:00:00 2001 From: Vilmos Nebehaj Date: Mon, 20 Jan 2020 13:49:52 -0800 Subject: [PATCH 1/3] Add support for v1.PodLogOptions --- node/api/logs.go | 90 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 71 insertions(+), 19 deletions(-) diff --git a/node/api/logs.go b/node/api/logs.go index c45793d4c..467b6f68f 100644 --- a/node/api/logs.go +++ b/node/api/logs.go @@ -16,8 +16,10 @@ package api import ( "context" + "fmt" "io" "net/http" + "net/url" "strconv" "time" @@ -33,10 +35,71 @@ type ContainerLogsHandlerFunc func(ctx context.Context, namespace, podName, cont // ContainerLogOpts are used to pass along options to be set on the container // log stream. type ContainerLogOpts struct { - Tail int - Since time.Duration - LimitBytes int - Timestamps bool + Tail int + LimitBytes int + Timestamps bool + Follow bool + Previous bool + SinceSeconds int + SinceTime time.Time +} + +func parseLogOptions(q url.Values) (opts ContainerLogOpts, err error) { + if tailLines := q.Get("tailLines"); tailLines != "" { + opts.Tail, err = strconv.Atoi(tailLines) + if err != nil { + return opts, errdefs.AsInvalidInput(errors.Wrap(err, "could not parse \"tailLines\"")) + } + if opts.Tail < 0 { + return opts, errdefs.InvalidInput(fmt.Sprintf("\"tailLines\" is %d", opts.Tail)) + } + } + if follow := q.Get("follow"); follow != "" { + opts.Follow, err = strconv.ParseBool(follow) + if err != nil { + return opts, errdefs.AsInvalidInput(errors.Wrap(err, "could not parse \"follow\"")) + } + } + if limitBytes := q.Get("limitBytes"); limitBytes != "" { + opts.LimitBytes, err = strconv.Atoi(limitBytes) + if err != nil { + return opts, errdefs.AsInvalidInput(errors.Wrap(err, "could not parse \"limitBytes\"")) + } + if opts.LimitBytes < 1 { + return opts, errdefs.InvalidInput(fmt.Sprintf("\"limitBytes\" is %d", opts.LimitBytes)) + } + } + if previous := q.Get("previous"); previous != "" { + opts.Previous, err = strconv.ParseBool(previous) + if err != nil { + return opts, errdefs.AsInvalidInput(errors.Wrap(err, "could not parse \"previous\"")) + } + } + if sinceSeconds := q.Get("sinceSeconds"); sinceSeconds != "" { + opts.SinceSeconds, err = strconv.Atoi(sinceSeconds) + if err != nil { + return opts, errdefs.AsInvalidInput(errors.Wrap(err, "could not parse \"sinceSeconds\"")) + } + if opts.SinceSeconds < 1 { + return opts, errdefs.InvalidInput(fmt.Sprintf("\"sinceSeconds\" is %d", opts.SinceSeconds)) + } + } + if sinceTime := q.Get("sinceTime"); sinceTime != "" { + opts.SinceTime, err = time.Parse(time.RFC3339, sinceTime) + if err != nil { + return opts, errdefs.AsInvalidInput(errors.Wrap(err, "could not parse \"sinceTime\"")) + } + if opts.SinceSeconds > 0 { + return opts, errdefs.InvalidInput("both \"sinceSeconds\" and \"sinceTime\" are set") + } + } + if timestamps := q.Get("timestamps"); timestamps != "" { + opts.Timestamps, err = strconv.ParseBool(timestamps) + if err != nil { + return opts, errdefs.AsInvalidInput(errors.Wrap(err, "could not parse \"timestamps\"")) + } + } + return opts, nil } // HandleContainerLogs creates an http handler function from a provider to serve logs from a pod @@ -55,22 +118,11 @@ func HandleContainerLogs(h ContainerLogsHandlerFunc) http.HandlerFunc { namespace := vars["namespace"] pod := vars["pod"] container := vars["container"] - tail := 10 - q := req.URL.Query() - if queryTail := q.Get("tailLines"); queryTail != "" { - t, err := strconv.Atoi(queryTail) - if err != nil { - return errdefs.AsInvalidInput(errors.Wrap(err, "could not parse \"tailLines\"")) - } - tail = t - } - - // TODO(@cpuguy83): support v1.PodLogOptions - // The kubelet decoding here is not straight forward, so this needs to be disected - - opts := ContainerLogOpts{ - Tail: tail, + query := req.URL.Query() + opts, err := parseLogOptions(query) + if err != nil { + return err } logs, err := h(ctx, namespace, pod, container, opts) From 7628c13aeb868afb2addccec79ebe09e2fb3613c Mon Sep 17 00:00:00 2001 From: Vilmos Nebehaj Date: Fri, 20 Mar 2020 13:46:50 -0700 Subject: [PATCH 2/3] Add tests for parseLogOptions() --- node/api/logs_test.go | 99 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 node/api/logs_test.go diff --git a/node/api/logs_test.go b/node/api/logs_test.go new file mode 100644 index 000000000..99f2ab7cf --- /dev/null +++ b/node/api/logs_test.go @@ -0,0 +1,99 @@ +package api + +import ( + "fmt" + "net/url" + "testing" + "time" + + "gotest.tools/assert" + is "gotest.tools/assert/cmp" +) + +//func parseLogOptions(q url.Values) (opts ContainerLogOpts, err error) +func TestParseLogOptions(t *testing.T) { + //tailLines + //follow + //limitBytes + //previous + //sinceSeconds + //sinceTime + //timestamps + sinceTime, _ := time.Parse(time.RFC3339, "2020-03-20T21:07:34Z") + fmt.Printf("%+v\n", sinceTime) + testCases := []struct { + Values url.Values + Failure bool + Result ContainerLogOpts + }{ + { + Values: url.Values{}, + Failure: false, + Result: ContainerLogOpts{}, + }, + { + Values: url.Values{ + "follow": {"true"}, + "limitBytes": {"123"}, + "previous": {"true"}, + "sinceSeconds": {"10"}, + "tailLines": {"99"}, + "timestamps": {"true"}, + }, + Failure: false, + Result: ContainerLogOpts{ + Follow: true, + LimitBytes: 123, + Previous: true, + SinceSeconds: 10, + Tail: 99, + Timestamps: true, + }, + }, + { + Values: url.Values{ + "sinceSeconds": {"10"}, + "sinceTime": {"2020-03-20T21:07:34Z"}, + }, + Failure: true, + }, + { + Values: url.Values{ + "sinceTime": {"2020-03-20T21:07:34Z"}, + }, + Failure: false, + Result: ContainerLogOpts{ + SinceTime: sinceTime, + }, + }, + { + Values: url.Values{ + "tailLines": {"-1"}, + }, + Failure: true, + }, + { + Values: url.Values{ + "limitBytes": {"0"}, + }, + Failure: true, + }, + { + Values: url.Values{ + "sinceSeconds": {"-10"}, + }, + Failure: true, + }, + } + // follow=true&limitBytes=1&previous=true&sinceSeconds=1&sinceTime=2020-03-20T21%3A07%3A34Z&tailLines=1×tamps=true + for i, tc := range testCases { + msg := fmt.Sprintf("test case #%d %+v failed", i+1, tc) + result, err := parseLogOptions(tc.Values) + if tc.Failure { + assert.Check(t, is.ErrorContains(err, ""), msg) + } else { + assert.NilError(t, err, msg) + assert.Check(t, is.Equal(result, tc.Result), msg) + } + } +} From 3e0d03c83382a7ee7bf0e294a369abda427d5a27 Mon Sep 17 00:00:00 2001 From: Vilmos Nebehaj Date: Tue, 28 Apr 2020 11:18:52 -0700 Subject: [PATCH 3/3] Use errdefs.InvalidInputf() for formatting --- node/api/logs.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/node/api/logs.go b/node/api/logs.go index 467b6f68f..9602dc72b 100644 --- a/node/api/logs.go +++ b/node/api/logs.go @@ -16,7 +16,6 @@ package api import ( "context" - "fmt" "io" "net/http" "net/url" @@ -51,7 +50,7 @@ func parseLogOptions(q url.Values) (opts ContainerLogOpts, err error) { return opts, errdefs.AsInvalidInput(errors.Wrap(err, "could not parse \"tailLines\"")) } if opts.Tail < 0 { - return opts, errdefs.InvalidInput(fmt.Sprintf("\"tailLines\" is %d", opts.Tail)) + return opts, errdefs.InvalidInputf("\"tailLines\" is %d", opts.Tail) } } if follow := q.Get("follow"); follow != "" { @@ -66,7 +65,7 @@ func parseLogOptions(q url.Values) (opts ContainerLogOpts, err error) { return opts, errdefs.AsInvalidInput(errors.Wrap(err, "could not parse \"limitBytes\"")) } if opts.LimitBytes < 1 { - return opts, errdefs.InvalidInput(fmt.Sprintf("\"limitBytes\" is %d", opts.LimitBytes)) + return opts, errdefs.InvalidInputf("\"limitBytes\" is %d", opts.LimitBytes) } } if previous := q.Get("previous"); previous != "" { @@ -81,7 +80,7 @@ func parseLogOptions(q url.Values) (opts ContainerLogOpts, err error) { return opts, errdefs.AsInvalidInput(errors.Wrap(err, "could not parse \"sinceSeconds\"")) } if opts.SinceSeconds < 1 { - return opts, errdefs.InvalidInput(fmt.Sprintf("\"sinceSeconds\" is %d", opts.SinceSeconds)) + return opts, errdefs.InvalidInputf("\"sinceSeconds\" is %d", opts.SinceSeconds) } } if sinceTime := q.Get("sinceTime"); sinceTime != "" {