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

Add FeatureGate framework to handle new features #1623

Merged
merged 1 commit into from
Mar 15, 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
14 changes: 8 additions & 6 deletions cmd/nfd-master/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@
"time"

"k8s.io/klog/v2"
klogutils "sigs.k8s.io/node-feature-discovery/pkg/utils/klog"

"sigs.k8s.io/node-feature-discovery/pkg/features"
master "sigs.k8s.io/node-feature-discovery/pkg/nfd-master"
"sigs.k8s.io/node-feature-discovery/pkg/utils"
klogutils "sigs.k8s.io/node-feature-discovery/pkg/utils/klog"
"sigs.k8s.io/node-feature-discovery/pkg/version"
)

Expand All @@ -42,6 +43,12 @@
printVersion := flags.Bool("version", false, "Print version and exit.")

args, overrides := initFlags(flags)
// Add FeatureGates flag
if err := features.NFDMutableFeatureGate.Add(features.DefaultNFDFeatureGates); err != nil {
klog.ErrorS(err, "failed to add default feature gates")
os.Exit(1)
}
features.NFDMutableFeatureGate.AddFlag(flags)

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

View check run for this annotation

Codecov / codecov/patch

cmd/nfd-master/main.go#L46-L51

Added lines #L46 - L51 were not covered by tests

_ = flags.Parse(os.Args[1:])
if len(flags.Args()) > 0 {
Expand Down Expand Up @@ -74,8 +81,6 @@
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, will be removed in a future release along with the deprecated gRPC API")
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 @@ -134,9 +139,6 @@
"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 a future release along with the deprecated gRPC API.")
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
13 changes: 9 additions & 4 deletions cmd/nfd-worker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@
"os"

"k8s.io/klog/v2"
klogutils "sigs.k8s.io/node-feature-discovery/pkg/utils/klog"

"sigs.k8s.io/node-feature-discovery/pkg/features"
worker "sigs.k8s.io/node-feature-discovery/pkg/nfd-worker"
"sigs.k8s.io/node-feature-discovery/pkg/utils"
klogutils "sigs.k8s.io/node-feature-discovery/pkg/utils/klog"
"sigs.k8s.io/node-feature-discovery/pkg/version"
)

Expand All @@ -39,6 +40,13 @@

printVersion := flags.Bool("version", false, "Print version and exit.")

// Add FeatureGates flag
if err := features.NFDMutableFeatureGate.Add(features.DefaultNFDFeatureGates); err != nil {
klog.ErrorS(err, "failed to add default feature gates")
os.Exit(1)
}
features.NFDMutableFeatureGate.AddFlag(flags)

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

View check run for this annotation

Codecov / codecov/patch

cmd/nfd-worker/main.go#L43-L49

Added lines #L43 - L49 were not covered by tests
args := parseArgs(flags, os.Args[1:]...)

if *printVersion {
Expand Down Expand Up @@ -124,9 +132,6 @@
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 a future release along with the deprecated gRPC API.")
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 @@ -80,7 +80,7 @@ rules:
- update
{{- end }}

{{- if and .Values.gc.enable .Values.gc.rbac.create (or .Values.enableNodeFeatureApi .Values.topologyUpdater.enable) }}
{{- if and .Values.gc.enable .Values.gc.rbac.create (or .Values.featureGates.NodeFeatureAPI .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.enableNodeFeatureApi .Values.topologyUpdater.enable) }}
{{- if and .Values.gc.enable .Values.gc.rbac.create (or .Values.featureGates.NodeFeatureAPI .Values.topologyUpdater.enable) }}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
Expand Down
7 changes: 5 additions & 2 deletions deployment/helm/node-feature-discovery/templates/master.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,8 @@ spec:
{{- if .Values.master.instance | empty | not }}
- "-instance={{ .Values.master.instance }}"
{{- end }}
{{- if not .Values.enableNodeFeatureApi }}
{{- if not .Values.featureGates.NodeFeatureAPI }}
- "-port={{ .Values.master.port | default "8080" }}"
- "-enable-nodefeature-api=false"
{{- else if gt (int .Values.master.replicaCount) 1 }}
- "-enable-leader-election"
{{- end }}
Expand Down Expand Up @@ -111,6 +110,10 @@ spec:
- "-key-file=/etc/kubernetes/node-feature-discovery/certs/tls.key"
- "-cert-file=/etc/kubernetes/node-feature-discovery/certs/tls.crt"
{{- end }}
# Go over featureGates and add the feature-gate flag
{{- range $key, $value := .Values.featureGates }}
- "-feature-gates={{ $key }}={{ $value }}"
{{- end }}
- "-metrics={{ .Values.master.metricsPort | default "8081" }}"
volumeMounts:
{{- if .Values.tls.enable }}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{- if and .Values.gc.enable (or .Values.enableNodeFeatureApi .Values.topologyUpdater.enable) -}}
{{- if and .Values.gc.enable (or .Values.featureGates.NodeFeatureAPI .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.enableNodeFeatureApi) .Values.master.enable }}
{{- if and (not .Values.featureGates.NodeFeatureAPI) .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.enableNodeFeatureApi .Values.topologyUpdater.enable) }}
{{- if and .Values.gc.enable .Values.gc.serviceAccount.create (or .Values.featureGates.NodeFeatureAPI .Values.topologyUpdater.enable) }}
---
apiVersion: v1
kind: ServiceAccount
Expand Down
9 changes: 6 additions & 3 deletions deployment/helm/node-feature-discovery/templates/worker.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,17 @@ spec:
command:
- "nfd-worker"
args:
{{- if not .Values.enableNodeFeatureApi }}
{{- if not .Values.featureGates.NodeFeatureAPI }}
- "-server={{ include "node-feature-discovery.fullname" . }}-master:{{ .Values.master.service.port }}"
- "-enable-nodefeature-api=false"
{{- end }}
{{- end }}
{{- if .Values.tls.enable }}
- "-ca-file=/etc/kubernetes/node-feature-discovery/certs/ca.crt"
- "-key-file=/etc/kubernetes/node-feature-discovery/certs/tls.key"
- "-cert-file=/etc/kubernetes/node-feature-discovery/certs/tls.crt"
{{- end }}
# Go over featureGate and add the feature-gate flag
{{- range $key, $value := .Values.featureGates }}
- "-feature-gates={{ $key }}={{ $value }}"
{{- end }}
- "-metrics={{ .Values.worker.metricsPort | default "8081"}}"
ports:
Expand Down
3 changes: 2 additions & 1 deletion deployment/helm/node-feature-discovery/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ nameOverride: ""
fullnameOverride: ""
namespaceOverride: ""

enableNodeFeatureApi: true
ArangoGutierrez marked this conversation as resolved.
Show resolved Hide resolved
featureGates:
NodeFeatureAPI: true

priorityClassName: ""

Expand Down
2 changes: 1 addition & 1 deletion docs/deployment/helm.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ Chart parameters are available.
| `tls.certManager` | bool | false | If enabled, requires [cert-manager](https://cert-manager.io/docs/) to be installed and will automatically create 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.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 |
| `enableNodeFeatureApi`| 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 |
| `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 |
| `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 |
| `priorityClassName` | string | | The name of the PriorityClass to be used for the NFD pods. |
Expand Down
27 changes: 27 additions & 0 deletions docs/reference/feature-gates.md
ArangoGutierrez marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
title: "Feature Gates"
layout: default
sort: 10
---

# Feature Gates
{: .no_toc}

---

Feature gates are a set of key-value pairs that control the behavior of NFD.
They are used to enable or disable certain features of NFD.
The feature gates are set using the `-feature-gates` command line flag or
`featureGates` value in the Helm chart. The following feature gates are available:

| Name | Default | Stage | Since | Until |
| --------------------- | ------- | ------ | ------- | ------ |
| `NodeFeatureAPI` | true | Beta | V0.14 | |

## NodeFeatureAPI

The `NodeFeatureAPI` feature gate enables the Node Feature API.
When enabled, NFD will register the Node Feature API with the Kubernetes API
server. The Node Feature API is used to expose node-specific hardware and
software features to the Kubernetes scheduler. The Node Feature API is a beta
feature and is enabled by default.
30 changes: 12 additions & 18 deletions docs/reference/master-commandline-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@ Print usage and exit.

Print version and exit.

### -feature-gates

The `-feature-gates` flag is used to enable or disable non GA features.
The list of available feature gates can be found in the [feature gates
documentation](../feature-gates.md).

Example:

```bash
nfd-master -feature-gates NodeFeatureAPI=false
```

### -prune

The `-prune` flag is a sub-command like option for cleaning up the cluster. It
Expand Down Expand Up @@ -163,24 +175,6 @@ 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

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

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
32 changes: 12 additions & 20 deletions docs/reference/worker-commandline-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@ Print usage and exit.

Print version and exit.

### -feature-gates

The `-feature-gates` flag is used to enable or disable non GA features.
The list of available feature gates can be found in the [feature gates
documentation](../feature-gates.md).

Example:

```bash
nfd-master -feature-gates NodeFeatureAPI=false
```

### -config

The `-config` flag specifies the path of the nfd-worker configuration file to
Expand Down Expand Up @@ -210,26 +222,6 @@ Example:
nfd-worker -label-sources=kernel,system,local
```

### -enable-nodefeature-api

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

The `-enable-nodefeature-api` flag enables/disables the
[NodeFeature](../usage/custom-resources.md#nodefeature) CRD API
for communicating with nfd-master. When enabled nfd-worker creates per-node
NodeFeature objects the contain all discovered node features and the set of
feature labels to be created. Setting the flag to false will enable
gRPC communication to nfd-master.

Default: true

Example:

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

### -metrics

The `-metrics` flag specifies the port on which to expose
Expand Down
2 changes: 1 addition & 1 deletion docs/usage/nfd-gc.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ default garbage collector interval is set to 1h which is the value when no

In Helm deployments (see
[garbage collector parameters](../deployment/helm.md#garbage-collector-parameters))
NFD-GC will only be deployed when `enableNodeFeatureApi` or
NFD-GC will only be deployed when `featureGates.NodeFeatureAPI` or
`topologyUpdater.enable` is set to true.
4 changes: 2 additions & 2 deletions docs/usage/nfd-master.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ received from nfd-worker instances through
[NodeFeature](custom-resources.md#nodefeature-custom-resource) objects.

> **NOTE**: when gRPC (**DEPRECATED**) is used for communicating
> the features (by setting the flag `-enable-nodefeature-api=false` on both
> nfd-master and nfd-worker, or via Helm values.enableNodeFeatureApi=false),
> the features (by setting the flag `-feature-gates NodeFeatureAPI=false` on both
> nfd-master and nfd-worker, or via Helm values.featureGates.NodeFeatureAPI=false),
> (re-)labelling only happens when a request is received from nfd-worker.
> That is, in practice rules are evaluated and labels for each node are created
> on intervals specified by the
Expand Down
37 changes: 37 additions & 0 deletions pkg/features/features.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
Copyright 2024 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package features

import (
"sigs.k8s.io/node-feature-discovery/pkg/utils/featuregate"
)

const (
NodeFeatureAPI featuregate.Feature = "NodeFeatureAPI"
)

var (
NFDMutableFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate()

// NFDFeatureGate is a shared global FeatureGate.
// Top-level commands/options setup that needs to modify this feature gate should use NFDMutableFeatureGate.
NFDFeatureGate featuregate.FeatureGate = NFDMutableFeatureGate
)

var DefaultNFDFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
NodeFeatureAPI: {Default: true, PreRelease: featuregate.Beta},
}
8 changes: 8 additions & 0 deletions pkg/nfd-master/nfd-master-internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ import (
fakecorev1client "k8s.io/client-go/kubernetes/typed/core/v1/fake"
clienttesting "k8s.io/client-go/testing"
"k8s.io/client-go/tools/cache"
"k8s.io/klog/v2"

nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/v1alpha1"
"sigs.k8s.io/node-feature-discovery/pkg/features"
fakenfdclient "sigs.k8s.io/node-feature-discovery/pkg/generated/clientset/versioned/fake"
nfdscheme "sigs.k8s.io/node-feature-discovery/pkg/generated/clientset/versioned/scheme"
nfdinformers "sigs.k8s.io/node-feature-discovery/pkg/generated/informers/externalversions"
Expand Down Expand Up @@ -685,6 +687,12 @@ extraLabelNs: ["added.ns.io"]
`)

noPublish := true
// Add FeatureGates flag
if err := features.NFDMutableFeatureGate.Add(features.DefaultNFDFeatureGates); err != nil {
klog.ErrorS(err, "failed to add default feature gates")
os.Exit(1)
}
_ = features.NFDMutableFeatureGate.OverrideDefault(features.NodeFeatureAPI, false)
m, err := NewNfdMaster(&Args{
ConfigFile: configFile,
Overrides: ConfigOverrideArgs{
Expand Down
Loading