test/e2e: migrate TestDeschedulingInterval to run descheduler as an image by Sanil2108 · Pull Request #1861 · kubernetes-sigs/descheduler

Sanil2108 · 2026-04-25T06:03:58Z

Summary

Per #1478, the e2e suite is being moved off in-process descheduler runs and onto deployments of the descheduler image so the tests exercise the real shipped artifact. `TestDeschedulingInterval` was one of the two remaining cases (along with `TestClientConnectionConfiguration`).

What this changes

`TestDeschedulingInterval` verifies the run-once-and-exit lifecycle when no `--descheduling-interval` is set. The previous version called `descheduler.RunDeschedulerStrategies` directly, which doesn't reflect the production code path. The new version:

Builds a one-shot Pod (`restartPolicy=Never`, no `--descheduling-interval` flag, no liveness probe — irrelevant for a one-shot run) via a new `newDeschedulerOneShotPod` helper.
Mounts an empty `DeschedulerPolicy` ConfigMap (we're testing the lifecycle, not any specific descheduling decision).
Asserts the Pod reaches `Succeeded` phase within 3 minutes — which is what `pkg/descheduler/descheduler.go:546` (`if rs.DeschedulingInterval.Seconds() == 0 { cancel() }`) guarantees.

The helper derives the Pod from the existing `deschedulerDeployment` template so the container image, security context, volume mounts, and SA stay in lock-step with the long-running tests.

Removed

The old in-process `TestDeschedulingInterval` in `test/e2e/e2e_test.go` and the now-unused `options` and `pkg/descheduler` imports it pulled in.

Out of scope

`TestClientConnectionConfiguration` is intentionally not migrated here — the maintainer noted it's "tricky" because the descheduler doesn't currently expose the custom client configuration. Happy to take that as a follow-up if you'd like to scope it.

Test plan

`go build -tags e2e ./test/e2e/...` clean
`go vet -tags e2e ./test/e2e/...` clean
Full e2e run on a real cluster (will rely on CI)

Refs #1478

…mage Per kubernetes-sigs#1478, the e2e suite is being moved off in-process descheduler runs and onto deployments of the descheduler image so the tests exercise the real shipped artifact. TestDeschedulingInterval verifies the run-once-and-exit lifecycle when no `--descheduling-interval` is set. The previous version called `descheduler.RunDeschedulerStrategies` directly, which doesn't reflect the production code path. The new version creates a one-shot Pod (restartPolicy=Never, no `--descheduling-interval` flag, no liveness probe — irrelevant for a one-shot run) and asserts the Pod reaches `Succeeded` phase within 3 minutes. A small `newDeschedulerOneShotPod` helper derives the Pod from the existing `deschedulerDeployment` template so the container image, security context, volume mounts, and other plumbing stay in lock-step with the long-running tests. Refs kubernetes-sigs#1478 Signed-off-by: Sanil2108 <sanilkhurana7@gmail.com>

k8s-ci-robot · 2026-04-25T06:04:04Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ricardomaraschini for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

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 · 2026-04-25T06:04:07Z

Welcome @Sanil2108!

It looks like this is your first PR to kubernetes-sigs/descheduler 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/descheduler has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2026-04-25T06:04:08Z

Hi @Sanil2108. 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.

Details

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.

ingvagabund · 2026-05-11T08:21:03Z

+//     meaningful, and a slow first probe could race the clean exit.
+func newDeschedulerOneShotPod(testName, policyConfigMapName string) *v1.Pod {
+	deploymentTemplate := deschedulerDeployment(testName)
+	podTemplate := deploymentTemplate.Spec.Template


Used only once, can be inlined

ingvagabund · 2026-05-11T08:23:42Z

+	deploymentTemplate := deschedulerDeployment(testName)
+	podTemplate := deploymentTemplate.Spec.Template
+
+	podSpec := podTemplate.Spec


This variable can be inlined as well. Even though this makes the path a bit longer keeping it makes the code easier to read.

ingvagabund · 2026-05-11T08:23:56Z

+	podSpec := podTemplate.Spec
+	podSpec.RestartPolicy = v1.RestartPolicyNever
+
+	container := podSpec.Containers[0]


The same here

ingvagabund · 2026-05-11T08:25:59Z

+
+	// Empty policy is fine — we are testing the run-once-and-exit lifecycle, not
+	// any specific descheduling decision.
+	deschedulerPolicyConfigMapObj, err := deschedulerPolicyConfigMap(&apiv1alpha2.DeschedulerPolicy{})


deschedulerPolicyConfigMapObj's .Name, resp. .Namespace are not changed. newDeschedulerOneShotPod can be simplified to keep the original CM name.

ingvagabund · 2026-05-11T08:35:50Z

+//     follows the run-once-and-exit code path.
+//   - No liveness probe — a one-shot run isn't long enough to make probing
+//     meaningful, and a slow first probe could race the clean exit.
+func newDeschedulerOneShotPod(testName, policyConfigMapName string) *v1.Pod {


Alternatively, you could define a new deschedulerPodSpec from deschedulerDeployment and have deschedulerDeployment invoke it internally. Then consume it here as well to avoid creating the whole deployment and dropping everything but the pod template.

ingvagabund · 2026-05-11T08:47:34Z

+
+	container := podSpec.Containers[0]
+	container.Args = []string{
+		"--policy-config-file", "/policy-dir/policy.yaml",


Instead of resetting the args you could have deschedulerPodSpec take the descheduling interval value (set to 0 in this case). To avoid exposing the actual arguments and keeping only a single place where the binary args are set.

There's already a test under e2e_evictioninbackground_test.go that resets the args. So you are not breaking any convention here. Yet, if there's a way to avoid exposing the arguments/implementation details it's still preferable.

ingvagabund · 2026-05-11T09:14:05Z

+		}
+	}
+
+	return &v1.Pod{


Have you considered running it as a Job? To avoid the cases where a pod gets evicted in-flight before it finishes? E.g. due to resource constraints. Creating a job can help with re-creating the pod in these cases. To avoid some of the potential flakes.

ingvagabund · 2026-05-11T09:17:30Z

/ok-to-test

Remove the policyConfigMapName parameter — deschedulerPolicyConfigMap always returns a CM named "descheduler-policy-configmap", which is exactly what deschedulerDeployment mounts, so the volume-name replacement loop was a no-op. Also inline the intermediate deploymentTemplate and podTemplate variables, each of which was used only once to reach the next field. Signed-off-by: Sanil Khurana <sanilkhurana7@gmail.com> Signed-off-by: SanilK2108 <sanil.khurana@atlan.com>

linux-foundation-easycla · 2026-05-15T06:36:20Z

✅ login: Sanil2108 / name: Sanil2108 (3aff022)
❌ - login: @SanilK2108 / name: SanilK2108. The commit (fa3bfaf) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please visit our EasyCLA portal and chat with our support bot.

Sanil2108 · 2026-05-15T06:36:25Z

Thanks for the thorough review, @ingvagabund! Pushed a follow-up that addresses the feedback:

Removed the policyConfigMapName parameter from newDeschedulerOneShotPod — deschedulerPolicyConfigMap always returns a CM named "descheduler-policy-configmap", which is exactly what deschedulerDeployment already mounts, so the volume-name replacement loop was a no-op.
Inlined the intermediate deploymentTemplate and podTemplate variables, each used only once to reach the next field in the chain.

On the two alternative suggestions:

deschedulerPodSpec helper: happy to go that route if you'd prefer — would mean touching e2e_test.go to extract the pod spec from deschedulerDeployment. Let me know and I'll follow up with that refactor.
Job instead of Pod: noted, and it's a good point about pod eviction. I kept it as a Pod to stay consistent with the existing pattern in e2e_evictioninbackground_test.go, but can convert to a Job if you'd prefer the extra resilience.

a7i · 2026-05-17T20:38:27Z

/retest-required

ingvagabund · 2026-05-18T11:19:27Z

Thank you for keeping the PR in a good shape :)

deschedulerPodSpec helper: happy to go that route if you'd prefer — would mean touching e2e_test.go to extract the pod spec from deschedulerDeployment. Let me know and I'll follow up with that refactor.

By all means :)

Job instead of Pod: noted, and it's a good point about pod eviction. I kept it as a Pod to stay consistent with the existing pattern in e2e_evictioninbackground_test.go, but can convert to a Job if you'd prefer the extra resilience.

Please proceed.

k8s-ci-robot · 2026-05-18T11:50:25Z

@Sanil2108: 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-descheduler-test-e2e-k8s-master-1-36	`fa3bfaf`	link	true	`/test pull-descheduler-test-e2e-k8s-master-1-36`

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.

Details

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. I understand the commands that are listed here.

k8s-ci-robot requested review from JaneLiuL and knelasevero April 25, 2026 06:04

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 25, 2026

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 25, 2026

ricardomaraschini mentioned this pull request May 6, 2026

fix(helm): remove default cpu resource limits to allow user-defined sizing #1864

Open

ingvagabund reviewed May 11, 2026

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 11, 2026

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 15, 2026

Conversation

Sanil2108 commented Apr 25, 2026

Summary

What this changes

Removed

Out of scope

Test plan

Uh oh!

k8s-ci-robot commented Apr 25, 2026

Uh oh!

k8s-ci-robot commented Apr 25, 2026

Uh oh!

k8s-ci-robot commented Apr 25, 2026

Uh oh!

ingvagabund May 11, 2026

Choose a reason for hiding this comment

Uh oh!

ingvagabund May 11, 2026

Choose a reason for hiding this comment

Uh oh!

ingvagabund May 11, 2026

Choose a reason for hiding this comment

Uh oh!

ingvagabund May 11, 2026

Choose a reason for hiding this comment

Uh oh!

ingvagabund May 11, 2026

Choose a reason for hiding this comment

Uh oh!

ingvagabund May 11, 2026

Choose a reason for hiding this comment

Uh oh!

ingvagabund May 11, 2026 • 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!

ingvagabund commented May 11, 2026

Uh oh!

linux-foundation-easycla Bot commented May 15, 2026

Uh oh!

Sanil2108 commented May 15, 2026

Uh oh!

a7i commented May 17, 2026

Uh oh!

ingvagabund commented May 18, 2026

Uh oh!

k8s-ci-robot commented May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ingvagabund May 11, 2026 •

edited

Loading