From 560bd11d8558ef9be06bfe1050f48bd1d24ef92e Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Wed, 15 May 2024 15:58:14 +0300 Subject: [PATCH] Re-add -enable-nodefeature-api cmdline flag Bring back the -enable-nodefeature-api command line flag and the corresponding enableNodeFeatureApi helm config value that were removed without deprecation when the NodeFeatureAPI feature gate was introduced. The thinking behind this change is to not break existing users (without warning) unless totally unavoidable. Now the -enable-nodefeature-api flag is marked as deprecated and slated for removal in NFD v0.17. The NodeFeatureAPI feature gate and the -enable-nodefeature-api flag work together so that the NodeFeature API is disabled (gRPC is enabled, instead) if either of them is set to false. This patch selectively reverts parts of 06c4733bc5c5c3881edd18875114e91ef922fe2f. --- cmd/nfd-master/main.go | 5 ++++ cmd/nfd-worker/main.go | 5 ++++ .../templates/clusterrole.yaml | 2 +- .../templates/clusterrolebinding.yaml | 2 +- .../templates/master.yaml | 2 +- .../templates/nfd-gc.yaml | 2 +- .../templates/service.yaml | 2 +- .../templates/serviceaccount.yaml | 2 +- .../templates/worker.yaml | 2 +- .../helm/node-feature-discovery/values.yaml | 2 ++ docs/deployment/helm.md | 1 + .../reference/master-commandline-reference.md | 21 ++++++++++++++ pkg/nfd-master/nfd-master.go | 27 ++++++++--------- pkg/nfd-worker/nfd-worker.go | 29 ++++++++++--------- 14 files changed, 70 insertions(+), 34 deletions(-) diff --git a/cmd/nfd-master/main.go b/cmd/nfd-master/main.go index 08a35324f9..a51309b4a3 100644 --- a/cmd/nfd-master/main.go +++ b/cmd/nfd-master/main.go @@ -81,6 +81,8 @@ func main() { 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") 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": @@ -139,6 +141,9 @@ func initFlags(flagset *flag.FlagSet) (*master.Args, *master.ConfigOverrideArgs) "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.") flagset.BoolVar(&args.CrdController, "featurerules-controller", true, "Enable NFD CRD API controller. DEPRECATED: use -crd-controller instead") flagset.BoolVar(&args.CrdController, "crd-controller", true, diff --git a/cmd/nfd-worker/main.go b/cmd/nfd-worker/main.go index 4031c6f54a..7d41aa562d 100644 --- a/cmd/nfd-worker/main.go +++ b/cmd/nfd-worker/main.go @@ -69,6 +69,8 @@ func main() { 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") case "server": klog.InfoS("-server is deprecated, will be removed in a future release along with the deprecated gRPC API") case "server-name-override": @@ -132,6 +134,9 @@ func initFlags(flagset *flag.FlagSet) (*worker.Args, *worker.ConfigOverrideArgs) 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, diff --git a/deployment/helm/node-feature-discovery/templates/clusterrole.yaml b/deployment/helm/node-feature-discovery/templates/clusterrole.yaml index e4a021b690..c6adcb543a 100644 --- a/deployment/helm/node-feature-discovery/templates/clusterrole.yaml +++ b/deployment/helm/node-feature-discovery/templates/clusterrole.yaml @@ -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 diff --git a/deployment/helm/node-feature-discovery/templates/clusterrolebinding.yaml b/deployment/helm/node-feature-discovery/templates/clusterrolebinding.yaml index 8623de6060..3f717988b3 100644 --- a/deployment/helm/node-feature-discovery/templates/clusterrolebinding.yaml +++ b/deployment/helm/node-feature-discovery/templates/clusterrolebinding.yaml @@ -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 diff --git a/deployment/helm/node-feature-discovery/templates/master.yaml b/deployment/helm/node-feature-discovery/templates/master.yaml index bb1f24938b..3a584209e9 100644 --- a/deployment/helm/node-feature-discovery/templates/master.yaml +++ b/deployment/helm/node-feature-discovery/templates/master.yaml @@ -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" diff --git a/deployment/helm/node-feature-discovery/templates/nfd-gc.yaml b/deployment/helm/node-feature-discovery/templates/nfd-gc.yaml index 75c223fc6d..4f4ac76c72 100644 --- a/deployment/helm/node-feature-discovery/templates/nfd-gc.yaml +++ b/deployment/helm/node-feature-discovery/templates/nfd-gc.yaml @@ -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: diff --git a/deployment/helm/node-feature-discovery/templates/service.yaml b/deployment/helm/node-feature-discovery/templates/service.yaml index a6e7678fb5..7191dca706 100644 --- a/deployment/helm/node-feature-discovery/templates/service.yaml +++ b/deployment/helm/node-feature-discovery/templates/service.yaml @@ -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: diff --git a/deployment/helm/node-feature-discovery/templates/serviceaccount.yaml b/deployment/helm/node-feature-discovery/templates/serviceaccount.yaml index 7401602841..59edc5e6c5 100644 --- a/deployment/helm/node-feature-discovery/templates/serviceaccount.yaml +++ b/deployment/helm/node-feature-discovery/templates/serviceaccount.yaml @@ -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 diff --git a/deployment/helm/node-feature-discovery/templates/worker.yaml b/deployment/helm/node-feature-discovery/templates/worker.yaml index 364259cacc..f2a2419fc3 100644 --- a/deployment/helm/node-feature-discovery/templates/worker.yaml +++ b/deployment/helm/node-feature-discovery/templates/worker.yaml @@ -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 }} diff --git a/deployment/helm/node-feature-discovery/values.yaml b/deployment/helm/node-feature-discovery/values.yaml index fae4b75497..e465a9e800 100644 --- a/deployment/helm/node-feature-discovery/values.yaml +++ b/deployment/helm/node-feature-discovery/values.yaml @@ -10,6 +10,8 @@ nameOverride: "" fullnameOverride: "" namespaceOverride: "" +enableNodeFeatureApi: true + featureGates: NodeFeatureAPI: true diff --git a/docs/deployment/helm.md b/docs/deployment/helm.md index efd6412a48..96e1211ecf 100644 --- a/docs/deployment/helm.md +++ b/docs/deployment/helm.md @@ -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 | diff --git a/docs/reference/master-commandline-reference.md b/docs/reference/master-commandline-reference.md index ffdd07e909..a64670e3a6 100644 --- a/docs/reference/master-commandline-reference.md +++ b/docs/reference/master-commandline-reference.md @@ -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. diff --git a/pkg/nfd-master/nfd-master.go b/pkg/nfd-master/nfd-master.go index ce2ee5b117..8af903edd4 100644 --- a/pkg/nfd-master/nfd-master.go +++ b/pkg/nfd-master/nfd-master.go @@ -108,15 +108,16 @@ type ConfigOverrideArgs struct { // 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 @@ -318,7 +319,7 @@ func (m *nfdMaster) Run() error { 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) } @@ -373,7 +374,7 @@ func (m *nfdMaster) Run() error { 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{}{} } @@ -471,7 +472,7 @@ func (m *nfdMaster) runGrpcServer(errChan chan<- error) { 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 updateNodes := make(map[string]struct{}) rateLimit := time.After(time.Second) for { @@ -1363,7 +1364,7 @@ func (m *nfdMaster) startNfdApiController() error { } 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, ResyncPeriod: m.config.ResyncPeriod.Duration, }) if err != nil { diff --git a/pkg/nfd-worker/nfd-worker.go b/pkg/nfd-worker/nfd-worker.go index 3575479123..9ffc1dbc53 100644 --- a/pkg/nfd-worker/nfd-worker.go +++ b/pkg/nfd-worker/nfd-worker.go @@ -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 } @@ -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() } @@ -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)