nfd-master: fix resync period config option

This PR fixes the resync-period configuration option of the nfd-master. In fact, previously, changes were not reflected in the nfd-master at runtime. e2e tests are also implemented to make sure that the fix is already working as expected. Signed-off-by: AhmedGrati <[email protected]>
kubernetes-sigs · Apr 28, 2023 · 6ac4e97 · 6ac4e97
1 parent e4dfa2d
commit 6ac4e97
Show file tree

Hide file tree

Showing 4 changed files with 111 additions and 20 deletions.
diff --git a/cmd/nfd-master/main.go b/cmd/nfd-master/main.go
@@ -68,7 +68,7 @@ func main() {
 			args.Overrides.EnableTaints = overrides.EnableTaints
 		case "no-publish":
 			args.Overrides.NoPublish = overrides.NoPublish
-		case "-resync-period":
+		case "resync-period":
 			args.Overrides.ResyncPeriod = overrides.ResyncPeriod
 		}
 	})

diff --git a/pkg/nfd-master/nfd-api-controller.go b/pkg/nfd-master/nfd-api-controller.go
@@ -31,6 +31,7 @@ import (
 	nfdscheme "sigs.k8s.io/node-feature-discovery/pkg/generated/clientset/versioned/scheme"
 	nfdinformers "sigs.k8s.io/node-feature-discovery/pkg/generated/informers/externalversions"
 	nfdlisters "sigs.k8s.io/node-feature-discovery/pkg/generated/listers/nfd/v1alpha1"
+	"sigs.k8s.io/node-feature-discovery/pkg/utils"
 )
 
 type nfdController struct {
@@ -44,8 +45,8 @@ type nfdController struct {
 }
 
 type nfdApiControllerOptions struct {
-	disableNodeFeature bool
-	resyncPeriod       time.Duration
+	DisableNodeFeature bool
+	ResyncPeriod       time.Duration
 }
 
 func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiControllerOptions) (*nfdController, error) {
@@ -56,10 +57,12 @@ func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiC
 	}
 
 	nfdClient := nfdclientset.NewForConfigOrDie(config)
-	informerFactory := nfdinformers.NewSharedInformerFactory(nfdClient, nfdApiControllerOptions.resyncPeriod)
+	utils.KlogDump(2, "NFD API controller options:", "  ", nfdApiControllerOptions)
+
+	informerFactory := nfdinformers.NewSharedInformerFactory(nfdClient, nfdApiControllerOptions.ResyncPeriod)
 
 	// Add informer for NodeFeature objects
-	if !nfdApiControllerOptions.disableNodeFeature {
+	if !nfdApiControllerOptions.DisableNodeFeature {
 		featureInformer := informerFactory.Nfd().V1alpha1().NodeFeatures()
 		if _, err := featureInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 			AddFunc: func(obj interface{}) {
@@ -89,23 +92,23 @@ func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiC
 		AddFunc: func(object interface{}) {
 			key, _ := cache.MetaNamespaceKeyFunc(object)
 			klog.V(2).Infof("NodeFeatureRule %v added", key)
-			if !nfdApiControllerOptions.disableNodeFeature {
+			if !nfdApiControllerOptions.DisableNodeFeature {
 				c.updateAllNodes()
 			}
 			// else: rules will be processed only when gRPC requests are received
 		},
 		UpdateFunc: func(oldObject, newObject interface{}) {
 			key, _ := cache.MetaNamespaceKeyFunc(newObject)
 			klog.V(2).Infof("NodeFeatureRule %v updated", key)
-			if !nfdApiControllerOptions.disableNodeFeature {
+			if !nfdApiControllerOptions.DisableNodeFeature {
 				c.updateAllNodes()
 			}
 			// else: rules will be processed only when gRPC requests are received
 		},
 		DeleteFunc: func(object interface{}) {
 			key, _ := cache.MetaNamespaceKeyFunc(object)
 			klog.V(2).Infof("NodeFeatureRule %v deleted", key)
-			if !nfdApiControllerOptions.disableNodeFeature {
+			if !nfdApiControllerOptions.DisableNodeFeature {
 				c.updateAllNodes()
 			}
 			// else: rules will be processed only when gRPC requests are received
@@ -119,7 +122,6 @@ func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiC
 	informerFactory.Start(c.stopChan)
 
 	utilruntime.Must(nfdv1alpha1.AddToScheme(nfdscheme.Scheme))
-
 	return c, nil
 }
 

diff --git a/pkg/nfd-master/nfd-master.go b/pkg/nfd-master/nfd-master.go
@@ -98,7 +98,6 @@ type Args struct {
 	Prune                bool
 	VerifyNodeName       bool
 	Options              string
-	ResyncPeriod         utils.DurationVal
 
 	Overrides ConfigOverrideArgs
 }
@@ -163,7 +162,6 @@ func NewNfdMaster(args *Args) (NfdMaster, error) {
 	if args.ConfigFile != "" {
 		nfd.configFilePath = filepath.Clean(args.ConfigFile)
 	}
-
 	return nfd, nil
 }
 
@@ -199,18 +197,10 @@ func (m *nfdMaster) Run() error {
 	}
 
 	if m.args.CrdController {
-		kubeconfig, err := m.getKubeconfig()
+		err := m.startNfdApiController()
 		if err != nil {
 			return err
 		}
-		klog.Info("starting nfd api controller")
-		m.nfdController, err = newNfdController(kubeconfig, nfdApiControllerOptions{
-			disableNodeFeature: !m.args.EnableNodeFeatureApi,
-			resyncPeriod:       m.args.ResyncPeriod.Duration,
-		})
-		if err != nil {
-			return fmt.Errorf("failed to initialize CRD controller: %w", err)
-		}
 	}
 
 	// Create watcher for config file
@@ -249,6 +239,18 @@ func (m *nfdMaster) Run() error {
 			if err := m.configure(m.configFilePath, m.args.Options); err != nil {
 				return err
 			}
+
+			// restart NFD API controller
+			if m.nfdController != nil {
+				klog.Info("stopping the nfd api controller")
+				m.nfdController.stop()
+			}
+			if m.args.CrdController {
+				err := m.startNfdApiController()
+				if err != nil {
+					return nil
+				}
+			}
 			// Update all nodes when the configuration changes
 			if m.nfdController != nil {
 				m.nfdController.updateAllNodesChan <- struct{}{}
@@ -1152,6 +1154,7 @@ func (m *nfdMaster) configure(filepath string, overrides string) error {
 		}
 		m.apihelper = apihelper.K8sHelpers{Kubeconfig: kubeconfig}
 	}
+
 	// Pre-process DenyLabelNS into 2 lists: one for normal ns, and the other for wildcard ns
 	normalDeniedNs, wildcardDeniedNs := preProcessDeniedNamespaces(c.DenyLabelNs)
 	m.deniedNs.normal = normalDeniedNs
@@ -1219,3 +1222,19 @@ func (m *nfdMaster) instanceAnnotation(name string) string {
 	}
 	return m.args.Instance + "." + name
 }
+
+func (m *nfdMaster) startNfdApiController() error {
+	kubeconfig, err := m.getKubeconfig()
+	if err != nil {
+		return err
+	}
+	klog.Info("starting nfd api controller")
+	m.nfdController, err = newNfdController(kubeconfig, nfdApiControllerOptions{
+		DisableNodeFeature: !m.args.EnableNodeFeatureApi,
+		ResyncPeriod:       m.config.ResyncPeriod.Duration,
+	})
+	if err != nil {
+		return fmt.Errorf("failed to initialize CRD controller: %w", err)
+	}
+	return nil
+}
diff --git a/test/e2e/node_feature_discovery_test.go b/test/e2e/node_feature_discovery_test.go
@@ -18,6 +18,7 @@ package e2e
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"path/filepath"
 	"strings"
@@ -33,6 +34,7 @@ import (
 	extclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
 	resourcev1 "k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	clientset "k8s.io/client-go/kubernetes"
 	taintutils "k8s.io/kubernetes/pkg/util/taints"
 	"k8s.io/kubernetes/test/e2e/framework"
@@ -41,6 +43,7 @@ import (
 	e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
 	admissionapi "k8s.io/pod-security-admission/api"
 
+	"sigs.k8s.io/node-feature-discovery/pkg/apihelper"
 	nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/v1alpha1"
 	nfdclient "sigs.k8s.io/node-feature-discovery/pkg/generated/clientset/versioned"
 	"sigs.k8s.io/node-feature-discovery/source/custom"
@@ -909,6 +912,73 @@ denyLabelNs: []
 					)).NotTo(HaveOccurred())
 				})
 			})
+
+			Context("and test whether resyncPeriod is passed successfully or not", func() {
+				BeforeEach(func(ctx context.Context) {
+					extraMasterPodSpecOpts = []testpod.SpecOption{
+						testpod.SpecWithConfigMap("nfd-master-conf", "/etc/kubernetes/node-feature-discovery"),
+					}
+					cm := testutils.NewConfigMap("nfd-master-conf", "nfd-master.conf", `
+resyncPeriod: "1s"
+`)
+					_, err := f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(ctx, cm, metav1.CreateOptions{})
+					Expect(err).NotTo(HaveOccurred())
+				})
+				It("labels should be restored to the original ones", func(ctx context.Context) {
+					// deploy node feature object
+					if !useNodeFeatureApi {
+						Skip("NodeFeature API not enabled")
+					}
+
+					nodes, err := getNonControlPlaneNodes(ctx, f.ClientSet)
+					Expect(err).NotTo(HaveOccurred())
+
+					targetNodeName := nodes[0].Name
+					Expect(targetNodeName).ToNot(BeEmpty(), "No suitable worker node found")
+
+					// Apply Node Feature object
+					By("Creating NodeFeature object")
+					nodeFeatures, err := testutils.CreateOrUpdateNodeFeaturesFromFile(ctx, nfdClient, "nodefeature-1.yaml", f.Namespace.Name, targetNodeName)
+					Expect(err).NotTo(HaveOccurred())
+
+					By("Verifying node labels from NodeFeature object #1")
+					expectedLabels := map[string]k8sLabels{
+						targetNodeName: {
+							nfdv1alpha1.FeatureLabelNs + "/e2e-nodefeature-test-1": "obj-1",
+							nfdv1alpha1.FeatureLabelNs + "/e2e-nodefeature-test-2": "obj-1",
+							nfdv1alpha1.FeatureLabelNs + "/fake-fakefeature3":      "overridden",
+						},
+					}
+					Expect(checkForNodeLabels(ctx, f.ClientSet,
+						expectedLabels, nodes,
+					)).NotTo(HaveOccurred())
+
+					patches, err := json.Marshal(
+						[]apihelper.JsonPatch{
+							apihelper.NewJsonPatch(
+								"replace",
+								"/metadata/labels",
+								nfdv1alpha1.FeatureLabelNs+"/e2e-nodefeature-test-1",
+								"randomValue",
+							),
+						},
+					)
+					Expect(err).NotTo(HaveOccurred())
+
+					_, err = f.ClientSet.CoreV1().Nodes().Patch(ctx, targetNodeName, types.JSONPatchType, patches, metav1.PatchOptions{})
+					Expect(err).NotTo(HaveOccurred())
+
+					Expect(checkForNodeLabels(ctx,
+						f.ClientSet,
+						expectedLabels,
+						nodes,
+					)).NotTo(HaveOccurred())
+
+					By("Deleting NodeFeature object")
+					err = nfdClient.NfdV1alpha1().NodeFeatures(f.Namespace.Name).Delete(ctx, nodeFeatures[0], metav1.DeleteOptions{})
+					Expect(err).NotTo(HaveOccurred())
+				})
+			})
 		})
 	}