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

Use instance availability zone for AWS EBS #10181

Merged
merged 1 commit into from Jul 6, 2015
Merged

Use instance availability zone for AWS EBS #10181

merged 1 commit into from Jul 6, 2015

Conversation

ghost
Copy link

@ghost ghost commented Jun 22, 2015

This simplifies the volumeID line and makes it similar to the pdName entry for GCE Persistent disks.
@justinsb What do you think of this ?

@k8s-bot
Copy link

k8s-bot commented Jun 22, 2015

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

@j3ffml
Copy link
Contributor

j3ffml commented Jun 22, 2015

@justinsb is this something you'd want for v1 or can it wait?

@j3ffml j3ffml added this to the v1.0-candidate milestone Jun 22, 2015
@justinsb
Copy link
Member

I think this is a good change; it is much more user-friendly.

This is a breaking change for anyone using volumes also; so it might be better to just treat this as a shortcut: so vol-12345678 is a shortcut for aws:///vol-12345678, which is itself a shortcut for aws:///vol-12345678. That way we don't break any docs / blogs etc.

@swagiaal this is just to be more user-friendly, right (it isn't a correctness issue)? I think we should still do it if we can accept both forms.

Signed-off-by: Sami Wagiaalla <swagiaal@redhat.com>
@ghost
Copy link
Author

ghost commented Jun 25, 2015

@justinsb yeah this is just for the purpose of being more user friendly.
I have updated the patch. This should now accept aws://zone/vol-ID and just vol-ID

@goltermann goltermann modified the milestones: v1.0, v1.0-candidate Jun 30, 2015
@goltermann goltermann added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Jun 30, 2015
@justinsb
Copy link
Member

justinsb commented Jul 5, 2015

This is nice; much more user-friendly and doesn't break anything.

LGTM

@justinsb justinsb added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 5, 2015
@bprashanth
Copy link
Contributor

This seems pretty isolated to aws, and it sounds like we want it for 1.0 if possible. @quinton-hoole for ok-to-merge.

@bprashanth bprashanth assigned ghost and unassigned justinsb Jul 5, 2015
@ghost
Copy link
Author

ghost commented Jul 6, 2015

Yup, ok-to-merge, thanks!

@ghost ghost added the ok-to-merge label Jul 6, 2015
yujuhong added a commit that referenced this pull request Jul 6, 2015
Use instance availability zone for AWS EBS
@yujuhong yujuhong merged commit 736b3cb into kubernetes:master Jul 6, 2015
@ghost ghost removed their assignment Aug 12, 2015
@sils
Copy link
Contributor

sils commented Nov 30, 2015

@sils
Copy link
Contributor

sils commented Nov 30, 2015

filed bug at #17957

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. priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants