search

akuity / kargo

Application lifecycle orchestration
https://kargo.akuity.io/
Apache License 2.0
1.39k stars 114 forks source link

fix: Multi-source Helm ArgoCD app support #2160

Open gnadaban opened 2 weeks ago

gnadaban commented 2 weeks ago

Changes in PR:

netlify[bot] commented 2 weeks ago

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

Name Link
Latest commit 68abfe37b831899dc21ab77826df1c13033c5ae9
Latest deploy log https://app.netlify.com/sites/docs-kargo-akuity-io/deploys/667f1d3d31a47600080934d1
Deploy Preview https://deploy-preview-2160.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.

gnadaban commented 2 weeks ago

@krancour , this is a WIP but I've made some good progress fixing the issues I found.

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 91.73554% with 10 lines in your changes missing coverage. Please review.

Project coverage is 46.52%. Comparing base (d5c5590) to head (68abfe3). Report is 12 commits behind head on main.

Files Patch % Lines
internal/controller/promotion/argocd.go 72.72% 8 Missing and 1 partial :warning:
internal/controller/stages/stages.go 91.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2160 +/- ## ========================================== + Coverage 46.30% 46.52% +0.22% ========================================== Files 242 242 Lines 16767 16856 +89 ========================================== + Hits 7764 7843 +79 - Misses 8632 8641 +9 - Partials 371 372 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

gnadaban commented 2 weeks ago

@hiddeco , any other concerns with this one, other than having to wait until #2156 is merged?

gnadaban commented 6 days ago

Is there anything else to address for this one @krancour or @hiddeco ?

hiddeco commented 6 days ago

This is on our list to look at, but given the (upstream) complexity it may take a while before we have time to validate this end-to-end. Please bear with us until then, thanks 🙇

gnadaban commented 5 days ago

Hey @hiddeco , I made some good progress on trying to test this with Argo Rollouts verifications, and found additional code bits in health.go that needed to be updated to support multi-source ArgoCD apps.

I think it would make sense to add those fixes in this PR, thoughts?

hiddeco commented 5 days ago

That's quite likely, as I think what you are working on compliments what someone else tried to do before in #2088.

gnadaban commented 4 days ago

Added health check fix and tests for multi-source app support.