[KEP-2400] Report swap capacity as part of node.status.nodeSystemInfo #129954

iholder101 · 2025-02-03T09:48:40Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Currently, there isn't an easy way to know the swap capacity for each node. It is available as part of Summary API, however it is both hard to read and demands certain permissions to use.

This PR adds the swap capacity into the node status, specifically under node.Status.NodeInfo.Swap.

Example for a node with 40GB swap capacity (trimmed for readability):

> k get node k8s-dev-worker -o yaml
apiVersion: v1
kind: Node
status:
  nodeInfo:
    architecture: amd64
    bootID: 5c5a1d6b-6447-45ac-ba88-b6a0c12c43f0
    containerRuntimeVersion: containerd://1.7.24
    kernelVersion: 6.11.9-100.fc39.x86_64
    kubeProxyVersion: v1.32.0
    kubeletVersion: v1.32.0
    machineID: 3050fbb203394830ad38c698ab0e261d
    operatingSystem: linux
    osImage: Debian GNU/Linux 12 (bookworm)
    swap:
      capacity: 42949664768
    systemUUID: 3ca8f8fd-1d9f-4bbc-99cb-d07cf9242484

Special notes for your reviewer:

I've tried to address this need in multiple past attempts.
In the first one, #123963, I've tried to add swap as another ReasourceName, hence to display it as part of kubectl describe node's status. This attempt was eventually denied with the main concern being that adding swap as a ResourceName is confusing since it cannot be directly/explicitly consumed (since the swap limit is auto-calculated).

In the second one, #125278, I've tried to add a swap condition to nodes. This PR was also denied with the main concern being that a condition is not suited to report such data, and that a field is recommended in this case.

I have also created another PR #129458 to add swap stats to kubectl top subcommands. However, this PR is currently blocked since it seems that using Summary API is problematic in terms of permissions, demonstrating again that a field for swap capacity is needed.

Does this PR introduce a user-facing change?

Start reporting swap capacity as part of node.status.nodeSystemInfo.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-node/2400-node-swap/README.md

k8s-ci-robot · 2025-02-03T09:48:44Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

iholder101 · 2025-03-12T13:31:08Z

@iholder101: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-kubernetes-unit 353dc8b link true /test pull-kubernetes-unit
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Unit tests fail locally on the master branch and do not seem related:

> make test WHAT=./staging/src/k8s.io/client-go/tools/cache/
go version go1.24.0 linux/amd64
+++ [0312 15:14:20] Set GOMAXPROCS automatically to 40
+++ [0312 15:14:20] Normalizing Go targets
+++ [0312 15:14:20] Running tests without code coverage and with -race
FAIL
FAIL	k8s.io/client-go/tools/cache	54.013s

=== Failed
=== FAIL: k8s.io/client-go/tools/cache TestGenericListerListMethod (0.00s)
    listers_test.go:126: unexpected object has been returned expected = [0xc000a64050 0xc000a64058 0xc000a64060] actual = [0xc000a64048 0xc000a64038 0xc000a64040], diff =   []runtime.Object{
        + 	&unstructured.Unstructured{
        + 		Object: map[string]any{
        + 			"apiVersion": string("group/version"),
        + 			"kind":       string("TheKind"),
        + 			"metadata":   map[string]any{"name": string("name-bar"), "namespace": string("ns-bar")},
        + 		},
        + 	},
          	&unstructured.Unstructured{Object: {"apiVersion": string("group/version"), "kind": string("TheKind"), "metadata": map[string]any{"name": string("name-foo"), "namespace": string("ns-foo")}}},
          	&unstructured.Unstructured{Object: {"apiVersion": string("group/version"), "kind": string("TheKind"), "metadata": map[string]any{"name": string("name-foo1"), "namespace": string("ns-foo")}}},
        - 	&unstructured.Unstructured{
        - 		Object: map[string]any{
        - 			"apiVersion": string("group/version"),
        - 			"kind":       string("TheKind"),
        - 			"metadata":   map[string]any{"name": string("name-bar"), "namespace": string("ns-bar")},
        - 		},
        - 	},
          }

serathius · 2025-03-12T14:19:13Z

Bisected to #125102

thockin · 2025-03-12T17:46:26Z

Shouldn't this have some validation? Is it OK to set 0 here? Is it OK to set a negative number? It looks like node.status validation is sloppy, that doesn't mean we should make it worse.

Whatever the validation rules are should be commented on the field and enforced in pkg/apis/core/validation/validation.go

iholder101 · 2025-03-13T10:34:27Z

Shouldn't this have some validation?

Yeah, I've added some. Thanks!

Is it OK to set 0 here? Is it OK to set a negative number?

If the capacity is 0 kubelet will leave the field to be nil.
So both 0 and negative numbers should be invalid.

Whatever the validation rules are should be commented on the field and enforced in pkg/apis/core/validation/validation.go

Thanks for the pointer!
Done (see last commit), PTAL 🙏

iholder101 · 2025-03-13T12:20:48Z

Failures look like CI outage.
/retest

thockin · 2025-03-13T17:41:39Z

pkg/apis/core/validation/validation_test.go

+	makeNode := func(swapCapacity *int64) core.Node {
+		node := makeNode("test-node", nil)
+		if swapCapacity != nil {
+			node.Status.NodeInfo.Swap = &core.NodeSwapStatus{


What about when node.Status.NodeInfo.Swap == nil ?

Perhaps you can reframe it as

makeNode := func(swap *NodeSwapStatus) *Node { ... } makeSwapStatus := func(capacity int64) *NodeSwapStatus { ... }

and then the testcases are:

makeNode(nil) // nil swap status makeNode(&NodeSwapStatus{}) // nil capacity makeNode(makeSwapStatus(123456)) makeNode(makeSwapStatus(0)) makeNode(makeSwapStatus(-123456))

@thockin Done. PTAL.

pkg/apis/core/validation/validation_test.go

Signed-off-by: Itamar Holder <[email protected]>

thockin · 2025-03-17T19:32:34Z

Thanks!

/lgtm
/approve

k8s-ci-robot · 2025-03-17T19:32:40Z

LGTM label has been added.

Git tree hash: ffe61db0e42a5a8e878f6b077438282d83703ae0

k8s-ci-robot · 2025-03-17T19:32:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dchen1107, iholder101, thockin

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:

~~api/OWNERS~~ [thockin]
~~pkg/apis/OWNERS~~ [thockin]
~~pkg/generated/openapi/OWNERS~~ [dchen1107,thockin]
~~pkg/kubelet/OWNERS~~ [dchen1107,thockin]
~~staging/src/k8s.io/api/OWNERS~~ [thockin]
~~staging/src/k8s.io/client-go/applyconfigurations/OWNERS~~ [dchen1107,thockin]

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

k8s-triage-robot · 2025-03-17T21:47:45Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

pacoxu · 2025-03-18T01:08:15Z

/test pull-kubernetes-integration

k8s-ci-robot requested review from caesarxuchao and deads2k and removed request for caesarxuchao and deads2k February 3, 2025 09:49

iholder101 force-pushed the swap/capacity-on-node-sys-info branch 4 times, most recently from fa23791 to 732802a Compare February 4, 2025 12:11

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Feb 4, 2025

iholder101 marked this pull request as ready for review February 4, 2025 12:50

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 4, 2025

k8s-ci-robot requested review from aojea and caesarxuchao February 4, 2025 12:50

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 13, 2025

iholder101 force-pushed the swap/capacity-on-node-sys-info branch from 7534356 to 42efd31 Compare March 13, 2025 10:27

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 13, 2025

thockin reviewed Mar 13, 2025

View reviewed changes

iholder101 force-pushed the swap/capacity-on-node-sys-info branch from 42efd31 to 8ae0b0d Compare March 16, 2025 09:59

iholder101 added 8 commits March 16, 2025 11:59

Add the Swap field to NodeSystemInfo

a53a517

Signed-off-by: Itamar Holder <[email protected]>

add auto-generated files: ./hack/update-codegen.sh

4bdaf6c

Signed-off-by: Itamar Holder <[email protected]>

add auto-generated files: ./hack/update-openapi-spec.sh

ee4ac3d

Signed-off-by: Itamar Holder <[email protected]>

add auto-generated files: make update

a9b0211

Signed-off-by: Itamar Holder <[email protected]>

populate the new node.status.nodeInfo.swapInfo field

16871a1

Signed-off-by: Itamar Holder <[email protected]>

Add unit tests

18139b9

Signed-off-by: Itamar Holder <[email protected]>

Ensure unit tests are guarded by a feature gate

9d38a85

Signed-off-by: Itamar Holder <[email protected]>

Add validation for the NodeSwapStatus field

1ae091e

Signed-off-by: Itamar Holder <[email protected]>

iholder101 force-pushed the swap/capacity-on-node-sys-info branch from 8ae0b0d to 1ae091e Compare March 16, 2025 09:59

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

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 17, 2025

k8s-ci-robot merged commit 2499a2c into kubernetes:master Mar 18, 2025
16 checks passed

k8s-ci-robot added this to the v1.33 milestone Mar 18, 2025

github-project-automation bot moved this from Needs Reviewer to Done in SIG Node: code and documentation PRs Mar 18, 2025

github-project-automation bot moved this from Needs Triage to Done in SIG Apps Mar 18, 2025

[KEP-2400] Report swap capacity as part of node.status.nodeSystemInfo #129954

[KEP-2400] Report swap capacity as part of node.status.nodeSystemInfo #129954

Uh oh!

Conversation

iholder101 commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

k8s-ci-robot commented Feb 3, 2025

Uh oh!

iholder101 commented Mar 12, 2025

Uh oh!

serathius commented Mar 12, 2025

Uh oh!

thockin commented Mar 12, 2025

Uh oh!

iholder101 commented Mar 13, 2025

Uh oh!

iholder101 commented Mar 13, 2025

Uh oh!

thockin Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

thockin Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

iholder101 Mar 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thockin commented Mar 17, 2025

Uh oh!

k8s-ci-robot commented Mar 17, 2025

Uh oh!

k8s-ci-robot commented Mar 17, 2025

Uh oh!

k8s-triage-robot commented Mar 17, 2025

Uh oh!

pacoxu commented Mar 18, 2025

Uh oh!

Uh oh!

Uh oh!

iholder101 commented Feb 3, 2025 •

edited

Loading