[GREP-291] Introduce end-to-end tests for the `OnDelete` update strategy by renormalize · Pull Request #469 · ai-dynamo/grove

renormalize · 2026-03-04T11:22:49Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Introduces end-to-end tests for the OnDelete update strategy introduced in #438.

Which issue(s) this PR fixes:

Fixes #291

Special notes for your reviewer:

This branch contains implementation code of the ondelete update strategy introduced in #438. Once that is merged, this PR will be rebased to only contain the E2E test code.

Does this PR introduce a API change?

NONE

Additional documentation e.g., enhancement proposals, usage docs, etc.:

NONE

copy-pr-bot · 2026-03-04T11:22:55Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Ronkahn21

Overall, this looks good. However, I’d prefer to keep the test files focused strictly on test logic rather than utility functions. Also, some functions are becoming quite large; could you refactor them so each function handles a single responsibility and calls helper functions where needed

Copilot

Pull request overview

This PR introduces end-to-end tests for the OnDelete update strategy for PodCliqueSet, complementing the implementation in PR #438. It also renames triggerPodCliqueRollingUpdate to triggerPodCliqueUpdate and the associated env var from ROLLING_UPDATE_TRIGGER to UPDATE_TRIGGER to be strategy-agnostic.

Changes:

Added 7 E2E test cases (OD-1 through OD-7) covering OnDelete strategy scenarios including no-auto-deletion, manual deletion with updated spec, scale-in preference for outdated pods, PCSG interactions, and multi-replica PCS
Renamed triggerPodCliqueRollingUpdate → triggerPodCliqueUpdate and env var ROLLING_UPDATE_TRIGGER → UPDATE_TRIGGER across existing rolling update tests/utils
Added CI matrix entry for ondelete_updates tests

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
operator/e2e/tests/ondelete_updates_test.go	New file with 7 OnDelete E2E tests and shared helper functions
operator/e2e/yaml/workload-ondelete.yaml	New workload manifest with `OnDelete` update strategy
operator/e2e/tests/rolling_update_utils.go	Renamed function and env var to be strategy-agnostic
operator/e2e/tests/rolling_updates_test.go	Updated call sites for the renamed function
.github/workflows/build-check-test.yaml	Added `ondelete_updates` test pattern to CI matrix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

unmarshall

1/n reviews

renormalize · 2026-03-16T17:42:25Z

@Ronkahn21 I've addressed your offline comments about the missing test cases.

Will refactor the utils in a subsequent PR since I would like to unblock the upcoming release which OnDelete needs to be a part of.

Thanks.

renormalize · 2026-03-18T14:33:28Z

@unmarshall I have refactored the codebase according to your suggestions offline. Please take a look and approve. Thanks.

renormalize · 2026-03-21T08:40:11Z

There seems to be a bug in the setup.go file (in lines which weren't modified). Will fix and push the change.

Signed-off-by: Saketh Kalaga <saketh.kalaga@sap.com>

…e_utils.go Signed-off-by: Saketh Kalaga <saketh.kalaga@sap.com>

Signed-off-by: Saketh Kalaga <saketh.kalaga@sap.com>

Removed the unneeded waitForOnDeleteUpdateCompleteDefault as its not required. Signed-off-by: Madhav Bhargava <madhav.bhargava@sap.com>

Signed-off-by: Madhav Bhargava <madhav.bhargava@sap.com>

…ersOutdatedPods` Signed-off-by: Saketh Kalaga <saketh.kalaga@sap.com>

Signed-off-by: Saketh Kalaga <saketh.kalaga@sap.com>

Signed-off-by: Madhav Bhargava <madhav.bhargava@sap.com>

renormalize self-assigned this Mar 4, 2026

renormalize force-pushed the ondelete-e2e branch 2 times, most recently from 3293602 to a8bf80b Compare March 9, 2026 20:30

renormalize force-pushed the ondelete-e2e branch from a8bf80b to 2e52d66 Compare March 12, 2026 05:02

renormalize marked this pull request as ready for review March 12, 2026 05:04

renormalize requested review from Ronkahn21, gflarity, sanjaychatterjee, shayasoolin and unmarshall as code owners March 12, 2026 05:04

renormalize force-pushed the ondelete-e2e branch from 2e52d66 to 5b27dac Compare March 12, 2026 08:06

Ronkahn21 reviewed Mar 12, 2026

View reviewed changes

Comment thread operator/e2e/tests/ondelete_updates_test.go Outdated

Comment thread operator/e2e/tests/ondelete_updates_test.go Outdated

Ronkahn21 requested a review from Copilot March 12, 2026 17:00

Copilot started reviewing on behalf of Ronkahn21 March 12, 2026 17:01 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

Comment thread operator/e2e/tests/ondelete_updates_test.go Outdated

renormalize requested a review from Ronkahn21 March 13, 2026 06:24

renormalize force-pushed the ondelete-e2e branch from bf700b9 to 1bf54e0 Compare March 13, 2026 06:25

unmarshall reviewed Mar 15, 2026

View reviewed changes

Comment thread operator/e2e/tests/ondelete_updates_test.go Outdated

Comment thread operator/e2e/tests/ondelete_updates_test.go Outdated

renormalize requested a review from unmarshall March 16, 2026 17:42

renormalize force-pushed the ondelete-e2e branch 3 times, most recently from 4b9711c to c09ad69 Compare March 18, 2026 14:28

unmarshall reviewed Mar 19, 2026

View reviewed changes

Comment thread operator/e2e/tests/update/ondelete_test.go

unmarshall reviewed Mar 19, 2026

View reviewed changes

Comment thread operator/e2e/tests/update/ondelete_test.go

Comment thread operator/e2e/tests/update/ondelete_test.go Outdated

unmarshall previously approved these changes Mar 20, 2026

View reviewed changes

Ronkahn21 previously approved these changes Mar 23, 2026

View reviewed changes

renormalize dismissed stale reviews from Ronkahn21 and unmarshall via 8333cff March 28, 2026 07:48

renormalize requested review from Ronkahn21 and unmarshall March 28, 2026 08:26

Ronkahn21 previously approved these changes Mar 28, 2026

View reviewed changes

unmarshall previously approved these changes Mar 31, 2026

View reviewed changes

unmarshall force-pushed the ondelete-e2e branch from 8333cff to b7fce93 Compare March 31, 2026 13:22

renormalize and others added 9 commits April 1, 2026 09:33

Introduce e2e tests for OnDelete

31696bc

Signed-off-by: Saketh Kalaga <saketh.kalaga@sap.com>

Add OnDelete tests to the e2e test workflow matrix

9c30d6a

Signed-off-by: Saketh Kalaga <saketh.kalaga@sap.com>

Move utilities to a utils file

b34dbd4

Signed-off-by: Saketh Kalaga <saketh.kalaga@sap.com>

Rename RollingUpdateTracker to UpdateTracker, and introduce updat…

e7445c5

…e_utils.go Signed-off-by: Saketh Kalaga <saketh.kalaga@sap.com>

Restructure the update end to end tests

faaa8df

Signed-off-by: Saketh Kalaga <saketh.kalaga@sap.com>

Renamed main_test.go => suite_test.go across e2e test packages.

045aff0

Removed the unneeded waitForOnDeleteUpdateCompleteDefault as its not required. Signed-off-by: Madhav Bhargava <madhav.bhargava@sap.com>

removed short assign where not required

4d9c744

Signed-off-by: Madhav Bhargava <madhav.bhargava@sap.com>

Remove redundant checks, reduce test runtime in `Test_OD3_ScaleInPref…

2201a14

…ersOutdatedPods` Signed-off-by: Saketh Kalaga <saketh.kalaga@sap.com>

Fix counter not being incremented

db3db11

Signed-off-by: Saketh Kalaga <saketh.kalaga@sap.com>

unmarshall dismissed stale reviews from Ronkahn21 and themself via db3db11 April 1, 2026 04:05

unmarshall force-pushed the ondelete-e2e branch from b7fce93 to db3db11 Compare April 1, 2026 04:05

unmarshall previously approved these changes Apr 1, 2026

View reviewed changes

fixed the logger variable as it is now exposed

81b5eb8

Signed-off-by: Madhav Bhargava <madhav.bhargava@sap.com>

unmarshall dismissed their stale review via 81b5eb8 April 1, 2026 04:34

unmarshall approved these changes Apr 1, 2026

View reviewed changes

sanjaychatterjee approved these changes Apr 1, 2026

View reviewed changes

unmarshall merged commit 8895a29 into ai-dynamo:main Apr 1, 2026
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GREP-291] Introduce end-to-end tests for the `OnDelete` update strategy#469

[GREP-291] Introduce end-to-end tests for the `OnDelete` update strategy#469
unmarshall merged 10 commits into
ai-dynamo:mainfrom
renormalize:ondelete-e2e

renormalize commented Mar 4, 2026

Uh oh!

copy-pr-bot Bot commented Mar 4, 2026

Uh oh!

Ronkahn21 left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

unmarshall left a comment

Uh oh!

Uh oh!

Uh oh!

renormalize commented Mar 16, 2026

Uh oh!

renormalize commented Mar 18, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

renormalize commented Mar 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

renormalize commented Mar 4, 2026

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 API change?

Additional documentation e.g., enhancement proposals, usage docs, etc.:

Uh oh!

copy-pr-bot Bot commented Mar 4, 2026

Uh oh!

Ronkahn21 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

unmarshall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

renormalize commented Mar 16, 2026

Uh oh!

renormalize commented Mar 18, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

renormalize commented Mar 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants