-
Notifications
You must be signed in to change notification settings - Fork 474
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
*: Add dedicated CRD helm chart #10828
base: main
Are you sure you want to change the base?
*: Add dedicated CRD helm chart #10828
Conversation
- helm create install/helm/kgateway-crds - Delete charts/ directory - Delete the templates/ directory - Remove all unrelated values.yaml Signed-off-by: timflannagan <[email protected]>
Signed-off-by: timflannagan <[email protected]>
Signed-off-by: timflannagan <[email protected]>
Signed-off-by: timflannagan <[email protected]>
Signed-off-by: timflannagan <[email protected]>
Signed-off-by: timflannagan <[email protected]>
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.
just a few comments/questions
# This is the chart version. This version number should be incremented each time you make changes | ||
# to the chart and its templates, including the app version. | ||
# Versions are expected to follow Semantic Versioning (https://semver.org/) | ||
version: 0.1.0 |
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 think I'm missing something, but is this version relevant?
package-kgateway-charts: package-kgateway-chart package-kgateway-crd-chart | ||
|
||
.PHONY: package-kgateway-chart | ||
package-kgateway-chart: ## Package the kgateway chart for testing |
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: is this comment accurate w.r.t. "for testing"?
Looks like this is the target used in the release workflow as well
@@ -1,6 +1,6 @@ | |||
apiVersion: v2 | |||
name: kgateway | |||
description: A Helm chart for Kubernetes | |||
description: A Helm chart for the kgateway project |
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.
👍
Description
Introduces a dedicated CRD helm chart in preparation for the 2.0 release. This aligns with helm best practices w.r.t CRD management. See https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#method-2-separate-charts for more information on this approach.
This is the first step towards auditing our install UX story. Having a dedicated chart before 2.0 is cut helps prevents any painful migrations for subsequent 2.x minor versions. See #10640 for some of the details captured there.
Checklist: