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

Bump plugin API version #19549

Merged
merged 1 commit into from Jan 22, 2016
Merged

Conversation

cpuguy83
Copy link
Member

This should have been updated with the changes to volume drivers (#16534).

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@tiborvass
Copy link
Contributor

LGTM

@vdemeester
Copy link
Member

LGTM 🐭

@ibuildthecloud
Copy link
Contributor

@cpuguy83 Whoa!? What is the impact of this? By bumping the version does this mean Docker 1.10 will reject drivers of version v1.1?

@ibuildthecloud
Copy link
Contributor

Sorry guys, I'm confused on the what this version is used for? It seems to be a global "plugins" version so I don't see why the version should change if the volume API changed. The is version seems to indicate the version of the plugin handshake.

@cpuguy83
Copy link
Member Author

This should be handled by the plugin...
But pinging @calavera
This is how I was told we handle these sorts of changes -- may not be ideal especially with networking in stable and using plugins.
At the time it was just volumes.

@mavenugo
Copy link
Contributor

there has been some extra fields added to the network plugin protocol as well.
They are just additions and it will NOT affect the backward compatibility.
Just making sure if this will impact existing plugins in anyway.

@ibuildthecloud
Copy link
Contributor

Not-LGTM? I don't think this version needs to be bumped as the handshake between daemon/plugin has not changed.

@calavera
Copy link
Contributor

I'm pretty sure we don't validate this field at the moment in any ways, but I might be mistaken.

We introduced changes in the volume plugins that are not (completely) backwards compatible afaik, so we should probably bump this version.

@calavera
Copy link
Contributor

This change LGTM.

We use this header to advertise that the docker daemon "Accepts" plugins with that version, but we don't reject responses from other versions.

calavera added a commit that referenced this pull request Jan 22, 2016
@calavera calavera merged commit ae8f7c6 into moby:master Jan 22, 2016
@cpuguy83 cpuguy83 deleted the bump_plugin_api_version branch January 22, 2016 17:33
@thaJeztah
Copy link
Member

Added impact labels

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

Successfully merging this pull request may close these issues.

None yet

8 participants