Split CRDs into a different chart so they could be fully managed by helm #252
+156
−29
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @gossion, thanks so so much for the contribution. I kicked off CI, and will hopefully have a chance to review later today :) |
EItanya
reviewed
Apr 18, 2025
| .PHONY: helm-publish | ||
| helm-publish: helm-version | ||
| helm push kagent-crds-$(VERSION).tgz oci://ghcr.io/kagent-dev/kagent/helm |
There was a problem hiding this comment.
Note to self: This may fail the release since the OCI repo won't exist beforehand, I can solve that if this happens.
There was a problem hiding this comment.
Thanks @EItanya , please help if that happen.
Comment on lines
+64
to
+72
| if strings.Contains(output, "exists and cannot be imported into the current release") { | ||
| s.Stop() | ||
| c.Println("Warning: CRDs already exist but not managed by helm, you might need to delete them manually to make them fully managed by helm.") | ||
| s.Start() | ||
| } else { | ||
| c.Println("Error installing kagent-crds:", output) | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
Not a requirement, but since we are in a repl anyway we could offer to do this for the user.
There was a problem hiding this comment.
That is a good point.
Here is what I am thinking:
- InstallCmd - still succeeds (to avoid breaking change) when CRDs are not managed by helm, but warns that users need to manually delete CRDs or run uninstall cmd to delete CRDs.
- UninstallCmd - adding steps in uninstall to delete CRDs (by kubctl?) if helm release not found but CRDs are there.
how do you think?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces the changes to the Helm chart structure and CLI commands for managing the Kagent application. The primary goal is to separate the Kagent Custom Resource Definitions (CRDs) into their own Helm chart, improving modularity, version control, and adherence to Helm best practices. Additionally, the CLI commands for installation and uninstallation have been updated to align with this new structure.
original helm release:
new helm releases:
It will also fix #234. Note: For the existing old CRDs deployed by old helm chart:
users need to delete those resources manually so the next
helm installcould use the separated CRDs chart.