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

AWS: use CoreOS for nodes #7905

Merged
merged 1 commit into from May 21, 2015
Merged

AWS: use CoreOS for nodes #7905

merged 1 commit into from May 21, 2015

Conversation

bakins
Copy link

@bakins bakins commented May 7, 2015

@justinsb

Adds ability to run CoreOS for the nodes. Currently is still just docker for the container runtime.

A bit of a WIP, but it all works (for me at least). The OS split is a bit of a kludge, TBH.

Based on the GCE work.

@justinsb
Copy link
Member

justinsb commented May 7, 2015

I think this is a good patch (although why can't I have a CoreOS master? And why can't I have rkt?)

I am concerned though that we're really hitting the maintenance limits of shell scripting by duplicating the create-minion function. Is the same thing needed with GCE? (What is/was the PR there?)

All in all, +1, because CoreOS & rkt support is important, but I really hope we can rethink these scripts...

@bakins
Copy link
Author

bakins commented May 7, 2015

The GCE work was done mostly in #7445 - uses the split Debian vs CoreOS helpers that I based mine on. rkt support should be fairly straightforward, but I wanted to get this out for discussion. I think some of the code in Ubuntu/CoreOS utils can be factored out.

I'm in agreement that the bash scripting needs to be reworked.

Anything in particular you think we should do before considering merging this?

@bakins
Copy link
Author

bakins commented May 7, 2015

Also, CoreOS master is a bit more thorny. I'd need to untangle the salt stuff to do it on CoreOS in a more "normal" way.

@justinsb
Copy link
Member

justinsb commented May 8, 2015

I was joking about CoreOS master & rkt - sorry! I know we'll get there, I just want it yesterday :-)

This LGTM. (I would love one day to eliminate the bash scripts & salt, and I think this gets us closer.)

@iterion
Copy link
Contributor

iterion commented May 8, 2015

+1 as well. It would be great to have CoreOS everywhere.

Are there any major blockers to a CoreOS master on AWS?

@bakins
Copy link
Author

bakins commented May 8, 2015

@justinsb ;) I tweaked it a bit so you can choose container runtime. Done same as GCE code provider. (Copying around code from provider to provider bugs me as well.)

@iterion Currently salt is used to set up master. Assumptions are made in the e2e about how the master is set up, and someone (me?) needs to "port" that from salt into something else for CoreOS. Obviously, you can run CoreOS as a master - see that various guides - but just need to spend the time to do it in the "kube-up" form.

@dchen1107
Copy link
Member

@justinsb I have a pending master.yaml for CoreOS master. It requires rkt integration with port supports. Once that is done, I will clean up this to have master node running coreos and rkt. :-)

@bakins
Copy link
Author

bakins commented May 11, 2015

@dchen1107 nice!

I'm rebasing and cleaning this up a bit. I'll squash and push in a bit.

@justinsb
Copy link
Member

I'm happy for this to merge whenever people want. When it does, we'll have to double check that we haven't lost any concurrent changes to the AWS scripts. But, as it is "opt-in", and won't/shouldn't break AWS/Ubuntu, I say we merge it whenever makes sense.

@bakins
Copy link
Author

bakins commented May 13, 2015

@justinsb let me do a final rebase and test run of both CoreOS and Ubuntu, just to be sure.

@bakins
Copy link
Author

bakins commented May 13, 2015

@justinsb tested on CoreOS stable. Also tested Ubuntu and it still works.

@iterion
Copy link
Contributor

iterion commented May 14, 2015

@bakins This looks good, excited to see it get merged!

@dchen1107
Copy link
Member

LGTM.

@iterion
Copy link
Contributor

iterion commented May 19, 2015

Any blockers to merge? Would love to see this merged so I can test it out.

@dchen1107 dchen1107 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 20, 2015
@dchen1107
Copy link
Member

We are temporarily on merge freeze to solve e2e test breakage.

@iterion
Copy link
Contributor

iterion commented May 20, 2015

Ahh, I was not aware! Sorry to be a bother!
On Tue, May 19, 2015 at 7:04 PM Dawn Chen notifications@github.com wrote:

We are temporarily on merge freeze to solve e2e test breakage.


Reply to this email directly or view it on GitHub
#7905 (comment)
.

dchen1107 added a commit that referenced this pull request May 21, 2015
@dchen1107 dchen1107 merged commit 04c4d25 into kubernetes:master May 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants