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

[17.06] Set ping version even on error #115

Merged
merged 2 commits into from Jul 13, 2017
Jump to file or symbol
Failed to load files and symbols.
+132 −33
Diff settings

Always

Just for now

Next

Set ping version even on error

In some cases a server may return an error on the ping response but
still provide version details. The client should use these values when
available.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit 27ef09a)
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
  • Loading branch information...
cpuguy83 authored and andrewhsu committed Jun 26, 2017
commit 10aa812ef5717df07055fddcd53ca7f0e830592b
@@ -20,13 +20,15 @@ func (cli *Client) Ping(ctx context.Context) (types.Ping, error) {
}
defer ensureReaderClosed(serverResp)
ping.APIVersion = serverResp.header.Get("API-Version")
if serverResp.header != nil {
ping.APIVersion = serverResp.header.Get("API-Version")
if serverResp.header.Get("Docker-Experimental") == "true" {
ping.Experimental = true
if serverResp.header.Get("Docker-Experimental") == "true" {
ping.Experimental = true
}
ping.OSType = serverResp.header.Get("OSType")
}
ping.OSType = serverResp.header.Get("OSType")
return ping, nil
err = cli.checkResponseErr(serverResp)
return ping, err
}
@@ -0,0 +1,82 @@
package client
import (
"errors"
"io/ioutil"
"net/http"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"golang.org/x/net/context"
)
// TestPingFail tests that when a server sends a non-successful response that we
// can still grab API details, when set.
// Some of this is just excercising the code paths to make sure there are no
// panics.
func TestPingFail(t *testing.T) {
var withHeader bool
client := &Client{
client: newMockClient(func(req *http.Request) (*http.Response, error) {
resp := &http.Response{StatusCode: http.StatusInternalServerError}
if withHeader {
resp.Header = http.Header{}
resp.Header.Set("API-Version", "awesome")
resp.Header.Set("Docker-Experimental", "true")
}
resp.Body = ioutil.NopCloser(strings.NewReader("some error with the server"))
return resp, nil
}),
}
ping, err := client.Ping(context.Background())
assert.Error(t, err)
assert.Equal(t, false, ping.Experimental)
assert.Equal(t, "", ping.APIVersion)
withHeader = true
ping2, err := client.Ping(context.Background())
assert.Error(t, err)
assert.Equal(t, true, ping2.Experimental)
assert.Equal(t, "awesome", ping2.APIVersion)
}
// TestPingWithError tests the case where there is a protocol error in the ping.
// This test is mostly just testing that there are no panics in this code path.
func TestPingWithError(t *testing.T) {
client := &Client{
client: newMockClient(func(req *http.Request) (*http.Response, error) {
resp := &http.Response{StatusCode: http.StatusInternalServerError}
resp.Header = http.Header{}
resp.Header.Set("API-Version", "awesome")
resp.Header.Set("Docker-Experimental", "true")
resp.Body = ioutil.NopCloser(strings.NewReader("some error with the server"))
return resp, errors.New("some error")
}),
}
ping, err := client.Ping(context.Background())
assert.Error(t, err)
assert.Equal(t, false, ping.Experimental)
assert.Equal(t, "", ping.APIVersion)
}
// TestPingSuccess tests that we are able to get the expected API headers/ping
// details on success.
func TestPingSuccess(t *testing.T) {
client := &Client{
client: newMockClient(func(req *http.Request) (*http.Response, error) {
resp := &http.Response{StatusCode: http.StatusInternalServerError}
resp.Header = http.Header{}
resp.Header.Set("API-Version", "awesome")
resp.Header.Set("Docker-Experimental", "true")
resp.Body = ioutil.NopCloser(strings.NewReader("some error with the server"))
return resp, nil
}),
}
ping, err := client.Ping(context.Background())
assert.Error(t, err)
assert.Equal(t, true, ping.Experimental)
assert.Equal(t, "awesome", ping.APIVersion)
}
@@ -24,6 +24,7 @@ type serverResponse struct {
body io.ReadCloser
header http.Header
statusCode int
reqURL *url.URL
}
// head sends an http request to the docker API using the method HEAD.
@@ -118,11 +119,18 @@ func (cli *Client) sendRequest(ctx context.Context, method, path string, query u
if err != nil {
return serverResponse{}, err
}
return cli.doRequest(ctx, req)
resp, err := cli.doRequest(ctx, req)
if err != nil {
return resp, err
}
if err := cli.checkResponseErr(resp); err != nil {
return resp, err
}
return resp, nil
}
func (cli *Client) doRequest(ctx context.Context, req *http.Request) (serverResponse, error) {
serverResp := serverResponse{statusCode: -1}
serverResp := serverResponse{statusCode: -1, reqURL: req.URL}
resp, err := ctxhttp.Do(ctx, cli.client, req)
if err != nil {
@@ -179,35 +187,42 @@ func (cli *Client) doRequest(ctx context.Context, req *http.Request) (serverResp
if resp != nil {
serverResp.statusCode = resp.StatusCode
serverResp.body = resp.Body
serverResp.header = resp.Header
}
return serverResp, nil
}
if serverResp.statusCode < 200 || serverResp.statusCode >= 400 {
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
return serverResp, err
}
if len(body) == 0 {
return serverResp, fmt.Errorf("Error: request returned %s for API route and version %s, check if the server supports the requested API version", http.StatusText(serverResp.statusCode), req.URL)
}
func (cli *Client) checkResponseErr(serverResp serverResponse) error {
if serverResp.statusCode >= 200 && serverResp.statusCode < 400 {
return nil
}
var errorMessage string
if (cli.version == "" || versions.GreaterThan(cli.version, "1.23")) &&
resp.Header.Get("Content-Type") == "application/json" {
var errorResponse types.ErrorResponse
if err := json.Unmarshal(body, &errorResponse); err != nil {
return serverResp, fmt.Errorf("Error reading JSON: %v", err)
}
errorMessage = errorResponse.Message
} else {
errorMessage = string(body)
}
body, err := ioutil.ReadAll(serverResp.body)
if err != nil {
return err
}
if len(body) == 0 {
return fmt.Errorf("Error: request returned %s for API route and version %s, check if the server supports the requested API version", http.StatusText(serverResp.statusCode), serverResp.reqURL)
}
return serverResp, fmt.Errorf("Error response from daemon: %s", strings.TrimSpace(errorMessage))
var ct string
if serverResp.header != nil {
ct = serverResp.header.Get("Content-Type")
}
serverResp.body = resp.Body
serverResp.header = resp.Header
return serverResp, nil
var errorMessage string
if (cli.version == "" || versions.GreaterThan(cli.version, "1.23")) && ct == "application/json" {
var errorResponse types.ErrorResponse
if err := json.Unmarshal(body, &errorResponse); err != nil {
return fmt.Errorf("Error reading JSON: %v", err)
}
errorMessage = errorResponse.Message
} else {
errorMessage = string(body)
}
return fmt.Errorf("Error response from daemon: %s", strings.TrimSpace(errorMessage))
}
func (cli *Client) addHeaders(req *http.Request, headers headers) *http.Request {
@@ -239,9 +254,9 @@ func encodeData(data interface{}) (*bytes.Buffer, error) {
}
func ensureReaderClosed(response serverResponse) {
if body := response.body; body != nil {
if response.body != nil {
// Drain up to 512 bytes and close the body to let the Transport reuse the connection
io.CopyN(ioutil.Discard, body, 512)
io.CopyN(ioutil.Discard, response.body, 512)
response.body.Close()
}
}
ProTip! Use n and p to navigate between commits in a pull request.