Skip to content
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

Smarter API version handling #1060

Closed
rhuss opened this issue Jul 31, 2018 · 6 comments
Closed

Smarter API version handling #1060

rhuss opened this issue Jul 31, 2018 · 6 comments
Assignees

Comments

@rhuss
Copy link
Collaborator

rhuss commented Jul 31, 2018

Description

As described in fabric8io/fabric8-maven-plugin#1200 the minimal version 1.18 used by d-m-p gets rejected for newer Docker daemons (e.g. those having 1.35).

Actually we are compliant with the newer versions (its a minimal version after all), so I see two solutions:

  • If possible, we don't send an API version. However when we use features which require a minimal version of 1.21 this still will still break.

  • We send a test request to the Daemon, and check whether it succeeds. In the return response there's a header Api-Version: which holds the actual Docker daemon version. So if this test request because of a too low version, we can just pick up that version and use it as minimal version for us.

@rohanKanojia
Copy link
Member

@rhuss : Could you please give some code pointer related to this?

@rhuss
Copy link
Collaborator Author

rhuss commented Jul 31, 2018

During initialization we check which features are used and then calculate the minimal version required:

this.minimalApiVersion = initImageConfiguration(getBuildTimestamp());

If nothing special is used its, the default of 1.18 for now.The docker client will be created with this version

String version = dockerAccessContext.getMinimalApiVersion() != null ? dockerAccessContext.getMinimalApiVersion() : API_VERSION;
DockerAccess access = new DockerAccessWithHcClient("v" + version, connectionParam.getUrl(),

Then, for each request this version is send to the daemon

return String.format("%s/%s/%s", baseUrl, apiVersion, path);

Actually when we get back an error that indicates that's a version mismatch like in the first post, we should adapt the minimal version to that provided in the response header.

Alternatively, and actually i like that better, we could send a single request to some version endpoint and pick up the api version. If this version is larger then our minimal version we just use that for our minimal version. otherwise we throw an error that we can't talk to the daemon (as our minimal version is higher of what the daemon provides).

@rohanKanojia rohanKanojia self-assigned this Aug 9, 2018
rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Sep 28, 2018
Send a test request to the Daemon, and check whether it succeeds. In the return response
there's a header Api-Version: which holds the actual Docker daemon version. If this test
request because of a too low version, we can just pick up that version and use it as
minimal version for us.
rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Oct 1, 2018
Send a test request to the Daemon, and check whether it succeeds. In the return response
there's a header Api-Version: which holds the actual Docker daemon version. If this test
request because of a too low version, we can just pick up that version and use it as
minimal version for us.
rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Oct 1, 2018
Send a test request to the Daemon, and check whether it succeeds. In the return response
there's a header Api-Version: which holds the actual Docker daemon version. If this test
request because of a too low version, we can just pick up that version and use it as
minimal version for us.
rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Oct 8, 2018
Send a test request to the Daemon, and check whether it succeeds. In the return response
there's a header Api-Version: which holds the actual Docker daemon version. If this test
request because of a too low version, we can just pick up that version and use it as
minimal version for us.
rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Oct 16, 2018
Send a test request to the Daemon, and check whether it succeeds. In the return response
there's a header Api-Version: which holds the actual Docker daemon version. If this test
request because of a too low version, we can just pick up that version and use it as
minimal version for us.
rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Dec 12, 2018
Send a test request to the Daemon, and check whether it succeeds. In the return response
there's a header Api-Version: which holds the actual Docker daemon version. If this test
request because of a too low version, we can just pick up that version and use it as
minimal version for us.
rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Dec 13, 2018
Send a test request to the Daemon, and check whether it succeeds. In the return response
there's a header Api-Version: which holds the actual Docker daemon version. If this test
request because of a too low version, we can just pick up that version and use it as
minimal version for us.
rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Dec 13, 2018
Send a test request to the Daemon, and check whether it succeeds. In the return response
there's a header Api-Version: which holds the actual Docker daemon version. If this test
request because of a too low version, we can just pick up that version and use it as
minimal version for us.
@rhuss rhuss closed this as completed in 7c2d535 Dec 13, 2018
@JoshLitt
Copy link

I know this is closed, but I just started using this plugin against an older Docker version (1.12.5, with Api-Version 1.24), and the fix here isn't good. Using 0.27.1 works OK. The issue is that DockerAccessWithHcClient.fetchApiVersionFromServer() assumes it will get an Api-Version: header. This was introduced at API version 1.25. Not sure if it was intentional to limit support to 1.25.

Since there is already code in DockerAccessWithHcClient to get the version from the JSON response, it's a bit odd that the plugin only works when the version is in the header.

@rohanKanojia
Copy link
Member

@JoshLitt : ah, apologies for the inconvenience. You're right, this change wasn't supposed to break backward compatibility. We should also handle the case when there is no apiVersion specified in header

rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Jan 13, 2019
Since Api-Version header was introduced in 1.25, we should send a minimal api-version
rather than just sending null. See this comment:
   fabric8io#1060 (comment)
rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Jan 13, 2019
Since Api-Version header was introduced in 1.25, we should send a minimal api-version
rather than just sending null. See this comment:
   fabric8io#1060 (comment)
rhuss pushed a commit that referenced this issue Jan 25, 2019
)

Since Api-Version header was introduced in 1.25, we should send a minimal api-version
rather than just sending null. See this comment:
   #1060 (comment)
@rhuss
Copy link
Collaborator Author

rhuss commented Jan 25, 2019

@JoshLitt should be fixed now with the next release (see later PR merged)

@dantran
Copy link

dantran commented Jun 20, 2019

I just ran into same issue with d-m-p 0.28 to 0.30 running against latest docker windows

How do fix this? my apology for not seeing the obvious in this discussion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants