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

Make AWS configuration file optional (fall-back to metadata service) #6370

Merged
merged 1 commit into from Apr 6, 2015

Conversation

justinsb
Copy link
Member

@justinsb justinsb commented Apr 2, 2015

The AWS configuration file currently only has the current AZ. It is easy to get that from the metadata service.

This brings AWS closer to GCE, and also avoids the need to create an aws.conf file for kubelet on the minions when we do EBS volumes.

@bprashanth
Copy link
Contributor

@mbforbes based off a previous review

@mbforbes
Copy link
Contributor

mbforbes commented Apr 2, 2015

Looks pretty good though I'm not familiar with this part of the code. Kicked travis and waiting on it.

@zmerlynn
Copy link
Member

zmerlynn commented Apr 2, 2015

Isn't the aws Salt config still setting this unconditionally, or did I miss a different PR?

@justinsb
Copy link
Member Author

justinsb commented Apr 2, 2015

@zmerlynn It is for kube-apiserver, but this is actually groundwork for kubelet. Kubelet must talk to AWS to mount EBS volumes. That work is in this PR #5138, which is getting really close...

Rather than add the aws.conf to kubelet (& minions), I made aws.conf optional.

I guess I could remove aws.conf everywhere now. This step-by-step approach feels a little safer though...

@justinsb
Copy link
Member Author

justinsb commented Apr 3, 2015

Rebased (really just to force the tests to re-run)

@mbforbes
Copy link
Contributor

mbforbes commented Apr 4, 2015

This LGTM and tests pass, so I'll mark as so, but given @zmerlynn has more context than me, please feel free to disagree & hold this.

@mbforbes mbforbes added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 4, 2015
@zmerlynn
Copy link
Member

zmerlynn commented Apr 5, 2015

Oh, sorry, LGTM, I just had that question.
On Apr 4, 2015 11:10 AM, "Maxwell Forbes" notifications@github.com wrote:

This LGTM and tests pass, so I'll mark as so, but given @zmerlynn
https://github.com/zmerlynn has more context than me, please feel free
to disagree & hold this.


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

rjnagal added a commit that referenced this pull request Apr 6, 2015
Make AWS configuration file optional (fall-back to metadata service)
@rjnagal rjnagal merged commit f2f3da1 into kubernetes:master Apr 6, 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

6 participants