AWS EBS volume support #5138

Merged
merged 40 commits into from Apr 10, 2015

Conversation

Projects
None yet
@justinsb
Member

justinsb commented Mar 6, 2015

Code definitely doesn't work yet, and an open discussion in #5129 on whether we should write server & client code once (with a provider approach) or N times (with a type-per-cloud approach).

But would greatly appreciate any comments particularly on the pkg/cloudprovider/aws/aws.go code. The extra code is non-trivial because AWS volume mounting is pretty painful: you have to specify an unused mount device.

@googlebot googlebot added the cla: yes label Mar 6, 2015

@justinsb

This comment has been minimized.

Show comment
Hide comment
@justinsb

justinsb Mar 6, 2015

Member

AWS code is mostly here: bd9021e

Member

justinsb commented Mar 6, 2015

AWS code is mostly here: bd9021e

pkg/api/types.go
// kubelet's host machine and then exposed to the pod.
- GCEPersistentDisk *GCEPersistentDiskVolumeSource `json:"persistentDisk"`
+ CloudPersistentDisk *CloudPersistentDiskVolumeSource `json:"persistentDisk"`

This comment has been minimized.

@markturansky

markturansky Mar 6, 2015

Member

I don't think this is going to work. You'll overload this struct for every type of cloud provider, which means you have to type it somehow. Some properties would be relevant to some but not all of the providers.

Better, I think, to have an explicit struct per type.

We agree there will be commonalities between them. Good code structure can promote reuse for the common pieces.

@markturansky

markturansky Mar 6, 2015

Member

I don't think this is going to work. You'll overload this struct for every type of cloud provider, which means you have to type it somehow. Some properties would be relevant to some but not all of the providers.

Better, I think, to have an explicit struct per type.

We agree there will be commonalities between them. Good code structure can promote reuse for the common pieces.

This comment has been minimized.

@justinsb

justinsb Mar 6, 2015

Member

Yeah, I'd started this before I saw that there were different viewpoints on #5129. Really looking for feedback on the AWS stuff specifically; once we figure out #5129 then I'll rebase the AWS work on to whatever we choose.

@justinsb

justinsb Mar 6, 2015

Member

Yeah, I'd started this before I saw that there were different viewpoints on #5129. Really looking for feedback on the AWS stuff specifically; once we figure out #5129 then I'll rebase the AWS work on to whatever we choose.

This comment has been minimized.

@swagiaal

swagiaal Mar 11, 2015

Contributor

@justinsb could you modify this so it implements AWS persistent volumes as sibling to GCE PD. Similar to other proposed persistent volume plugins: #4612, and #4601.
I think there is general consensus on doing that for now, then improving the abstraction to promote code sharing after a couple of them are merged. See the discussion here on https://github.com/GoogleCloudPlatform/kubernetes/pull/4601/files#r25993686

@swagiaal

swagiaal Mar 11, 2015

Contributor

@justinsb could you modify this so it implements AWS persistent volumes as sibling to GCE PD. Similar to other proposed persistent volume plugins: #4612, and #4601.
I think there is general consensus on doing that for now, then improving the abstraction to promote code sharing after a couple of them are merged. See the discussion here on https://github.com/GoogleCloudPlatform/kubernetes/pull/4601/files#r25993686

@markturansky

This comment has been minimized.

Show comment
Hide comment
Member

markturansky commented Mar 6, 2015

@piosz

This comment has been minimized.

Show comment
Hide comment
Member

piosz commented Mar 31, 2015

@justinsb

This comment has been minimized.

Show comment
Hide comment
@justinsb

justinsb Mar 31, 2015

Member

I should be ready with the updated code this week. I'm currently working on load balancer support, and maintaining two big feature branches simultaneously is too hard, so I'm till that merges before working on EBS.

Member

justinsb commented Mar 31, 2015

I should be ready with the updated code this week. I'm currently working on load balancer support, and maintaining two big feature branches simultaneously is too hard, so I'm till that merges before working on EBS.

@swagiaal

This comment has been minimized.

Show comment
Hide comment
@swagiaal

swagiaal Mar 31, 2015

Contributor

@justinsb I can work on this if you are too busy with other PRs. It was on my radar before you posted this.

Contributor

swagiaal commented Mar 31, 2015

@justinsb I can work on this if you are too busy with other PRs. It was on my radar before you posted this.

@justinsb

This comment has been minimized.

Show comment
Hide comment
@justinsb

justinsb Mar 31, 2015

Member

I think it's really close, and not sure if it makes any sense to attempt a hand-off.

Let me try to get some of my smaller PRs merged, so I only have to deal with this one and the ELB one... That should be manageable.

Member

justinsb commented Mar 31, 2015

I think it's really close, and not sure if it makes any sense to attempt a hand-off.

Let me try to get some of my smaller PRs merged, so I only have to deal with this one and the ELB one... That should be manageable.

@swagiaal

This comment has been minimized.

Show comment
Hide comment
@swagiaal

swagiaal Mar 31, 2015

Contributor

@justinsb Sounds good :)

Contributor

swagiaal commented Mar 31, 2015

@justinsb Sounds good :)

@justinsb justinsb changed the title from WIP: AWS EBS volume support to AWS EBS volume support Apr 3, 2015

@justinsb

This comment has been minimized.

Show comment
Hide comment
@justinsb

justinsb Apr 3, 2015

Member

This now "works for me", but I need to run the e2e tests (which I have extended to cover AWS).

Should I squash? I don't know if anyone really cares to see my thought process here :-) And there hasn't been that much review activity yet...

Member

justinsb commented Apr 3, 2015

This now "works for me", but I need to run the e2e tests (which I have extended to cover AWS).

Should I squash? I don't know if anyone really cares to see my thought process here :-) And there hasn't been that much review activity yet...

pkg/api/types.go
@@ -189,6 +189,9 @@ type VolumeSource struct {
// GCEPersistentDisk represents a GCE Disk resource that is attached to a
// kubelet's host machine and then exposed to the pod.
GCEPersistentDisk *GCEPersistentDiskVolumeSource `json:"gcePersistentDisk"`
+ // AWSPersistentDisk represents a AWS EBS disk that is attached to a
+ // kubelet's host machine and then exposed to the pod.
+ AWSPersistentDisk *AWSPersistentDiskVolumeSource `json:"awsPersistentDisk"`

This comment has been minimized.

@markturansky

markturansky Apr 4, 2015

Member

Would AWSElasticBlockStore be more accurately named?

@markturansky

markturansky Apr 4, 2015

Member

Would AWSElasticBlockStore be more accurately named?

This comment has been minimized.

@justinsb

justinsb Apr 7, 2015

Member

Perhaps. EBS is just a marketing term though, right? I'd rather think about the API in terms of what the user is trying to achieve - they want a persistent volume. Maybe it should be AWSPersistentVolume.

I guess the question is when Amazon announces ElasticNvram, which is still a block device, would we create a second entry in the API? (Or, if they had chosen to launch SSD under a different name). I'd argue we shouldn't, and as such we shouldn't mirror Amazon's trademark-du-jour...

@justinsb

justinsb Apr 7, 2015

Member

Perhaps. EBS is just a marketing term though, right? I'd rather think about the API in terms of what the user is trying to achieve - they want a persistent volume. Maybe it should be AWSPersistentVolume.

I guess the question is when Amazon announces ElasticNvram, which is still a block device, would we create a second entry in the API? (Or, if they had chosen to launch SSD under a different name). I'd argue we shouldn't, and as such we shouldn't mirror Amazon's trademark-du-jour...

This comment has been minimized.

@markturansky

markturansky Apr 7, 2015

Member

I believe @bgrant0607's intention is to remove all these types of disk from VolumeSource and from the end user (pod authors and the like). The user will make a request for a persistent volume (via PVClaim) and get something. They won't know what and they shouldn't care.

The cluster admin, OTOH, deals with specific infrastructure. He needs to create a specific kind of disk. AWSElasticBlockStore maps cleanly to the AWS API type EbsBlockDevice.

@markturansky

markturansky Apr 7, 2015

Member

I believe @bgrant0607's intention is to remove all these types of disk from VolumeSource and from the end user (pod authors and the like). The user will make a request for a persistent volume (via PVClaim) and get something. They won't know what and they shouldn't care.

The cluster admin, OTOH, deals with specific infrastructure. He needs to create a specific kind of disk. AWSElasticBlockStore maps cleanly to the AWS API type EbsBlockDevice.

This comment has been minimized.

@justinsb

justinsb Apr 7, 2015

Member

OK, I'll change it. Avoiding another rebase is more important to me than nomenclature :-)

@justinsb

justinsb Apr 7, 2015

Member

OK, I'll change it. Avoiding another rebase is more important to me than nomenclature :-)

pkg/api/types.go
+// A AWS EBS disk can only be mounted as read/write once.
+type AWSPersistentDiskVolumeSource struct {
+ // Unique name of the PD resource. Used to identify the disk in AWS
+ PDName string `json:"pdName"`

This comment has been minimized.

@markturansky

markturansky Apr 4, 2015

Member

Similarly, the AWS API has "volumeId", not PDName.

I see why you were thinking of 1 "cloud disk" for re-use across both GCE and AWS. But so long as they must be separate implementations, I think the names of fields can map cleanly and obviously to the AWS API as opposed to mapping to the GCE API. My $0.02.

@markturansky

markturansky Apr 4, 2015

Member

Similarly, the AWS API has "volumeId", not PDName.

I see why you were thinking of 1 "cloud disk" for re-use across both GCE and AWS. But so long as they must be separate implementations, I think the names of fields can map cleanly and obviously to the AWS API as opposed to mapping to the GCE API. My $0.02.

This comment has been minimized.

@justinsb

justinsb Apr 7, 2015

Member

Fair enough! Making the change now. VolumeId feels like a better name even for GCE :-)

@justinsb

justinsb Apr 7, 2015

Member

Fair enough! Making the change now. VolumeId feels like a better name even for GCE :-)

This comment has been minimized.

@justinsb

justinsb Apr 7, 2015

Member

Also, this isn't just the AWS ID. We qualify it with the AZ e.g. aws://us-east-1a/vol-12345678

@justinsb

justinsb Apr 7, 2015

Member

Also, this isn't just the AWS ID. We qualify it with the AZ e.g. aws://us-east-1a/vol-12345678

pkg/volume/aws_pd/aws_pd.go
+var _ volume.VolumePlugin = &awsPersistentDiskPlugin{}
+
+const (
+ awsPersistentDiskPluginName = "kubernetes.io/aws-pd"

This comment has been minimized.

@markturansky

markturansky Apr 4, 2015

Member

aws-ebs?

@markturansky

markturansky Apr 4, 2015

Member

aws-ebs?

This comment has been minimized.

@justinsb

justinsb Apr 7, 2015

Member

My inclination is to keep this named-by-intent, not named by product-name.

@justinsb

justinsb Apr 7, 2015

Member

My inclination is to keep this named-by-intent, not named by product-name.

This comment has been minimized.

@markturansky

markturansky Apr 7, 2015

Member

Perhaps this is changed to match what consensus comes regarding the name in VolumeSource?

"aws-pd", to me, requires parsing and thought while "aws-ebs" is known AWS infrastructure.

@markturansky

markturansky Apr 7, 2015

Member

Perhaps this is changed to match what consensus comes regarding the name in VolumeSource?

"aws-pd", to me, requires parsing and thought while "aws-ebs" is known AWS infrastructure.

@markturansky

This comment has been minimized.

Show comment
Hide comment
@justinsb

This comment has been minimized.

Show comment
Hide comment
@justinsb

justinsb Apr 7, 2015

Member

Added fuzzer, rebased to avoid conflicts with iSCSI

Member

justinsb commented Apr 7, 2015

Added fuzzer, rebased to avoid conflicts with iSCSI

@justinsb

This comment has been minimized.

Show comment
Hide comment
@justinsb

justinsb Apr 9, 2015

Member

Now with passing e2e tests also!

http://public-kubernetes-meteor.s3-website-us-east-1.amazonaws.com/builds/kubernetes-e2e-aws/6/_1.xml

In particular:

<testcase name="PD should schedule a pod w/ a RW PD, remove it, then schedule it on another host" classname="Kubernetes e2e Suite run 1 of 1" time="60.902563537"/>
Member

justinsb commented Apr 9, 2015

Now with passing e2e tests also!

http://public-kubernetes-meteor.s3-website-us-east-1.amazonaws.com/builds/kubernetes-e2e-aws/6/_1.xml

In particular:

<testcase name="PD should schedule a pod w/ a RW PD, remove it, then schedule it on another host" classname="Kubernetes e2e Suite run 1 of 1" time="60.902563537"/>
+ if context.Provider == "aws" {
+ awsConfig := "[Global]\n"
+ if cloudConfig.Zone == "" {
+ glog.Error("gce_zone must be specified for AWS")

This comment has been minimized.

@markturansky

markturansky Apr 9, 2015

Member

is "gce_zone" correct here?

@markturansky

markturansky Apr 9, 2015

Member

is "gce_zone" correct here?

This comment has been minimized.

@justinsb

justinsb Apr 9, 2015

Member

This is tricky. I'm not sure whether we want flags for each provider (i.e. potentially duplicate kube_master, gce_project, gce_zone -> aws_kube_master, aws_gce_project, aws_gce_zone) or whether we should treat these as "cloud_project" & "cloud_zone". If the latter, a secondary question is whether we should rename the flags.

Edit: I don't much care either way. I'm guessing we'll end up with duplicating (i.e. aws_zone)

@justinsb

justinsb Apr 9, 2015

Member

This is tricky. I'm not sure whether we want flags for each provider (i.e. potentially duplicate kube_master, gce_project, gce_zone -> aws_kube_master, aws_gce_project, aws_gce_zone) or whether we should treat these as "cloud_project" & "cloud_zone". If the latter, a secondary question is whether we should rename the flags.

Edit: I don't much care either way. I'm guessing we'll end up with duplicating (i.e. aws_zone)

This comment has been minimized.

@markturansky

markturansky Apr 9, 2015

Member

You can only have one cloud provider at a time, right? Perhaps this is a good place for generic cloud patterns, so long as it's not planned to allow a cluster span infrastructure providers... If that's the case, that's probably an easy small PR instead of adding more to this one.

@markturansky

markturansky Apr 9, 2015

Member

You can only have one cloud provider at a time, right? Perhaps this is a good place for generic cloud patterns, so long as it's not planned to allow a cluster span infrastructure providers... If that's the case, that's probably an easy small PR instead of adding more to this one.

This comment has been minimized.

@thockin

thockin Apr 9, 2015

Member

As long as the concepts fundamentally match up, I'm in favor of generification.

@thockin

thockin Apr 9, 2015

Member

As long as the concepts fundamentally match up, I'm in favor of generification.

This comment has been minimized.

@thockin

thockin Apr 10, 2015

Member

I'm fine with not renaming in this PR, but please file a bug on this. We just need to figure out the ripple of renaming them.

@thockin

thockin Apr 10, 2015

Member

I'm fine with not renaming in this PR, but please file a bug on this. We just need to figure out the ripple of renaming them.

pkg/api/types.go
@@ -208,6 +211,9 @@ type PersistentVolumeSource struct {
// GCEPersistentDisk represents a GCE Disk resource that is attached to a
// kubelet's host machine and then exposed to the pod.
GCEPersistentDisk *GCEPersistentDiskVolumeSource `json:"gcePersistentDisk"`
+ // AWSElasticBlockStore represents a AWS EBS disk that is attached to a

This comment has been minimized.

@markturansky

markturansky Apr 9, 2015

Member

s/a/an for each types.go and struct

@markturansky

markturansky Apr 9, 2015

Member

s/a/an for each types.go and struct

This comment has been minimized.

@justinsb

justinsb Apr 9, 2015

Member

Fixed - thanks!

@justinsb

justinsb Apr 9, 2015

Member

Fixed - thanks!

@markturansky

This comment has been minimized.

Show comment
Hide comment
@markturansky

markturansky Apr 9, 2015

Member

LGTM for what I can review. You'll need @thockin for the rest.

Member

markturansky commented Apr 9, 2015

LGTM for what I can review. You'll need @thockin for the rest.

@@ -189,6 +189,9 @@ type VolumeSource struct {
// GCEPersistentDisk represents a GCE Disk resource that is attached to a
// kubelet's host machine and then exposed to the pod.
GCEPersistentDisk *GCEPersistentDiskVolumeSource `json:"gcePersistentDisk"`
+ // AWSElasticBlockStore represents an AWS EBS disk that is attached to a
+ // kubelet's host machine and then exposed to the pod.
+ AWSElasticBlockStore *AWSElasticBlockStoreVolumeSource `json:"awsElasticBlockStore"`

This comment has been minimized.

@thockin

thockin Apr 9, 2015

Member

@bgrant0607 Are you OK with AWS support here to parallel GCE? It seems appropriate. We can EOL them both when persistent volumes are up to snuff.

@thockin

thockin Apr 9, 2015

Member

@bgrant0607 Are you OK with AWS support here to parallel GCE? It seems appropriate. We can EOL them both when persistent volumes are up to snuff.

pkg/cloudprovider/aws/aws.go
+}
+
+// TODO: Also return number of mounts allowed?
+func (self *awsInstanceType) getEbsMountDevices() []string {

This comment has been minimized.

@thockin

thockin Apr 9, 2015

Member

Here and elsewhere: Go style would say "EBS"

@thockin

thockin Apr 9, 2015

Member

Here and elsewhere: Go style would say "EBS"

pkg/cloudprovider/aws/aws.go
+ ec2 EC2
+
+ // id in AWS
+ awsId string

This comment has been minimized.

@thockin

thockin Apr 9, 2015

Member

nit: ID

@thockin

thockin Apr 9, 2015

Member

nit: ID

pkg/cloudprovider/aws/aws.go
+
+// Implements Volumes.AttachDisk
+func (aws *AWSCloud) AttachDisk(instanceName string, diskName string, readOnly bool) (string, error) {
+ disk, err := newAwsDisk(aws.ec2, diskName)

This comment has been minimized.

@thockin

thockin Apr 9, 2015

Member

AWS vs Aws

@thockin

thockin Apr 9, 2015

Member

AWS vs Aws

pkg/volume/aws_pd/aws_pd.go
+var _ volume.VolumePlugin = &awsElasticBlockStorePlugin{}
+
+const (
+ awsElasticBlockStorePluginName = "kubernetes.io/aws-ebs"

This comment has been minimized.

@thockin

thockin Apr 9, 2015

Member

This whole pkg and the files should be called aws_ebs

@thockin

thockin Apr 9, 2015

Member

This whole pkg and the files should be called aws_ebs

@thockin

This comment has been minimized.

Show comment
Hide comment
@thockin

thockin Apr 9, 2015

Member

LGTM but for naming stuff

Member

thockin commented Apr 9, 2015

LGTM but for naming stuff

@justinsb

This comment has been minimized.

Show comment
Hide comment
@justinsb

justinsb Apr 10, 2015

Member

So I think we're mainly waiting on @bgrant0607 to comment on the API (?) #5138 (comment)

And I'm not entirely sure what we want to do about the parameter names in e2e.go, at least in this PR? (#5138 (comment))

Member

justinsb commented Apr 10, 2015

So I think we're mainly waiting on @bgrant0607 to comment on the API (?) #5138 (comment)

And I'm not entirely sure what we want to do about the parameter names in e2e.go, at least in this PR? (#5138 (comment))

@thockin

This comment has been minimized.

Show comment
Hide comment
@thockin

thockin Apr 10, 2015

Member

Don't wait too long for Brian, I am pretty sure it's a case of ", ok"
:)

On Fri, Apr 10, 2015 at 1:03 PM, Justin Santa Barbara <
notifications@github.com> wrote:

So I think we're mainly waiting on @bgrant0607
https://github.com/bgrant0607 to comment on the API (?) #5138 (comment)
#5138 (comment)

And I'm not entirely sure what we want to do about the parameter names in
e2e.go, at least in this PR? (#5138 (comment)
#5138 (comment)
)


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

Member

thockin commented Apr 10, 2015

Don't wait too long for Brian, I am pretty sure it's a case of ", ok"
:)

On Fri, Apr 10, 2015 at 1:03 PM, Justin Santa Barbara <
notifications@github.com> wrote:

So I think we're mainly waiting on @bgrant0607
https://github.com/bgrant0607 to comment on the API (?) #5138 (comment)
#5138 (comment)

And I'm not entirely sure what we want to do about the parameter names in
e2e.go, at least in this PR? (#5138 (comment)
#5138 (comment)
)


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

pkg/volume/aws_ebs/aws_pd.go
+limitations under the License.
+*/
+
+package aws_ebs

This comment has been minimized.

@thockin

thockin Apr 10, 2015

Member

nit: file should be renamed to match dir

@thockin

thockin Apr 10, 2015

Member

nit: file should be renamed to match dir

pkg/volume/aws_ebs/aws_pd_test.go
@@ -0,0 +1,156 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.

This comment has been minimized.

@thockin

thockin Apr 10, 2015

Member

Same rename comment - file name.

@thockin

thockin Apr 10, 2015

Member

Same rename comment - file name.

@thockin

This comment has been minimized.

Show comment
Hide comment
@thockin

thockin Apr 10, 2015

Member

Needs a rebase too, otherwise LGTM

Member

thockin commented Apr 10, 2015

Needs a rebase too, otherwise LGTM

justinsb added some commits Mar 6, 2015

Fix AWS region vs zone
We were specifying a region, but naming it as a zone in util.sh

The zone matters just as much as the region, e.g. for EBS volumes.

We also change the config to require a Zone, not a Region.
But we fallback to get the information from the metadata service.

justinsb added some commits Apr 2, 2015

More logging around error causes
Come back exceptions, all is forgiven!
Parse the pdName from the volume mount
Don't assume there are no slashes!
Default attachment status to detached
So no-attachments = status detached
@justinsb

This comment has been minimized.

Show comment
Hide comment
@justinsb

justinsb Apr 10, 2015

Member

Opened issue #6699 to track renaming the e2e cmd arguments.

Member

justinsb commented Apr 10, 2015

Opened issue #6699 to track renaming the e2e cmd arguments.

thockin added a commit that referenced this pull request Apr 10, 2015

@thockin thockin merged commit 4a7b0ee into kubernetes:master Apr 10, 2015

3 checks passed

Shippable Shippable builds completed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.57%) to 53.65%
Details
@lavalamp

This comment has been minimized.

Show comment
Hide comment
@lavalamp

lavalamp Apr 10, 2015

Member

@thockin, @justinsb -- this definitely could have used a squash

Member

lavalamp commented Apr 10, 2015

@thockin, @justinsb -- this definitely could have used a squash

@thockin

This comment has been minimized.

Show comment
Hide comment
@thockin

thockin Apr 10, 2015

Member

Yeah, I messed up - meant to ask for it, forgot.

I wonder if there are tools that could help here?

On Fri, Apr 10, 2015 at 3:48 PM, Daniel Smith notifications@github.com
wrote:

@thockin https://github.com/thockin, @justinsb
https://github.com/justinsb -- this definitely could have used a squash


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

Member

thockin commented Apr 10, 2015

Yeah, I messed up - meant to ask for it, forgot.

I wonder if there are tools that could help here?

On Fri, Apr 10, 2015 at 3:48 PM, Daniel Smith notifications@github.com
wrote:

@thockin https://github.com/thockin, @justinsb
https://github.com/justinsb -- this definitely could have used a squash


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

@justinsb

This comment has been minimized.

Show comment
Hide comment
@justinsb

justinsb Apr 11, 2015

Member

Sorry, I should definitely have squashed it once we were getting closer. But squashing also obliterates history. I have looked, and haven't found any great solution. Even with squashing, I still don't love the merge-commits that the button produces.

I hear code.google.com will soon be available. Let's build v2 (on gerrit this time!)

Or we could probably figure out a script that squashes the commits, changes the author and does a push.

Member

justinsb commented Apr 11, 2015

Sorry, I should definitely have squashed it once we were getting closer. But squashing also obliterates history. I have looked, and haven't found any great solution. Even with squashing, I still don't love the merge-commits that the button produces.

I hear code.google.com will soon be available. Let's build v2 (on gerrit this time!)

Or we could probably figure out a script that squashes the commits, changes the author and does a push.

@bgrant0607

This comment has been minimized.

Show comment
Hide comment
@bgrant0607

bgrant0607 Apr 11, 2015

Member

Sorry for the late response. Github notifications are useless, since I get hundreds per day.

Fortunately, I'm ok with this. Next time, please ask someone to ping me not through github for an API change.

Member

bgrant0607 commented Apr 11, 2015

Sorry for the late response. Github notifications are useless, since I get hundreds per day.

Fortunately, I'm ok with this. Next time, please ask someone to ping me not through github for an API change.

@manolitto

This comment has been minimized.

Show comment
Hide comment
@manolitto

manolitto Apr 27, 2015

Contributor

My feeling is, it should be possible to support multiple clusters inside one VPC. So, perhaps it is better to identify the VPC via the "Name" tag (as it was prior to v0.15) or introduce a new variable "VPC_NAME"?

Contributor

manolitto commented on cluster/aws/util.sh in 034412a Apr 27, 2015

My feeling is, it should be possible to support multiple clusters inside one VPC. So, perhaps it is better to identify the VPC via the "Name" tag (as it was prior to v0.15) or introduce a new variable "VPC_NAME"?

@delianides

This comment has been minimized.

Show comment
Hide comment
@delianides

delianides Apr 30, 2015

Is there going to be any documentation for this? How exactly does this work with availability zones? Should the volume exists in the same AZ as the master?

Is there going to be any documentation for this? How exactly does this work with availability zones? Should the volume exists in the same AZ as the master?

@tslater

This comment has been minimized.

Show comment
Hide comment
@tslater

tslater Jun 4, 2015

I would love to see some documentation as well.

tslater commented Jun 4, 2015

I would love to see some documentation as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment