(eks): deploying helm chart to cluster created in another stack fails with circular dependency
What is the problem?
I'm attempting to deploy a helm chart, using assets, to a cluster which I created in a separate stack. It fails with a circular dependency error due to the kubectl barrier.
I can see the original issue was addressed in #7322 with a test added for a simple kubernetes manifest. This looks to have been reverted a few months later in https://github.com/aws/aws-cdk/commit/4fad9bcbd75702e89eea02a140aa010f8952329a
I'm not even sure if this should be considered a bug since that later commit quite clearly states everything should await that new barrier object. However this does to me seem to break out-of-stack helm deployments using assets. If it is the expected behaviour it'd be helpful to call it out on the HelmChart docs page.
If this looks feature worthy (I'm not sure how feasible it actually is), let me know and I'll stick a request in.
Reproduction Steps
// Run this in stack2, with a cluster created in stack1
const chartAsset = new s3Assets.Asset(this, "ChartAsset", {
path: "./helm",
});
new eks.HelmChart(this, "foobar", {
cluster,
chartAsset: chartAsset
});
What did you expect to happen?
I expected to be able to deploy a helm chart from a different stack
What actually happened?
'stack2' depends on 'stack1' ("stack2/foobar/Resource/Default" depends on "stack1/cluster/KubectlReadyBarrier"). Adding this dependency (stack1/@aws-cdk--aws-eks.KubectlProvider -> stack2/AssetParameters/guidlookingthing/S3Bucket.Ref) would create a cyclic reference.
CDK CLI Version
1.149.0
Framework Version
No response
Node.js Version
16.3.0
OS
MacOs 12.3 (M1)
Language
Typescript
Language Version
No response
Other information
No response
It's also worth mentioning that the test here adds to the confusion - https://github.com/aws/aws-cdk/blob/43efe5976f7442cae81c83886ee3869695ae7e90/packages/%40aws-cdk/aws-eks/test/cluster.test.ts#L1244
The description suggests that it's testing kubectl resources can be created in separate stacks. Depending on the actual desired behaviour we might want to consider changing this to also test the s3 assets as noted in the bug report, or renaming/removing this test.