Create extended resources with NodeFeatureRule #1099

ArangoGutierrez · 2023-03-28T14:28:16Z

Add support for management of extended resources with the
NodeFeatureRule CRD API.

There are usage scenarios where users want to advertise features
as extended resources instead of labels (or annotations).

This patch enables the discovery of extended resources, via annotation
and patch of node.status.capacity and node.status.allocatable By using
the NodeFeatureRule API.

Co-authored-by: Carlos Eduardo Arango Gutierrez eduardoa@nvidia.com
Co-authored-by: Fabiano Fidêncio fabiano.fidencio@intel.com

netlify · 2023-03-28T14:28:47Z

✅ Deploy Preview for kubernetes-sigs-nfd ready!

Name	Link
🔨 Latest commit	`250aea4`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-nfd/deploys/6430257e9940c70008e4a4cd
😎 Deploy Preview	https://deploy-preview-1099--kubernetes-sigs-nfd.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

ArangoGutierrez · 2023-03-28T14:32:31Z

/assign marquiz

ArangoGutierrez · 2023-03-28T14:36:40Z

using

apiVersion: nfd.k8s-sigs.io/v1alpha1
kind: NodeFeatureRule
metadata:
  name: my-sample-rule-object
spec:
  rules:
    - name: "my sample rule"
      extendedResources: 
        - "kernel-version.major"
      matchFeatures:
        - feature: kernel.version
          matchExpressions:
            major: {op: Exists}

I was able to get

Capacity:
  cpu:                                              4
  ephemeral-storage:                                17784752Ki
  feature.node.kubernetes.io/kernel-version.major:  5
  hugepages-2Mi:                                    0
  memory:                                           8135008Ki
  pods:                                             110
Allocatable:
  cpu:                                              4
  ephemeral-storage:                                17784752Ki
  feature.node.kubernetes.io/kernel-version.major:  5
  hugepages-2Mi:                                    0
  memory:                                           8135008Ki
  pods:                                             110

Annotations:        kubeadm.alpha.kubernetes.io/cri-socket: unix:///var/run/crio/crio.sock
                    nfd.node.kubernetes.io/extended-resources: kernel-version.major

(used kernel-version just bc is an int, so it was easy to use as example)

ArangoGutierrez · 2023-03-29T05:33:13Z

/cc @fidencio

k8s-ci-robot · 2023-03-29T05:33:14Z

@ArangoGutierrez: GitHub didn't allow me to request PR reviews from the following users: fidencio.

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 @fidencio

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.

docs/usage/customization-guide.md

fidencio · 2023-03-29T11:19:37Z

Using this PR, combined with mine , with the following rule ...

apiVersion: nfd.k8s-sigs.io/v1alpha1
kind: NodeFeatureRule
metadata:
  name: tdx-total-keys
spec:
  rules:
    - name: "TDX total keys rule"
      extendedResources:
        - "cpu-security.tdx.total_keys"
      matchFeatures:
        - feature: cpu.security
          matchExpressions:
            tdx.enabled: {op: Exists}

... I was able to get ...

{
  "cpu": "128",
  "ephemeral-storage": "383957772Ki",
  "feature.node.kubernetes.io/cpu-security.tdx.total_keys": "31",
  "hugepages-1Gi": "0",
  "hugepages-2Mi": "0",
  "memory": "131192364Ki",
  "pods": "110"
}

cmd/nfd-master/main.go

zvonkok · 2023-03-30T09:51:53Z

@fidencio Thanks for the examples here and on the depending PR, this explains the bigger picture.

@ArangoGutierrez AMD told me that the only countable resources for SNP is the number of ASIDs. We can do the same for SNP what Fabiano has done here for TDX.

zvonkok · 2023-03-30T09:52:17Z

/lgtm

k8s-ci-robot · 2023-03-30T09:52:24Z

LGTM label has been added.

Git tree hash: 9a0dd119a383f5e5304659a1a8e951d5caba7721

mythi · 2023-03-30T10:01:19Z

@fidencio Thanks for the examples here and on the depending PR, this explains the bigger picture.

@zvonkok one hiccup we noticed yesterday was that these extended resources are not correctly shown in nodes' Allocatable. However, the accounting works fine. See kubernetes/kubernetes#115190

pkg/apis/nfd/v1alpha1/annotations_labels.go

pkg/nfd-master/nfd-master.go

docs/usage/customization-guide.md

marquiz

Big hand for this work @ArangoGutierrez. I'd be happy to get this in 😎 Really cool new feature and nice e2e coverage, too. +1

k8s-ci-robot · 2023-04-06T16:17:05Z

[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:

~~OWNERS~~ [marquiz]

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

ArangoGutierrez · 2023-04-06T16:20:08Z

@PiotrProkop @zvonkok @fidencio PR is ready for your lgtm now :)

marquiz · 2023-04-06T16:24:54Z

ping @fidencio @PiotrProkop @mythi @zvonkok

marquiz · 2023-04-06T16:36:11Z

Hmm, e2e failed
/hold

ArangoGutierrez · 2023-04-06T16:47:54Z

Hmm, e2e failed /hold

Ran 8 of 263 Specs in 279.768 seconds
SUCCESS! -- 8 Passed | 0 Failed | 0 Pending | 255 Skipped
--- PASS: TestE2E (279.77s)
PASS
ok      sigs.k8s.io/node-feature-discovery/test/e2e     279.813s

I got a successful run before pushing

marquiz · 2023-04-06T18:28:27Z

There's an ancient bug in nfd-master that this PR exposed. Fixed by #1119. I'd suggest to merge that one first either way is fine, we need both of these. Anyway, this PR (1099) still legit
/unhold

ArangoGutierrez · 2023-04-06T19:23:57Z

There's an ancient bug in nfd-master that this PR exposed. Fixed by #1119. I'd suggest to merge that one first either way is fine, we need both of these. Anyway, this PR (1099) still legit /unhold

And we are back in business here!

ArangoGutierrez · 2023-04-06T19:24:04Z

/test all

PiotrProkop · 2023-04-07T07:59:02Z

I am happy to LGTM this PR but something is wrong with Netlify :(

PiotrProkop · 2023-04-07T08:00:20Z

/retest

ArangoGutierrez · 2023-04-07T08:40:37Z

/retest

Prow comments don't retrigger GitHub actions.
And I don't think is the PR changes fault for Netify errors .... I think jejeje 🤞

Add support for management of Extended Resources via the NodeFeatureRule CRD API. There are usage scenarios where users want to advertise features as extended resources instead of labels (or annotations). This patch enables the discovery of extended resources, via annotation and patch of node.status.capacity and node.status.allocatable. By using the NodeFeatureRule API. Co-authored-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com> Co-authored-by: Markus Lehtonen <markus.lehtonen@intel.com> Co-authored-by: Fabiano Fidêncio <fabiano.fidencio@intel.com> Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com> Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>

ArangoGutierrez · 2023-04-07T14:34:22Z

@PiotrProkop Netlify is happy now

PiotrProkop · 2023-04-07T15:08:45Z

/lgtm

Good job! @ArangoGutierrez @fidencio

k8s-ci-robot · 2023-04-07T15:08:50Z

LGTM label has been added.

Git tree hash: 01d6880d520986fbccdd492edea49c36d4fbc501

mythi · 2023-04-11T06:18:22Z

@ArangoGutierrez @fidencio thanks!

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 28, 2023

k8s-ci-robot requested review from jjacobelli and zvonkok March 28, 2023 14:28

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 28, 2023

ArangoGutierrez force-pushed the extended_resources_v2 branch from f2ba2b7 to 2c1f56d Compare March 28, 2023 14:31

k8s-ci-robot assigned marquiz Mar 28, 2023

PiotrProkop reviewed Mar 29, 2023

View reviewed changes

docs/usage/customization-guide.md Outdated Show resolved Hide resolved

docs/usage/customization-guide.md Outdated Show resolved Hide resolved

ArangoGutierrez force-pushed the extended_resources_v2 branch from 4dcbd2d to 52856ef Compare March 29, 2023 08:12

ArangoGutierrez requested review from PiotrProkop and removed request for zvonkok and jjacobelli March 29, 2023 08:13

PiotrProkop mentioned this pull request Mar 29, 2023

Advertise RDT L3 num_closid #1100

Merged

ArangoGutierrez force-pushed the extended_resources_v2 branch from 52856ef to cafebc4 Compare March 29, 2023 11:17

fidencio mentioned this pull request Mar 29, 2023

cpu: Expose the total number of keys for TDX #1079

Merged

ArangoGutierrez force-pushed the extended_resources_v2 branch from cafebc4 to 8ffddca Compare March 29, 2023 11:21

AhmedGrati reviewed Mar 29, 2023

View reviewed changes

cmd/nfd-master/main.go Outdated Show resolved Hide resolved

ArangoGutierrez force-pushed the extended_resources_v2 branch from 8ffddca to 96af09a Compare March 30, 2023 09:13

k8s-ci-robot assigned zvonkok Mar 30, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 30, 2023

mythi reviewed Mar 30, 2023

View reviewed changes

pkg/apis/nfd/v1alpha1/annotations_labels.go Outdated Show resolved Hide resolved

pkg/nfd-master/nfd-master.go Outdated Show resolved Hide resolved

docs/usage/customization-guide.md Outdated Show resolved Hide resolved

ArangoGutierrez force-pushed the extended_resources_v2 branch 2 times, most recently from 0c67733 to 96ddbae Compare April 6, 2023 12:59

ArangoGutierrez requested a review from marquiz April 6, 2023 12:59

ArangoGutierrez force-pushed the extended_resources_v2 branch 2 times, most recently from b586bc9 to 04707d9 Compare April 6, 2023 15:52

marquiz approved these changes Apr 6, 2023

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 6, 2023

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 6, 2023

marquiz mentioned this pull request Apr 6, 2023

nfd-master: fix node update #1119

Merged

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 6, 2023

ArangoGutierrez force-pushed the extended_resources_v2 branch from 04707d9 to 250aea4 Compare April 7, 2023 14:15

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 7, 2023

k8s-ci-robot merged commit ad07829 into kubernetes-sigs:master Apr 7, 2023

marquiz mentioned this pull request Apr 12, 2023

Release v0.13.0 #1068

Closed

24 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create extended resources with NodeFeatureRule #1099

Create extended resources with NodeFeatureRule #1099

ArangoGutierrez commented Mar 28, 2023 •

edited

Loading

netlify bot commented Mar 28, 2023 •

edited

Loading

ArangoGutierrez commented Mar 28, 2023

ArangoGutierrez commented Mar 28, 2023

ArangoGutierrez commented Mar 29, 2023

k8s-ci-robot commented Mar 29, 2023

fidencio commented Mar 29, 2023 •

edited

Loading

zvonkok commented Mar 30, 2023 •

edited

Loading

zvonkok commented Mar 30, 2023

k8s-ci-robot commented Mar 30, 2023

mythi commented Mar 30, 2023

marquiz left a comment

k8s-ci-robot commented Apr 6, 2023

ArangoGutierrez commented Apr 6, 2023

marquiz commented Apr 6, 2023

marquiz commented Apr 6, 2023

ArangoGutierrez commented Apr 6, 2023

marquiz commented Apr 6, 2023

ArangoGutierrez commented Apr 6, 2023

ArangoGutierrez commented Apr 6, 2023

PiotrProkop commented Apr 7, 2023

PiotrProkop commented Apr 7, 2023

ArangoGutierrez commented Apr 7, 2023

ArangoGutierrez commented Apr 7, 2023

PiotrProkop commented Apr 7, 2023

k8s-ci-robot commented Apr 7, 2023

mythi commented Apr 11, 2023

Create extended resources with NodeFeatureRule #1099

Create extended resources with NodeFeatureRule #1099

Conversation

ArangoGutierrez commented Mar 28, 2023 • edited Loading

netlify bot commented Mar 28, 2023 • edited Loading

✅ Deploy Preview for kubernetes-sigs-nfd ready!

ArangoGutierrez commented Mar 28, 2023

ArangoGutierrez commented Mar 28, 2023

ArangoGutierrez commented Mar 29, 2023

k8s-ci-robot commented Mar 29, 2023

fidencio commented Mar 29, 2023 • edited Loading

zvonkok commented Mar 30, 2023 • edited Loading

zvonkok commented Mar 30, 2023

k8s-ci-robot commented Mar 30, 2023

mythi commented Mar 30, 2023

marquiz left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 6, 2023

ArangoGutierrez commented Apr 6, 2023

marquiz commented Apr 6, 2023

marquiz commented Apr 6, 2023

ArangoGutierrez commented Apr 6, 2023

marquiz commented Apr 6, 2023

ArangoGutierrez commented Apr 6, 2023

ArangoGutierrez commented Apr 6, 2023

PiotrProkop commented Apr 7, 2023

PiotrProkop commented Apr 7, 2023

ArangoGutierrez commented Apr 7, 2023

ArangoGutierrez commented Apr 7, 2023

PiotrProkop commented Apr 7, 2023

k8s-ci-robot commented Apr 7, 2023

mythi commented Apr 11, 2023

ArangoGutierrez commented Mar 28, 2023 •

edited

Loading

netlify bot commented Mar 28, 2023 •

edited

Loading

fidencio commented Mar 29, 2023 •

edited

Loading

zvonkok commented Mar 30, 2023 •

edited

Loading