Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

Node Pool Nested Stack Name #187

Merged
merged 1 commit into from
Jan 6, 2017
Merged

Node Pool Nested Stack Name #187

merged 1 commit into from
Jan 6, 2017

Conversation

icereval
Copy link
Contributor

Nest node pool stack name by cluster name to avoid conflict when running multiple clusters.

e.g.

cluster-1
cluster-1-pool-1
cluster-1-pool-2
cluster-2
cluster-2-pool-1
cluster-2-pool-2

@codecov-io
Copy link

codecov-io commented Dec 31, 2016

Current coverage is 69.11% (diff: 66.66%)

Merging #187 into master will not change coverage

@@             master       #187   diff @@
==========================================
  Files             4          4          
  Lines          1130       1130          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits            781        781          
  Misses          261        261          
  Partials         88         88          

Powered by Codecov. Last update dec2402...db304dd

@@ -693,7 +693,7 @@ type Config struct {

// CloudFormation stack name which is unique in an AWS account.
// This is intended to be used to reference stack name from cloud-config as the target of awscli or cfn-bootstrap-tools commands e.g. `cfn-init` and `cfn-signal`
func (c Config) StackName() string {
func (c Cluster) StackName() string {
return c.ClusterName
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Any specific reason to move this method from Config to Cluster? (I guess for now there's no specific reason/it doesn't relate to the main purpose of this PR but you just thought it might be better to do so?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only that StackName was not available in the context of Cluster functions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it! I somehow missed from where the ClusterName field is coming.

@mumoshu
Copy link
Contributor

mumoshu commented Jan 6, 2017

E2E passed. LGTM 👍
Thanks for your contribution @icereval

@mumoshu mumoshu merged commit 37c2421 into kubernetes-retired:master Jan 6, 2017
@mumoshu mumoshu added this to the v0.9.3-rc.3 milestone Jan 6, 2017
@cknowles
Copy link
Contributor

cknowles commented Jan 8, 2017

@mumoshu do we want to also update the name tags of subresources?

kylehodgetts pushed a commit to HotelsDotCom/kube-aws that referenced this pull request Mar 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants