Closed mochizuki875 closed 5 months ago
/cc @adrianludwin
/ok-to-test
Thanks @rjbez17 ! I've addressed all feedback except one.
Sorry for the delay here. Finally found some time to pull the branch and play with the templating. The ONLY thing left that I can find is can you please make:
hrq:
enabled: false
true
by default?
Building the manifests (make manifests
) shows we are enabling by default there and this would keep us inline. Thanks again for all the work here, I really appreciate it.
apiVersion: apps/v1
kind: Deployment
metadata:
@@ -789,21 +214,20 @@
spec:
containers:
- args:
+ - --excluded-namespace=hnc-system
+ - --excluded-namespace=kube-system
+ - --excluded-namespace=kube-public
+ - --excluded-namespace=kube-node-lease
- --webhook-server-port=9443
- --metrics-addr=:8080
- --max-reconciles=10
- --apiserver-qps-throttle=50
- - --excluded-namespace=kube-system
- - --excluded-namespace=kube-public
- - --excluded-namespace=hnc-system
- - --excluded-namespace=kube-node-lease
- --nopropagation-label=cattle.io/creator=norman
- --enable-internal-cert-management
- --cert-restart-on-secret-refresh
- - --enable-hrq
@rjbez17
I recognize HRQ is beta status and not enabled by default(default.yaml
) at v1.1.0.
As beta feature, it is enabled in hrq.yaml
.
https://github.com/kubernetes-sigs/hierarchical-namespaces/releases/tag/v1.1.0
Of course, it's ok to set hrq.enabled: true
as helm default value, should we do it?
@rjbez17 I recognize HRQ is beta status and not enabled by default(
default.yaml
) at v1.1.0. As beta feature, it is enabled inhrq.yaml
. https://github.com/kubernetes-sigs/hierarchical-namespaces/releases/tag/v1.1.0Of course, it's ok to set
hrq.enabled: true
as helm default value, should we do it?
Yeah, I was on the fence with it, but it seems that current manifest generation turns it on by default so I was thinking we'd do the same. We can keep at as false
for now since it is an easy change for users to override it on.
/lgtm /approve
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: mochizuki875, rjbez17
The full list of commands accepted by this bot can be found here.
The pull request process is described here
@rjbez17 Thanks for your reviews and approve!
Thanks guys, I'm really looking forward to see this helm chart as artefact hosted on github pages!
What type of PR is this?:
/kind feature
What this PR does / why we need it:
Provide HNC installation method via Helm.
Previously, it was disccussed in #46 and #62. However, the problem that how to keep the chart up-to-date with changes in the config directory has not been resolved. In this PR, I've added a mechanism to auto generate chart from manifest file generated from config directory using kustomize.
How to use:
Clone this repository.
Set environment variables. They will be actually set in release process.
Generate chart.
Install chart. You can do any setting in
charts/hnc/values.yaml
before installation. eg: setting hncExcludeNamespaces, enable HRQ, etc.Tested:
E2E tests has passed.
Related:
46
62
Special notes for your reviewer:
Packaging and publishing to chart repository are not included. In many cases, charts are placed in gh-pages branch and published as Helm repository using GitHub Pages.
eg:
https://github.com/prometheus-community/helm-charts/tree/gh-pages
Onece gh-pages branch is created add published as GitHub Pages, I think we can include these process in
cloudbuild.yaml
.