-
Notifications
You must be signed in to change notification settings - Fork 196
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
feat: migrate helm charts repo from odigos-charts to here #1429
Conversation
f88506a
to
d61b5d3
Compare
d61b5d3
to
6670bf4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
@edeNFed do you want to have a look as well?
# Retry to avoid flakiness (due to CRD race conditions in helm). Timeout after 60s. | ||
while ! helm upgrade --install odigos /tmp/odigos-charts/charts/odigos --create-namespace --namespace odigos-test-ns --set image.tag=e2e-test; do | ||
while ! helm upgrade --install odigos $GITHUB_WORKSPACE/helm/odigos --create-namespace --namespace odigos-test-ns --set image.tag=e2e-test; do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I used /tmp
to easily be able to run chainsaw tests locally outside Github Actions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that.
We need to somehow find the project root. Chainsaw just changes the current working directory and, outside of GHA, I don't even know how to retrieve it.
Do you know a way to reference project root in chainsaw tests that is not GHA-specific?
Alternatively we could just hardcode ../../
and have the tests break when there are changes in chainsaw structure.
EDIT: Now thinking about it, I think making it work locally is more valuable than making it resistant to folder structure changes. I'll make that change. Thanks for pointing this out! 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in c44935b
After merging this we will be managing helm charts in this repo fully. The old index will then stay up and working but will be stale.
This has a benefit of being able to update helm and code in the same PR, simplifying the whole release process and having e2e tests run on the latest unreleased helm version.
This PR will also change the chart versioning(currently
0.3.x
) to match the application version(currently1.0.x
) on every release.This includes 2bd8d7b of odigos-charts which is the last commit at the moment of creation of this PR.
Commits: