cpu: Expose the total number of keys for TDX

[fidencio]

The total amount of keys that can be used on a specific TDX system is exposed via the cgroups misc.capacity. See:

$ cat /sys/fs/cgroup/misc.capacity
tdx 31

The first step to properly manage the amount of keys present in a node is exposing it via the NFD, and that's exactly what this commit does.

An example of how it ends up being exposed via the NFD:

$ kubectl get node 984fee00befb.jf.intel.com -o jsonpath='{.metadata.labels}'  | jq | grep tdx.total_keys
  "feature.node.kubernetes.io/cpu-security.tdx.total_keys": "31",

[k8s-ci-robot]

Hi @fidencio. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[netlify]

✅ Deploy Preview for kubernetes-sigs-nfd ready!

Name	Link
🔨 Latest commit	10672e1
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-nfd/deploys/642687fc9f4058000886158b
😎 Deploy Preview	https://deploy-preview-1079--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.

[marquiz]

Thanks @fidencio for the PR.

How are you planning to utilize this label in practice? So is the label actually useful for anything by itself or are you planning to turn that into an extended resource or smth? What I'm trying to understand if this is smth that we want to have that as a built-in feature label, enabled by default for all users.

Also, we need to document this new feature in docs/usage/features.md
and docs/usage/customization-guide.md

[fidencio]

How are you planning to utilize this label in practice? So is the label actually useful for anything by itself or are you planning to turn that into an extended resource or smth?

That's a very good question, and sorry for my lack of explanation on this.
I wat to turn that into an exteded resource, so I'm not totally sure if this needs to be exposed as it's right now or not.
What would you suggest, @marquiz?

[marquiz]

That's a very good question, and sorry for my lack of explanation on this.

No problem, that's what the reviews are for

I wat to turn that into an exteded resource, so I'm not totally sure if this needs to be exposed as it's right now or not.
What would you suggest, @marquiz?

I think my kinda (so far hidden) idea for filling these kind of usage scenarios involving extended resources would be leverage NodeFeatureRule (and possibly NodeFeature) CRD. I.e. to have support for specifying ExtendedResources in the NodeFeatureRule object, basically following the lines of Labels, Taints (#540) and Annotations (#863).

But we don't have that capability, yet. Just realized that there's not even an issue tracking that, yet -> will create one.

How would NodeFeatureRule serve your use case? One plan could be to merge this PR without the labeling part and try to squeeze in support for ExtendedResources (in NodeFeatureRule CRD) for the next v0.13.0 release.

[marquiz]

/ok-to-test

[ArangoGutierrez]

just one nit

[zvonkok]

@ArangoGutierrez We need that for SNP as well, the total number of ASIDs

[marquiz]

@ArangoGutierrez We need that for SNP as well, the total number of ASIDs

You mean extended resources (#1081)?

[zvonkok]

@marquiz Yes and No. (1) We need the extended resource via Rule (2) cpu: Expose the total number of ASIDs for SNP

You can only run a specific amount of VMs on a host, so we need to expose that as well.

[fidencio]

@marquiz, I've updated the PR and I'll test it together with @ArangoGutierrez's work and report back very soon.

[ArangoGutierrez]

rm trailing white space

[fidencio]

Using @ArangoGutierrez's PR, 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 ...

kubectl get node 984fee00befb.jf.intel.com -o jsonpath='{.status.capacity}' | jq
{
  "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"
}

[fidencio]

There's still one bit missing, though, that's getting the amount of used keys. I'm working on that right now.

[fidencio]

There's still one bit missing, though, that's getting the amount of used keys. I'm working on that right now.

No, nevermind.

I've done some tests here, let me share the results:

I'm using the follow Kata Containers runtime class definition:

---
kind: RuntimeClass
apiVersion: node.k8s.io/v1
metadata:
    name: kata
handler: kata
overhead:
    podFixed:
        memory: "160Mi"
        cpu: "250m"
        feature.node.kubernetes.io/cpu-security.tdx.total_keys: "1"

Doing a kubectl node describe I can see:

Allocated resources:
  (Total limits may be over 100 percent, i.e., overcommitted.)
  Resource                                                Requests    Limits
  --------                                                --------    ------
  cpu                                                     950m (0%)   0 (0%)
  memory                                                  290Mi (0%)  340Mi (0%)
  ephemeral-storage                                       0 (0%)      0 (0%)
  hugepages-1Gi                                           0 (0%)      0 (0%)
  hugepages-2Mi                                           0 (0%)      0 (0%)
  feature.node.kubernetes.io/cpu-security.tdx.total_keys  0           0

Then when I start a pod, which has the follow definition:

apiVersion: v1
kind: Pod
metadata:
  name: nginx-qemu-tdx
spec:
  runtimeClassName: kata
  containers:
  - name: nginx-qemu-tdx
    image: nginx:1.14.2
    ports:
    - containerPort: 80

I can then see, as a result of kubectl node describe:

Allocated resources:
  (Total limits may be over 100 percent, i.e., overcommitted.)
  Resource                                                Requests    Limits
  --------                                                --------    ------
  cpu                                                     1200m (0%)  0 (0%)
  memory                                                  450Mi (0%)  340Mi (0%)
  ephemeral-storage                                       0 (0%)      0 (0%)
  hugepages-1Gi                                           0 (0%)      0 (0%)
  hugepages-2Mi                                           0 (0%)      0 (0%)
  feature.node.kubernetes.io/cpu-security.tdx.total_keys  1           0

If I try to start a second pod, now requesting 30 keys (+1 coming from the podOverhead definition), using the definition shown below:

apiVersion: v1
kind: Pod
metadata:
  name: nginx-qemu-tdx-2
spec:
  runtimeClassName: kata
  containers:
  - name: nginx-qemu-tdx-2
    image: nginx:1.14.2
    resources:
      limits:
        feature.node.kubernetes.io/cpu-security.tdx.total_keys: 30
    ports:
    - containerPort: 80

Then I can see it doesn't start:

Events:
  Type     Reason            Age   From               Message
  ----     ------            ----  ----               -------
  Warning  FailedScheduling  50s   default-scheduler  0/1 nodes are available: 1 Insufficient feature.node.kubernetes.io/cpu-security.tdx.total_keys.

It means the book keeping is correctly done.

[fidencio]

One thing that we may figure out later why it happens is that although the bookkeeping is correctly done the values of the "feature.node.kubernetes.io/cpu-security.tdx.total_keys" (coming from kubectl describe node), as part of the Capacity and Allocatable, are never updated:

Capacity:
  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
Allocatable:
  cpu:                                                     127
  ephemeral-storage:                                       353855482090
  feature.node.kubernetes.io/cpu-security.tdx.total_keys:  31
  hugepages-1Gi:                                           0
  hugepages-2Mi:                                           0
  memory:                                                  130565676Ki
  pods:                                                    110

[zvonkok]

/lgtm

Last famous words @marquiz

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 9a1ce5e4555613fa6cbdcf08fa38d76fd3f53a37

[zvonkok]

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 175450f2c7e94f409a6f2eccdd2f0101a0e50b1a

[marquiz]

Thanks @fidencio for the enhancement, looks good to me 👍

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fidencio, 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