From 69468bf42fd033d02c4509f6df42efd8505cd53a Mon Sep 17 00:00:00 2001 From: Nathan LeClaire Date: Mon, 9 Jan 2017 18:08:00 -0800 Subject: [PATCH 1/3] Begin adding system container log support to diagnostics Signed-off-by: Nathan LeClaire --- alpine/packages/diagnostics/capture.go | 3 +- alpine/packages/diagnostics/http.go | 53 ++++++------ .../diagnostics/system_log_capture.go | 84 +++++++++++++++++++ 3 files changed, 115 insertions(+), 25 deletions(-) create mode 100644 alpine/packages/diagnostics/system_log_capture.go diff --git a/alpine/packages/diagnostics/capture.go b/alpine/packages/diagnostics/capture.go index 85685c48f..8f1c31c35 100644 --- a/alpine/packages/diagnostics/capture.go +++ b/alpine/packages/diagnostics/capture.go @@ -15,7 +15,8 @@ import ( ) const ( - defaultCommandTimeout = 5 * time.Second + defaultCaptureTimeout = 5 * time.Second + defaultCommandTimeout = defaultCaptureTimeout // Might eventually have some pretty long (~30s) traces in here, so 35 // seconds seems reasonable. diff --git a/alpine/packages/diagnostics/http.go b/alpine/packages/diagnostics/http.go index 588b9c28c..42f76e5d2 100644 --- a/alpine/packages/diagnostics/http.go +++ b/alpine/packages/diagnostics/http.go @@ -19,7 +19,7 @@ import ( ) var ( - errDockerPingNotOK = errors.New("Docker /_ping did not return OK") + errDockerRespNotOK = errors.New("Docker API call did not return OK") ) const ( @@ -39,12 +39,38 @@ func init() { for _, c := range commonCmdCaptures { cloudCaptures = append(cloudCaptures, c) } + + cloudCaptures = append(cloudCaptures, SystemContainerCapturer{}) } // HTTPDiagnosticListener sets a health check and optional diagnostic endpoint // for cloud editions. type HTTPDiagnosticListener struct{} +func dockerHTTPGet(ctx context.Context, url string) (*http.Response, error) { + client := &http.Client{ + Transport: &UnixSocketRoundTripper{}, + } + + req, err := http.NewRequest(http.MethodGet, url, nil) + if err != nil { + return nil, err + } + + req = req.WithContext(ctx) + + resp, err := client.Do(req) + if err != nil { + return resp, err + } + + if resp.StatusCode != http.StatusOK { + return resp, errDockerRespNotOK + } + + return resp, err +} + // UnixSocketRoundTripper provides a way to make HTTP request to Docker socket // directly. type UnixSocketRoundTripper struct{} @@ -63,35 +89,14 @@ func (u UnixSocketRoundTripper) RoundTrip(req *http.Request) (*http.Response, er // Listen starts the HTTPDiagnosticListener and sets up handlers for its endpoints func (h HTTPDiagnosticListener) Listen() { http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - client := &http.Client{ - Transport: &UnixSocketRoundTripper{}, - } - - req, err := http.NewRequest(http.MethodGet, "/_ping", nil) - if err != nil { - http.Error(w, "Error creating HTTP request to talk to Docker", http.StatusInternalServerError) - return - } - ctx, cancel := context.WithTimeout(context.Background(), healthcheckTimeout) defer cancel() - req = req.WithContext(ctx) - errCh := make(chan error) go func() { - resp, err := client.Do(req) - if err != nil { - errCh <- err - return - } - defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - errCh <- errDockerPingNotOK - return - } - errCh <- nil + _, err := dockerHTTPGet(ctx, "/_ping") + errCh <- err }() select { diff --git a/alpine/packages/diagnostics/system_log_capture.go b/alpine/packages/diagnostics/system_log_capture.go new file mode 100644 index 000000000..a513ccc58 --- /dev/null +++ b/alpine/packages/diagnostics/system_log_capture.go @@ -0,0 +1,84 @@ +package main + +import ( + "archive/tar" + "bytes" + "context" + "encoding/json" + "io/ioutil" + "log" +) + +const ( + systemLogDir = "editionslogs" + systemContainerLabel = "system" +) + +// SystemContainerCapturer gets the logs from containers which are run +// specifically for Docker Editions. +type SystemContainerCapturer struct{} + +// Capture writes output from a CommandCapturer to a tar archive +func (s SystemContainerCapturer) Capture(parentCtx context.Context, w *tar.Writer) { + done := make(chan struct{}) + + ctx, cancel := context.WithTimeout(context.Background(), defaultCaptureTimeout) + defer cancel() + + go func() { + resp, err := dockerHTTPGet(ctx, "/containers/json?all=1&label="+systemContainerLabel) + if err != nil { + log.Println("ERROR:", err) + return + } + + defer resp.Body.Close() + + names := []struct { + ID string `json:"id"` + Names []string + }{} + + if err := json.NewDecoder(resp.Body).Decode(&names); err != nil { + log.Println("ERROR:", err) + return + } + + for _, c := range names { + resp, err := dockerHTTPGet(ctx, "/containers/"+c.ID+"/logs?stderr=1&stdout=1×tamps=1") + if err != nil { + log.Println("ERROR:", err) + continue + } + + defer resp.Body.Close() + + logLines, err := ioutil.ReadAll(resp.Body) + if err != nil { + log.Println("ERROR:", err) + continue + } + + // Docker API returns a Names array where the names are + // like this: ["/foobar", "/quux"] + // + // Use the first one from it. + // + // Additionally, the slash from it helps delimit a path + // separator in the tar archive. + // + // TODO(nathanleclaire): This seems fragile, but I'm + // not sure what approach would be much better. + tarWrite(w, bytes.NewBuffer(logLines), systemLogDir+c.Names[0]) + } + + done <- struct{}{} + }() + + select { + case <-ctx.Done(): + log.Println("System container log capture error", ctx.Err()) + case <-done: + log.Println("System container log capture finished") + } +} From 1ca9096f5555ad9d6d6b93e38d464df634d38fa6 Mon Sep 17 00:00:00 2001 From: Nathan LeClaire Date: Tue, 10 Jan 2017 10:48:02 -0800 Subject: [PATCH 2/3] Fix streaming API request error Signed-off-by: Nathan LeClaire --- alpine/packages/diagnostics/http.go | 32 +++++++++++++-- .../diagnostics/system_log_capture.go | 39 +++++++++++++------ 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/alpine/packages/diagnostics/http.go b/alpine/packages/diagnostics/http.go index 42f76e5d2..be37464d6 100644 --- a/alpine/packages/diagnostics/http.go +++ b/alpine/packages/diagnostics/http.go @@ -52,6 +52,10 @@ func dockerHTTPGet(ctx context.Context, url string) (*http.Response, error) { Transport: &UnixSocketRoundTripper{}, } + return dockerHTTPGetWithClient(ctx, url, client) +} + +func dockerHTTPGetWithClient(ctx context.Context, url string, client *http.Client) (*http.Response, error) { req, err := http.NewRequest(http.MethodGet, url, nil) if err != nil { return nil, err @@ -69,11 +73,24 @@ func dockerHTTPGet(ctx context.Context, url string) (*http.Response, error) { } return resp, err + } // UnixSocketRoundTripper provides a way to make HTTP request to Docker socket // directly. -type UnixSocketRoundTripper struct{} +type UnixSocketRoundTripper struct { + Stream bool + conn *httputil.ClientConn +} + +// Close will close the connection if the caller needs to clean up after +// themselves in a streaming request. +func (u UnixSocketRoundTripper) Close() error { + if u.conn != nil { + return u.conn.Close() + } + return nil +} // RoundTrip dials the Docker UNIX socket to make a HTTP request. func (u UnixSocketRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { @@ -81,9 +98,16 @@ func (u UnixSocketRoundTripper) RoundTrip(req *http.Request) (*http.Response, er if err != nil { return nil, err } - conn := httputil.NewClientConn(dial, nil) - defer conn.Close() - return conn.Do(req) + u.conn = httputil.NewClientConn(dial, nil) + + // If the client makes a streaming request (e.g., /container/x/logs) + // it's their responsibility to close the connection, because it needs + // to remain open to stream the response body. + if !u.Stream { + defer u.conn.Close() + } + + return u.conn.Do(req) } // Listen starts the HTTPDiagnosticListener and sets up handlers for its endpoints diff --git a/alpine/packages/diagnostics/system_log_capture.go b/alpine/packages/diagnostics/system_log_capture.go index a513ccc58..e6515ea69 100644 --- a/alpine/packages/diagnostics/system_log_capture.go +++ b/alpine/packages/diagnostics/system_log_capture.go @@ -7,6 +7,7 @@ import ( "encoding/json" "io/ioutil" "log" + "net/http" ) const ( @@ -20,15 +21,15 @@ type SystemContainerCapturer struct{} // Capture writes output from a CommandCapturer to a tar archive func (s SystemContainerCapturer) Capture(parentCtx context.Context, w *tar.Writer) { - done := make(chan struct{}) - ctx, cancel := context.WithTimeout(context.Background(), defaultCaptureTimeout) defer cancel() + errCh := make(chan error) + go func() { resp, err := dockerHTTPGet(ctx, "/containers/json?all=1&label="+systemContainerLabel) if err != nil { - log.Println("ERROR:", err) + errCh <- err return } @@ -40,22 +41,35 @@ func (s SystemContainerCapturer) Capture(parentCtx context.Context, w *tar.Write }{} if err := json.NewDecoder(resp.Body).Decode(&names); err != nil { - log.Println("ERROR:", err) + errCh <- err return } for _, c := range names { - resp, err := dockerHTTPGet(ctx, "/containers/"+c.ID+"/logs?stderr=1&stdout=1×tamps=1") + transport := &UnixSocketRoundTripper{ + Stream: true, + } + + client := &http.Client{ + Transport: transport, + } + + resp, err := dockerHTTPGetWithClient(ctx, "/containers/"+c.ID+"/logs?stderr=1&stdout=1×tamps=1&tail=all", client) if err != nil { - log.Println("ERROR:", err) + log.Println("ERROR (get request):", err) continue } defer resp.Body.Close() + // logs makes streaming request where the original http + // conn is left open so we must clean up after + // ourselves when we're done reading + defer transport.Close() + logLines, err := ioutil.ReadAll(resp.Body) if err != nil { - log.Println("ERROR:", err) + log.Println("ERROR (reading response):", err) continue } @@ -72,13 +86,16 @@ func (s SystemContainerCapturer) Capture(parentCtx context.Context, w *tar.Write tarWrite(w, bytes.NewBuffer(logLines), systemLogDir+c.Names[0]) } - done <- struct{}{} + errCh <- nil }() select { case <-ctx.Done(): - log.Println("System container log capture error", ctx.Err()) - case <-done: - log.Println("System container log capture finished") + log.Println("System container log capture context error", ctx.Err()) + case err := <-errCh: + if err != nil { + log.Println("System container log capture error", err) + } + log.Println("System container log capture finished successfully") } } From e1026a1f954a1d85591c77eff3529634c55941fd Mon Sep 17 00:00:00 2001 From: Nathan LeClaire Date: Tue, 10 Jan 2017 13:29:47 -0800 Subject: [PATCH 3/3] Change system container label Signed-off-by: Nathan LeClaire --- alpine/packages/diagnostics/system_log_capture.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/alpine/packages/diagnostics/system_log_capture.go b/alpine/packages/diagnostics/system_log_capture.go index e6515ea69..415c8503d 100644 --- a/alpine/packages/diagnostics/system_log_capture.go +++ b/alpine/packages/diagnostics/system_log_capture.go @@ -12,7 +12,7 @@ import ( const ( systemLogDir = "editionslogs" - systemContainerLabel = "system" + systemContainerLabel = "com.docker.editions.system" ) // SystemContainerCapturer gets the logs from containers which are run