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

source/cpu: make cpuid configurable #224

Merged
merged 2 commits into from
May 14, 2019

Conversation

marquiz
Copy link
Contributor

@marquiz marquiz commented Mar 5, 2019

Add 'cpuid/attributeBlacklist' and 'cpuid/attributeWhitelist' config
options for the cpu feature source. These can be used to filter the set
of cpuid capabilities that get published. The intention is to reduce
clutter in the NFD label space, getting rid of "obvious" or misleading
cpuid labels. Whitelisting has higher priority, i.e. only whitelist
takes effect if both attributeWhitelist and attributeBlacklist are
specified.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 5, 2019
@k8s-ci-robot
Copy link
Contributor

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 5, 2019
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 5, 2019
}

func (f keyFilter) unmask(k string) bool {
if f.whitelist {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be simplified to one if. you need to return true when whitelist and ok is true or when whitelist and ok are false. Thus,

if _, ok := f.keys[k]; f.whitelist == ok {
...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True (I didn't even think about that), but, I think think that this optimization clearly worsens the readability of the code. In the future, it may be hard to understand the logic behind f.whitelist == ok. So, I'd leave it as it is (and let the compiler do the optimization)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kad ping

@marquiz
Copy link
Contributor Author

marquiz commented Apr 18, 2019

ping @kad @zvonkok

README.md Show resolved Hide resolved
@marquiz
Copy link
Contributor Author

marquiz commented May 6, 2019

Rebased.

ping @kad @zvonkok

marquiz added 2 commits May 13, 2019 17:17
Add 'cpuid/attributeBlacklist' and 'cpuid/attributeWhitelist' config
options for the cpu feature source. These can be used to filter the set
of cpuid capabilities that get published. The intention is to reduce
clutter in the NFD label space, getting rid of "obvious" or misleading
cpuid labels. Whitelisting has higher priority, i.e.  only whitelist
takes effect if both attributeWhitelist and attributeBlacklist are
specified.
@marquiz marquiz changed the title source/cpuid: make cpuid configurable source/cpu: make cpuid configurable May 13, 2019
@marquiz
Copy link
Contributor Author

marquiz commented May 13, 2019

Rebased and adapted to #217

@kad
Copy link
Contributor

kad commented May 14, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 14, 2019
@k8s-ci-robot k8s-ci-robot merged commit 7126b02 into kubernetes-sigs:master May 14, 2019
@marquiz marquiz mentioned this pull request May 14, 2019
10 tasks
@marquiz marquiz deleted the devel/cpuid branch May 15, 2019 15:27
ArangoGutierrez pushed a commit to ArangoGutierrez/node-feature-discovery that referenced this pull request Nov 18, 2020
Version 0.4.0

Node-feature-discovery was migrated into a new repository under the
kubernetes-sigs organization in Github (kubernetes-sigs#175). Related to the migration,
the final container image registry/repo hasn't been dediced yet (kubernetes-sigs#177) –
for this release we still use the old repo.

Major changes
- Split NFD into client and server (kubernetes-sigs#209)
- Changes in labels:
  - NFD label namespace was changed to 'feature.node.kubernetes.io/' (kubernetes-sigs#176)
    - 'nfd-' prefix was dropped from all feature labels
    - NFD version label
      (feature.node.kubernetes.io/node-feature-discovery.version) was
      replaced by an annotation (nfd.node.kubernetes.io/version)
  - network SRIOV labels were changed (kubernetes-sigs#173):
    - 'network-sriov' -> 'network-sriov.capable'
    - 'network-sriov-configured' -> 'network-sriov.configured'
  - selinux detection was moved to kernel feature source
    - 'selinux' -> 'kernel-selinux.enabled'
  - cpuid, pstate and RDT labels moved under cpu feature source (kubernetes-sigs#217)
    - 'cpuid-<cpuid flag>' -> 'cpu-cpuid.<cpuid flag>'
    - 'pstate-turbo' -> 'cpu-pstate.turbo'
    - 'rdt-<rdt feature>' -> 'cpu-rdt.<rdt feature>'
- Support for config file (kubernetes-sigs#169). Currently with three configurable
  feature sources i.e. cpu (kubernetes-sigs#224), kernel (kubernetes-sigs#157) and pci (kubernetes-sigs#168)
- Support for non-binary labels, with arbitrary values other than plain
  'true'
- PCI device detection (kubernetes-sigs#168)
- Kernel version detection (kubernetes-sigs#157)
- Kernel config option detection (kubernetes-sigs#146)
- Support for custom feature-detector hooks (kubernetes-sigs#144)
- Support OS version detection (kubernetes-sigs#149, kubernetes-sigs#211)
- Detection of hardware multithreading, such as Intel Hyper-Threading
  Technology (kubernetes-sigs#147)
- Arm64 support for CPUID detection (kubernetes-sigs#194)
- Validation of feature label names and values (kubernetes-sigs#199, kubernetes-sigs#219)
- Detection of NVDIMM devices (kubernetes-sigs#214)
- Get labels by reading from file in 'local' source (kubernetes-sigs#228)
- Detection of Intel SST-BF (Speed Select Technology - Base Frequency) (kubernetes-sigs#235)
- Make it possible to create feature labels in non-default namespace
  (kubernetes-sigs#231). Currently possible for using the local source (hooks and files).

Miscellaneous
- Template specs converted from json to yaml
- Documentation updates and fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants