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

nfd-master: fix resync period config option #1185

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
2 changes: 1 addition & 1 deletion cmd/nfd-master/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
})
Expand Down
18 changes: 10 additions & 8 deletions pkg/nfd-master/nfd-api-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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) {
Expand All @@ -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)
AhmedGrati marked this conversation as resolved.
Show resolved Hide resolved

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{}) {
Expand Down Expand Up @@ -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
Expand All @@ -119,7 +122,6 @@ func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiC
informerFactory.Start(c.stopChan)

utilruntime.Must(nfdv1alpha1.AddToScheme(nfdscheme.Scheme))

return c, nil
}

Expand Down
43 changes: 31 additions & 12 deletions pkg/nfd-master/nfd-master.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ type Args struct {
Prune bool
VerifyNodeName bool
Options string
ResyncPeriod utils.DurationVal

Overrides ConfigOverrideArgs
}
Expand Down Expand Up @@ -163,7 +162,6 @@ func NewNfdMaster(args *Args) (NfdMaster, error) {
if args.ConfigFile != "" {
nfd.configFilePath = filepath.Clean(args.ConfigFile)
}

return nfd, nil
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -249,8 +239,20 @@ func (m *nfdMaster) Run() error {
if err := m.configure(m.configFilePath, m.args.Options); err != nil {
return err
}
// Update all nodes when the configuration changes

// restart NFD API controller
if m.nfdController != nil {
klog.Info("stopping the nfd api controller")
m.nfdController.stop()
AhmedGrati marked this conversation as resolved.
Show resolved Hide resolved
}
if m.args.CrdController {
err := m.startNfdApiController()
if err != nil {
return nil
}
}
// Update all nodes when the configuration changes
if m.nfdController != nil && m.args.EnableNodeFeatureApi {
m.nfdController.updateAllNodesChan <- struct{}{}
}
case <-m.stop:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
AhmedGrati marked this conversation as resolved.
Show resolved Hide resolved
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
}
70 changes: 70 additions & 0 deletions test/e2e/node_feature_discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package e2e

import (
"context"
"encoding/json"
"fmt"
"path/filepath"
"strings"
Expand All @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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())
AhmedGrati marked this conversation as resolved.
Show resolved Hide resolved

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())
})
})
})
}

Expand Down