CLI should honor API version header when _ping returns unsuccessful status code

[aaronlehmann]

When _ping returns a non-200 HTTP status code, the CLI defaults to the latest API version. This can result in errors like:

your client is too new (API version 1.27). The newest supported API version is 1.26.0

...when the CLI moves on to performing the actual operation.

The CLI should still honor the version header, so _ping returning a bad status code doesn't prevent the CLI from working at all. Right now the client's Ping method just returns an error, but it should still return the version information in this case. Maybe the HTTP status code should be moved to a field on types.Ping instead of triggering an error.

I ran into this issue running the CLI against Docker Datacenter. Its _ping endpoint was returning a 500 status code because the cluster was considered unhealthy. But this caused the CLI to use a newer API version than was supported, so CLI commands didn't work at all while the cluster was in this state.

[vdemeester]

@aaronlehmann isn't this already fix in master ? 👼

[aaronlehmann]

I believe there is a fix in moby's client package, but I don't know if cli has been updated with it yet.

[aaronlehmann]

The vendored client library was updated 366d3ec. However, I think some CLI changes are necessary. The CLI appears to only use the ping value when Ping does not return an error:

        if ping, err := cli.client.Ping(context.Background()); err == nil {
                cli.server = ServerInfo{
                        HasExperimental: ping.Experimental,
                        OSType:          ping.OSType,
                }

                cli.client.NegotiateAPIVersionPing(ping)
        } else {
                // Default to true if we fail to connect to daemon
                cli.server = ServerInfo{HasExperimental: true}
        }

It needs to be updated to call NegotiateAPIVersionPing even when Ping returns an error. I think the following would be correct:

        if ping, err := cli.client.Ping(context.Background()); err == nil {
                cli.server = ServerInfo{
                        HasExperimental: ping.Experimental,
                        OSType:          ping.OSType,
                }

                cli.client.NegotiateAPIVersionPing(ping)
        } else {
                // Default to true if we fail to connect to daemon
                cli.server = ServerInfo{HasExperimental: true}

                // An API version may be returned, even on error. Use it
                // if it is present. Otherwise, do not call
                // NegotiateAPIVersionPing, because this will revert to
                // an API version that predates the versioning headers.
                if ping.APIVersion != "" {
                        cli.client.NegotiateAPIVersionPing(ping)
                }
        }