Skip to content
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

Re-add -enable-nodefeature-api cmdline flag #1708

Merged
merged 1 commit into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions cmd/nfd-master/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@
args.Overrides.ResyncPeriod = overrides.ResyncPeriod
case "nfd-api-parallelism":
args.Overrides.NfdApiParallelism = overrides.NfdApiParallelism
case "enable-nodefeature-api":
klog.InfoS("-enable-nodefeature-api is deprecated and will be removed in the next release, use -feature-gate NodeFeatureAPI instead")

Check warning on line 85 in cmd/nfd-master/main.go

View check run for this annotation

Codecov / codecov/patch

cmd/nfd-master/main.go#L84-L85

Added lines #L84 - L85 were not covered by tests
case "ca-file":
klog.InfoS("-ca-file is deprecated, will be removed in a future release along with the deprecated gRPC API")
case "cert-file":
Expand Down Expand Up @@ -139,6 +141,9 @@
"Config file to use.")
flagset.StringVar(&args.Kubeconfig, "kubeconfig", "",
"Kubeconfig to use")
flagset.BoolVar(&args.EnableNodeFeatureApi, "enable-nodefeature-api", true,
"Enable the NodeFeature CRD API for receiving node features. This will automatically disable the gRPC communication."+
" DEPRECATED: will be removed in NFD v0.17. Use -feature-gate NodeFeatureAPI instead.")

Check warning on line 146 in cmd/nfd-master/main.go

View check run for this annotation

Codecov / codecov/patch

cmd/nfd-master/main.go#L144-L146

Added lines #L144 - L146 were not covered by tests
flagset.BoolVar(&args.CrdController, "featurerules-controller", true,
"Enable NFD CRD API controller. DEPRECATED: use -crd-controller instead")
flagset.BoolVar(&args.CrdController, "crd-controller", true,
Expand Down
5 changes: 5 additions & 0 deletions cmd/nfd-worker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@
klog.InfoS("-cert-file is deprecated, will be removed in a future release along with the deprecated gRPC API")
case "key-file":
klog.InfoS("-key-file is deprecated, will be removed in a future release along with the deprecated gRPC API")
case "enable-nodefeature-api":
klog.InfoS("-enable-nodefeature-api is deprecated and will be removed in the next release, use -feature-gate NodeFeatureAPI instead")

Check warning on line 73 in cmd/nfd-worker/main.go

View check run for this annotation

Codecov / codecov/patch

cmd/nfd-worker/main.go#L72-L73

Added lines #L72 - L73 were not covered by tests
case "server":
klog.InfoS("-server is deprecated, will be removed in a future release along with the deprecated gRPC API")
case "server-name-override":
Expand Down Expand Up @@ -132,6 +134,9 @@
flagset.StringVar(&args.KeyFile, "key-file", "",
"Private key matching -cert-file."+
" DEPRECATED: will be removed in a future release along with the deprecated gRPC API.")
flagset.BoolVar(&args.EnableNodeFeatureApi, "enable-nodefeature-api", true,
"Enable the NodeFeature CRD API for communicating with nfd-master. This will automatically disable the gRPC communication."+
" DEPRECATED: will be removed in NFD v0.17. Use -feature-gate NodeFeatureAPI instead.")
flagset.StringVar(&args.Kubeconfig, "kubeconfig", "",
"Kubeconfig to use")
flagset.BoolVar(&args.Oneshot, "oneshot", false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ rules:
- update
{{- end }}

{{- if and .Values.gc.enable .Values.gc.rbac.create (or .Values.featureGates.NodeFeatureAPI .Values.topologyUpdater.enable) }}
{{- if and .Values.gc.enable .Values.gc.rbac.create (or (and .Values.featureGates.NodeFeatureAPI .Values.enableNodeFeatureApi) .Values.topologyUpdater.enable) }}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ subjects:
namespace: {{ include "node-feature-discovery.namespace" . }}
{{- end }}

{{- if and .Values.gc.enable .Values.gc.rbac.create (or .Values.featureGates.NodeFeatureAPI .Values.topologyUpdater.enable) }}
{{- if and .Values.gc.enable .Values.gc.rbac.create (or (and .Values.featureGates.NodeFeatureAPI .Values.enableNodeFeatureApi) .Values.topologyUpdater.enable) }}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ spec:
{{- if .Values.master.instance | empty | not }}
- "-instance={{ .Values.master.instance }}"
{{- end }}
{{- if not .Values.featureGates.NodeFeatureAPI }}
{{- if not (and .Values.featureGates.NodeFeatureAPI .Values.enableNodeFeatureApi) }}
- "-port={{ .Values.master.port | default "8080" }}"
{{- else if gt (int .Values.master.replicaCount) 1 }}
- "-enable-leader-election"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{- if and .Values.gc.enable (or .Values.featureGates.NodeFeatureAPI .Values.topologyUpdater.enable) -}}
{{- if and .Values.gc.enable (or (and .Values.featureGates.NodeFeatureAPI .Values.enableNodeFeatureApi) .Values.topologyUpdater.enable) -}}
apiVersion: apps/v1
kind: Deployment
metadata:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{- if and (not .Values.featureGates.NodeFeatureAPI) .Values.master.enable }}
{{- if and (not (and .Values.featureGates.NodeFeatureAPI .Values.enableNodeFeatureApi)) .Values.master.enable }}
apiVersion: v1
kind: Service
metadata:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ metadata:
{{- end }}
{{- end }}

{{- if and .Values.gc.enable .Values.gc.serviceAccount.create (or .Values.featureGates.NodeFeatureAPI .Values.topologyUpdater.enable) }}
{{- if and .Values.gc.enable .Values.gc.serviceAccount.create (or (and .Values.featureGates.NodeFeatureAPI .Values.enableNodeFeatureApi) .Values.topologyUpdater.enable) }}
---
apiVersion: v1
kind: ServiceAccount
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ spec:
command:
- "nfd-worker"
args:
{{- if not .Values.featureGates.NodeFeatureAPI }}
{{- if not (and .Values.featureGates.NodeFeatureAPI .Values.enableNodeFeatureApi) }}
- "-server={{ include "node-feature-discovery.fullname" . }}-master:{{ .Values.master.service.port }}"
{{- end }}
{{- if .Values.tls.enable }}
Expand Down
2 changes: 2 additions & 0 deletions deployment/helm/node-feature-discovery/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ nameOverride: ""
fullnameOverride: ""
namespaceOverride: ""

enableNodeFeatureApi: true

featureGates:
NodeFeatureAPI: true

Expand Down
1 change: 1 addition & 0 deletions docs/deployment/helm.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ Chart parameters are available.
| `tls.certManager.certManagerCertificate.issuerName` | string | | If specified, it will use a pre-existing issuer instead for the required TLS certificates. **NOTE**: this parameter is related to the deprecated gRPC API and will be removed with it in a future release |
| `tls.certManager.certManagerCertificate.issuerKind` | string | | Specifies on what kind of issuer is used, can be either ClusterIssuer or Issuer (default). Requires `tls.certManager.certManagerCertificate.issuerName` to be set. **NOTE**: this parameter is related to the deprecated gRPC API and will be removed with it in a future release |
| `featureGates.NodeFeatureAPI`| bool | true | Enable the [NodeFeature](../usage/custom-resources.md#nodefeature) CRD API for communicating node features. This will automatically disable the gRPC communication. **NOTE**: this parameter is related to the deprecated gRPC API and will be removed with it in a future release |
| `enableNodeFeatureApi`| bool | true | Enable the [NodeFeature](../usage/custom-resources.md#nodefeature) CRD API for communicating node features. This will automatically disable the gRPC communication. **DEPRECATED**: will be removed in NFD v0.17, use `featureGates.NodeFeatureAPI` instead. |
| `prometheus.enable` | bool | false | Specifies whether to expose metrics using prometheus operator |
| `prometheus.labels` | dict | {} | Specifies labels for use with the prometheus operator to control how it is selected |
| `prometheus.scrapeInterval` | string | 10s | Specifies the interval by which metrics are scraped |
Expand Down
21 changes: 21 additions & 0 deletions docs/reference/master-commandline-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,27 @@ nfd-master -verify-node-name -ca-file=/opt/nfd/ca.crt \
-cert-file=/opt/nfd/master.crt -key-file=/opt/nfd/master.key
```

### -enable-nodefeature-api

**DEPRECATED**: will be removed in NFD v0.17. Use'
[`-feature-gates`](#-feature-gates)
[NodeFeatureAPI](feature-gates.md#nodefeatureapi) instead.

> **NOTE** the gRPC API is deprecated and will be removed in a future release.

The `-enable-nodefeature-api` flag enables/disables the
[NodeFeature](../usage/custom-resources.md#nodefeature) CRD API for receiving
feature requests. This will also automatically disable/enable the gRPC
interface.

Default: true

Example:

```bash
nfd-master -enable-nodefeature-api=false
```

### -enable-leader-election

The `-enable-leader-election` flag enables leader election for NFD-Master.
Expand Down
27 changes: 14 additions & 13 deletions pkg/nfd-master/nfd-master.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,16 @@

// Args holds command line arguments
type Args struct {
CaFile string
CertFile string
ConfigFile string
Instance string
KeyFile string
Klog map[string]*utils.KlogFlagVal
Kubeconfig string
CrdController bool
Port int
CaFile string
CertFile string
ConfigFile string
Instance string
KeyFile string
Klog map[string]*utils.KlogFlagVal
Kubeconfig string
CrdController bool
EnableNodeFeatureApi bool
Port int
// GrpcHealthPort is only needed to avoid races between tests (by skipping the health server).
// Could be removed when gRPC labler service is dropped (when nfd-worker tests stop running nfd-master).
GrpcHealthPort int
Expand Down Expand Up @@ -318,7 +319,7 @@
grpcErr := make(chan error)
// If the NodeFeature API is enabled, don'tregister the labeler API
// server. Otherwise, register the labeler server.
if !nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.NodeFeatureAPI) {
if !nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.NodeFeatureAPI) || !m.args.EnableNodeFeatureApi {
go m.runGrpcServer(grpcErr)
}

Expand Down Expand Up @@ -373,7 +374,7 @@
m.nodeUpdaterPool.start(m.config.NfdApiParallelism)

// Update all nodes when the configuration changes
if m.nfdController != nil && nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.NodeFeatureAPI) {
if m.nfdController != nil && nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.NodeFeatureAPI) && m.args.EnableNodeFeatureApi {
m.nfdController.updateAllNodesChan <- struct{}{}
}

Expand Down Expand Up @@ -471,7 +472,7 @@
func (m *nfdMaster) nfdAPIUpdateHandler() {
// We want to unconditionally update all nodes at startup if gRPC is
// disabled (i.e. NodeFeature API is enabled)
updateAll := nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.NodeFeatureAPI)
updateAll := nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.NodeFeatureAPI) && m.args.EnableNodeFeatureApi

Check warning on line 475 in pkg/nfd-master/nfd-master.go

View check run for this annotation

Codecov / codecov/patch

pkg/nfd-master/nfd-master.go#L475

Added line #L475 was not covered by tests
updateNodes := make(map[string]struct{})
rateLimit := time.After(time.Second)
for {
Expand Down Expand Up @@ -1363,7 +1364,7 @@
}
klog.InfoS("starting the nfd api controller")
m.nfdController, err = newNfdController(kubeconfig, nfdApiControllerOptions{
DisableNodeFeature: !nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.NodeFeatureAPI),
DisableNodeFeature: !nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.NodeFeatureAPI) || !m.args.EnableNodeFeatureApi,

Check warning on line 1367 in pkg/nfd-master/nfd-master.go

View check run for this annotation

Codecov / codecov/patch

pkg/nfd-master/nfd-master.go#L1367

Added line #L1367 was not covered by tests
ResyncPeriod: m.config.ResyncPeriod.Duration,
})
if err != nil {
Expand Down
29 changes: 15 additions & 14 deletions pkg/nfd-worker/nfd-worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,18 +96,19 @@ type Labels map[string]string

// Args are the command line arguments of NfdWorker.
type Args struct {
CaFile string
CertFile string
ConfigFile string
KeyFile string
Klog map[string]*utils.KlogFlagVal
Kubeconfig string
Oneshot bool
Options string
Server string
ServerNameOverride string
MetricsPort int
GrpcHealthPort int
CaFile string
CertFile string
ConfigFile string
EnableNodeFeatureApi bool
KeyFile string
Klog map[string]*utils.KlogFlagVal
Kubeconfig string
Oneshot bool
Options string
Server string
ServerNameOverride string
MetricsPort int
GrpcHealthPort int

Overrides ConfigOverrideArgs
}
Expand Down Expand Up @@ -316,7 +317,7 @@ func (w *nfdWorker) Run() error {
return err
}
// Manage connection to master
if w.config.Core.NoPublish || !features.NFDFeatureGate.Enabled(features.NodeFeatureAPI) {
if w.config.Core.NoPublish || !features.NFDFeatureGate.Enabled(features.NodeFeatureAPI) || !w.args.EnableNodeFeatureApi {
w.grpcDisconnect()
}

Expand Down Expand Up @@ -660,7 +661,7 @@ func getFeatureLabels(source source.LabelSource, labelWhiteList regexp.Regexp) (

// advertiseFeatures advertises the features of a Kubernetes node
func (w *nfdWorker) advertiseFeatures(labels Labels) error {
if features.NFDFeatureGate.Enabled(features.NodeFeatureAPI) {
if features.NFDFeatureGate.Enabled(features.NodeFeatureAPI) && w.args.EnableNodeFeatureApi {
// Create/update NodeFeature CR object
if err := w.updateNodeFeatureObject(labels); err != nil {
return fmt.Errorf("failed to advertise features (via CRD API): %w", err)
Expand Down