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

Programmatical Subnets for EKS #421

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

weastel
Copy link
Contributor

@weastel weastel commented Jul 31, 2020

No description provided.

@weastel
Copy link
Contributor Author

weastel commented Jul 31, 2020

@geekodour This pr is work in progress.

@weastel weastel force-pushed the automatic_subnet_ids branch 2 times, most recently from 8385ec1 to a4a1554 Compare August 28, 2020 12:45
@weastel
Copy link
Contributor Author

weastel commented Aug 28, 2020

@geekodour This PR ready to be reviewed.

@weastel weastel changed the title [WIP] Programmatical Subnets for EKS Programmatical Subnets for EKS Aug 28, 2020
Signed-off-by: Drumil Patel <[email protected]>
Signed-off-by: Drumil Patel <[email protected]>
Signed-off-by: Drumil Patel <[email protected]>
Signed-off-by: Drumil Patel <[email protected]>
Signed-off-by: Drumil Patel <[email protected]>
Signed-off-by: Drumil Patel <[email protected]>
Signed-off-by: Drumil Patel <[email protected]>
@@ -203,6 +222,157 @@ func (c *EKS) K8SDeploymentsParse(*kingpin.ParseContext) error {
return nil
}

// listdownSubnetIds lists all subnet ids for given cluster name, if vpc for given cluster does not exists it creates vpc with given configuration.
func (c *EKS) listdownSubnetIds(clusterName string) ([]string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func (c *EKS) listdownSubnetIds(clusterName string) ([]string, error) {
func (c *EKS) listSubnetIds(clusterName string) ([]string, error) {

Copy link
Contributor

@geekodour geekodour left a comment

Choose a reason for hiding this comment

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

only nits :)

@@ -1,4 +1,5 @@
INFRA_CMD ?= ../infra/infra
PROVIDER ?= gke
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we re-declaring PROVIDER here? :)

projectid: {{ .GKE_PROJECT_ID }}
=======
projectid: {{ .PROJECT_ID }}
>>>>>>> Chnage as per review
Copy link
Contributor

Choose a reason for hiding this comment

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

Some conflicts didn't get resolved :octocat:

@bwplotka
Copy link
Member

Do you mind rebasing and fixing old rebase issues? It looks like a super useful change! Thanks! 🤗

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants