Skip to content

Commit

Permalink
fix(eks): KubernetesPatch and FargateCluster creates a circular d…
Browse files Browse the repository at this point in the history
…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*
  • Loading branch information
iliapolo authored Sep 25, 2020
1 parent e349004 commit b23ce03
Show file tree
Hide file tree
Showing 4 changed files with 1,415 additions and 2 deletions.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-eks/lib/k8s-patch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class KubernetesPatch extends Construct {
super(scope, id);

const stack = Stack.of(this);
const provider = KubectlProvider.getOrCreate(scope, props.cluster);
const provider = KubectlProvider.getOrCreate(this, props.cluster);

new CustomResource(this, 'Resource', {
serviceToken: provider.serviceToken,
Expand Down
Loading

0 comments on commit b23ce03

Please sign in to comment.