Support CRDs for custom label rules

[marquiz]

What would you like to be added:

An idea I've been pondering to make adding new rules for the custom source easier, without the need to add any new mounts or anything. NFD would specify a new CRD and watch for objects. Not sure what would be the best way to implement this:

1. NFD worker instances would watch the CRs directly themselves
2. NFD master would watch the CRs and communicate rules via gRPC
3. NFD master would watch the CRs and update a predefined ConfigMap that is mounted on the worker containers
4. Do rule processing on master side: NFD worker sends "raw" features to master. Master watches CRs, performs rule processing/matching against the "raw" features and creates labels.

Option 1 would be the obvious simplest choice even if it opens up direct communication of the worker to the control plane.

Why is this needed:

Make dropping in vendor/user specific custom label rules easier, as simple as kubectl apply -f <rules.yaml>

List of individual PRs:

• source/custom: refactor kconfig rule internal representation #543
• source: define source names as consts #544
• source: rename FeatureSource to LabelSource #596
• source: make sources register themselves #597
• source: introduce FeatureSource interface #601
• source: rename FeatureSource to LabelSource #596
• pkg/api/feature: small improvements #642
• source: implement FeatureSource interface #604
• source/custom: expression based label rules #639
• More extensive and expressive custom rules #464
• grpc: extend the API to send raw features #646
• specify CRD for custom labeling rules #653
• source/custom: move rule matching to pkg/apis/nfd #654
• Add code for interacting with CRD API #655
• nfd-master: implement controller for NodeFeatureRule CRs #656
• CRD-based custom node labeling #553

Adjacent PRs, building on top and extending the functionality:

• source/storage: implement FeatureSource #649
• source/network: implement FeatureSource #660
• source/memory: implement FeatureSource #661
• Templating of custom label names #550
• Add variables to feature rule spec and support backrefs  #663
• nfd-worker: add core.featureSources config option #605
• Wildcard for matching against all keys in label rules #662

[zvonkok]

etcd has the 1.5 MiB object limit size which corresponds to roughly 750 pages of text. I think this should be enough to hold enough rules.

[zvonkok]

Regarding 1. does it mean we would have 1 CR that all nfd-workers would read and report the labels ... If we have several hundreds of nodes, how will the API behave is several hundred nfd-workers are reading from the very same CR at the same time? Should we add a random startup time?

[zvonkok]

Regarding 3. only the master would read the CR and the load would be distributed to each node, update of the CM...

[marquiz]

Regarding 1. does it mean we would have 1 CR that all nfd-workers would read and report the labels ...

Yes, all workers will need read that config (CR)

If we have several hundreds of nodes, how will the API behave is several hundred nfd-workers are reading from the very same CR at the same time? Should we add a random startup time?

This is a fair concern. Unfortunately I don't have access to such a cluster. It's not just startup time but any time a new CR is added (or old one updated)

[marquiz]

Regarding 3. only the master would read the CR and the load would be distributed to each node, update of the CM...

Yes. Point of stress would be moved to the configmap

[zvonkok]

Regarding 2. This could create some load on the "network" if going this route we definitely would need something like "OnChange" trigger and not do this periodically.

[marquiz]

Regarding 2. This could create some load on the "network" if going this route we definitely would need something like "OnChange" trigger and not do this periodically

Yeah, I don't like this as it complicates the worker-master communication and looks like a well of sync issues.

[marquiz]

Added option 4 which might make sense: do rule processing of the CRs on the master side.

[ArangoGutierrez]

I know I am biassed, but I feel this issue is more suited for the Operator than the Operand side of NFD
thoughts?

[marquiz]

I know I am biassed, but I feel this issue is more suited for the Operator than the Operand side of NFD
thoughts?

I think this would be orthogonal to / independent of the operator and should be handled by nfd-master. Allowing the creation of rules independent of the deployment mechanism, be it the operator, Helm or templates/kustomize.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[marquiz]

Been working on a prototype building on top of #464 and #550. I'll submit a RFC version soon.
/remove-lifecycle stale

[marquiz]

Been working on a prototype building on top of #464 and #550. I'll submit a RFC version soon.

...and we have #553

[ArangoGutierrez]

we are getting ready for V0.11.0 !!!!

[marquiz]

With #663 all the essential parts are now merged and I declare this issue as implemented
/close

[k8s-ci-robot]

@marquiz: Closing this issue.

In response to this:

With #663 all the essential parts are now merged and I declare this issue as implemented
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[eero-t]

With #663 all the essential parts are now merged and I declare this issue as implemented /close

@marquiz In one of the PRs you mentioned documentation update being a separate PR, but I do not see one listed above. could you you give a pointer to the associated doc update?

[marquiz]

@marquiz In one of the PRs you mentioned documentation update being a separate PR, but I do not see one listed above. could you you give a pointer to the associated doc update?

It's still under construction, there's no PR yet on that