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

feat: add CF stackNameOverride in cluster.yaml #1484

Merged
merged 2 commits into from
Nov 9, 2018

Conversation

koen92
Copy link
Contributor

@koen92 koen92 commented Nov 7, 2018

Kube-aws 0.11.x introduced a separate CloudFormation network stack. AWS VPCs are not movable between two CloudFormation stacks. This means that Kube-aws 0.11.x will create a new VPC when upgrading.

Because we added some extra LoadBalancers and RDS Databases that we cannot easily migrate to another VPC without downtime, creating a new VPC is not desired for us.

Therefore, we decided to keep the network stack in the original control plane stack and instead moved the control plane to a new separate stack. This means that we can stick with the original VPC.

The easiest way to do this with Kube-aws without adding legacy code, is by allowing the user to override the CF stacks names of the control plane, network and etcd stacks in cluster.yaml.

This PR adds an option in cluster.yaml to override the name of the CF controlPlane,
network and etcd stacks.

Add option in cluster.yaml to override the name of CF controlPlane,
network and etcd stacks.

This is useful for migration of CF stacks from kube-aws <0.11.x without
creating a new VPC.
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 7, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: mumoshu

If they are not already assigned, you can assign the PR to them by writing /assign @mumoshu in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 7, 2018
@codecov-io
Copy link

codecov-io commented Nov 7, 2018

Codecov Report

Merging #1484 into master will decrease coverage by 0.05%.
The diff coverage is 23.52%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1484      +/-   ##
==========================================
- Coverage   37.92%   37.86%   -0.06%     
==========================================
  Files          75       75              
  Lines        4596     4608      +12     
==========================================
+ Hits         1743     1745       +2     
- Misses       2611     2620       +9     
- Partials      242      243       +1
Impacted Files Coverage Δ
core/controlplane/config/config.go 62.88% <21.42%> (-0.62%) ⬇️
core/controlplane/cluster/cluster.go 50.22% <33.33%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bfa9f54...4626d27. Read the comment docs.

@koen92
Copy link
Contributor Author

koen92 commented Nov 7, 2018

/assign @mumoshu

@mumoshu
Copy link
Contributor

mumoshu commented Nov 8, 2018

Hey! Thanks for submitting this.

we added some extra LoadBalancers and RDS Databases that we cannot easily migrate

Ahhh... this is something I strongly suggest NOT to do. Sorry If I wasn't very clear about that in the kube-aws docs or related GitHub issues.

Please give me a moment and I'll try to understand what you're going to do. Thanks.

@mumoshu
Copy link
Contributor

mumoshu commented Nov 8, 2018

@koen92 Would you also mind submitting this to v0.11.x and v0.12.x branches? So that we can cut v0.11.2 and v0.12.1 with this.

The current master is for v0.13.x development.

Edit: Fixed the wrong version nums in my prev comment

@mumoshu mumoshu added this to the v0.13.0 milestone Nov 8, 2018
# In this scenario the name of the networkstack becomes "ControlPlane". (which is the name of the stack which holds the vpc components in a kube-aws pre 0.11 cluster)
# For clearity it is advised also rename the stack of the controlPlane components to, for example, "masters"
# Note that this will keep some legacy naming around in your configs and CF stacks...
#stackNameOverride:
Copy link
Contributor

@mumoshu mumoshu Nov 8, 2018

Choose a reason for hiding this comment

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

Nit but:

cloudformation:
  stackNameOverrides:
    ...

sounds slightly nicer to me because it expresses you can one or more stack names customized, and it works in term of CloudFormation.

Because it is part of the cloudformation config in cluster.yaml.
@koen92
Copy link
Contributor Author

koen92 commented Nov 8, 2018

Ahhh... this is something I strongly suggest NOT to do. Sorry If I wasn't very clear about that in the kube-aws docs or related GitHub issues.

Yeah this maybe was a mistake on our side that we're now stuck with. Luckily, this PR fixes the problems for us for now!

@koen92 Would you also mind submitting this to v0.11.x and v0.12.x branches? So that we can cut v0.11.2 and v0.12.1 with this.

I submitted two PRs for the v0.11.x and v0.12.x branches:

Nit but:

cloudformation:
  stackNameOverrides:
    ...

sounds slightly nicer to me because it expresses you can one or more stack names customized, and it works in term of CloudFormation.

Thnx for the feedback! I fixed this in a5a359b.

@mumoshu Let me know, thanks!

Copy link
Contributor

@mumoshu mumoshu left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot for contributing this!

@mumoshu mumoshu merged commit 6e68d41 into kubernetes-retired:master Nov 9, 2018
mumoshu pushed a commit to mumoshu/kube-aws that referenced this pull request Nov 9, 2018
kevtaylor pushed a commit to HotelsDotCom/kube-aws that referenced this pull request Jan 9, 2019
)

* feat: add CF stackNameOverride in cluster.yaml

Add option in cluster.yaml to override the name of CF controlPlane,
network and etcd stacks.

This is useful for migration of CF stacks from kube-aws <0.11.x without
creating a new VPC.

* wrap stackNameOverrides into cloudformation config

Because it is part of the cloudformation config in cluster.yaml.
@koen92 koen92 deleted the cf-stack-name-override branch January 14, 2019 12:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants