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

Remove azure driver temporarily + Godeps for new azure driver #3157

Merged
merged 1 commit into from Mar 7, 2016
Merged

Conversation

ahmetb
Copy link
Contributor

@ahmetb ahmetb commented Mar 7, 2016

This commit temporarily removes Azure driver and its dependencies from the source tree and adds dependencies for the new Azure driver (so that Azure driver PR will not have godeps changes and will be
easier to review).

Very shortly after this is merged I will send the actual PR for the new azure driver.

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "azure" git@github.com:ahmetalpbalkan/machine.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Ammending updates the existing PR. You DO NOT need to open a new one.

This commit temporarily removes Azure driver and its dependencies
from the source tree and adds dependencies for the new Azure driver
(so that Azure driver PR will not have godeps changes and will be
easier to review).

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
@nathanleclaire
Copy link
Contributor

LGTM

nathanleclaire added a commit that referenced this pull request Mar 7, 2016
Remove azure driver temporarily + Godeps for new azure driver
@nathanleclaire nathanleclaire merged commit 8a17878 into docker:master Mar 7, 2016
@ahmetb
Copy link
Contributor Author

ahmetb commented Mar 7, 2016

thanks aaand here's the actual PR #3159

@nathanleclaire
Copy link
Contributor

OK @dgageot @jeanlaurent I went ahead and merged this but it occurs to me that there there are probably at least a few instances of existing azure machines in the wild. Based on the numbers I see it's not very common with probably about 88 failed create errors (compare to ~3000 for the most popular cloud drivers) in the past month over ~10 different hosts. I wonder what we should do, since @ahmetalpbalkan new driver and this PR seems definitely to be the right direction in general. Any ideas?

@friism
Copy link

friism commented Mar 8, 2016

Maybe Azure just works way better than the other clouds and that's why there are orders of magnitude fever errors. :trollface:

@ahmetb
Copy link
Contributor Author

ahmetb commented Mar 8, 2016

@nathanleclaire haha I wouldn't be surprised if those errors were me while trying to develop this. 😛 Azure is not super big on Linux just yet (we're about a 25% Linux cloud) but that's growing rapidly. I'm constantly getting emails from customers using docker-machine on Azure, it appears like there is a demand for it (that's why we did this after all).

I think the new driver is solid from Azure perspective, there might be still some flakiness that we can address over time but yesterday I created about 250 machines and all failures were due to non-Azure issues.

@nathanleclaire
Copy link
Contributor

@ahmetalpbalkan Yup, no doubt that it could be a very popular driver -- my concern is largely just that we shouldn't leave existing users of the azure driver in the lurch. Since it doesn't seem very common, we may not need to take drastic action, e.g. a large note about breaking changes in the release notes for the next version might suffice, but we should come up with and execute a game plan on it.

@ahmetb
Copy link
Contributor Author

ahmetb commented Mar 8, 2016

@nathanleclaire Yes, a large note about the breaking change will be really useful. We can suggest sticking to older versions of machine for existing VMs or re-importing the VM to the generic driver perhaps.

@nathanleclaire
Copy link
Contributor

@ahmetalpbalkan If possible, we should add a "safety gate" to refuse to move forward with any Machine operations given the new azure driver in the presence of an old config, and also protects the user's config.json, allowing them to switch back to an older version of Machine until they are ready to upgrade. I know it's non-trivial to engineer such a thing, but it's something to consider.

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

Successfully merging this pull request may close these issues.

None yet

4 participants