Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(chart): Updated release to not duplicate AH config #6022

Merged
merged 1 commit into from
Apr 21, 2024

Conversation

stevehipwell
Copy link
Contributor

@stevehipwell stevehipwell commented Apr 11, 2024

Fixes #N/A

Description
This PR fixes the duplication of the Artifact Hub config file for each chart release resulting in the artifacthub.io tag having a new digest after each release. I've also added a README and fixed the CRDs chart config.

There should be an untagged image for each chart release with AH config prior to the current release, these should be deleted unless there is an automated cleanup policy in place.

How was this change tested?
I switched to this pattern for my own Helm chart logic and have tested the logic there.

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

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

@stevehipwell stevehipwell requested a review from a team as a code owner April 11, 2024 09:10
@stevehipwell stevehipwell requested a review from bwagner5 April 11, 2024 09:10
@stevehipwell
Copy link
Contributor Author

CC @jonathan-innis

Copy link

netlify bot commented Apr 11, 2024

Deploy Preview for karpenter-docs-prod ready!

Name Link
🔨 Latest commit 5b83a09
🔍 Latest deploy log https://app.netlify.com/sites/karpenter-docs-prod/deploys/66227b790dfcd70008d03b9e
😎 Deploy Preview https://deploy-preview-6022--karpenter-docs-prod.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@stevehipwell stevehipwell force-pushed the fix-ah-config-duplication branch from 42d7f77 to 23e5e99 Compare April 12, 2024 11:43
@stevehipwell stevehipwell force-pushed the fix-ah-config-duplication branch from 23e5e99 to 44fee69 Compare April 15, 2024 13:09
@jonathan-innis jonathan-innis self-assigned this Apr 16, 2024
@stevehipwell stevehipwell force-pushed the fix-ah-config-duplication branch from 44fee69 to 9aab8df Compare April 17, 2024 12:54
@jonathan-innis
Copy link
Contributor

Otherwise, changes look reasonable to me

@coveralls
Copy link

coveralls commented Apr 19, 2024

Pull Request Test Coverage Report for Build 8754972446

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 82.015%

Totals Coverage Status
Change from base Build 8733550496: 0.02%
Covered Lines: 5372
Relevant Lines: 6550

💛 - Coveralls

@stevehipwell stevehipwell force-pushed the fix-ah-config-duplication branch from 9aab8df to 5b83a09 Compare April 19, 2024 14:11
Copy link
Contributor

@jonathan-innis jonathan-innis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@jonathan-innis jonathan-innis enabled auto-merge (squash) April 21, 2024 03:47
@jonathan-innis
Copy link
Contributor

jonathan-innis commented Apr 21, 2024

@stevehipwell Looks like the helm charts still have updated digests whenever they get pushed with the same tag. I haven't done a ton of looking but there's perhaps some work that we can do here to get a reproducible build so that the digest is always the same here.

What do you think about creating an issue for reproducible builds for the helm charts as well as the AH config (by eventually finding a build flag to pass into the manifest file) so that we can track down those fixes eventually in our CI systems?

@jonathan-innis jonathan-innis merged commit bc653f0 into aws:main Apr 21, 2024
18 checks passed
@stevehipwell stevehipwell deleted the fix-ah-config-duplication branch April 22, 2024 09:05
@stevehipwell
Copy link
Contributor Author

@jonathan-innis do you mean the digest of the Helm chart created from the same raw files and uploaded with a helm push? If so then I suspect that this is an ORAS limitation as that's what Helm is using under the hood too. I'll take a look at if I can manually set the OCI metadata for ORAS and solve it at that layer. For Helm I can open a PR there for reproducible builds (although this is likely reproducible pushes.

For Karpenter you (probably) could manually implement the helm push with oras as I've done for the AH config to solve this today.

jmdeal added a commit to jmdeal/karpenter-provider-aws that referenced this pull request May 29, 2024
This partially reverts commit bc653f0.
@jmdeal jmdeal mentioned this pull request May 29, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants