Feature/import clusters #696
Conversation
shapirov103
requested review from
aws-ia-ci,
a team,
parkand1,
youngjeong46 and
elamaran11
as code owners
There was a problem hiding this comment.
@shapirov103 This is awesome work. Could we have a doc update with a section to explain to users how to import an existing cluster to CDK Blueprints framework for deploying addons?
Yeah, it is coming. I am thinking I will add a ImportClusterProvider to the blueprint and a pattern for it as well. |
|
/do-e2e-tests |
There was a problem hiding this comment.
@shapirov103 Looks good to me. Great work and much needed feature. We might need to have a new doc like import-cluster-provider.md showing how to using this feature. I know we are also working on a a pattern but a doc would help.
That's for sure. I am testing it out now in the patterns and there will be a bit of back and forth on this PR. Once ready with examples I will add the docs. |
|
/do-e2e-tests |
|
@elamaran11 please take another look for review, it is ready now. |
|
/do-e2e-tests |
There was a problem hiding this comment.
@shapirov103 Great work. Couple of minor comments for better clarity to users.
| **Note:** `blueprints.describeCluster() is an asynchronous function, you should either use `await` or handle promise resolution chain. | ||
|
|
||
| ```typescript | ||
| const sdkCluster = await blueprints.describeCluster(clusterName, region); // get cluster information using EKS APIs |
There was a problem hiding this comment.
Is clusterName a variable. If so we should also add that and show people of a sample populated value.
There was a problem hiding this comment.
The downside of that is people mindlessly copying this to their env just to discover that it does not work, similar to what we had with the update-kubeconfig frm the blog post. But for consistency I will add sample data.
| This option is convenient if you already know the VPC Id of the target cluster. It also requires `eks:DescribeCluster` permission at build-time: | ||
|
|
||
| ```typescript | ||
| const kubectlRole: iam.IRole = blueprints.getNamedResource('my-role'); |
There was a problem hiding this comment.
Is clusterName a variable. If so we should also add that and show people of a sample populated value.
| ```typescript | ||
|
|
||
| const importClusterProvider3 = new ImportClusterProvider({ | ||
| clusterName: myClusterName, |
There was a problem hiding this comment.
show variable population like myClusterName, API Server Endpoint with example.
|
@elamaran11 addressed feedback |
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.