Skip to content
This repository has been archived by the owner on Sep 26, 2021. It is now read-only.

Docker Machine v0.5.1 breaks compatibility with v0.5.0 drivers #2325

Closed
janeczku opened this issue Nov 18, 2015 · 9 comments
Closed

Docker Machine v0.5.1 breaks compatibility with v0.5.0 drivers #2325

janeczku opened this issue Nov 18, 2015 · 9 comments

Comments

@janeczku
Copy link
Contributor

This particularly affects use of drivers not distributed with the machine binary.

For simplicity this is demonstrated with the virtualbox driver.

Steps to reproduce

  1. $ git clone https://github.com/docker/machine.git
  2. $ cd machine && git checkout v0.5.0
  3. $ go build -a -o /usr/local/bin/docker-machine-driver-virtualbox cmd/machine-driver-virtualbox.go
  4. $ git checkout v0.5.1
  5. $ go build -a -o /usr/local/bin/docker-machine cmd/machine.go
  6. $ docker-machine create -d virtualbox

Expected result

Machine continuous to work

Actual result

Error occurs and Machine exits

WARNING >>> Error attempting heartbeat call to plugin server: rpc: can't find service RPCServerDriver.Heartbeat
Error attempting to invoke binary for plugin 'virtualbox': rpc: can't find service RPCServerDriver.GetVersion

This is probably caused by commit 89d9854 which changed the RPC API interface breaking compatibility with all drivers compiled against v0.5.0 or earlier.

Reference PR #2321
/cc @nathanleclaire @dmp42

@nathanleclaire
Copy link
Contributor

Ah, of course, the linting changes. Thanks for the report @janeczku.

I'm pretty upset about this (and I really am profusely sorry) as it wasn't intended to break compatibility but I think the linting changes are over (new PRs are being run through golint) and there is nothing we can do about it now unfortunately. In the future we will be more disciplined about not breaking the API.

@dgageot
Copy link
Member

dgageot commented Nov 18, 2015

Yes, that's bad. We had to do it. We should have done it before 0.5.0 and didn't have time.

@legal90
Copy link
Contributor

legal90 commented Nov 20, 2015

That's really sad that backward & forward incompatible stuff was done in the scope of patch level release. May be, it is reasonable to bump a minor version at least? (0.6.0)

@allingeek
Copy link
Contributor

You can still fix this AND keep the linting change. There is no reason to tie the API service and method names to Go types. Please consider reopening, making the change, and restoring the API.

Should I send a PR?

@nathanleclaire
Copy link
Contributor

@allingeek Interesting, I had not considered that as an alternative. Would love to see such a PR.

allingeek added a commit to allingeek/machine that referenced this issue Nov 23, 2015
Addresses docker#2325

Signed-off-by: Jeff Nickoloff <jeff@allingeek.com>

	modified:   libmachine/drivers/plugin/register_driver.go
	modified:   libmachine/drivers/rpc/client_driver.go
@allingeek
Copy link
Contributor

I've verified that a docker-machine binary with this patch will work with both plugin generations.

@janeczku
Copy link
Contributor Author

@allingeek you rock! 👏 📣

allingeek added a commit to allingeek/machine that referenced this issue Nov 25, 2015
Added compatibility for both 5.0 and 5.1 RPC plugins.
Addresses docker#2325

Signed-off-by: Jeff Nickoloff <jeff@allingeek.com>

	modified:   libmachine/drivers/plugin/register_driver.go
	modified:   libmachine/drivers/rpc/client_driver.go
@allingeek
Copy link
Contributor

Can this be closed after the patch?

@nathanleclaire
Copy link
Contributor

Yes I think so!

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

No branches or pull requests

6 participants