specify FeatureGates in helm chart #1314

B1F030 · 2023-11-08T08:32:40Z

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?

Allow configuring featureGates on helm charts.

netlify · 2023-11-08T08:32:48Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`216505c`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/655325700e77fa00079817e9

k8s-ci-robot · 2023-11-08T08:32:51Z

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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

charts/kueue/values.yaml

charts/kueue/templates/manager/manager.yaml

B1F030 · 2023-11-08T10:52:36Z

/hold

kerthcet · 2023-11-08T11:16:36Z

/kind feature
/remove-kind cleanup

kerthcet

/lgtm
/approve

k8s-ci-robot · 2023-11-09T03:56:00Z

LGTM label has been added.

Git tree hash: 44563f2f2a9e779b4211b0f77278545149996b8e

B1F030 · 2023-11-09T17:53:30Z

@tenzen-y Thanks for review. I fixed that! There are no empty lines any more.

tenzen-y · 2023-11-09T18:03:29Z

I found it! I need to add another with:

    {{- with .Values.controllerManager.featureGates }}
      {{- include "kueue.featureGates" . | nindent 8 }}
    {{- end }}

It's a great point. An empty line issue is resolved. However, once I remove comment out, I face the following errors:
Does this chart work well on your local?

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 {}

B1F030 · 2023-11-10T01:45:03Z

I found it! I need to add another with:

    {{- with .Values.controllerManager.featureGates }}
      {{- include "kueue.featureGates" . | nindent 8 }}
    {{- end }}

It's a great point. An empty line issue is resolved. However, once I remove comment out, I face the following errors: Does this chart work well on your local?

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... :(
I have changed .value to .enabled, now it should be all right.

B1F030 · 2023-11-10T03:31:03Z

/unhold

B1F030 · 2023-11-13T09:01:58Z

/meow

k8s-ci-robot · 2023-11-13T09:02:01Z

@B1F030:

In response to this:

/meow

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.

kerthcet · 2023-11-13T09:24:42Z

ping @tenzen-y

tenzen-y · 2023-11-13T22:08:47Z

@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)

B1F030 · 2023-11-14T02:01:13Z

@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:

 featureGates:
   - name: PartialAdmission
     enabled: true

It seems more reasonable to use enabled instead of value.

tenzen-y · 2023-11-14T05:56:45Z

@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:

 featureGates:
   - name: PartialAdmission
     enabled: true

It seems more reasonable to use enabled instead of value.

I was using enabled field since I was using your latest commit, as I showed in the above. Can you build this helm on your local?

# 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 <kerthcet@gmail.com> Co-authored-by: B1F030 <646337422@qq.com> Signed-off-by: B1F030 <646337422@qq.com>

B1F030 · 2023-11-14T08:05:24Z

@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:

 featureGates:
   - name: PartialAdmission
     enabled: true

It seems more reasonable to use enabled instead of value.

I was using enabled field since I was using your latest commit, as I showed in the above. Can you build this helm on your local?

# 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:
...

Really thanks for your patience, sorry I didn't perfectly fix that empty line bug before.
After looking through helm chart's official document, I found that it is becacuse of nindent, the nindent function is the same as the indent function, but prepends a new line to the beginning of the string.
Last time when I try to fix that problem, I use another {{with}}, but end up with another bug.
Sorry again for my careless, next time I shall be more careful, and test well in my local environment.

controllerManager:
 #featureGates:
 #  - name: PartialAdmission
 #    enabled: true
 #  - name: FlavorFungibility
 #    enabled: true

      containers:
      - args:
        - --config=/controller_manager_config.yaml
        - --zap-log-level=2
        {{- include "kueue.featureGates" . | indent 8 }}

Now I replaced nindent with indent, it works, and this PR should be ready to merge. Appreciate again! @tenzen-y

tenzen-y · 2023-11-14T10:06:23Z

@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:
 featureGates:
   - name: PartialAdmission
     enabled: true
It seems more reasonable to use enabled instead of value.
I was using enabled field since I was using your latest commit, as I showed in the above. Can you build this helm on your local?
# 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:
...
Really thanks for your patience, sorry I didn't perfectly fix that empty line bug before. After looking through helm chart's official document, I found that it is becacuse of nindent, the nindent function is the same as the indent function, but prepends a new line to the beginning of the string. Last time when I try to fix that problem, I use another {{with}}, but end up with another bug. Sorry again for my careless, next time I shall be more careful, and test well in my local environment.
controllerManager:
 #featureGates:
 #  - name: PartialAdmission
 #    enabled: true
 #  - name: FlavorFungibility
 #    enabled: true
      containers:
      - args:
        - --config=/controller_manager_config.yaml
        - --zap-log-level=2
        {{- include "kueue.featureGates" . | indent 8 }}
Now I replaced nindent with indent, it works, and this PR should be ready to merge. Appreciate again! @tenzen-y

That makes sense. The latest commit should work well.
Thank you for the great work :)

/lgtm
/approve

/release-note-edit

Allow configuring featureGates on helm charts.

k8s-ci-robot · 2023-11-14T10:06:26Z

@tenzen-y: /release-note-edit must be used with a release note block.

In response to this:

@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:
 featureGates:
   - name: PartialAdmission
     enabled: true
It seems more reasonable to use enabled instead of value.
I was using enabled field since I was using your latest commit, as I showed in the above. Can you build this helm on your local?
# 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:
...
Really thanks for your patience, sorry I didn't perfectly fix that empty line bug before. After looking through helm chart's official document, I found that it is becacuse of nindent, the nindent function is the same as the indent function, but prepends a new line to the beginning of the string. Last time when I try to fix that problem, I use another {{with}}, but end up with another bug. Sorry again for my careless, next time I shall be more careful, and test well in my local environment.
controllerManager:
 #featureGates:
 #  - name: PartialAdmission
 #    enabled: true
 #  - name: FlavorFungibility
 #    enabled: true
      containers:
      - args:
        - --config=/controller_manager_config.yaml
        - --zap-log-level=2
        {{- include "kueue.featureGates" . | indent 8 }}
Now I replaced nindent with indent, it works, and this PR should be ready to merge. Appreciate again! @tenzen-y
That makes sense. The latest commit should work well.
Thank you for the great work :)

/lgtm
/approve

/release-note-edit
Allow configuring featureGates on helm charts.

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.

k8s-ci-robot · 2023-11-14T10:06:29Z

LGTM label has been added.

Git tree hash: 2dd541dfb8205c649183db8c212cee42a2d4aa98

k8s-ci-robot · 2023-11-14T10:06:31Z

[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:

~~OWNERS~~ [tenzen-y]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tenzen-y · 2023-11-14T10:08:06Z

/release-note-edit Allow configuring featureGates on helm charts.

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. labels Nov 8, 2023

k8s-ci-robot requested review from mimowo and tenzen-y November 8, 2023 08:32

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 8, 2023

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 8, 2023

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 8, 2023

mimowo reviewed Nov 8, 2023

View reviewed changes

charts/kueue/values.yaml Outdated Show resolved Hide resolved

kerthcet reviewed Nov 8, 2023

View reviewed changes

charts/kueue/values.yaml Outdated Show resolved Hide resolved

charts/kueue/templates/manager/manager.yaml Outdated Show resolved Hide resolved

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 8, 2023

B1F030 force-pushed the helm-chart branch from b22f972 to 2b46c57 Compare November 9, 2023 03:54

kerthcet reviewed Nov 9, 2023

View reviewed changes

k8s-ci-robot assigned kerthcet Nov 9, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 9, 2023

B1F030 force-pushed the helm-chart branch from 2b46c57 to 8f97712 Compare November 9, 2023 03:57

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 9, 2023

k8s-ci-robot requested a review from kerthcet November 9, 2023 03:57

B1F030 force-pushed the helm-chart branch from 8f97712 to 7df21b1 Compare November 9, 2023 05:20

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 9, 2023

B1F030 force-pushed the helm-chart branch from 7df21b1 to e22d6d2 Compare November 9, 2023 05:30

k8s-ci-robot requested a review from tenzen-y November 9, 2023 17:39

B1F030 force-pushed the helm-chart branch from ca8765e to 36139c1 Compare November 9, 2023 17:49

B1F030 force-pushed the helm-chart branch from 36139c1 to 44c2af6 Compare November 10, 2023 01:42

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 10, 2023

specify feature gates in helm chart

216505c

Co-authored-by: kerthcet <kerthcet@gmail.com> Co-authored-by: B1F030 <646337422@qq.com> Signed-off-by: B1F030 <646337422@qq.com>

B1F030 force-pushed the helm-chart branch from 44c2af6 to 216505c Compare November 14, 2023 07:44

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 14, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 14, 2023

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Nov 14, 2023

k8s-ci-robot merged commit cf25c71 into kubernetes-sigs:main Nov 14, 2023
14 checks passed

k8s-ci-robot added this to the v0.6 milestone Nov 14, 2023

B1F030 deleted the helm-chart branch November 15, 2023 02:40

B1F030 mentioned this pull request Feb 4, 2024

REQUEST: New membership for B1F030 kubernetes/org#4732

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

specify FeatureGates in helm chart #1314

specify FeatureGates in helm chart #1314

B1F030 commented Nov 8, 2023 •

edited

netlify bot commented Nov 8, 2023 •

edited

k8s-ci-robot commented Nov 8, 2023

B1F030 commented Nov 8, 2023

kerthcet commented Nov 8, 2023

kerthcet left a comment

k8s-ci-robot commented Nov 9, 2023

B1F030 commented Nov 9, 2023

tenzen-y commented Nov 9, 2023

B1F030 commented Nov 10, 2023

B1F030 commented Nov 10, 2023

B1F030 commented Nov 13, 2023

k8s-ci-robot commented Nov 13, 2023

kerthcet commented Nov 13, 2023

tenzen-y commented Nov 13, 2023

B1F030 commented Nov 14, 2023

tenzen-y commented Nov 14, 2023

B1F030 commented Nov 14, 2023

tenzen-y commented Nov 14, 2023

k8s-ci-robot commented Nov 14, 2023

k8s-ci-robot commented Nov 14, 2023

k8s-ci-robot commented Nov 14, 2023

tenzen-y commented Nov 14, 2023

specify FeatureGates in helm chart #1314

specify FeatureGates in helm chart #1314

Conversation

B1F030 commented Nov 8, 2023 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

netlify bot commented Nov 8, 2023 • edited

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

k8s-ci-robot commented Nov 8, 2023

B1F030 commented Nov 8, 2023

kerthcet commented Nov 8, 2023

kerthcet left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 9, 2023

B1F030 commented Nov 9, 2023

tenzen-y commented Nov 9, 2023

B1F030 commented Nov 10, 2023

B1F030 commented Nov 10, 2023

B1F030 commented Nov 13, 2023

k8s-ci-robot commented Nov 13, 2023

kerthcet commented Nov 13, 2023

tenzen-y commented Nov 13, 2023

B1F030 commented Nov 14, 2023

tenzen-y commented Nov 14, 2023

B1F030 commented Nov 14, 2023

tenzen-y commented Nov 14, 2023

k8s-ci-robot commented Nov 14, 2023

k8s-ci-robot commented Nov 14, 2023

k8s-ci-robot commented Nov 14, 2023

tenzen-y commented Nov 14, 2023

B1F030 commented Nov 8, 2023 •

edited

netlify bot commented Nov 8, 2023 •

edited