-
Notifications
You must be signed in to change notification settings - Fork 480
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
Changes from 6 commits
24f925d
2f3590b
81d5a53
ddb572a
3df1978
0d3e90e
065ce40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -493,14 +493,31 @@ envoy-wrapper-distroless-docker: $(ENVOYINIT_OUTPUT_DIR)/envoyinit-linux-$(GOARC | |
# Helm | ||
#---------------------------------------------------------------------------------- | ||
|
||
.PHONY: package-kgateway-chart | ||
HELM ?= helm | ||
HELM_PACKAGE_ARGS ?= --version $(VERSION) | ||
package-kgateway-chart: ## Package the new kgateway helm chart for testing | ||
HELM_CHART_DIR=install/helm/kgateway | ||
HELM_CHART_DIR_CRD=install/helm/kgateway-crds | ||
|
||
.PHONY: package-kgateway-charts | ||
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 commentThe reason will be displayed to describe this comment to others. Learn more. nit: is this comment accurate w.r.t. "for testing"? |
||
mkdir -p $(TEST_ASSET_DIR); \ | ||
$(HELM) package $(HELM_PACKAGE_ARGS) --destination $(TEST_ASSET_DIR) install/helm/kgateway; \ | ||
$(HELM) package $(HELM_PACKAGE_ARGS) --destination $(TEST_ASSET_DIR) $(HELM_CHART_DIR); \ | ||
$(HELM) repo index $(TEST_ASSET_DIR); | ||
|
||
.PHONY: package-kgateway-crd-chart | ||
package-kgateway-crd-chart: ## Package the kgateway crd chart for testing | ||
mkdir -p $(TEST_ASSET_DIR); \ | ||
$(HELM) package $(HELM_PACKAGE_ARGS) --destination $(TEST_ASSET_DIR) $(HELM_CHART_DIR_CRD); \ | ||
$(HELM) repo index $(TEST_ASSET_DIR); | ||
|
||
.PHONY: lint-kgateway-charts | ||
lint-kgateway-charts: ## Lint the kgateway charts | ||
$(HELM) lint $(HELM_CHART_DIR) | ||
$(HELM) lint $(HELM_CHART_DIR_CRD) | ||
|
||
#---------------------------------------------------------------------------------- | ||
# Release | ||
#---------------------------------------------------------------------------------- | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
# Patterns to ignore when building packages. | ||
# This supports shell glob matching, relative path matching, and | ||
# negation (prefixed with !). Only one pattern per line. | ||
.DS_Store | ||
# Common VCS dirs | ||
.git/ | ||
.gitignore | ||
.bzr/ | ||
.bzrignore | ||
.hg/ | ||
.hgignore | ||
.svn/ | ||
# Common backup files | ||
*.swp | ||
*.bak | ||
*.tmp | ||
*.orig | ||
*~ | ||
# Various IDEs | ||
.project | ||
.idea/ | ||
*.tmproj | ||
.vscode/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
apiVersion: v2 | ||
name: kgateway-crds | ||
description: A Helm chart for the kgateway project CRDs | ||
|
||
# A chart can be either an 'application' or a 'library' chart. | ||
# | ||
# Application charts are a collection of templates that can be packaged into versioned archives | ||
# to be deployed. | ||
# | ||
# Library charts provide useful utilities or functions for the chart developer. They're included as | ||
# a dependency of application charts to inject those utilities and functions into the rendering | ||
# pipeline. Library charts do not define any templates and therefore cannot be deployed. | ||
type: application | ||
|
||
# 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 commentThe reason will be displayed to describe this comment to others. Learn more. I think I'm missing something, but is this version relevant? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's relevant, but we always provide a --version flag when publishing helm charts, so the chart There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll update the value of this to be consistent with the kgateway chart.yaml. Just pushed that change in the most recent commit. |
||
|
||
# This is the version number of the application being deployed. This version number should be | ||
# incremented each time you make changes to the application. Versions are not expected to | ||
# follow Semantic Versioning. They should reflect the version the application is using. | ||
# It is recommended to use it with quotes. | ||
appVersion: "1.16.0" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
# Default values for kgateway-crds. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
package helmutils | ||
|
||
const ( | ||
ChartName = "kgateway" | ||
ChartName = "kgateway" | ||
CRDChartName = "kgateway-crds" | ||
|
||
DefaultChartUri = "oci://ghcr.io/kgateway-dev/charts/kgateway" | ||
DefaultChartUri = "oci://ghcr.io/kgateway-dev/charts/kgateway" | ||
DefaultCRDChartUri = "oci://ghcr.io/kgateway-dev/charts/kgateway-crds" | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -148,10 +148,23 @@ func (i *TestInstallation) InstallKgatewayFromLocalChart(ctx context.Context) { | |
return | ||
} | ||
|
||
chartUri, err := helper.GetLocalChartPath(helmutils.ChartName) | ||
// install the CRD chart first | ||
crdChartURI, err := helper.GetLocalChartPath(helmutils.CRDChartName) | ||
i.Assertions.Require.NoError(err) | ||
err = i.Actions.Helm().WithReceiver(os.Stdout).Install( | ||
ctx, | ||
helmutils.InstallOpts{ | ||
CreateNamespace: true, | ||
ReleaseName: helmutils.CRDChartName, | ||
Namespace: i.Metadata.InstallNamespace, | ||
ChartUri: crdChartURI, | ||
}) | ||
i.Assertions.Require.NoError(err) | ||
|
||
err = i.Actions.Helm().Install( | ||
// and then install the main chart | ||
chartUri, err := helper.GetLocalChartPath(helmutils.ChartName) | ||
i.Assertions.Require.NoError(err) | ||
err = i.Actions.Helm().WithReceiver(os.Stdout).Install( | ||
ctx, | ||
helmutils.InstallOpts{ | ||
Namespace: i.Metadata.InstallNamespace, | ||
|
@@ -175,15 +188,29 @@ func (i *TestInstallation) UninstallKgateway(ctx context.Context) { | |
if testutils.ShouldSkipInstall() { | ||
return | ||
} | ||
|
||
// uninstall the main chart first | ||
err := i.Actions.Helm().Uninstall( | ||
ctx, | ||
helmutils.UninstallOpts{ | ||
Namespace: i.Metadata.InstallNamespace, | ||
ReleaseName: helmutils.ChartName, | ||
}, | ||
) | ||
i.Assertions.Require.NoError(err) | ||
i.Assertions.Require.NoError(err, "failed to uninstall main chart") | ||
i.Assertions.EventuallyKgatewayUninstallSucceeded(ctx) | ||
|
||
// uninstall the CRD chart | ||
err = i.Actions.Helm().Uninstall( | ||
ctx, | ||
helmutils.UninstallOpts{ | ||
Namespace: i.Metadata.InstallNamespace, | ||
ReleaseName: helmutils.CRDChartName, | ||
}, | ||
) | ||
i.Assertions.Require.NoError(err, "failed to uninstall CRD chart") | ||
|
||
// TODO: Remove the namespace. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need this? |
||
} | ||
|
||
// PreFailHandler is the function that is invoked if a test in the given TestInstallation fails | ||
|
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.
are there advantages of doing it as a separate chart (where we need to issue 2 separate install/upgrade/uninstall commands) vs. having the crd chart being a subchart of kgateway? if it's a subchart, we can guarantee the versions wouldn't get out of sync