Skip to content

Commit

Permalink
nfd-master: add DisableAutoPrefix feature gate
Browse files Browse the repository at this point in the history
Now that we have support for feature gates deprecate the autoDefaultNs
config option of nfd-master and replace it with a new alpha feature gate
DisableAutoPrefix (defaults to false). Using a feature gate to handle
and communicate these kind of changes, where the default behavior is
intended to be changed in a future release, feels much more natural than
using random flags/options.

The combined logic of the feature gate and the config option is a
logical OR over disabling auto-prefixing. That is, auto-prefixing is
disabled if either the feature gate or the config options is used set to
disable it:

                       | DisableAutoPrefix (feature gate)
                       | false | true
  -------------------- | --------------------------------
  autoDefaultNs   true |  ON   | OFF
  (config opt)   false |  OFF  | OFF
  • Loading branch information
marquiz committed May 15, 2024
1 parent eb7a0ad commit 1213454
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 7 deletions.
27 changes: 26 additions & 1 deletion docs/reference/feature-gates.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ The feature gates are set using the `-feature-gates` command line flag or

| Name | Default | Stage | Since | Until |
| --------------------- | ------- | ------ | ------- | ------ |
| `NodeFeatureAPI` | true | Beta | V0.14 | |
| `NodeFeatureAPI` | true | Beta | V0.14 | |
| `DisableAutoPrefix` | false | Alpha | V0.16 | |

## NodeFeatureAPI

Expand All @@ -25,3 +26,27 @@ 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.

## DisableAutoPrefix

The `DisableAutoPrefix` feature gate controls the automatic prefixing of names.
When enabled nfd-master does not automatically add the default
`feature.node.kubernetes.io/` prefix to unprefixed labels, annotations and
extended resources. Automatic prefixing is the default behavior in NFD v0.16
and earlier.

Note that enabling the feature gate effectively causes unprefixed names to be
filtered out as NFD does not allow unprefixed names of labels, annotations or
extended resources. For example, with the `DisableAutoPrefix` feature gate set
to `false`, a NodeFeatureRule with

```yaml
labels:
foo: bar
```
will turn into `feature.node.kubernetes.io/foo=bar` node label. With
`DisableAutoPrefix` set to `true`, no prefix is added and the label will be
filtered out.

Note that taint keys are not affected by this feature gate.
3 changes: 3 additions & 0 deletions docs/reference/master-configuration-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ denyLabelNs: ["denied.ns.io","denied.kubernetes.io"]

## autoDefaultNs

**DEPRECATED**: Will be removed in NFD v0.17. Use the
[DisableAutoPrefix](feature-gates.md#disableautoprefix) feature gate instead.

The `autoDefaultNs` option controls the automatic prefixing of names. When set
to true (the default in NFD version {{ site.version }}) nfd-master
automatically adds the default `feature.node.kubernetes.io/` prefix to
Expand Down
6 changes: 4 additions & 2 deletions pkg/features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import (
)

const (
NodeFeatureAPI featuregate.Feature = "NodeFeatureAPI"
NodeFeatureAPI featuregate.Feature = "NodeFeatureAPI"
DisableAutoPrefix featuregate.Feature = "DisableAutoPrefix"
)

var (
Expand All @@ -33,5 +34,6 @@ var (
)

var DefaultNFDFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
NodeFeatureAPI: {Default: true, PreRelease: featuregate.Beta},
NodeFeatureAPI: {Default: true, PreRelease: featuregate.Beta},
DisableAutoPrefix: {Default: false, PreRelease: featuregate.Alpha},
}
4 changes: 4 additions & 0 deletions pkg/nfd-master/nfd-master-internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,10 @@ func TestRemovingExtResources(t *testing.T) {

func TestSetLabels(t *testing.T) {
Convey("When servicing SetLabels request", t, func() {
// Add feature gates as running nfd-master depends on that
err := features.NFDMutableFeatureGate.Add(features.DefaultNFDFeatureGates)
So(err, ShouldBeNil)

testNode := newTestNode()
// We need to populate the node with some annotations or the patching in the fake client fails
testNode.Labels["feature.node.kubernetes.io/foo"] = "bar"
Expand Down
8 changes: 4 additions & 4 deletions pkg/nfd-master/nfd-master.go
Original file line number Diff line number Diff line change
Expand Up @@ -801,12 +801,12 @@ func (m *nfdMaster) nfdAPIUpdateOneNode(cli k8sclient.Interface, node *corev1.No
// NOTE: changing the rule api to support handle multiple objects instead
// of merging would probably perform better with lot less data to copy.
features = objs[0].Spec.DeepCopy()
if m.config.AutoDefaultNs {
if !nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.DisableAutoPrefix) && m.config.AutoDefaultNs {

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L804 was not covered by tests
features.Labels = addNsToMapKeys(features.Labels, nfdv1alpha1.FeatureLabelNs)
}
for _, o := range objs[1:] {
s := o.Spec.DeepCopy()
if m.config.AutoDefaultNs {
if !nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.DisableAutoPrefix) && m.config.AutoDefaultNs {

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L809 was not covered by tests
s.Labels = addNsToMapKeys(s.Labels, nfdv1alpha1.FeatureLabelNs)
}
s.MergeInto(features)
Expand Down Expand Up @@ -864,7 +864,7 @@ func filterExtendedResource(name, value string, features *nfdv1alpha1.Features)
}

func (m *nfdMaster) refreshNodeFeatures(cli k8sclient.Interface, node *corev1.Node, labels map[string]string, features *nfdv1alpha1.Features) error {
if m.config.AutoDefaultNs {
if !nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.DisableAutoPrefix) && m.config.AutoDefaultNs {
labels = addNsToMapKeys(labels, nfdv1alpha1.FeatureLabelNs)
} else if labels == nil {
labels = make(map[string]string)
Expand Down Expand Up @@ -1041,7 +1041,7 @@ func (m *nfdMaster) processNodeFeatureRule(nodeName string, features *nfdv1alpha
l := ruleOut.Labels
e := ruleOut.ExtendedResources
a := ruleOut.Annotations
if m.config.AutoDefaultNs {
if !nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.DisableAutoPrefix) && m.config.AutoDefaultNs {

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L1044 was not covered by tests
l = addNsToMapKeys(ruleOut.Labels, nfdv1alpha1.FeatureLabelNs)
e = addNsToMapKeys(ruleOut.ExtendedResources, nfdv1alpha1.ExtendedResourceNs)
a = addNsToMapKeys(ruleOut.Annotations, nfdv1alpha1.FeatureAnnotationNs)
Expand Down

0 comments on commit 1213454

Please sign in to comment.