Merge pull request #1708 from marquiz/devel/nf-feature-gate

k8s-ci-robot · web-flow · commit eda85439fa54 · 2024-05-16T01:11:57.000-07:00
Re-add -enable-nodefeature-api cmdline flag
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -10,6 +10,8 @@ nameOverride: ""
 fullnameOverride: ""
 namespaceOverride: ""
 
+enableNodeFeatureApi: true
+
 featureGates:
   NodeFeatureAPI: true
 
diff --git 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
@@ -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
@@ -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
@@ -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)

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-{{- if and .Values.gc.enable (or .Values.featureGates.NodeFeatureAPI .Values.topologyUpdater.enable) -}}`
	`1`	`+{{- if and .Values.gc.enable (or (and .Values.featureGates.NodeFeatureAPI .Values.enableNodeFeatureApi) .Values.topologyUpdater.enable) -}}`
`2`	`2`	`apiVersion: apps/v1`
`3`	`3`	`kind: Deployment`
`4`	`4`	`metadata:`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-{{- if and (not .Values.featureGates.NodeFeatureAPI) .Values.master.enable }}`
	`1`	`+{{- if and (not (and .Values.featureGates.NodeFeatureAPI .Values.enableNodeFeatureApi)) .Values.master.enable }}`
`2`	`2`	`apiVersion: v1`
`3`	`3`	`kind: Service`
`4`	`4`	`metadata:`