test: Add test case for createNodesOp #131607

utam0k · 2025-05-05T12:56:35Z

What type of PR is this?

/kind cleanup
/sig scheduling

What this PR does / why we need it:

To enhance scheduler_perf testing

Which issue(s) this PR fixes:

A part of #127745

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

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

k8s-ci-robot · 2025-05-05T12:56:42Z

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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-sigs/prow repository.

utam0k · 2025-05-05T12:58:11Z

test/integration/scheduler_perf/scheduler_perf_test.go

+			name:      "Zero Nodes",
+			nodeCount: 0,


Should this be an error in the validation? If so, I'll create the issue and fix it after merging this PR.

Yeah, 0 on count doesn't make sense, which could be checked at a validation phase.

I've created the issue and will try it after merging this pr.
#131828

utam0k · 2025-05-06T02:54:13Z

/cc @macsko @sanposhiho

test/integration/scheduler_perf/scheduler_perf_test.go

utam0k · 2025-05-09T22:38:10Z

/retest-required

test/integration/scheduler_perf/scheduler_perf_test.go

sanposhiho

nice, lookin' a lot better

test/integration/scheduler_perf/scheduler_perf_test.go

sanposhiho

only nits

test/integration/scheduler_perf/scheduler_perf_test.go

macsko · 2025-06-05T13:04:09Z

test/utils/runners.go

+	if newNode.Status.Allocatable == nil {
+		newNode.Status.Allocatable = make(v1.ResourceList)
+	}


Why do we need this change?

You're right to question this. In production, kubelet always initializes Allocatable, but test nodes might not have it set.
Would you prefer if I add a comment explaining this, or should we enforce that test templates always include Allocatable?

Without this nil check, the test TestRunOp/Create_Nodes_with_Node_Allocatable_Strategy fails with:

panic: assignment to entry in nil map at test/utils/runners.go:893 in NodeAllocatableStrategy.PreparePatch

Could you try just initializing Allocatable in makeBaseNode?

Sure. Updated.

@macsko PTAL 🙏

macsko · 2025-06-11T07:52:04Z

/test pull-kubernetes-scheduler-perf

test/utils/runners.go

utam0k · 2025-06-11T07:58:37Z

/test pull-kubernetes-scheduler-perf

utam0k · 2025-06-11T10:40:02Z

@macsko This PR has passedpull-kubernetes-scheduler-perf test 🎉

test/integration/scheduler_perf/scheduler_perf_test.go

Signed-off-by: utam0k <[email protected]>

utam0k · 2025-06-11T12:29:46Z

/test pull-kubernetes-scheduler-perf

macsko · 2025-06-11T12:31:10Z

/approve

Leaving final stamp to @sanposhiho

utam0k · 2025-06-11T12:37:13Z

@macsko Thanks for your review 🙏

sanposhiho

/lgtm

Lovely, thanks!

k8s-ci-robot · 2025-06-11T18:12:12Z

LGTM label has been added.

Git tree hash: 3447806cc240e8d7d25034654dee8ef785634a38

k8s-ci-robot · 2025-06-11T18:12:18Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: macsko, sanposhiho, utam0k

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:

~~test/integration/scheduler_perf/OWNERS~~ [macsko,sanposhiho]

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 added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels May 5, 2025

k8s-ci-robot requested review from kerthcet and macsko May 5, 2025 12:56

k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels May 5, 2025

utam0k commented May 5, 2025

View reviewed changes

utam0k force-pushed the test-nodes-with-node-allocatable-strategy branch from 93c541b to c487678 Compare May 5, 2025 22:39

k8s-ci-robot requested a review from sanposhiho May 6, 2025 02:54

utam0k force-pushed the test-nodes-with-node-allocatable-strategy branch from c487678 to 35eceae Compare May 6, 2025 03:48

sanposhiho reviewed May 6, 2025

View reviewed changes

test/integration/scheduler_perf/scheduler_perf_test.go Outdated Show resolved Hide resolved

test/integration/scheduler_perf/scheduler_perf_test.go Outdated Show resolved Hide resolved

test/integration/scheduler_perf/scheduler_perf_test.go Outdated Show resolved Hide resolved

utam0k requested a review from sanposhiho May 6, 2025 12:50

utam0k force-pushed the test-nodes-with-node-allocatable-strategy branch from b63e59f to 4f965bc Compare May 8, 2025 12:59

utam0k changed the title ~~test: Add test case for runCreateNodesOp~~ test: Add test case for createNodesOp May 8, 2025

utam0k force-pushed the test-nodes-with-node-allocatable-strategy branch from 4f965bc to b30c176 Compare May 9, 2025 12:36

sanposhiho reviewed May 10, 2025

View reviewed changes

utam0k force-pushed the test-nodes-with-node-allocatable-strategy branch 2 times, most recently from 17fca56 to 00eb343 Compare May 11, 2025 12:36

utam0k requested a review from sanposhiho May 11, 2025 12:36

sanposhiho reviewed May 11, 2025

View reviewed changes

test/integration/scheduler_perf/scheduler_perf_test.go Outdated Show resolved Hide resolved

sanposhiho mentioned this pull request May 12, 2025

add tests for scheduler-perf itself #127745

Open

utam0k mentioned this pull request May 17, 2025

scheduler-perf: Validation of createNodeOp when the number of nodes is 0 #131828

Open

sanposhiho reviewed May 19, 2025

View reviewed changes

test/integration/scheduler_perf/scheduler_perf_test.go Show resolved Hide resolved

sanposhiho reviewed May 19, 2025

View reviewed changes

test/integration/scheduler_perf/scheduler_perf_test.go Outdated Show resolved Hide resolved

test/integration/scheduler_perf/scheduler_perf_test.go Outdated Show resolved Hide resolved

test/integration/scheduler_perf/scheduler_perf_test.go Outdated Show resolved Hide resolved

macsko reviewed May 20, 2025

View reviewed changes

utam0k mentioned this pull request May 23, 2025

scheduler/perf: Add validation for Count=0 in createNodesOp #131933

Closed

macsko reviewed May 29, 2025

View reviewed changes

test/integration/scheduler_perf/scheduler_perf_test.go Outdated Show resolved Hide resolved

test/integration/scheduler_perf/scheduler_perf_test.go Outdated Show resolved Hide resolved

utam0k requested review from sanposhiho and macsko June 2, 2025 07:53

macsko reviewed Jun 5, 2025

View reviewed changes

utam0k requested a review from macsko June 9, 2025 12:33

macsko reviewed Jun 11, 2025

View reviewed changes

test/utils/runners.go Outdated Show resolved Hide resolved

utam0k force-pushed the test-nodes-with-node-allocatable-strategy branch from 7c89a7d to 174d424 Compare June 11, 2025 07:57

utam0k requested a review from macsko June 11, 2025 07:58

macsko reviewed Jun 11, 2025

View reviewed changes

test/integration/scheduler_perf/scheduler_perf_test.go Outdated Show resolved Hide resolved

test: Add test case for createNodeOp

57d55fb

Signed-off-by: utam0k <[email protected]>

utam0k force-pushed the test-nodes-with-node-allocatable-strategy branch from 174d424 to 57d55fb Compare June 11, 2025 12:29

utam0k requested a review from macsko June 11, 2025 12:29

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

sanposhiho approved these changes Jun 11, 2025

View reviewed changes

k8s-ci-robot assigned sanposhiho Jun 11, 2025

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

k8s-ci-robot merged commit 05592c3 into kubernetes:master Jun 11, 2025
15 checks passed

k8s-ci-robot added this to the v1.34 milestone Jun 11, 2025

test: Add test case for createNodesOp #131607

test: Add test case for createNodesOp #131607

Conversation

utam0k commented May 5, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

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 May 5, 2025

Uh oh!

utam0k May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanposhiho May 6, 2025

Choose a reason for hiding this comment

Uh oh!

utam0k May 17, 2025

Choose a reason for hiding this comment

Uh oh!

utam0k commented May 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

utam0k commented May 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sanposhiho left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sanposhiho left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

macsko Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

utam0k Jun 7, 2025

Choose a reason for hiding this comment

Uh oh!

macsko Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

utam0k Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

utam0k Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

macsko commented Jun 11, 2025

Uh oh!

Uh oh!

utam0k commented Jun 11, 2025

Uh oh!

utam0k commented Jun 11, 2025

Uh oh!

Uh oh!

utam0k commented Jun 11, 2025

Uh oh!

utam0k May 5, 2025 •

edited

Loading