-
Notifications
You must be signed in to change notification settings - Fork 230
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
specify FeatureGates in helm chart #1314
Conversation
✅ Deploy Preview for kubernetes-sigs-kueue canceled.
|
Hi @B1F030. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/hold |
/kind feature |
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
/approve
LGTM label has been added. Git tree hash: 44563f2f2a9e779b4211b0f77278545149996b8e
|
@tenzen-y Thanks for review. I fixed that! There are no empty lines any more. |
It's a great point. An empty line issue is resolved. However, once I remove comment out, I face the following errors: values.yaml: # Declare variables to be passed into your templates.
nameOverride: ""
fullnameOverride: ""
# Enable each function, like kustomize https://github.com/kubernetes-sigs/kueue/blob/main/config/default/kustomization.yaml
enablePrometheus: false
# Enable x509 automated certificate management using cert-manager (cert-manager.io)
enableCertManager: false
# Customize controlerManager
controllerManager:
featureGates:
- name: PartialAdmission
value: true
kubeRbacProxy:
image:
... Error: template: kueue/templates/manager/manager.yaml:27:14: executing "kueue/templates/manager/manager.yaml" at <include "kueue.featureGates" .>: error calling include: template: kueue/templates/_helpers.tpl:69:17: executing "kueue.featureGates" at <.Values.controllerManager.featureGates>: can't evaluate field Values in type []interface {} |
Sorry I missed that... :( |
/unhold |
/meow |
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
ping @tenzen-y |
@B1F030 I'm still facing the same error. Did you push the latest commit into this branch? $ helm template ./charts/kueue --values ./charts/kueue/values.yaml
Error: template: kueue/templates/manager/manager.yaml:27:14: executing "kueue/templates/manager/manager.yaml" at <include "kueue.featureGates" .>: error calling include: template: kueue/templates/_helpers.tpl:69:17: executing "kueue.featureGates" at <.Values.controllerManager.featureGates>: can't evaluate field Values in type []interface {}
Use --debug flag to render out invalid YAML
$
$ git log --oneline
44c2af6 (HEAD, B1F030/helm-chart) specify feature gates in helm chart
b47bf03 (origin/main, origin/HEAD, main) Updated the tool to generate client-go libraries (#1151)
b95392a correct some comments (#1305)
2a21bb4 List features of Kueue in README and website/overview (#1286) |
now it should be like:
It seems more reasonable to use |
I was using # Default values for kueue.
# This is a YAML-formatted file.
# Declare variables to be passed into your templates.
nameOverride: ""
fullnameOverride: ""
# Enable each function, like kustomize https://github.com/kubernetes-sigs/kueue/blob/main/config/default/kustomization.yaml
enablePrometheus: false
# Enable x509 automated certificate management using cert-manager (cert-manager.io)
enableCertManager: false
# Customize controlerManager
controllerManager:
featureGates:
- name: PartialAdmission
enabled: true
kubeRbacProxy:
... |
Co-authored-by: kerthcet <[email protected]> Co-authored-by: B1F030 <[email protected]> Signed-off-by: B1F030 <[email protected]>
Really thanks for your patience, sorry I didn't perfectly fix that empty line bug before.
Now I replaced |
That makes sense. The latest commit should work well. /lgtm /release-note-edit
|
@tenzen-y: /release-note-edit must be used with a release note block. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
LGTM label has been added. Git tree hash: 2dd541dfb8205c649183db8c212cee42a2d4aa98
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: B1F030, kerthcet, tenzen-y The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/release-note-edit |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Let Helm chart specify which FeatureGates should be used
Which issue(s) this PR fixes:
Fixes #1230
Special notes for your reviewer:
Does this PR introduce a user-facing change?