integration: refactor and split scheduler tests #109834

aojea · 2022-05-05T16:28:52Z

/kind cleanup
/kind failing-test
Fixes #109783

NONE

The scheduler jobs on integration are close to the timeout of 10m (default in golang) , causing that sometimes the test panic and fails because of the timeout.

Splitting the tests in subdirectories, will run the subtests independently, with a 10m timeout per subtest

k8s-ci-robot · 2022-05-05T16:28:59Z

@aojea: 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/test-infra repository.

aojea · 2022-05-05T16:33:26Z

/assign @wojtek-t @alculquicondor

This will stop the bleeding, sig-scheduling can work independently on improving the tests after this if they want, but we should be less impacted by the global timeout, waiting for the results to confirm though ... 👀

test/integration/util/util.go

test/integration/scheduler/plugin/plugin_test.go

wojtek-t · 2022-05-05T18:18:34Z

test/integration/scheduler/plugin/plugin_test.go

-			pod, err := createPausePod(testCtx.ClientSet,
-				initPausePod(&pausePodConfig{Name: "test-pod", Namespace: testCtx.NS.Name}))
+			pod, err := testutils.CreatePausePod(testCtx.ClientSet,
+				testutils.InitPausePod(&testutils.PausePodConfig{Name: "test-pod", Namespace: testCtx.NS.Name}))


Also maybe those function's aren't super controversial, but I would actually move them not to testutils, but rather to test/integration/scheduler/utils

+1
I just said the same before I saw your comment 😅

it seems that the integrations/util is mainly by scheduler folks already

https://github.com/kubernetes/kubernetes/pull/109834/files#r866610966

alculquicondor

There are a lot of failures... I couldn't find any obvious mistakes.

But one of the failures is a timeout, even after the split? That sounds crazy.

test/integration/scheduler/predicates/predicates_test.go

test/integration/scheduler/plugin/main_test.go

test/integration/scheduler/priorities/priorities_test.go

alculquicondor · 2022-05-05T18:22:23Z

test/integration/util/util.go

+
+// InitTest initializes a test environment and creates API server and scheduler with default
+// configuration.
+func InitTest(t *testing.T, nsPrefix string, opts ...scheduler.Option) *TestContext {


Perhaps InitTestScheduler?

heh, already exists 😄
more reason to merge this to avoid drifting

test/integration/scheduler/extender/extender_test.go

test/integration/util/util.go

alculquicondor · 2022-05-05T18:30:44Z

test/integration/util/util.go

+
+// InitDisruptionController initializes and runs a Disruption Controller to properly
+// update PodDisuptionBudget objects.
+func InitDisruptionController(t *testing.T, testCtx *TestContext) *disruption.DisruptionController {


These are a lot of function that are now under the ownership of sig-testing. Are you comfortable with that?

Maybe we can have a test/integration/scheduler/util package?

scheduler are the main users of the library already

grep -r "k8s.io/kubernetes/test/integration/util" test/integration/ test/integration/ipamperf/cloud.go: "k8s.io/kubernetes/test/integration/util" test/integration/ipamperf/ipam_test.go: "k8s.io/kubernetes/test/integration/util" test/integration/scheduler/preemption_test.go: testutils "k8s.io/kubernetes/test/integration/util" test/integration/scheduler/priorities_test.go: testutils "k8s.io/kubernetes/test/integration/util" test/integration/scheduler/queue_test.go: testutils "k8s.io/kubernetes/test/integration/util" test/integration/scheduler/scheduler_test.go: testutils "k8s.io/kubernetes/test/integration/util" test/integration/scheduler/taint_test.go: testutils "k8s.io/kubernetes/test/integration/util" test/integration/scheduler/extender_test.go: testutils "k8s.io/kubernetes/test/integration/util" test/integration/scheduler/framework_test.go: testutils "k8s.io/kubernetes/test/integration/util" test/integration/scheduler/predicates_test.go: testutils "k8s.io/kubernetes/test/integration/util" test/integration/scheduler/util/util.go: testutils "k8s.io/kubernetes/test/integration/util" test/integration/scheduler_perf/util.go: "k8s.io/kubernetes/test/integration/util" test/integration/node/lifecycle_test.go: testutils "k8s.io/kubernetes/test/integration/util"

and there are already scheduler only things like

kubernetes/test/integration/util/util.go

Lines 384 to 396 in a330d3b

// InitTestScheduler initializes a test environment and creates a scheduler with default

// configuration.

func InitTestScheduler(

t *testing.T,

testCtx *TestContext,

) *TestContext {

// Pod preemption is enabled by default scheduler configuration.

return InitTestSchedulerWithOptions(t, testCtx)

}

// InitTestSchedulerWithOptions initializes a test environment and creates a scheduler with default

// configuration and other options.

func InitTestSchedulerWithOptions(

I think that is simpler to have only one util package, I don't think that we suffers of bottlenecks on reviews and approvals in the integration framework, so if you don't mind I prefer to have only one package

aojea · 2022-05-05T21:47:54Z

the failures may be related to something wrong I did, I will redo it tomorrow following your comments

wojtek-t · 2022-05-06T10:19:09Z

There is a bunch of cleanup I would like to see in those utility functions, but that's not the request for this PR anyway...

/lgtm
/approve

Thanks!

aojea · 2022-05-06T10:37:06Z

this reduces the times significatively

grep testsuite junit_20220506-095056.xml  | grep scheduler
        <testsuite tests="9" failures="0" time="68.750000" name="k8s.io/kubernetes/test/integration/scheduler" timestamp="">
        <testsuite tests="1" failures="0" time="5.900000" name="k8s.io/kubernetes/test/integration/scheduler/extender" timestamp="">
        <testsuite tests="37" failures="0" time="242.950000" name="k8s.io/kubernetes/test/integration/scheduler/filters" timestamp="">
        <testsuite tests="70" failures="0" time="304.949000" name="k8s.io/kubernetes/test/integration/scheduler/plugins" timestamp="">
        <testsuite tests="34" failures="0" time="225.040000" name="k8s.io/kubernetes/test/integration/scheduler/preemption" timestamp="">
        <testsuite tests="13" failures="0" time="73.020000" name="k8s.io/kubernetes/test/integration/scheduler/scoring" timestamp="">
        <testsuite tests="9" failures="0" time="36.479000" name="k8s.io/kubernetes/test/integration/scheduler/taint" timestamp="">
        <testsuite tests="0" failures="0" time="0.000000" name="k8s.io/kubernetes/test/integration/scheduler_perf" timestamp="">

I have lint errors because of unused variables :/

…_test.go

aojea · 2022-05-06T11:19:58Z

k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters: TestTimeoutWithLogging expand_less 0s
{Failed === RUN TestTimeoutWithLogging
timeout_test.go:358: got res.StatusCode 200; expected 503
--- FAIL: TestTimeoutWithLogging (0.01s)

/test pull-kubernetes-unit

aojea · 2022-05-06T13:05:18Z

Kubernetes e2e suite: [sig-api-machinery] ResourceQuota should create a ResourceQuota and capture the life of a service. [Conformance] expand_more

k8s.io/kubernetes/test/integration/daemonset: TestDSCUpdatesPodLabelAfterDedupCurHistories expand_more

timeout error unrelated to this Pr

/retest

wojtek-t · 2022-05-06T13:30:25Z

/lgtm
/approve

k8s-ci-robot · 2022-05-06T13:31:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, wojtek-t

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/OWNERS~~ [aojea,wojtek-t]

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

alculquicondor · 2022-05-06T14:42:49Z

Thanks! I'll take some days to clean up the mess.

alculquicondor · 2022-08-03T17:02:13Z

We wouldn't cherry-pick this, would we?

…4-upstream-release-1.24 Automated cherry pick of #109834: integration: refactor and split scheduler tests

k8s-ci-robot requested review from chendave and johnSchnake May 5, 2022 16:30

k8s-ci-robot assigned alculquicondor and wojtek-t May 5, 2022

aojea commented May 5, 2022

View reviewed changes

test/integration/util/util.go Show resolved Hide resolved

wojtek-t reviewed May 5, 2022

View reviewed changes

alculquicondor reviewed May 5, 2022

View reviewed changes

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

integration: scheduler: migrate PDB from v1beta1 to v1

ad765c1

aojea force-pushed the split_scheduling_integration branch from 86ff0a2 to 277bcdb Compare May 6, 2022 09:25

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

aojea added 4 commits May 6, 2022 11:27

merge test integration scheduler util

3bb965b

integration: migrate extender tests

69acad2

integration: migrate plugings tests

6a64e24

integration: migrate preemption tests

63d64b7

aojea added 2 commits May 6, 2022 11:27

integration: migrate scoring tests

0dbb9f4

integration: migrate taint tests

9dbe4c8

aojea force-pushed the split_scheduling_integration branch from 277bcdb to 9dbe4c8 Compare May 6, 2022 09:27

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 6, 2022

aojea added 3 commits May 6, 2022 12:47

remove unused function on b/test/integration/util/util.go

ef410d1

remove unused variables in test/integration/scheduler/util.go

46fc39f

remove unused variables in test/integration/scheduler/filters/filters…

6a0e55d

…_test.go

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 6, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 6, 2022

k8s-ci-robot merged commit 83b25e3 into kubernetes:master May 6, 2022

k8s-ci-robot added this to the v1.25 milestone May 6, 2022

sanposhiho mentioned this pull request May 8, 2022

Retry when it fails to update pods status on scheduling loop #109832

Merged

Huang-Wei mentioned this pull request May 8, 2022

Flaky scheduler integration tests #109783

Closed

liggitt mentioned this pull request Dec 21, 2022

update release-1.24 to go1.19 #113956

Merged

aojea mentioned this pull request Dec 21, 2022

Automated cherry pick of #109834: integration: refactor and split scheduler tests #114643

Merged

k8s-ci-robot added a commit that referenced this pull request Dec 21, 2022

Merge pull request #114643 from aojea/automated-cherry-pick-of-#10983…

54cea65

…4-upstream-release-1.24 Automated cherry pick of #109834: integration: refactor and split scheduler tests

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

integration: refactor and split scheduler tests #109834

integration: refactor and split scheduler tests #109834

aojea commented May 5, 2022 •

edited

k8s-ci-robot commented May 5, 2022

aojea commented May 5, 2022

wojtek-t May 5, 2022

alculquicondor May 5, 2022

aojea May 6, 2022 •

edited

alculquicondor left a comment

alculquicondor May 5, 2022

aojea May 6, 2022 •

edited

alculquicondor May 5, 2022

aojea May 6, 2022

aojea commented May 5, 2022

wojtek-t commented May 6, 2022

aojea commented May 6, 2022

aojea commented May 6, 2022

aojea commented May 6, 2022

wojtek-t commented May 6, 2022

k8s-ci-robot commented May 6, 2022

alculquicondor commented May 6, 2022

alculquicondor commented Aug 3, 2022

	// InitTestScheduler initializes a test environment and creates a scheduler with default
	// configuration.
	func InitTestScheduler(
	t *testing.T,
	testCtx *TestContext,
	) *TestContext {
	// Pod preemption is enabled by default scheduler configuration.
	return InitTestSchedulerWithOptions(t, testCtx)
	}

	// InitTestSchedulerWithOptions initializes a test environment and creates a scheduler with default
	// configuration and other options.
	func InitTestSchedulerWithOptions(

integration: refactor and split scheduler tests #109834

integration: refactor and split scheduler tests #109834

Conversation

aojea commented May 5, 2022 • edited

k8s-ci-robot commented May 5, 2022

aojea commented May 5, 2022

wojtek-t May 5, 2022

Choose a reason for hiding this comment

alculquicondor May 5, 2022

Choose a reason for hiding this comment

aojea May 6, 2022 • edited

Choose a reason for hiding this comment

alculquicondor left a comment

Choose a reason for hiding this comment

alculquicondor May 5, 2022

Choose a reason for hiding this comment

aojea May 6, 2022 • edited

Choose a reason for hiding this comment

alculquicondor May 5, 2022

Choose a reason for hiding this comment

aojea May 6, 2022

Choose a reason for hiding this comment

aojea commented May 5, 2022

wojtek-t commented May 6, 2022

aojea commented May 6, 2022

aojea commented May 6, 2022

aojea commented May 6, 2022

wojtek-t commented May 6, 2022

k8s-ci-robot commented May 6, 2022

alculquicondor commented May 6, 2022

alculquicondor commented Aug 3, 2022

aojea commented May 5, 2022 •

edited

aojea May 6, 2022 •

edited

aojea May 6, 2022 •

edited