Skip to content

Conversation

@britaniar
Copy link
Collaborator

@britaniar britaniar commented Nov 5, 2025

Description of your changes

Fixes #

I have:

  • Updated API to make ResourceSnapshotIndex optional. in staged update run

  • Update the initialization stage of staged update run to check if ResourceSnapshotIndex is defined to find that specific resource snapshot. Otherwise, if not defined it will use the latest resource snapshot.

  • Added an integration test case where relevant.

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

  • Integration test

Special notes for your reviewer

…ex is unset

Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>
Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>
@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

❌ Patch coverage is 63.33333% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/controllers/updaterun/initialization.go 62.06% 8 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

@britaniar britaniar marked this pull request as ready for review November 6, 2025 00:37
@Arvindthiru
Copy link
Collaborator

Arvindthiru commented Nov 6, 2025

We also need to account for resourceSnapshotIndex access here https://github.com/britaniar/kubefleet/blob/bff950842f01cab0eb0041401ed4ef7134581848/pkg/controllers/updaterun/execution.go#L99.

I think it's better to populate the ResourceSnapshotIndex field in the spec by a defaulter instead from the latest resource snapshot. Ideally once set ResourceSnapshotIndex should not change in the middle of an updateRun.

Edit: after offline discussion it's better to store resourceSnapshotName as a field on the updateRun status instead we don't want to change spec since it's an user facing object

Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>
@britaniar britaniar force-pushed the updateStagedUpdateRunResourceIndex branch from e34c1db to 1c44de9 Compare November 7, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants