Skip to content

Commit 070f5ec

Browse files
authored
fix(eks): reuse chart name as chart dir for helmchart deployment from OCI repository (#23392)
Fixes #22341 It migrates assigning chart_dir from release name to chart name, to be consistent with helm-pull's target directory. I also added to integration test yet another chart deployment of a different chart. Why it's separate? Because of the way of how testing flow is working - if i understand it correctly it provisions previous snapshot, and finally after that provisions a new version of assets and test code **on top of existing**. Thanks to that i'm unable to cover my case in existing chart deployment, because when i change release name it conflicts with previously deployed (during old snapshot provisioning) chart (`UPDATE_FAILED (The following resource(s) failed to update: [Clustercharttestocichart9C188967]. ): Received response status [FAILED] from custom resource. Message returned: Error: b'Release "s3-chart-release" does not exist. Installing it now.\nError: rendered manifests contain a resource that already exists. Unable to continue with install: ServiceAccount "ack-s3-controller" in namespace "ack-system" exists and cannot be imported into the current release: invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-name" must equal "s3-chart-release": current value is "s3-chart"\n'`). Actually i successfully ran the modified integration test, but i didn't commit changes caused by yarn integ command, because i don't know which i have to commit. ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Construct Runtime Dependencies: * [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 660198b commit 070f5ec

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+479
-370
lines changed

packages/@aws-cdk/aws-eks/lib/kubectl-handler/helm/__init__.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ def helm_handler(event, context):
8282

8383
if repository is not None and repository.startswith('oci://'):
8484
tmpdir = tempfile.TemporaryDirectory()
85-
chart_dir = get_chart_from_oci(tmpdir.name, release, repository, version)
85+
chart_dir = get_chart_from_oci(tmpdir.name, repository, version)
8686
chart = chart_dir
8787

8888
helm('upgrade', release, chart, repository, values_file, namespace, version, wait, timeout, create_namespace)
@@ -123,7 +123,7 @@ def get_oci_cmd(repository, version):
123123
return cmnd
124124

125125

126-
def get_chart_from_oci(tmpdir, release, repository = None, version = None):
126+
def get_chart_from_oci(tmpdir, repository = None, version = None):
127127

128128
cmnd = get_oci_cmd(repository, version)
129129

@@ -135,7 +135,9 @@ def get_chart_from_oci(tmpdir, release, repository = None, version = None):
135135
output = subprocess.check_output(cmnd, stderr=subprocess.STDOUT, cwd=tmpdir, shell=True)
136136
logger.info(output)
137137

138-
return os.path.join(tmpdir, release)
138+
# effectively returns "$tmpDir/$lastPartOfOCIUrl", because this is how helm pull saves OCI artifact.
139+
# Eg. if we have oci://9999999999.dkr.ecr.us-east-1.amazonaws.com/foo/bar/pet-service repository, helm saves artifact under $tmpDir/pet-service
140+
return os.path.join(tmpdir, repository.rpartition('/')[-1])
139141
except subprocess.CalledProcessError as exc:
140142
output = exc.output
141143
if b'Broken pipe' in output:
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ def helm_handler(event, context):
8282

8383
if repository is not None and repository.startswith('oci://'):
8484
tmpdir = tempfile.TemporaryDirectory()
85-
chart_dir = get_chart_from_oci(tmpdir.name, release, repository, version)
85+
chart_dir = get_chart_from_oci(tmpdir.name, repository, version)
8686
chart = chart_dir
8787

8888
helm('upgrade', release, chart, repository, values_file, namespace, version, wait, timeout, create_namespace)
@@ -123,7 +123,7 @@ def get_oci_cmd(repository, version):
123123
return cmnd
124124

125125

126-
def get_chart_from_oci(tmpdir, release, repository = None, version = None):
126+
def get_chart_from_oci(tmpdir, repository = None, version = None):
127127

128128
cmnd = get_oci_cmd(repository, version)
129129

@@ -135,7 +135,7 @@ def get_chart_from_oci(tmpdir, release, repository = None, version = None):
135135
output = subprocess.check_output(cmnd, stderr=subprocess.STDOUT, cwd=tmpdir, shell=True)
136136
logger.info(output)
137137

138-
return os.path.join(tmpdir, release)
138+
return os.path.join(tmpdir, repository.rpartition('/')[-1])
139139
except subprocess.CalledProcessError as exc:
140140
output = exc.output
141141
if b'Broken pipe' in output:

packages/@aws-cdk/aws-eks/test/integ.eks-helm-asset.js.snapshot/asset.7215c88dd3e638d28329d4538b36cdbfb54233a4d972181795814f8b904d1037/outbound.js

Lines changed: 0 additions & 45 deletions
This file was deleted.

0 commit comments

Comments
 (0)