Add a DOCKER_API_VERSION env var #15964

duglin · 2015-08-31T21:46:53Z

Closes: #11486

Just for @ahmetalpbalkan :-)

Added DOCKER_API_VERSION env var and ApiVersion property in docker's config.json.

Cleaned up some comments that were > 80 chars.

Signed-off-by: Doug Davis dug@us.ibm.com

thaJeztah · 2015-08-31T21:54:08Z

@duglin would this be something the client-config could be useful for?

duglin · 2015-08-31T22:03:08Z

@thaJeztah good point - forgot that it should be an option in there too... will work on that in a bit.

ahmetb · 2015-08-31T22:16:48Z

thaJeztah · 2015-08-31T22:18:17Z

Awwwwwwww, LOL

duglin · 2015-09-01T01:59:40Z

ok added the config file property.

thaJeztah · 2015-09-01T05:39:29Z

docs/reference/commandline/cli.md

@@ -105,7 +112,8 @@ Following is a sample `config.json` file:
      "HttpHeaders": {
        "MyHeader": "MyValue"
      },
-      "psFormat": "table {{.ID}}\\t{{.Image}}\\t{{.Command}}\\t{{.Labels}}"
+      "psFormat": "table {{.ID}}\\t{{.Image}}\\t{{.Command}}\\t{{.Labels}}",
+	  "ApiVersion": "1.19"


This could be just GitHub, but the indentation looks off here (tabs/spaces?)

yea there was a tab in there - fixed.

thaJeztah · 2015-09-01T05:56:34Z

Thanks @duglin! Added some comments

calavera · 2015-09-01T07:40:46Z

If we do this, I think we should check that the version set by the user is lower than the client version, just in case.

calavera · 2015-09-01T07:41:38Z

I'm not completely fond of the env var, but I understand that it can be useful for automation.

duglin · 2015-09-01T11:56:08Z

aside from the 'XXX' string I mentioned before, I can see using this to test some v "2.0" APIs before they're officially in the build (e.g. I only have it enabled for certain APIs so I don't want to change ALL docker calls to 2.0 - just some via env var). So checking for a version string <= current version would be too limiting.

tiborvass · 2015-09-04T00:07:41Z

I don't understand the usecase for an envvar

ahmetb · 2015-09-04T00:10:27Z

@tiborvass head over to #11486.

icecrime · 2015-10-08T18:26:08Z

Sorry, meant to comment on the PR, please see here: #11486 (comment)

icecrime · 2015-10-27T22:15:31Z

Still very much not convinced... But I guess: design LGTM assuming this remains a debugging tool.

tiborvass · 2015-10-30T01:26:19Z

I found a usecase: allow people to have a quick workaround for scripts using the docker CLI and that are expecting output from older API versions such as for docker inspect -f ... that is always dependent on the latest API version.

tiborvass · 2015-10-30T01:37:02Z

I would be fine with the envvar, but I don't see a need for the docker config.

tiborvass · 2015-12-07T15:45:08Z

@duglin I suggest to have a Version field in api/client.DockerCli and also one of the following:

Keep the Version constant in api/common.go, and set DockerCli.Version's default to api.Version
Same as 1, but rename api.Version to api.ServerVersion or api.DefaultVersion
Move Version constant to api/server, and set the default of DockerCli.Version manually to the same number as api/server.Version, but add a test to make sure the defaults are always equal for the server and the client.

Closes: moby#11486 Just for @ahmetalpbalkan :-) Fixed some comment formatting too while in there. Signed-off-by: Doug Davis <dug@us.ibm.com>

duglin · 2015-12-14T21:26:27Z

@tiborvass I went with your 2nd option. Option 3 was really tempting but I couldn't bring myself to define the same value twice at this time. If we ever do move the cli into a separate repo then option 3 might make more sense.

However, as for option 2 (and 1), I came across an example of why this still bugs me. Look at:
https://github.com/docker/docker/pull/15964/files#diff-1d801b8b2dcffcde77b5995194bb3171L60
Notice I switched the reference from api.Version to use the cli's version. Sounds good (I hope). However, I think its important to ask how I knew to even change that at all? This PR was all about changing the URL's version string, which means I could have very well just changed the stuff here: https://github.com/docker/docker/pull/15964/files#diff-bcf4431eed51e55b4dca7deafdca48b0L106 and its possible the PR would have been merged and @ahmetalpbalkan would have been happy.

But, I just happen to know/remember that docker version also printed a version string. So, if no reviewer happened to remember that api.Version was use in this other spot then we would have missed this use of api.Version. Having the same info available at two locations is asking for this type of situation to happen again. Maybe it'll never happen, and maybe the next time someone adds a client-side reference to api.DefaultVersion someone will hopefully remember to use the cli.client.ClientVersion() method instead, but counting on hope makes me uncomfortable. But, its time to move on....

@calavera ready for review

calavera · 2015-12-14T21:59:48Z

I like it. Thanks @duglin.

LGTM.

runcom · 2015-12-16T12:18:04Z

This will allow anyone to have docker installed on their systemd (say 1.10-dev) and a docker machine with 1.9.1. This way we won't get anymore Error response from daemon: client is newer than server (client API version: 1.22, server API version: 1.21)

LGTM, moving to doc review
ping @thaJeztah

thaJeztah · 2015-12-16T12:29:41Z

docs/reference/commandline/cli.md

@@ -38,6 +38,7 @@ the [installation](../../installation/index.md) instructions for your operating
 For easy reference, the following list of environment variables are supported
 by the `docker` command line:

+* `DOCKER_API_VERSION` The API version to use (e.g. `1.19`)


Per earlier discussions (e.g., "advanced/power user feature", no validation of the format, overriding the API version can lead to unexpected results), should we add a warning here?

Don't think its necessary since people would need to go out of their way to make this happen so they should already know the risks.

Besides, I wouldn't want the warning message to interfere with any expected output.

Oh, sorry, didn't mean a warning on the command line (that was discussed), but something in the docs. Do you think that's redundant?

yea, I think it would be redundant.

tiborvass · 2015-12-16T13:01:22Z

@duglin can't we write a test (unit?) to ensure the two versions are the same?

duglin · 2015-12-16T13:17:52Z

@tiborvass I'm not sure how a unit test will help find uses of api.DefaultVersion that should really be cli.client.ClientVersion().

thaJeztah · 2015-12-16T14:03:57Z

Alright, let's not uphold this PR by additional warnings in the docs.

docs LGTM

calavera · 2015-12-16T22:23:35Z

docs, (or lack of them ) LGTM. Merging 🎉

Add a DOCKER_API_VERSION env var

ahmetb · 2015-12-16T23:15:16Z

yayy! thanks y'all

runcom · 2015-12-17T06:43:33Z

🎉

GordonTheTurtle added the status/0-triage label Aug 31, 2015

duglin mentioned this pull request Aug 31, 2015

Proposal: Override ApiVersion in client #11486

Closed

duglin force-pushed the APIVersion branch 2 times, most recently from 642d8ac to cf20009 Compare September 1, 2015 00:22

thaJeztah reviewed Sep 1, 2015
View reviewed changes

calavera added status/1-design-review and removed status/0-triage labels Sep 1, 2015

duglin force-pushed the APIVersion branch 2 times, most recently from 2e6a39c to dd4d6b5 Compare September 1, 2015 11:53

This was referenced Sep 8, 2015

Expose the CLI reference pages a bit better from the index docker/compose#1966

Merged

Docker version drift docker/machine#1839

Closed

thaJeztah mentioned this pull request Oct 30, 2015

In windows can't use arrow navigation exec bash into container #13817

Closed

duglin force-pushed the APIVersion branch from db6bee7 to 6d49eb1 Compare November 30, 2015 18:40

duglin force-pushed the APIVersion branch from 6d49eb1 to 30c12e9 Compare December 14, 2015 20:38

Add a DOCKER_API_VERSION env var

6287ec9

Closes: moby#11486 Just for @ahmetalpbalkan :-) Fixed some comment formatting too while in there. Signed-off-by: Doug Davis <dug@us.ibm.com>

duglin force-pushed the APIVersion branch from 30c12e9 to 6287ec9 Compare December 14, 2015 20:45

runcom added status/3-docs-review and removed status/2-code-review labels Dec 16, 2015

thaJeztah reviewed Dec 16, 2015
View reviewed changes

calavera added a commit that referenced this pull request Dec 16, 2015

Merge pull request #15964 from duglin/APIVersion

905f333

Add a DOCKER_API_VERSION env var

calavera merged commit 905f333 into moby:master Dec 16, 2015

thaJeztah added the impact/changelog label Dec 16, 2015

thaJeztah added this to the 1.10 milestone Dec 16, 2015

thaJeztah mentioned this pull request Dec 25, 2015

ARM + PC mix Error response from daemon: client is newer than server docker-archive/classicswarm#1344

Closed

thaJeztah mentioned this pull request Jan 13, 2016

Update response content for volume mounts #19252

Merged

ikedaj mentioned this pull request Feb 16, 2016

error: can't connect to unix:///docker.sock dreamcat4/docker-images#17

Closed

thaJeztah mentioned this pull request Feb 18, 2016

Support multiple versions of the client protocol in the cli #20405

Closed

duglin deleted the APIVersion branch February 19, 2016 19:27

schmunk42 mentioned this pull request Apr 26, 2016

Allow installation of a specific version via generic driver docker/machine#3374

Open

thaJeztah mentioned this pull request Nov 8, 2016

allow client to talk to an older server #27745

Merged

thaJeztah mentioned this pull request Dec 14, 2016

Removal of the -f flags in docker tag break compatibility with server 1.9.x #29410

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a DOCKER_API_VERSION env var #15964

Add a DOCKER_API_VERSION env var #15964

duglin commented Aug 31, 2015

thaJeztah commented Aug 31, 2015

duglin commented Aug 31, 2015

ahmetb commented Aug 31, 2015

thaJeztah commented Aug 31, 2015

duglin commented Sep 1, 2015

thaJeztah Sep 1, 2015

duglin Sep 1, 2015

thaJeztah commented Sep 1, 2015

calavera commented Sep 1, 2015

calavera commented Sep 1, 2015

duglin commented Sep 1, 2015

tiborvass commented Sep 4, 2015

ahmetb commented Sep 4, 2015

icecrime commented Oct 8, 2015

icecrime commented Oct 27, 2015

tiborvass commented Oct 30, 2015

tiborvass commented Oct 30, 2015

tiborvass commented Dec 7, 2015

duglin commented Dec 14, 2015

calavera commented Dec 14, 2015

runcom commented Dec 16, 2015

thaJeztah Dec 16, 2015

duglin Dec 16, 2015

thaJeztah Dec 16, 2015

duglin Dec 16, 2015

tiborvass commented Dec 16, 2015

duglin commented Dec 16, 2015

thaJeztah commented Dec 16, 2015

calavera commented Dec 16, 2015

ahmetb commented Dec 16, 2015

runcom commented Dec 17, 2015

Add a DOCKER_API_VERSION env var #15964

Add a DOCKER_API_VERSION env var #15964

Conversation

duglin commented Aug 31, 2015

thaJeztah commented Aug 31, 2015

duglin commented Aug 31, 2015

ahmetb commented Aug 31, 2015

thaJeztah commented Aug 31, 2015

duglin commented Sep 1, 2015

thaJeztah Sep 1, 2015

Choose a reason for hiding this comment

duglin Sep 1, 2015

Choose a reason for hiding this comment

thaJeztah commented Sep 1, 2015

calavera commented Sep 1, 2015

calavera commented Sep 1, 2015

duglin commented Sep 1, 2015

tiborvass commented Sep 4, 2015

ahmetb commented Sep 4, 2015

icecrime commented Oct 8, 2015

icecrime commented Oct 27, 2015

tiborvass commented Oct 30, 2015

tiborvass commented Oct 30, 2015

tiborvass commented Dec 7, 2015

duglin commented Dec 14, 2015

calavera commented Dec 14, 2015

runcom commented Dec 16, 2015

thaJeztah Dec 16, 2015

Choose a reason for hiding this comment

duglin Dec 16, 2015

Choose a reason for hiding this comment

thaJeztah Dec 16, 2015

Choose a reason for hiding this comment

duglin Dec 16, 2015

Choose a reason for hiding this comment

tiborvass commented Dec 16, 2015

duglin commented Dec 16, 2015

thaJeztah commented Dec 16, 2015

calavera commented Dec 16, 2015

ahmetb commented Dec 16, 2015

runcom commented Dec 17, 2015