-
Notifications
You must be signed in to change notification settings - Fork 245
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
More extensive and expressive custom rules #464
More extensive and expressive custom rules #464
Conversation
Skipping CI for Draft Pull Request. |
[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:
Approvers can indicate their approval by writing |
fce3173
to
542988c
Compare
Even if in the proto stage, any feedback on the concept, data model, configuration model etc would be more than welcome 😉 |
542988c
to
9f7496a
Compare
Fixed unit tests and added |
dd57a37
to
fd0080e
Compare
fd0080e
to
69a6d66
Compare
514ef2d
to
2ebc7e3
Compare
/assign |
2ebc7e3
to
87f2ff0
Compare
8de67dc
to
fc5d2a7
Compare
fc5d2a7
to
d4d71e4
Compare
#604 was merged so we're one step away from this |
d4d71e4
to
269f253
Compare
Implement generic feature matchers that cover all feature sources (that implement the FeatureSource interface). The implementation relies on the unified data model provided by the FeatureSource interface as well as the generic expression-based rule processing framework that was added to the source/custom/expression package. With this patch any new features added will be automatically available for custom rules, without any additional work. Rule hierarchy follows the source/feature hierarchy by design. This patch introduces a new format for custom rule specifications, dropping the 'value' field and introducing new 'labels' field which makes it possible to specify multiple labels per rule. Also, in the new format the 'name' field is just for reference and no matching label is created. The new generic rules are available in this new rule format under a 'matchFeatures. MatchFeatures implements a logical AND over an array of per-feature matchers - i.e. a match for all of the matchers is required. The goal of the new rule format is to make it better follow K8s API design guidelines and make it extensible for future enhancements (e.g. addition of templating, taints, annotations, extended resources etc). The old rule format (with cpuID, kConfig, loadedKMod, nodename, pciID, usbID rules) is still supported. The rule format (new vs. old) is determined at config parsing time based on the existence of the 'matchOn' field. The new rule format and the configuration format for the new matchFeatures field is - name: <rule-name> labels: <key>: <value> ... matchFeatures: - feature: <domain>.<feature> matchExpressions: <attribute>: op: <operator> value: - <list-of-values> - feature: <domain>.<feature> ... Currently, "cpu", "kernel", "pci", "system", "usb" and "local" sources are covered by the matshers/feature selectors. Thus, the following features are available for matching with this patch: - cpu.cpuid: <cpuid-flag>: <exists/does-not-exist> - cpu.cstate: enabled: <bool> - cpu.pstate: status: <string> turbo: <bool> scaling_governor: <string> - cpu.rdt: <rdt-feature>: <exists/does-not-exist> - cpu.sst: bf.enabled: <bool> - cpu.topology: hardware_multithreading: <bool> - kernel.config: <flag-name>: <string> - kernel.loadedmodule: <module-name>: <exists/does-not-exist> - kernel.selinux: enabled: <bool> - kernel.version: major: <int> minor: <int> revision: <int> full: <string> - system.osrelease: <key-name>: <string> VERSION_ID.major: <int> VERSION_ID.minor: <int> - system.name: nodename: <string> - pci.device: <device-instance>: class: <string> vendor: <string> device: <string> subsystem_vendor: <string> susbystem_device: <string> sriov_totalvfs: <int> - usb.device: <device-instance>: class: <string> vendor: <string> device: <string> serial: <string> - local.label: <label-name>: <string> The configuration also supports some "shortforms" for convenience: matchExpressions: [<attr-1>, <attr-2>=<val-2>] --- matchExpressions: <attr-3>: <attr-4>: <val-4> is equal to: matchExpressions: <attr-1>: {op: Exists} <attr-2>: {op: In, value: [<val-2>]} --- matchExpressions: <attr-3>: {op: Exists} <attr-4>: {op: In, value: [<val-4>]} In other words: - feature: kernel.config matchExpressions: ["X86", "INIT_ENV_ARG_LIMIT=32"] - feature: pci.device matchExpressions: vendor: "8086" is the same as: - feature: kernel.config matchExpressions: X86: {op: Exists} INIT_ENV_ARG_LIMIT: {op: In, values: ["32"]} - feature: pci.device matchExpressions: vendor: {op: In, value: ["8086"] Some configuration examples below. In order to match a CPUID feature the following snippet can be used: - name: cpu-test-1 labels: cpu-custom-feature: "true" matchFeatures: - feature: cpu.cpuid matchExpressions: AESNI: {op: Exists} AVX: {op: Exists} In order to match against a loaded kernel module and OS version: - name: kernel-test-1 labels: kernel-custom-feature: "true" matchFeatures: - feature: kernel.loadedmodule matchExpressions: e1000: {op: Exists} - feature: system.osrelease matchExpressions: NAME: {op: InRegexp, values: ["^openSUSE"]} VERSION_ID.major: {op: Gt, values: ["14"]} In order to require a kernel module and both of two specific PCI devices: - name: multi-device-test labels: multi-device-feature: "true" matchFeatures: - feature: kernel.loadedmodule matchExpressions: driver-module: {op: Exists} - pci.device: vendor: "8086" device: "1234" - pci.device: vendor: "8086" device: "abcd"
Implement a new 'matchAny' directive in the new rule format, building on top of the previously implemented 'matchFeatures' matcher. MatchAny applies a logical OR over multiple matchFeatures directives. That is, it allows specifying multiple alternative matchers (at least one of which must match) in a single label rule. The configuration format for the new matchers is matchAny: - matchFeatures: - feature: <domain>.<feature> matchExpressions: <attribute>: op: <operator> value: - <list-of-values> - matchFeatures: ... A configuration example. In order to require a cpu feature, kernel module and one of two specific PCI devices (taking use of the shortform notation): - name: multi-device-test labels: multi-device-feature: "true" matchFeatures: - feature: kernel.loadedmodule matchExpressions: [driver-module] - feature: cpu.cpuid matchExpressions: [AVX512F] matchAny: - matchFeatures: - feature; pci.device matchExpressions: vendor: "8086" device: "1234" - matchFeatures: - feature: pci.device matchExpressions: vendor: "8086" device: "abcd"
Print out the result of applying an expression. Also, truncate the output to max 10 elements (of items matched against) unless '-v 4' verbosity level is in use.
269f253
to
52d9aa2
Compare
First manual test Name: minikube
Roles: control-plane,master
Labels: beta.kubernetes.io/arch=amd64
beta.kubernetes.io/os=linux
feature.node.kubernetes.io/cpu-cpuid.ADX=true
feature.node.kubernetes.io/cpu-cpuid.AESNI=true
feature.node.kubernetes.io/cpu-cpuid.AVX=true
feature.node.kubernetes.io/cpu-cpuid.AVX2=true
feature.node.kubernetes.io/cpu-cpuid.FMA3=true
feature.node.kubernetes.io/cpu-cpuid.HYPERVISOR=true
feature.node.kubernetes.io/cpu-cpuid.IBPB=true
feature.node.kubernetes.io/cpu-cpuid.MPX=true
feature.node.kubernetes.io/cpu-cpuid.STIBP=true
feature.node.kubernetes.io/cpu-cpuid.VMX=true
feature.node.kubernetes.io/cpu-hardware_multithreading=false
feature.node.kubernetes.io/kernel-config.NO_HZ=true
feature.node.kubernetes.io/kernel-config.NO_HZ_IDLE=true
feature.node.kubernetes.io/kernel-version.full=4.19.202
feature.node.kubernetes.io/kernel-version.major=4
feature.node.kubernetes.io/kernel-version.minor=19
feature.node.kubernetes.io/kernel-version.revision=202
feature.node.kubernetes.io/system-os_release.ID=buildroot
feature.node.kubernetes.io/system-os_release.VERSION_ID=2021.02.4
feature.node.kubernetes.io/system-os_release.VERSION_ID.major=2021
feature.node.kubernetes.io/system-os_release.VERSION_ID.minor=02
kubernetes.io/arch=amd64
kubernetes.io/hostname=minikube
kubernetes.io/os=linux
minikube.k8s.io/commit=76b94fb3c4e8ac5062daf70d60cf03ddcc0a741b
minikube.k8s.io/name=minikube
minikube.k8s.io/updated_at=2021_11_11T11_09_13_0700
minikube.k8s.io/version=v1.24.0
node-role.kubernetes.io/control-plane=
node-role.kubernetes.io/master=
node.kubernetes.io/exclude-from-external-load-balancers= |
ete-test using the test config file Ran 3 of 230 Specs in 36.625 seconds
SUCCESS! -- 3 Passed | 0 Failed | 0 Pending | 227 Skipped
--- PASS: TestE2E (36.65s)
PASS
ok sigs.k8s.io/node-feature-discovery/test/e2e 36.681s test go test ./cmd/... ./pkg/... ./source/...
? sigs.k8s.io/node-feature-discovery/cmd/nfd-master [no test files]
ok sigs.k8s.io/node-feature-discovery/cmd/nfd-topology-updater 0.011s
ok sigs.k8s.io/node-feature-discovery/cmd/nfd-worker 0.005s
? sigs.k8s.io/node-feature-discovery/pkg/api/feature [no test files]
? sigs.k8s.io/node-feature-discovery/pkg/apihelper [no test files]
? sigs.k8s.io/node-feature-discovery/pkg/cpuid [no test files]
ok sigs.k8s.io/node-feature-discovery/pkg/kubeconf 0.015s
? sigs.k8s.io/node-feature-discovery/pkg/labeler [no test files]
? sigs.k8s.io/node-feature-discovery/pkg/nfd-client [no test files]
ok sigs.k8s.io/node-feature-discovery/pkg/nfd-client/topology-updater 0.017s
ok sigs.k8s.io/node-feature-discovery/pkg/nfd-client/worker 3.091s
ok sigs.k8s.io/node-feature-discovery/pkg/nfd-master 0.019s
? sigs.k8s.io/node-feature-discovery/pkg/podres [no test files]
ok sigs.k8s.io/node-feature-discovery/pkg/resourcemonitor 0.033s
? sigs.k8s.io/node-feature-discovery/pkg/topologypolicy [no test files]
? sigs.k8s.io/node-feature-discovery/pkg/topologyupdater [no test files]
ok sigs.k8s.io/node-feature-discovery/pkg/utils 0.040s
? sigs.k8s.io/node-feature-discovery/pkg/version [no test files]
ok sigs.k8s.io/node-feature-discovery/source 0.004s
ok sigs.k8s.io/node-feature-discovery/source/cpu 0.005s
ok sigs.k8s.io/node-feature-discovery/source/custom 0.005s
ok sigs.k8s.io/node-feature-discovery/source/custom/expression 0.004s
? sigs.k8s.io/node-feature-discovery/source/custom/rules [no test files]
? sigs.k8s.io/node-feature-discovery/source/fake [no test files]
? sigs.k8s.io/node-feature-discovery/source/iommu [no test files]
ok sigs.k8s.io/node-feature-discovery/source/kernel 0.007s
ok sigs.k8s.io/node-feature-discovery/source/local 0.008s
? sigs.k8s.io/node-feature-discovery/source/memory [no test files]
? sigs.k8s.io/node-feature-discovery/source/network [no test files]
ok sigs.k8s.io/node-feature-discovery/source/pci 0.010s
? sigs.k8s.io/node-feature-discovery/source/storage [no test files]
ok sigs.k8s.io/node-feature-discovery/source/system 0.005s
ok sigs.k8s.io/node-feature-discovery/source/usb 0.005s |
tested with |
I think this PR is good to go, but to double check, lest all put eyes on @zvonkok |
Thanks for the review @ArangoGutierrez 👀 |
As this PR does not consider the documentation we need to be aware to explicitly document the new API with some examples of |
Tested several combinations of
|
/lgtm |
Overview
The main goal of this PR is to greatly expand the capabilities of the custom source, and, to make easier to extend in the future. After this change basically all features from the
cpu
,kernel
,system
pci
,usb
andlocal
source are available for custom rule processing. This is achieved in three stepscpu
,kernel
,system
pci
,usb
andlocal
in this PR)This PR introduces a new format for custom rules where the new capabilities are available under new
matchFeatures
directive. The old rule format (withmatchOn
) is still supported to provice backwards compatibility.Rule syntax
A new format for custom rule specifications is introduced, dropping the 'value' field and introducing new 'labels' field which makes it possible to specify multiple labels per rule. Also, in the new format the 'name' field is just for reference and no matching label is created.
Currently supported
op
s areIn
,NotIn
,InRegexp
,Exists
,DoesNotExist
,Gt
Lt
,GtLt
,IsTrue
,IsFalse
.The list of available
<domain>.<feature>
's is:<cpuid-flag>: <exists/does-not-exist>
enabled: <bool>
status: <string>
turbo: <bool>
scaling_governor: <string>
<rdt-feature>: <exists/does-not-exist>
bf.enabled: <bool>
hardware_multithreading: <bool>
<flag-name>: <string>
<module-name>: <exists/does-not-exist>
enabled: <bool>
major: <int>
minor: <int>
revision: <int>
full: <string>
<key-name>: <string>
VERSION_ID.major: <int>
VERSION_ID.minor: <int>
nodename: <string>
<device-instance>:
class: <string>
vendor: <string>
device: <string>
subsystem_vendor: <string>
susbystem_device: <string>
sriov_totalvfs: <int>
<device-instance>:
class: <string>
vendor: <string>
device: <string>
serial: <string>
<label-name>: <string>
Examples
The following example below demonstrates the new capabilities:
The example above also demonstrates some "shortcuts / shortforms" that are wired in the rule parsing
Example 1:
is the same as
Example 2:
is the same as
is the same as
Example 3:
is the same as
matchAny
In addition to
matchFeatures
, the PR also implements a new 'matchAny' directive. MatchAny applies a logical OR over multiple matchFeatures matchers. That is, it allows specifying multiple alternative matchers (at least one of which must match) in a single label rule.The configuration format for the new matcher is
A configuration example. In order to require a cpu feature, kernel
module and one of two specific PCI devices (taking use of the shortform
notation):
Notes
Documentation is out of the scope of this PR. Documentation will require major changes and will be addressed in a separate PR, also covering #550 and #553.