Create control plane with cloudformation #126
Conversation
nckturner
commented
Jul 18, 2018
nckturner
commented
- retains old behavior with flag but defaults to new
- copied assets into a new asset dir 1.10.3/2018-07-18
- Fixes Create EKS cluster using Cloudformation instead of raw API calls #76
|
This is great, thanks a lot! I was just thinking of looking into it myself. I will review later on. |
|
@nckturner actually, I'm not sure we need the flag, because it's a change in implementation details, and users should rely on such a thing, unless you thinks it fundamentally different? |
pkg/eks/cfn.go
Outdated
| @@ -289,6 +290,81 @@ func (c *ClusterProvider) DeleteStackServiceRole() error { | |||
| return nil | |||
| } | |||
|
|
|||
| func (c *ClusterProvider) stackNameDefaultControlPlane() string { | |||
| return "EKS-" + c.cfg.ClusterName + "-DefaultControlPlane" | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
@errordeveloper yeah I would have left the flag out had it been my project, but I wasn't sure if backwards compatibility was required, so I threw it in for good measure. Would you like me to leave it in to be removed in the next release or so, or just remove it now? I'm not sure why someone wouldn't want a cfn resource for their cluster, but perhaps there could be scenarios. It does (should) make deleting the cluster different, as you should do it through the stack. |
nckturner
force-pushed
the
cloudformation-control-plane
branch
from
4920c60
to
550574b
Compare
|
I am with you. Don't think there is a need to keep clusters creation
backwards compatible, but let's make sure deletions are backwards
compatible. We don't need a flag for deletions, we can just attempt
deleting the cluster if is present, otherwise delete the CFN stack. And we
can break this compatibility in the next release for sure.
…On Wed, 18 Jul 2018, 10:04 pm Nicholas Turner, ***@***.***> wrote:
@errordeveloper <https://github.com/errordeveloper> yeah I would have
left the flag out had it been my project, but I wasn't sure if backwards
compatibility was required, so I threw it in for good measure. Would you
like me to leave it in to be removed in the next release or so, or just
remove it now? I'm not sure why someone *wouldn't* want a cfn resource
for their cluster, but perhaps there could be scenarios. It does (should)
make deleting the cluster different, as you should do it through the stack.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#126 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAPWSyuisjUiLGd5GPd76729JXSiVXFXks5uH6LPgaJpZM4VUmq4>
.
|
|
I think the refactoring PR will take a while. We can land this right after 0.1.0, so hopefully before the end of this week! Do you want to try rebasing? |
errordeveloper
force-pushed
the
cloudformation-control-plane
branch
from
550574b
to
0fdfd2f
Compare
|
@nckturner I 've rebased and added one more commit in your branch, I'm happy to land this right after 0.1.0. |
|
And of course, now we have deletion also. |
errordeveloper
force-pushed
the
cloudformation-control-plane
branch
2 times, most recently
from
8274d26
to
c9b0b17
Compare
|
Ah thanks, I was just working on this last night, but you beat me to it! Sounds good to me! |
errordeveloper
force-pushed
the
cloudformation-control-plane
branch
2 times, most recently
from
9d855c3
to
8d0c30c
Compare
- retains old behavior with flag but defaults to new - copied assets into a new asset dir 1.10.3/2018-07-18
errordeveloper
force-pushed
the
cloudformation-control-plane
branch
from
8d0c30c
to
719f33b
Compare
Wait for volume to become available
