apis/nfd: fix multiple matcher terms targeting the same feature #1468
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
the
cncf-cla: yes
✅ Deploy Preview for kubernetes-sigs-nfd ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
k8s-ci-robot
added
approved
|
/assign @ArangoGutierrez |
Fix NodeFeatureRule templating in cases where multiple matchFeatures
terms are targeting the same feature. Previously, only matched feature
elements from the last matcher terms were used as the input to the
template. However, the input should contain all matched elements from
all matcher terms.
For example, consider the example rule snippet below:
...
labelsTemplate: |
{{ range .pci.device }}vendor.io/pci-device.{{ .class }}-{{ .device }}=exists
{{ end }}
matchFeatures:
- feature: pci.device
matchExpressions:
class: {op: InRegexp, value: ["^03"]}
vendor: {op: In, value: ["1234"]}
- feature: pci.device
matchExpressions:
class: {op: InRegexp, value: ["^12"]}
This rule matches if both a pci device of class 03 from vendor 1234
exists and a pci device of class 12 (from any vendor) exists.
Previously, the template would only generate labels from the devices in
class 12 (as that's the last term). With this patch the template creates
device labels from devices in both classes 03 and 12.
marquiz
force-pushed
the
devel/nfr-template-fix
branch
from
d78cadb
to
63c2255
Compare
|
/retest |
1 similar comment
|
/retest |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: bd38d2acd37507e3bc0fb41cd7925b60621f1f36
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ArangoGutierrez, 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 |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1468 +/- ##
==========================================
+ Coverage 30.55% 32.10% +1.54%
==========================================
Files 58 58
Lines 7527 7662 +135
==========================================
+ Hits 2300 2460 +160
+ Misses 4978 4937 -41
- Partials 249 265 +16
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix NodeFeatureRule templating in cases where multiple matchFeatures terms are targeting the same feature. Previously, only matched feature elements from the last matcher terms were used as the input to the template. However, the input should contain all matched elements from all matcher terms.
For example, consider the example rule snippet below:
This rule matches if both a pci device of class
03from vendor1234exists and a pci device of class12(from any vendor) exists. Previously, the template would only generate labels from the devices in class12(as that's the last term). With this patch the template creates device labels from devices in both classes03and12.