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!: play nicer with helm #1450

Merged
merged 7 commits into from
Feb 2, 2024

Conversation

krancour
Copy link
Member

@krancour krancour commented Feb 2, 2024

Fixes #1281

May partially address #1280, although some aspects of that issue are strictly UI-related.

Supersedes #1294

Draft because I am still working out some kinks.

Summary

With respect to Helm charts, the term "registry" has proven intuitive to people very familiar with OCI registries, but much more befuddling to those who are more familiar with classic (https/s-based) chart repositories. Confusion is compounded by Argo CD and Helm itself (e.g. in dependencies section of a chart.yaml file) continuing to exclusively use the classic repository parlance.

This PR implements a strategy refined through extensive discussion in #1294 and strikes "registry" from the Kargo vernacular.

  • When subscribing a warehouse to a chart repo you:

    • Use URL + name for classic repos. (Classic repos can contain differently named charts, so the name field provides necessary qualification.)
    • Use URL only for oci repos. (OCI repo URLs imply the chart's name.)
  • When defining promotion mechanisms:

    • Updates to a chart.yaml simply require you to match repository and name fields exactly as they appear in the chart.yaml. Kargo will figure out the rest.
    • Updates to an Application source require you to match the repoURL and chart fields exactly as they appear in the Application source.

Incidentally, this PR also changes some function names that contained the phrase "getLatestChart" to "selectChart." "Latest" implies something that isn't necessarily true since chart selection can be constrained with semver constraints.

Copy link

netlify bot commented Feb 2, 2024

Deploy Preview for docs-kargo-akuity-io ready!

Name Link
🔨 Latest commit 39c178f
🔍 Latest deploy log https://app.netlify.com/sites/docs-kargo-akuity-io/deploys/65bd27c9cb9702000830df88
😎 Deploy Preview https://deploy-preview-1450.kargo.akuity.io
📱 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.

@krancour krancour added this to the v0.4.0 milestone Feb 2, 2024
Copy link

codecov bot commented Feb 2, 2024

Codecov Report

Attention: 38 lines in your changes are missing coverage. Please review.

Comparison is base (397fc3e) 48.77% compared to head (39c178f) 48.84%.

Files Patch % Lines
internal/helm/helm.go 28.57% 14 Missing and 1 partial ⚠️
internal/controller/warehouses/helm.go 55.17% 11 Missing and 2 partials ⚠️
internal/controller/stages/health.go 53.33% 5 Missing and 2 partials ⚠️
internal/api/query_freights_v1alpha1.go 71.42% 2 Missing ⚠️
internal/controller/promotion/argocd.go 85.71% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1450      +/-   ##
==========================================
+ Coverage   48.77%   48.84%   +0.06%     
==========================================
  Files         131      131              
  Lines       12026    12087      +61     
==========================================
+ Hits         5866     5904      +38     
- Misses       5959     5978      +19     
- Partials      201      205       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@krancour krancour force-pushed the krancour/alt-helm-improvements branch 2 times, most recently from 8523cbf to ff26e0f Compare February 2, 2024 15:13
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
@krancour krancour force-pushed the krancour/alt-helm-improvements branch 2 times, most recently from 48383bc to 683b7ac Compare February 2, 2024 15:33
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
@krancour krancour force-pushed the krancour/alt-helm-improvements branch 2 times, most recently from bb2c5a2 to fb315d8 Compare February 2, 2024 15:53
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
@krancour krancour force-pushed the krancour/alt-helm-improvements branch from fb315d8 to 39c178f Compare February 2, 2024 17:35
@krancour krancour marked this pull request as ready for review February 2, 2024 19:00
@krancour krancour requested a review from a team as a code owner February 2, 2024 19:00
Copy link
Member

@jessesuen jessesuen left a comment

Choose a reason for hiding this comment

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

Great improvement

@krancour krancour added this pull request to the merge queue Feb 2, 2024
Merged via the queue into akuity:main with commit 9a4c04d Feb 2, 2024
16 checks passed
@krancour krancour deleted the krancour/alt-helm-improvements branch February 2, 2024 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm Chart.yaml based promotion not working
2 participants