-
Notifications
You must be signed in to change notification settings - Fork 38.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding endpoint for log retrieval on the minion #1318
Changes from all commits
f351691
6da2653
f3f5d02
9f34eae
960f1b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,7 @@ import ( | |
"sort" | ||
"strconv" | ||
"strings" | ||
"io" | ||
|
||
"github.com/GoogleCloudPlatform/kubernetes/pkg/api" | ||
"github.com/fsouza/go-dockerclient" | ||
|
@@ -46,6 +47,7 @@ type DockerInterface interface { | |
StartContainer(id string, hostConfig *docker.HostConfig) error | ||
StopContainer(id string, timeout uint) error | ||
PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error | ||
Logs(opts docker.LogsOptions) error | ||
} | ||
|
||
// DockerID is an ID of docker container. It is a type to make it clear when we're working with docker container Ids | ||
|
@@ -202,6 +204,30 @@ func GetRecentDockerContainersWithNameAndUUID(client DockerInterface, podFullNam | |
return result, nil | ||
} | ||
|
||
// GetKubeletDockerContainerLogs returns logs of specific container | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. improve documentation of this function to document the use of parameters (especially the "tail is only used if follow is false" part. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated in second commit jhadvig@6da2653 |
||
// By default the function will return snapshot of the container log | ||
// Log streaming is possible if 'follow' param is set to true | ||
// Log tailing is possible when number of tailed lines are set and only if 'follow' is false | ||
func GetKubeletDockerContainerLogs(client DockerInterface, containerID, tail string, follow bool, writer io.Writer) (err error) { | ||
opts := docker.LogsOptions{ | ||
Container: containerID, | ||
Stdout: true, | ||
Stderr: true, | ||
OutputStream: writer, | ||
ErrorStream: writer, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to at least allow the option of passing a separate error stream? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it really necessary to have separate stream for stdout and stderr ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a utility function, you can always pass the same stream to both the output and err parameters, as you are doing inside the function, we're better off creating a generally useful helper function, rather than one that is hand-crafted to the specifics of returning the data over HTTP. Additionally, I could easily imagine converting this http function to take a 'json' parameter, which would return structured data that separates the two streams, and places each as a member in a json object. |
||
Timestamps: true, | ||
RawTerminal: true, | ||
Follow: follow, | ||
} | ||
|
||
if !follow { | ||
opts.Tail = tail | ||
} | ||
|
||
err = client.Logs(opts) | ||
return | ||
} | ||
|
||
// ErrNoContainersInPod is returned when there are no containers for a given pod | ||
var ErrNoContainersInPod = errors.New("no containers exist for this pod") | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: all the other methods are verbs - GetLogs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dockerclient does not implement GetLogs method.
This interface is defining methods which are going to be used from the DockerClient, so they must have the same name as in the DockerClient.