Skip to content

Enable NodeFeature API by default in GFD #1504

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Enable NodeFeature API by default in GFD #1504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/gpu-feature-discovery/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ func main() {
},
&cli.BoolFlag{
Name: "use-node-feature-api",
Copy link
Contributor Author

@rajathagasthya rajathagasthya Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we deprecate this flag too for future removal?

Copy link
Collaborator

@ArangoGutierrez ArangoGutierrez Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think is sane to do, in a couple more releases, NFD will start deprecating the old method.

Copy link
Collaborator

@ArangoGutierrez ArangoGutierrez Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say a depreciation path will involve not fully removing enableNodeFeatureApi from the Helm charts but adding a logic to helm helper to print a warning mentioning that the new default and the flag will go away soon

Copy link
Contributor Author

@rajathagasthya rajathagasthya Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed enableNodeFeatureApi from the helm chart because that value is not functional right now. And giving users the option to set enableNodeFeatureApi=false will actually leave the deployment broken because it doesn't create the necessary role/rolebinding/serviceaccount objects in the chart. So I'm not sure adding a warning there is of much help.

Let me know what you think!

Copy link
Collaborator

@ArangoGutierrez ArangoGutierrez Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am ok with removing it, NFD has been with NodeFeatureApi enabled by default for a long time now, most users must be on it already.

Value: true,
Usage: "Use NFD NodeFeature API to publish labels",
EnvVars: []string{"GFD_USE_NODE_FEATURE_API", "USE_NODE_FEATURE_API"},
},
Expand Down
6 changes: 1 addition & 5 deletions deployments/helm/nvidia-device-plugin/templates/daemonset-gfd.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
{{- if .Values.gfd.enabled }}
---
{{- $options := (include "nvidia-device-plugin.options" . | fromJson) }}
{{- $useServiceAccount := or ( $options.hasConfigMap ) ( and .Values.gfd.enabled .Values.nfd.enableNodeFeatureApi ) }}
{{- $useServiceAccount := or $options.hasConfigMap .Values.gfd.enabled }}
{{- $configMapName := (include "nvidia-device-plugin.configMapName" .) | trim }}
{{- $daemonsetName := printf "%s-gpu-feature-discovery" (include "nvidia-device-plugin.fullname" .) | trunc 63 | trimSuffix "-" }}
apiVersion: apps/v1
Expand Down Expand Up @@ -161,10 +161,6 @@ spec:
- name: GFD_SLEEP_INTERVAL
value: {{ .Values.sleepInterval | quote }}
{{- end }}
{{- if typeIs "bool" .Values.nfd.enableNodeFeatureApi }}
- name: GFD_USE_NODE_FEATURE_API
value: {{ .Values.nfd.enableNodeFeatureApi | quote }}
{{- end }}
{{- if $options.hasConfigMap }}
- name: CONFIG_FILE
value: /config/config.yaml
Expand Down
2 changes: 1 addition & 1 deletion deployments/helm/nvidia-device-plugin/templates/role-binding.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
{{- $options := (include "nvidia-device-plugin.options" . | fromJson) }}
{{- if or $options.hasConfigMap ( and .Values.gfd.enabled .Values.nfd.enableNodeFeatureApi ) }}
{{- if or $options.hasConfigMap .Values.gfd.enabled }}
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
Expand Down
4 changes: 2 additions & 2 deletions deployments/helm/nvidia-device-plugin/templates/role.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
{{- $options := (include "nvidia-device-plugin.options" . | fromJson) }}
{{- if or $options.hasConfigMap ( and .Values.gfd.enabled .Values.nfd.enableNodeFeatureApi ) }}
{{- if or $options.hasConfigMap .Values.gfd.enabled }}
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
Expand All @@ -11,7 +11,7 @@ rules:
- apiGroups: [""]
resources: ["nodes"]
verbs: ["get", "list", "watch"]
{{- if and .Values.gfd.enabled .Values.nfd.enableNodeFeatureApi }}
{{- if .Values.gfd.enabled }}
- apiGroups: ["nfd.k8s-sigs.io"]
resources: ["nodefeatures"]
verbs: ["get", "list", "watch", "create", "update"]
Expand Down
2 changes: 1 addition & 1 deletion deployments/helm/nvidia-device-plugin/templates/service-account.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
{{- $options := (include "nvidia-device-plugin.options" . | fromJson) }}
{{- if or $options.hasConfigMap ( and .Values.gfd.enabled .Values.nfd.enableNodeFeatureApi ) }}
{{- if or $options.hasConfigMap .Values.gfd.enabled }}
apiVersion: v1
kind: ServiceAccount
metadata:
Expand Down
1 change: 0 additions & 1 deletion deployments/helm/nvidia-device-plugin/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ gfd:
# Helm dependency
nfd:
nameOverride: node-feature-discovery
enableNodeFeatureApi: false
master:
serviceAccount:
name: node-feature-discovery
Expand Down