Skip to content

Commit 7bad0d5

Browse files
committed
feat/nfd-master: support CR restrictions
Signed-off-by: AhmedGrati <[email protected]>
1 parent c1db298 commit 7bad0d5

File tree

6 files changed

+453
-28
lines changed

6 files changed

+453
-28
lines changed

go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ module sigs.k8s.io/node-feature-discovery
22

33
go 1.22.2
44

5+
toolchain go1.22.0
6+
57
require (
68
github.com/fsnotify/fsnotify v1.7.0
79
github.com/golang/protobuf v1.5.4

pkg/nfd-master/nfd-api-controller.go

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package nfdmaster
1818

1919
import (
20+
"context"
2021
"fmt"
2122
"time"
2223

@@ -26,7 +27,6 @@ import (
2627
"k8s.io/client-go/tools/cache"
2728
"k8s.io/klog/v2"
2829

29-
nfdclientset "sigs.k8s.io/node-feature-discovery/api/generated/clientset/versioned"
3030
nfdscheme "sigs.k8s.io/node-feature-discovery/api/generated/clientset/versioned/scheme"
3131
nfdinformers "sigs.k8s.io/node-feature-discovery/api/generated/informers/externalversions"
3232
nfdinformersv1alpha1 "sigs.k8s.io/node-feature-discovery/api/generated/informers/externalversions/nfd/v1alpha1"
@@ -65,6 +65,8 @@ func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiC
6565
updateOneNodeChan: make(chan string),
6666
updateAllNodeFeatureGroupsChan: make(chan struct{}, 1),
6767
updateNodeFeatureGroupChan: make(chan string),
68+
k8sClient: nfdApiControllerOptions.K8sClient,
69+
nodeFeatureNamespaceSelector: nfdApiControllerOptions.NodeFeatureNamespaceSelector,
6870
}
6971

7072
nfdClient := nfdclientset.NewForConfigOrDie(config)
@@ -89,25 +91,28 @@ func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiC
8991
AddFunc: func(obj interface{}) {
9092
nfr := obj.(*nfdv1alpha1.NodeFeature)
9193
klog.V(2).InfoS("NodeFeature added", "nodefeature", klog.KObj(nfr))
92-
c.updateOneNode("NodeFeature", nfr)
93-
if !nfdApiControllerOptions.DisableNodeFeatureGroup {
94-
c.updateAllNodeFeatureGroups()
94+
if c.isNamespaceSelected(nfr.Namespace) {
95+
c.updateOneNode("NodeFeature", nfr)
96+
} else {
97+
klog.InfoS("NodeFeature not in selected namespace", "namespace", nfr.Namespace, "name", nfr.Name)
9598
}
9699
},
97100
UpdateFunc: func(oldObj, newObj interface{}) {
98101
nfr := newObj.(*nfdv1alpha1.NodeFeature)
99102
klog.V(2).InfoS("NodeFeature updated", "nodefeature", klog.KObj(nfr))
100-
c.updateOneNode("NodeFeature", nfr)
101-
if !nfdApiControllerOptions.DisableNodeFeatureGroup {
102-
c.updateAllNodeFeatureGroups()
103+
if c.isNamespaceSelected(nfr.Namespace) {
104+
c.updateOneNode("NodeFeature", nfr)
105+
} else {
106+
klog.InfoS("NodeFeature not in selected namespace", "namespace", nfr.Namespace, "name", nfr.Name)
103107
}
104108
},
105109
DeleteFunc: func(obj interface{}) {
106110
nfr := obj.(*nfdv1alpha1.NodeFeature)
107111
klog.V(2).InfoS("NodeFeature deleted", "nodefeature", klog.KObj(nfr))
108-
c.updateOneNode("NodeFeature", nfr)
109-
if !nfdApiControllerOptions.DisableNodeFeatureGroup {
110-
c.updateAllNodeFeatureGroups()
112+
if c.isNamespaceSelected(nfr.Namespace) {
113+
c.updateOneNode("NodeFeature", nfr)
114+
} else {
115+
klog.InfoS("NodeFeature not in selected namespace", "namespace", nfr.Namespace, "name", nfr.Name)
111116
}
112117
},
113118
}); err != nil {
@@ -209,6 +214,37 @@ func (c *nfdController) updateOneNode(typ string, obj metav1.Object) {
209214
c.updateOneNodeChan <- nodeName
210215
}
211216

217+
func (c *nfdController) isNamespaceSelected(namespace string) bool {
218+
// no namespace restrictions are made here
219+
if c.nodeFeatureNamespaceSelector == nil {
220+
return true
221+
}
222+
223+
labelMap, err := metav1.LabelSelectorAsSelector(c.nodeFeatureNamespaceSelector)
224+
if err != nil {
225+
klog.ErrorS(err, "failed to convert label selector to map", "selector", c.nodeFeatureNamespaceSelector)
226+
return false
227+
}
228+
229+
listOptions := metav1.ListOptions{
230+
LabelSelector: labelMap.String(),
231+
}
232+
233+
namespaces, err := c.k8sClient.CoreV1().Namespaces().List(context.Background(), listOptions)
234+
if err != nil {
235+
klog.ErrorS(err, "failed to query namespaces", "listOptions", listOptions)
236+
return false
237+
}
238+
239+
for _, ns := range namespaces.Items {
240+
if ns.Name == namespace {
241+
return true
242+
}
243+
}
244+
245+
return false
246+
}
247+
212248
func (c *nfdController) updateAllNodes() {
213249
select {
214250
case c.updateAllNodesChan <- struct{}{}:

pkg/nfd-master/nfd-api-controller_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import (
2323

2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2525
nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/api/nfd/v1alpha1"
26+
fakeclient "k8s.io/client-go/kubernetes/fake"
27+
corev1 "k8s.io/api/core/v1"
2628
)
2729

2830
func TestGetNodeNameForObj(t *testing.T) {
@@ -42,3 +44,57 @@ func TestGetNodeNameForObj(t *testing.T) {
4244
assert.Nil(t, err)
4345
assert.Equal(t, n, "node-1")
4446
}
47+
48+
func newTestNamespace(name string) *corev1.Namespace{
49+
return &corev1.Namespace{
50+
ObjectMeta: metav1.ObjectMeta{
51+
Name: name,
52+
Labels: map[string]string{
53+
"name": name,
54+
},
55+
},
56+
}
57+
}
58+
59+
func TestIsNamespaceAllowed(t *testing.T) {
60+
fakeCli := fakeclient.NewSimpleClientset(newTestNamespace("fake"))
61+
c := &nfdController{
62+
k8sClient: fakeCli,
63+
}
64+
65+
testcases := []struct {
66+
name string
67+
objectNamespace string
68+
nodeFeatureNamespaceSelector *metav1.LabelSelector
69+
expectedResult bool
70+
}{
71+
{
72+
name: "namespace not allowed",
73+
objectNamespace: "random",
74+
nodeFeatureNamespaceSelector: &metav1.LabelSelector{
75+
MatchExpressions: []metav1.LabelSelectorRequirement{
76+
{
77+
Key: "name",
78+
Operator: metav1.LabelSelectorOpIn,
79+
Values: []string{"fake"},
80+
},
81+
},
82+
},
83+
expectedResult: false,
84+
},
85+
{
86+
name: "namespace is allowed",
87+
objectNamespace: "fake",
88+
nodeFeatureNamespaceSelector: &metav1.LabelSelector{
89+
MatchLabels: map[string]string{"name": "fake"},
90+
},
91+
expectedResult: false,
92+
},
93+
}
94+
95+
for _, tc := range testcases {
96+
c.nodeFeatureNamespaceSelector = tc.nodeFeatureNamespaceSelector
97+
res := c.isNamespaceSelected(tc.name)
98+
assert.Equal(t, res, tc.expectedResult)
99+
}
100+
}

pkg/nfd-master/nfd-master-internal_test.go

Lines changed: 76 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -509,35 +509,36 @@ func TestFilterLabels(t *testing.T) {
509509
func TestCreatePatches(t *testing.T) {
510510
Convey("When creating JSON patches", t, func() {
511511
existingItems := map[string]string{"key-1": "val-1", "key-2": "val-2", "key-3": "val-3"}
512+
overwriteKeys := true
512513
jsonPath := "/root"
513514

514-
Convey("When when there are neither itmes to remoe nor to add or update", func() {
515-
p := createPatches([]string{"foo", "bar"}, existingItems, map[string]string{}, jsonPath)
515+
Convey("When there are neither itmes to remoe nor to add or update", func() {
516+
p := createPatches([]string{"foo", "bar"}, existingItems, map[string]string{}, jsonPath, overwriteKeys)
516517
So(len(p), ShouldEqual, 0)
517518
})
518519

519-
Convey("When when there are itmes to remoe but none to add or update", func() {
520-
p := createPatches([]string{"key-2", "key-3", "foo"}, existingItems, map[string]string{}, jsonPath)
520+
Convey("When there are itmes to remoe but none to add or update", func() {
521+
p := createPatches([]string{"key-2", "key-3", "foo"}, existingItems, map[string]string{}, jsonPath, overwriteKeys)
521522
expected := []utils.JsonPatch{
522523
utils.NewJsonPatch("remove", jsonPath, "key-2", ""),
523524
utils.NewJsonPatch("remove", jsonPath, "key-3", ""),
524525
}
525526
So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected))
526527
})
527528

528-
Convey("When when there are no itmes to remove but new items to add", func() {
529+
Convey("When there are no itmes to remove but new items to add", func() {
529530
newItems := map[string]string{"new-key": "new-val", "key-1": "new-1"}
530-
p := createPatches([]string{"key-1"}, existingItems, newItems, jsonPath)
531+
p := createPatches([]string{"key-1"}, existingItems, newItems, jsonPath, overwriteKeys)
531532
expected := []utils.JsonPatch{
532533
utils.NewJsonPatch("add", jsonPath, "new-key", newItems["new-key"]),
533534
utils.NewJsonPatch("replace", jsonPath, "key-1", newItems["key-1"]),
534535
}
535536
So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected))
536537
})
537538

538-
Convey("When when there are items to remove add and update", func() {
539+
Convey("When there are items to remove add and update", func() {
539540
newItems := map[string]string{"new-key": "new-val", "key-2": "new-2", "key-4": "val-4"}
540-
p := createPatches([]string{"key-1", "key-2", "key-3", "foo"}, existingItems, newItems, jsonPath)
541+
p := createPatches([]string{"key-1", "key-2", "key-3", "foo"}, existingItems, newItems, jsonPath, overwriteKeys)
541542
expected := []utils.JsonPatch{
542543
utils.NewJsonPatch("add", jsonPath, "new-key", newItems["new-key"]),
543544
utils.NewJsonPatch("add", jsonPath, "key-4", newItems["key-4"]),
@@ -547,6 +548,16 @@ func TestCreatePatches(t *testing.T) {
547548
}
548549
So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected))
549550
})
551+
552+
Convey("When overwrite of keys is denied and there is already an existant key", func() {
553+
overwriteKeys = false
554+
newItems := map[string]string{"key-1": "new-2", "key-4": "val-4"}
555+
p := createPatches([]string{}, existingItems, newItems, jsonPath, overwriteKeys)
556+
expected := []utils.JsonPatch{
557+
utils.NewJsonPatch("add", jsonPath, "key-4", newItems["key-4"]),
558+
}
559+
So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected))
560+
})
550561
})
551562
}
552563

@@ -896,3 +907,60 @@ func TestGetDynamicValue(t *testing.T) {
896907
})
897908
}
898909
}
910+
911+
func TestFilterTaints(t *testing.T) {
912+
testcases := []struct {
913+
name string
914+
taints []corev1.Taint
915+
maxTaints int
916+
expectedResult []corev1.Taint
917+
}{
918+
{
919+
name: "no restriction on the number of taints",
920+
taints: []corev1.Taint{
921+
{
922+
Key: "feature.node.kubernetes.io/key1",
923+
Value: "dummy",
924+
Effect: corev1.TaintEffectNoSchedule,
925+
},
926+
},
927+
maxTaints: 0,
928+
expectedResult: []corev1.Taint{
929+
{
930+
Key: "feature.node.kubernetes.io/key1",
931+
Value: "dummy",
932+
Effect: corev1.TaintEffectNoSchedule,
933+
},
934+
},
935+
},
936+
{
937+
name: "max of 1 Taint should be generated",
938+
taints: []corev1.Taint{
939+
{
940+
Key: "feature.node.kubernetes.io/key1",
941+
Value: "dummy",
942+
Effect: corev1.TaintEffectNoSchedule,
943+
},
944+
{
945+
Key: "feature.node.kubernetes.io/key2",
946+
Value: "dummy",
947+
Effect: corev1.TaintEffectNoSchedule,
948+
},
949+
},
950+
maxTaints: 1,
951+
expectedResult: []corev1.Taint{},
952+
},
953+
}
954+
955+
mockMaster := newFakeMaster(nil)
956+
957+
for _, tc := range testcases {
958+
t.Run(tc.name, func(t *testing.T) {
959+
mockMaster.config.Restrictions.MaxTaintsPerCR = tc.maxTaints
960+
res := mockMaster.filterTaints(tc.taints)
961+
Convey("The expected number of taints should be correct", t, func() {
962+
So(len(res), ShouldEqual, len(tc.expectedResult))
963+
})
964+
})
965+
}
966+
}

0 commit comments

Comments
 (0)