GREP-291: `OnDelete` update strategy for `PodCliqueSet` by renormalize · Pull Request #403 · ai-dynamo/grove

renormalize · 2026-02-06T10:15:30Z

What type of PR is this?

What this PR does / why we need it:

/kind feature
/kind api

Introduces GREP-291.

#291 mentions certain realities of performing day 2 operations on a PodCliqueSet. The author mentions changing nodeAffinity in the pod template of the PodCliqueSet to ensure that pods are not scheduled onto failed nodes.

However, changing nodeAffinity will cause all pods of the PodClique to roll, which is undesirable since there might only be a small subset of pods that were on failed nodes. Therefore, to avoid the rolling of all pods in the PodClique, taking inspiration from Kubernetes StatefulSet, I would like to introduce an OnDelete update strategy for PodCliqueSet, where users can then update the nodeAffinity, and avoid rolling of all the pods in a PodClique.

Which issue(s) this PR fixes:

Fixes #291

Special notes for your reviewer:

N/A

Does this PR introduce a API change?

Introduce the new `spec.updateStrategy` field to specify `OnDelete` update strategy for `PodCliqueSet`

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

Introduce GREP-291 for the `OnDelete` update strategy for `PodCliqueSet`

copy-pr-bot · 2026-02-06T10:15:34Z

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.

renormalize · 2026-02-09T17:52:14Z

In a discussion with the maintainers, they had expressed the following opinions:

Change the scope of the GREP to define OnDelete for both PodCliques and ScalingGroups. Implementation can be split into different stages.
Rename RollingUpdate to RollingRecreate, and define what RollingRecreate is. The update strategy currently implemented in grove is not strictly RollingUpdate as referred to by the community, and therefore it would be incorrect to call it that.
Keep update strategy only at the PodCliqueSet level. In the future, update strategy can be defined at each hierarchy to specify different behavior for different PodCliques/ScalingGroups. This would align with the convention used for topology configuration in PCS.
ScalingGroups and (standalone) PodCliques both will follow the same strategy. This can be configured hierarchically at all levels later on if needed, as mentioned above.

@unmarshall @sanjaychatterjee @Ronkahn21 @gflarity had suggested the above.

Please refrain from reviews before I adapt the GREP with the above suggestions, thanks.

sanjaychatterjee

Mostly looks good to me. Not convinced about the need for the extra condition.

sanjaychatterjee

Looks good to me. Thanks!

renormalize · 2026-02-16T17:13:20Z

After a proof of concept implementation, I had second thoughts about certain fields. I've reworked the GREP based on these opinions.

I've removed the .spec changes to PodClique and PodCliqueScalingGroup. I feel we should not include the update strategy as a part of the spec for these resources now, and only include them when absolutely necessary. The update strategy can be fetched from the PodCliqueSet resource. I think this aligns with the original comments from the codeowners as well.
I've renamed the rollingUpdateProgress status fields to updateProgress, to more generically track information about updates no matter what the update strategy is.

Ronkahn21

Looks good overall. Thoughts on the API break—do we introduce it now or wait for v1? @sanjaychatterjee @unmarshall

renormalize · 2026-02-17T15:41:51Z

Looks good overall. Thoughts on the API break—do we introduce it now or wait for v1? @sanjaychatterjee @unmarshall

~~@Ronkahn21 thanks for your review! Can you please elaborate on what you mean by the API break in this case? My proposed changes are backwards compatible.~~

We are indeed changing the status API that we expose to consumers by renaming this field.

@julienmancuso Could you let us know if renaming this field will cause issues for dynamo/operator?

renormalize · 2026-02-18T08:51:36Z

@Ronkahn21 I've introduced UpdateProgress as a separate field after discussion with @sanjaychatterjee and @unmarshall. We deprecate RollingUpdateProgress, and will continue to function as it is currently for the next 2 releases; after which it will be removed.

Once I have an approval for this GREP, I will open the implementation PR for review. Thanks.

sanjaychatterjee · 2026-02-18T08:59:23Z

Looks good overall. Thoughts on the API break—do we introduce it now or wait for v1? @sanjaychatterjee @unmarshall

~~@Ronkahn21 thanks for your review! Can you please elaborate on what you mean by the API break in this case? My proposed changes are backwards compatible.~~

We are indeed changing the status API that we expose to consumers by renaming this field.

@julienmancuso Could you let us know if renaming this field will cause issues for dynamo/operator?

Julien already confirmed on slack that Dynamo is not using the field but still we need to follow deprecation process as part of changelog notifications for two consecutive releases. cc @danbar2

unmarshall

1/2 review comments

gflarity

Still making my way through it, minor comments so far.

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

* Remove (unnecessary) API changes proposed to `PodClique` and `PodCliqueScalingGroup` * Rename `*RollingUpdateProgress` status fields to `*UpdateProgress` status fields to more generically track update information. Signed-off-by: Saketh Kalaga <saketh.kalaga@sap.com>

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

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

renormalize self-assigned this Feb 6, 2026

renormalize requested review from Ronkahn21, gflarity, sanjaychatterjee, shayasoolin and unmarshall as code owners February 6, 2026 10:15

renormalize marked this pull request as draft February 9, 2026 17:52

renormalize force-pushed the 291-ondelete-update-standalone-podclique branch from 9bbebcf to 1aed557 Compare February 10, 2026 13:20

renormalize changed the title ~~GREP-291: OnDelete update strategy for standalone PodCliques.~~ GREP-291: OnDelete update strategy for PodCliqueSet Feb 10, 2026

renormalize marked this pull request as ready for review February 10, 2026 13:22

sanjaychatterjee reviewed Feb 11, 2026

View reviewed changes

Comment thread docs/proposals/291-ondelete-update/README.md

Comment thread docs/proposals/291-ondelete-update/README.md Outdated

Comment thread docs/proposals/291-ondelete-update/README.md Outdated

sanjaychatterjee reviewed Feb 12, 2026

View reviewed changes

Comment thread docs/proposals/291-ondelete-update/README.md Outdated

renormalize requested a review from sanjaychatterjee February 12, 2026 10:10

sanjaychatterjee previously approved these changes Feb 14, 2026

View reviewed changes

renormalize dismissed sanjaychatterjee’s stale review via fd45f16 February 16, 2026 17:10

renormalize requested a review from sanjaychatterjee February 16, 2026 17:13

Ronkahn21 reviewed Feb 17, 2026

View reviewed changes

renormalize mentioned this pull request Feb 17, 2026

[GREP-291] OnDelete implementation for standalone PodCliques and PodCliqueScalingGroups #438

Merged

renormalize requested a review from Ronkahn21 February 18, 2026 08:48

sanjaychatterjee previously approved these changes Feb 18, 2026

View reviewed changes

Ronkahn21 previously approved these changes Feb 18, 2026

View reviewed changes

unmarshall reviewed Feb 21, 2026

View reviewed changes

gflarity reviewed Feb 21, 2026

View reviewed changes

renormalize dismissed stale reviews from Ronkahn21 and sanjaychatterjee via 8f90a19 February 23, 2026 22:30

renormalize requested review from Ronkahn21, gflarity, sanjaychatterjee and unmarshall February 23, 2026 22:31

unmarshall reviewed Feb 24, 2026

View reviewed changes

renormalize requested a review from unmarshall February 24, 2026 11:20

unmarshall approved these changes Feb 24, 2026

View reviewed changes

sanjaychatterjee approved these changes Feb 24, 2026

View reviewed changes

unmarshall removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 25, 2026

renormalize added 10 commits February 25, 2026 12:59

GREP-291: OnDelete update strategy for standalone PodCliques.

885e29c

Adapt structure to adhere to the proposal convention.

de38738

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

Address feedback from maintainers

c7f2847

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

Rename directory

8002f17

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

make update-toc

22982ed

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

Remove redundant condition

764be2f

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

Introduce UpdateProgress as a seperate field, and deprecate `Rollin…

8a97c25

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

Rephrase the document according to review comments

f1c8eb9

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

Address more comments.

c14c44c

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

renormalize force-pushed the 291-ondelete-update-standalone-podclique branch from afd1541 to c14c44c Compare February 25, 2026 07:29

renormalize added the run-e2e label Feb 25, 2026

renormalize merged commit 0b7a4c4 into ai-dynamo:main Feb 25, 2026
11 checks passed

Conversation

renormalize commented Feb 6, 2026 • 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:

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 Feb 6, 2026

Uh oh!

renormalize commented Feb 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sanjaychatterjee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sanjaychatterjee left a comment

Choose a reason for hiding this comment

Uh oh!

renormalize commented Feb 16, 2026

Uh oh!

Ronkahn21 left a comment

Choose a reason for hiding this comment

Uh oh!

renormalize commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

renormalize commented Feb 18, 2026

Uh oh!

sanjaychatterjee commented Feb 18, 2026

Uh oh!

unmarshall left a comment • 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!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gflarity 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

renormalize commented Feb 6, 2026 •

edited

Loading

renormalize commented Feb 17, 2026 •

edited

Loading

unmarshall left a comment •

edited

Loading