feat(eks): support adding k8s resources to imported clusters #9802

eladb · 2020-08-18T16:26:59Z

Allow adding Kubernetes resources such as manifests and Helm charts to imported clusters (eks.Cluster.fromAttributes).

To enable this behavior, when the cluster is imported, users will have to specify additional information:

kubectlRole - an IAM role that can issue kubectl commands against the cluster
kubectlEnvironment (optional) - environment variables for kubectl.
kubectlPrivateSubnets and kubectlSecurityGroup - required if the cluster's k8s endpoint is private

Resolves #5383

BREAKING CHANGE: when importing EKS clusters using eks.Cluster.fromClusterAttributes, the clusterArn attribute is not supported anymore, and will always be derived from clusterName.

eks: Only a single eks.Cluster is allowed per CloudFormation stack.
eks: The securityGroups attribute of ClusterAttributes is now securityGroupIds.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

Allow adding Kubernetes resources such as manifests and Helm charts to imported clusters (`eks.Cluster.fromAttributes`). To enable this behavior, when the cluster is imported, users will have to specify additional information: - `kubectlRole` - an IAM role that can issue kubectl commands against the cluster - `kubectlEnvironment` (optional) - environment variables for `kubectl`. - `kubectlPrivateSubnets` and `kubectlSecurityGroup` - required if the cluster's k8s endpoint is private Resolves #5383

packages/@aws-cdk/aws-eks/README.md

packages/@aws-cdk/aws-eks/lib/cluster.ts

packages/@aws-cdk/aws-eks/lib/kubectl-provider.ts

… replacement

…nifest-to-imported-cluster

iliapolo · 2020-09-02T10:09:44Z

@Mergifyio refresh

mergify · 2020-09-02T10:10:16Z

Command refresh: success

iliapolo · 2020-09-02T10:10:23Z

@eladb Any reason the do-not-merge label is still here?

eladb · 2020-09-02T11:27:10Z

@Mergifyio refresh

mergify · 2020-09-02T11:27:12Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2020-09-02T11:27:15Z

Command refresh: success

mergify · 2020-09-02T12:06:46Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2020-09-02T12:07:21Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: fa0473f
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

pahud · 2020-09-02T12:13:49Z

This is amazing! Thank you for the great work!

eladb · 2020-09-02T12:54:56Z

This is amazing! Thank you for the great work!

Thanks! Looking forward for feedback :-)

itajaja · 2020-09-15T07:17:32Z

hey, just updated. we have two clusters in our stack. does this mean we cannot update, or is there a way to go around this?

iliapolo · 2020-09-15T08:39:35Z

@itajaja This limitation actually short-circuits some problems you might encounter with multiple clusters in a stack with regards to endpoint access configuration. Is it possible for you to split the cluster to different stacks?

itajaja · 2020-09-15T08:46:31Z

what kinds of problems? are they solvable through aws-cdk alone?

Is it possible for you to split the cluster to different stacks?

it would be quite awkward.

iliapolo · 2020-09-15T09:03:08Z

@itajaja

what kinds of problems? are they solvable through aws-cdk alone?

Currently no. The problem is that having multiple clusters in the same stack would force those clusters to have a publicly accessible endpoint (i.e only EndpointAccess.PUBLIC).

Apologies for the inconvenience, we have created an issue to gather feedback on this limitation and think of possible solutions. Would be great if you can share your use case.

…ependency and breaks deployment (#10536) In version [`1.62.0`](https://github.com/aws/aws-cdk/releases/tag/v1.62.0) we introduced the ability to run `kubectl` commands on imported clusters. (See #9802). Part of this change included some refactoring with regards to how we use and create the `KubectlProvider`. Looks like we didn't consistently apply the same logic across all constructs that use it. Case in point: https://github.com/aws/aws-cdk/blob/e349004a522e2123c1e93bd3402dd7c3f9c5c17c/packages/%40aws-cdk/aws-eks/lib/k8s-manifest.ts#L58 Notice that here we use `this` as the scope to the `getOrCreate` call. Same goes for: https://github.com/aws/aws-cdk/blob/e349004a522e2123c1e93bd3402dd7c3f9c5c17c/packages/%40aws-cdk/aws-eks/lib/k8s-object-value.ts#L64 However, `KubernetesPatch` use `scope` instead. https://github.com/aws/aws-cdk/blob/e349004a522e2123c1e93bd3402dd7c3f9c5c17c/packages/%40aws-cdk/aws-eks/lib/k8s-patch.ts#L74 This means that the entire `scope` of the `KubernetesPatch` now depends, among others, on the `kubectlBarrier`. The scope will usually be either the cluster itself (when using `FargateCluster`), or the entire stack (when using `new KubernetesPatch`). In any case, the scope will most likely contain the cluster VPC. This creates the following dependency cycle: `Cluster => ClusterVpc => KubectlBarrier => Cluster`. The fix aligns the `KubernetesPatch` behavior to all other `kubectl` constructs and uses `this` as the scope, which will only add dependency on the barrier to the custom resource representing the patch. Fixes #10528 Fixes #10537 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

mergify bot added the contribution/core This is a PR that came from AWS. label Aug 18, 2020

SomayaB assigned iliapolo Aug 18, 2020

bvtujo mentioned this pull request Aug 18, 2020

[aws-s3-assets] ci build fails due to docker hub rate limit #9803

Closed

Elad Ben-Israel and others added 8 commits August 19, 2020 14:35

Update README.md

096705b

Merge branch 'master' into benisrae/eks-add-manifest-to-imported-cluster

4963422

integ tests working

2e10a16

some refactor

667a5ea

mid work

77db13e

Merge branch 'master' into benisrae/eks-add-manifest-to-imported-cluster

09769dc

typos

9422bbd

Merge branch 'master' into benisrae/eks-add-manifest-to-imported-cluster

379ee80

eladb mentioned this pull request Sep 1, 2020

feat(eks): kubectl layer customization #10090

Merged

Elad Ben-Israel added 3 commits September 1, 2020 13:26

unit tests pass

62bd355

allow cluster provider role to assume admin role

8cbf061

update integ test expectations

9d0e326

iliapolo requested changes Sep 2, 2020

View reviewed changes

readme updates following cr

a085671

eladb requested a review from iliapolo September 2, 2020 08:25

Elad Ben-Israel added 3 commits September 2, 2020 11:33

additional CR fixes

ed99f9f

revert IAM role to the ClusterResource scope to preserve ID and avoid…

ff02937

… replacement

update tests

5a612be

eladb added the pr/do-not-merge This PR should not be merged at this time. label Sep 2, 2020

Elad Ben-Israel added 2 commits September 2, 2020 12:05

Merge branch 'master' into benisrae/eks-add-manifest-to-imported-cluster

f3ca25c

Merge remote-tracking branch 'origin/master' into benisrae/eks-add-ma…

0f2177a

…nifest-to-imported-cluster

iliapolo approved these changes Sep 2, 2020

View reviewed changes

eladb removed the pr/do-not-merge This PR should not be merged at this time. label Sep 2, 2020

Merge branch 'master' into benisrae/eks-add-manifest-to-imported-cluster

fa0473f

mergify bot merged commit 4439481 into master Sep 2, 2020

mergify bot deleted the benisrae/eks-add-manifest-to-imported-cluster branch September 2, 2020 12:06

stefanolczak mentioned this pull request Sep 10, 2020

[aws-eks] private subnets from other Cluster or Vpc can't be used when importing cluster #10287

Closed

iliapolo mentioned this pull request Sep 15, 2020

[aws-eks] Allow multiple EKS clusters in the same stack #10073

Closed

iliapolo mentioned this pull request Sep 25, 2020

fix(eks): KubernetesPatch and FargateCluster creates a circular dependency and breaks deployment #10536

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(eks): support adding k8s resources to imported clusters #9802

feat(eks): support adding k8s resources to imported clusters #9802

eladb commented Aug 18, 2020 •

edited

iliapolo commented Sep 2, 2020

mergify bot commented Sep 2, 2020

iliapolo commented Sep 2, 2020

eladb commented Sep 2, 2020

mergify bot commented Sep 2, 2020

mergify bot commented Sep 2, 2020

mergify bot commented Sep 2, 2020

aws-cdk-automation commented Sep 2, 2020

pahud commented Sep 2, 2020

eladb commented Sep 2, 2020

itajaja commented Sep 15, 2020

iliapolo commented Sep 15, 2020

itajaja commented Sep 15, 2020

iliapolo commented Sep 15, 2020

feat(eks): support adding k8s resources to imported clusters #9802

feat(eks): support adding k8s resources to imported clusters #9802

Conversation

eladb commented Aug 18, 2020 • edited

iliapolo commented Sep 2, 2020

mergify bot commented Sep 2, 2020

iliapolo commented Sep 2, 2020

eladb commented Sep 2, 2020

mergify bot commented Sep 2, 2020

mergify bot commented Sep 2, 2020

mergify bot commented Sep 2, 2020

aws-cdk-automation commented Sep 2, 2020

AWS CodeBuild CI Report

pahud commented Sep 2, 2020

eladb commented Sep 2, 2020

itajaja commented Sep 15, 2020

iliapolo commented Sep 15, 2020

itajaja commented Sep 15, 2020

iliapolo commented Sep 15, 2020

eladb commented Aug 18, 2020 •

edited