CRD-based custom node labeling

[marquiz]

Enable Custom Resource based label creation in nfd-master. This extends
the previously implemented controller stub for watching NodeFeatureRule
objects. NFD-master watches NodeFeatureRule objects in the cluster and
processes the rules on every incoming labeling request from workers.
The functionality relies on the "raw features" (identical to how
nfd-worker handles custom rules) submitted by nfd-worker, making it
independent of the label source configuration of the worker. This means
that the labeling functions as expected even if all sources in the
worker are disabled.

NOTE: nfd-master is stateless and re-labeling only happens on the
reception of SetLabelsRequest from the worker – i.e. on intervals
specified by the core.sleepInterval configuration option (or
-sleep-interval cmdline flag) of each nfd-worker instance. This means
that modification/creation of NodeFeatureRule objects does not
automatically update the node labels. Instead, the changes only come
visible when workers send their labeling requests.

This PR implements #468, i.e. CRD-based custom node labeling, and builds on top of previous work:

• 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

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: marquiz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [marquiz]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[kailun-qin]

/cc @kailun-qin

[k8s-ci-robot]

@kailun-qin: GitHub didn't allow me to request PR reviews from the following users: kailun-qin.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @kailun-qin

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.

[marquiz]

All feedback is very much appreciated. Especially on the soundness of the whole concept.

Some concerns/questions from the top of my head below, just talking out loud...

Data format

I've been pondering the data representation of "raw" features, i.e. is it good (enough). In this patchset NFD now under the hood uses a unified data model of all the "raw features" (not talking about labels here) it is detecting.

Basically we divide feature types into three categories:

1. "key-only features": features that don't have any value other than "present" or "not present". Examples are CPUID flags or loaded kernel nodules
2. "value features": features that have a value associated with it. Examples are kernel config flags or os-release information.
3. "instance features": features that cannot be represented by a single key-value pair (or key-only) but consist of mulitple attributes (key-value pairs). Examples are pci and usb devices

On the higher level, data representation of all detected features (from all features sources) is fixed into a three level hierarchy:
<source-name>.<feature-name>.<single-feature-instance>.

In go code this is a bit more complicated looks something like this (in a pseudo-ish golang):

// Name of the feature source e.g. "cpu" or "pci"
type FeatureSourceName string

// Name of the feature under the feature source e.g. "cpuid" (cpu) or "device" (pci)
type FeatureName string

// All features from all the feature sources
type AllFeatures map[FeatureSourceName]struct {
	// "key-only" features like cpuid or loaded kernel modules
	Keys map[FeatureName]struct {
		Features map[string]struct{}
	}

	// "key-value" features e.g. kernel version or kernel config flags
	Values map[FeatureName]struct {
		Features map[string]string
	}

	// "instance features" like pci or usb devices
	Instances map[FeatureName]struct {
		Features []struct {
			Attributes map[string]string
		}
	}
}

This data representation trickles down everywhere and to the rule format and CRD, too. The unified representation is nice in the sense that it reduces the lines of code dramatically and also simplifies the CRD schema plus makes it stable. Basically any new features added to NFD is automatically supported (because of the unified data representation and unified rule schema) with essentially no changes required in the CRD spec or rules.

My only concern is that is this flexible enough as we're fixed with the three-level hierarchy and cannot specify arbitrary data structures for the features. For example, Intel Speed Select Technology we now have cpu["sst"].Features["bf.enabled"] even though cpu.sst.bf.enabled (just nested structs and a bool in the end) might be more logical representation. However, the downside is that every feature needs a rule type of its own, similar to what we currently have in master (the custom rules). This also means that any added feature potentially changes the CRD API making is potentially way messier.

MatchExpressions

The key attribute of matchexpressions is implicit os the set of expressions are are presented as a map - the map key is used as the key attribute of the matchexpression. This has the limitation that you cannot specify more than one expression per key. I originally thought that this does not matter but now realized that there is actually one case where you would need this property: if you want to check that a value is between two values - in this case you'd need both Lt and Gt expressions. Talking out loud: maybe this should be changed...

Naming

I'm not happy with all the terms/naming that I've come up with. Need to think through it.

[ArangoGutierrez]

Not yet ready for review?
I want to jump into this PR as soon as it moves out of draft !

[marquiz]

Not yet ready for review?
I want to jump into this PR as soon as it moves out of draft !

#464 would be the first part. But even that is not ready yet, missing unit tests and documentation. I will work on these in the coming weeks, for sure, as I want to get these finally in shape, too.

[ArangoGutierrez]

/assign

[ArangoGutierrez]

@marquiz rebase needed

[marquiz]

#656 was merged -> rebased and PR description updated
/unhold

[marquiz]

/retest

[zvonkok]

/lgtm