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

Issue #1947 CoreOS provisioner does not properly bootstrap etcd #1971

Closed
wants to merge 1 commit into from
Closed

Issue #1947 CoreOS provisioner does not properly bootstrap etcd #1971

wants to merge 1 commit into from

Conversation

sylus
Copy link

@sylus sylus commented Oct 11, 2015

This referrence issue: #1947

This is my first go at getting the new CoreOS provisioner to properly bootstrap ETCD. It works right now for the generation of the discovery token and using the Public IP. I'd welcome any and all feedback as is my first PR to docker.

Ideally we can also support the --digitalocean-private-networking and use the Private IP as well. Looking at the ec2 driver as a guide since I think it supports both. Right now the template enforces just the Public IP.

Signed-off-by: William Hearn <sylus1984@gmail.com>
@nathanleclaire nathanleclaire changed the title Issue #1947 CoreOS provisioner does not properly bootstrap ETCD Issue #1947 CoreOS provisioner does not properly bootstrap etcd Oct 12, 2015
@nathanleclaire
Copy link
Contributor

cc @mschygulla please take a look and let us know your thoughts.

@mschygulla
Copy link
Contributor

I had a look and I have to say that I don't think this is a good way to configure the etcd service.

With this pull request the etcd configuration is hardcoded in the docker-machine executable. You cannot modify or adjust the etcd service settings to your personal needs. The hardcoded URL (https://discovery.etcd.io) to generate the discovery token might also not work for everyone (e.g. if you run your own etcd cluster in a corporate environment, or simply on your laptop inside virtualbox without internet connection).

I think the better way to configure the etcd service is to use cloud-config as described here:

https://coreos.com/os/docs/latest/booting-on-ec2.html

@sylus
Copy link
Author

sylus commented Oct 12, 2015

I see your point about the etcd url being different and some of the hard coded values only able to be changed on the running droplet. Could you provide more explanation on how you see docker machine provisioning using cloud-config such that using the docker-machine create command would work?

For instance I don’t know how docker machine would provision the cloud-config on digitalocean which I don’t think exposes an API to set it ahead of time so running docker-machine create would work. I think the real problem is currently when running docker-machine create on a droplet for coreos it prevents the ability to add a cloud-init file through the UI later as the option never appears. I am looking for a programmatic way of solving this without relying on external steps through the cloud provider UI.

I could just have docker-machine read from an external file with a new flag but if given more information will try to work on a more acceptable way. ^_^ Thanks for your time!

@mschygulla
Copy link
Contributor

In my eyes the docker-machine driver needs to be extended to allow an option to provide a cloud-config file. For example, take a look at my PR-1479 for the VMwareFusion driver. I can imaging something similar for other cloud providers, at least where it's technical possible.

@so0k
Copy link

so0k commented Oct 26, 2015

I've always been using Docker on CoreOS (and I'm on Hyper-V). I rely heavily on user-data to pull binaries (experimental docker releases, swarm, kubernetes, ...) at the moment I provision my docker hosts with powershell scripts (locally) and doctl.. (cloud) but I'd like to use Machine.

I've never written go before, but...

for digital ocean you can provide string for cloud-config in createDropletRequest: https://github.com/digitalocean/godo/blob/master/droplets.go#L133
so, should just update driver to accept user data string and pass it on to godo here:
https://github.com/docker/machine/blob/master/drivers/digitalocean/digitalocean.go#L166

However, @mschygulla added support for specifying url to config drive, while the above would take a userdata string for cloud providers.., wouldn't it be more consistent to always accept a string and do something like: https://github.com/kelseyhightower/isod/blob/master/main.go to generate the iso? (or would this not cross compile and work on windows/osx?)

any suggestions, comments?

@janeczku
Copy link
Contributor

janeczku commented Nov 2, 2015

However, @mschygulla added support for specifying url to config drive, while the above would take a userdata string for cloud providers..

And thats why the provisioner is the wrong place for implementing this stuff in my opinion. This should be done on the driver level.

@nathanleclaire
Copy link
Contributor

I think @janeczku has a pretty valid point about keeping this out of the provisioners in general, however if there's any way we can bootstrap the CoreOS node with a basic sane configuration and that isn't being done right now, I'm for it. However, I don't want to make heavy modifications elsewhere to support it. Maybe cloud config as an optional parameters is something we could tackle in the core drivers for the 0.6.0 release.

@dgageot dgageot added this to the 0.6.0 milestone Nov 19, 2015
@dgageot dgageot modified the milestones: 0.5.5, 0.6.0 Dec 28, 2015
@dgageot dgageot modified the milestones: 0.5.6, 0.6.0 Jan 11, 2016
@dgageot dgageot modified the milestones: 0.6.0, 0.7.0 Jan 19, 2016
@sylus
Copy link
Author

sylus commented Feb 9, 2016

Is it possible to get more information about the direction this should go? As the earlier comment mentioned I just want sane configuration. If the direction is this should be solved exclusively on the driver level then that is fine but just want to make sure we are all on the same page. It really is unfortunate we have to resort to custom scripts rather then leveraging docker-machine which is just phenomenal in its ease of use. I myself am not a go expert but can get my way around though just want to make sure that the direction we choose is appropriate.

@sylus
Copy link
Author

sylus commented Feb 9, 2016

I wonder if the CoreOS folks have any insight on this (sorry for the ping) @crawford @philips

etcd-request-timeout: 15
units:
- name: etcd.service
command: start
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to use etcd2

@philips
Copy link

philips commented Feb 9, 2016

Does docker machine know the IP of the machines before they boot? (It seems it does based on the cloud-config). If so you can just use static bootstrap: https://coreos.com/etcd/docs/latest/clustering.html#static

Adding @gyuho to help assist further if needed.

@dgageot
Copy link
Member

dgageot commented Feb 18, 2016

@sylus Do you plan to improve the PR based on @philips's feedback? Or are you willing to close it?

@sylus
Copy link
Author

sylus commented Feb 18, 2016

I plan to improve it I am going to try to take a look at it this weekend.

@nathanleclaire
Copy link
Contributor

@sylus Any update on this?

@sylus
Copy link
Author

sylus commented Mar 22, 2016

Hey! Sorry been so slow getting back to this issue. I have made the corresponding fixes mentioned by the CoreOS folks but this still doesn't solve the earlier concerns that this was the wrong approach to do in the provisioner and should be in the drivers logic instead. I think the digital ocean driver might be simplest based on what @so0k mentioned in the earlier comments. However the virtualbox driver looks to be completely integrated with boot2docker so that would need some reoorg to get that working with CoreOS.

I'll close this P.R. though as is the wrong approach and this is ultimately going to need code fixes done on a particular driver level. I will try to reopen a new P.R. against digitalocean at some point. Sorry for keeping this issue open so long in the queue. Many thanks!

@sylus sylus closed this Mar 22, 2016
@nathanleclaire
Copy link
Contributor

Why not start by revising this PR to provide a basic sane configuration as is? Maybe static bootstrap like Brandon suggested?

If we're going to implement cloudinit support in drivers, it should be done as across the board (for each driver) as much as possible, which is a massive undertaking.

@so0k
Copy link

so0k commented Mar 23, 2016

I'm interested to implemement cloud-init for digitalocean & hyper-v drivers
(the ones I use the most)

On Wed, Mar 23, 2016 at 5:33 AM, Nathan LeClaire notifications@github.com
wrote:

Why not start by revising this PR to provide a basic sane configuration as
is? Maybe static bootstrap like Brandon suggested?

If we're going to implement cloudinit support in drivers, it should be
done as across the board (for each driver) as much as possible, which is a
massive undertaking.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1971 (comment)

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.

8 participants