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

feat: support dynamic values for labels in the NodeFeatureRule #1226

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
32 changes: 32 additions & 0 deletions docs/usage/customization-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,38 @@ The `.name` field is required and used as an identifier of the rule.

The `.labels` is a map of the node labels to create if the rule matches.

Take this rule as a referential example:

```yaml
apiVersion: nfd.k8s-sigs.io/v1alpha1
kind: NodeFeatureRule
metadata:
name: my-sample-rule-object
spec:
rules:
- name: "my dynamic label value rule"
labels:
linux-lsm-enabled: "@kernel.config.LSM"
custom-label: "customlabel"
```

AhmedGrati marked this conversation as resolved.
Show resolved Hide resolved
Label `linux-lsm-enabled` uses the `@` notation for dynamic values.
The value of the label will be the value of the attribute `LSM`
of the feature `kernel.config`.

The `@<feature-name>.<element-name>` format can be used to inject values of
detected features to the label. See
[available features](#available-features) for possible values to use.

This will yield into the following node label:

```yaml
labels:
...
feature.node.kubernetes.io/linux-lsm-enabled: apparmor
feature.node.kubernetes.io/custom-label: "customlabel"
```

#### Labels template

The `.labelsTemplate` field specifies a text template for dynamically creating
Expand Down
29 changes: 29 additions & 0 deletions pkg/nfd-master/nfd-master-internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8sclient "k8s.io/client-go/kubernetes"
"sigs.k8s.io/node-feature-discovery/pkg/apihelper"
"sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/v1alpha1"
nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/v1alpha1"
"sigs.k8s.io/node-feature-discovery/pkg/labeler"
"sigs.k8s.io/node-feature-discovery/pkg/utils"
Expand Down Expand Up @@ -438,6 +439,34 @@ func TestSetLabels(t *testing.T) {
})
}

func TestFilterLabels(t *testing.T) {
mockHelper := &apihelper.MockAPIHelpers{}
mockMaster := newMockMaster(mockHelper)

Convey("When using dynamic values", t, func() {
labelName := "testLabel"
labelValue := "@test.feature.LSM"
features := nfdv1alpha1.Features{
Attributes: map[string]nfdv1alpha1.AttributeFeatureSet{
"test.feature": v1alpha1.AttributeFeatureSet{
Elements: map[string]string{
"LSM": "123",
},
},
},
}
labelValue, err := mockMaster.filterFeatureLabel(labelName, labelValue, &features)

Convey("Operation should succeed", func() {
So(err, ShouldBeNil)
})

Convey("Label value should change", func() {
So(labelValue, ShouldEqual, "123")
})
})
}

func TestCreatePatches(t *testing.T) {
Convey("When creating JSON patches", t, func() {
existingItems := map[string]string{"key-1": "val-1", "key-2": "val-2", "key-3": "val-3"}
Expand Down
79 changes: 50 additions & 29 deletions pkg/nfd-master/nfd-master.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,13 +483,13 @@ func (m *nfdMaster) updateMasterNode() error {
// into extended resources. This function also handles proper namespacing of
// labels and ERs, i.e. adds the possibly missing default namespace for labels
// arriving through the gRPC API.
func (m *nfdMaster) filterFeatureLabels(labels Labels) (Labels, ExtendedResources) {
func (m *nfdMaster) filterFeatureLabels(labels Labels, features *nfdv1alpha1.Features) (Labels, ExtendedResources) {
outLabels := Labels{}
for name, value := range labels {
// Add possibly missing default ns
name := addNs(name, nfdv1alpha1.FeatureLabelNs)

if err := m.filterFeatureLabel(name, value); err != nil {
if value, err := m.filterFeatureLabel(name, value, features); err != nil {
klog.ErrorS(err, "ignoring label", "labelKey", name, "labelValue", value)
} else {
outLabels[name] = value
Expand All @@ -515,10 +515,11 @@ func (m *nfdMaster) filterFeatureLabels(labels Labels) (Labels, ExtendedResource
return outLabels, extendedResources
}

func (m *nfdMaster) filterFeatureLabel(name, value string) error {
func (m *nfdMaster) filterFeatureLabel(name, value string, features *nfdv1alpha1.Features) (string, error) {

//Validate label name
if errs := k8svalidation.IsQualifiedName(name); len(errs) > 0 {
return fmt.Errorf("invalid name %q: %s", name, strings.Join(errs, "; "))
return "", fmt.Errorf("invalid name %q: %s", name, strings.Join(errs, "; "))
}

marquiz marked this conversation as resolved.
Show resolved Hide resolved
// Check label namespace, filter out if ns is not whitelisted
Expand All @@ -528,22 +529,53 @@ func (m *nfdMaster) filterFeatureLabel(name, value string) error {
// If the namespace is denied, and not present in the extraLabelNs, label will be ignored
if isNamespaceDenied(ns, m.deniedNs.wildcard, m.deniedNs.normal) {
if _, ok := m.config.ExtraLabelNs[ns]; !ok {
return fmt.Errorf("namespace %q is not allowed", ns)
return "", fmt.Errorf("namespace %q is not allowed", ns)
}
}
}

// Skip if label doesn't match labelWhiteList
if !m.config.LabelWhiteList.Regexp.MatchString(base) {
return fmt.Errorf("%s (%s) does not match the whitelist (%s)", base, name, m.config.LabelWhiteList.Regexp.String())
return "", fmt.Errorf("%s (%s) does not match the whitelist (%s)", base, name, m.config.LabelWhiteList.Regexp.String())
}

var filteredLabel string

// Dynamic Value
if strings.HasPrefix(value, "@") {
dynamicValue, err := getDynamicValue(value, features)
if err != nil {
return "", err
}
marquiz marked this conversation as resolved.
Show resolved Hide resolved
filteredLabel = dynamicValue
} else {
filteredLabel = value
}

// Validate the label value
if errs := k8svalidation.IsValidLabelValue(value); len(errs) > 0 {
return fmt.Errorf("invalid value %q: %s", value, strings.Join(errs, "; "))
if errs := k8svalidation.IsValidLabelValue(filteredLabel); len(errs) > 0 {
return "", fmt.Errorf("invalid value %q: %s", filteredLabel, strings.Join(errs, "; "))
}
return filteredLabel, nil
}

return nil
func getDynamicValue(value string, features *nfdv1alpha1.Features) (string, error) {
// value is a string in the form of attribute.featureset.elements
split := strings.SplitN(value[1:], ".", 3)
if len(split) != 3 {
return "", fmt.Errorf("value %s is not in the form of '@domain.feature.element'", value)
}
featureName := split[0] + "." + split[1]
elementName := split[2]
attrFeatureSet, ok := features.Attributes[featureName]
if !ok {
return "", fmt.Errorf("feature %s not found", featureName)
}
element, ok := attrFeatureSet.Elements[elementName]
if !ok {
return "", fmt.Errorf("element %s not found on feature %s", elementName, featureName)
}
return element, nil
}

func filterTaints(taints []corev1.Taint) []corev1.Taint {
Expand Down Expand Up @@ -750,26 +782,15 @@ func filterExtendedResource(name, value string, features *nfdv1alpha1.Features)

// Dynamic Value
if strings.HasPrefix(value, "@") {
// value is a string in the form of attribute.featureset.elements
split := strings.SplitN(value[1:], ".", 3)
if len(split) != 3 {
return "", fmt.Errorf("value %s is not in the form of '@domain.feature.element'", value)
}
featureName := split[0] + "." + split[1]
elementName := split[2]
attrFeatureSet, ok := features.Attributes[featureName]
if !ok {
return "", fmt.Errorf("feature %s not found", featureName)
}
element, ok := attrFeatureSet.Elements[elementName]
if !ok {
return "", fmt.Errorf("element %s not found on feature %s", elementName, featureName)
}
q, err := k8sQuantity.ParseQuantity(element)
if err != nil {
return "", fmt.Errorf("invalid value %s (from %s): %w", element, value, err)
if element, err := getDynamicValue(value, features); err != nil {
return "", err
} else {
q, err := k8sQuantity.ParseQuantity(element)
if err != nil {
return "", fmt.Errorf("invalid value %s (from %s): %w", element, value, err)
}
return q.String(), nil
}
return q.String(), nil
}
// Static Value (Pre-Defined at the NodeFeatureRule)
q, err := k8sQuantity.ParseQuantity(value)
Expand All @@ -794,7 +815,7 @@ func (m *nfdMaster) refreshNodeFeatures(cli *kubernetes.Clientset, nodeName stri

// Remove labels which are intended to be extended resources via
// -resource-labels or their NS is not whitelisted
labels, extendedResources := m.filterFeatureLabels(labels)
labels, extendedResources := m.filterFeatureLabels(labels, features)

// Mix in CR-originated extended resources with -resource-labels
for k, v := range crExtendedResources {
Expand Down
1 change: 1 addition & 0 deletions test/e2e/data/nodefeaturerule-2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ spec:
- name: "e2e-matchany-test-1"
labels:
e2e-matchany-test-1: "true"
dynamic-label: "@rule.matched.e2e-attribute-test-1"
vars:
e2e-instance-test-1.not: "false"
matchFeatures:
Expand Down
1 change: 1 addition & 0 deletions test/e2e/node_feature_discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,7 @@ core:
expectedLabels["*"][nfdv1alpha1.FeatureLabelNs+"/e2e-matchany-test-1"] = "true"
expectedLabels["*"][nfdv1alpha1.FeatureLabelNs+"/e2e-template-test-1-instance_1"] = "found"
expectedLabels["*"][nfdv1alpha1.FeatureLabelNs+"/e2e-template-test-1-instance_2"] = "found"
expectedLabels["*"][nfdv1alpha1.FeatureLabelNs+"/dynamic-label"] = "true"

By("Verifying node labels from NodeFeatureRules #1 and #2")
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
Expand Down