Skip to content

Add new status conditions to replication status #826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 10, 2025

Conversation

Nikhil-Ladha
Copy link
Contributor

Added new status conditions which reflect the current replication status as returned from the storage vendor.

@mergify mergify bot added api Change to the API, requires extra care vendor Pull requests that update vendored dependencies labels Jun 27, 2025
@Madhu-1
Copy link
Member

Madhu-1 commented Jun 27, 2025

@ShyamsundarR PTAL. @Nikhil-Ladha please check with Ramen team to ensure there is no regression in Ramen because of new condition

@Nikhil-Ladha Nikhil-Ladha force-pushed the add-state-condition branch from 164e12f to 3de7136 Compare June 28, 2025 06:41
@Nikhil-Ladha
Copy link
Contributor Author

@ShyamsundarR PTAL. @Nikhil-Ladha please check with Ramen team to ensure there is no regression in Ramen because of new condition

From what I have understood, ramen checks for individual conditions from VR status, so a new condition won't hamper anything unless used explicitly. But, I will still let @ShyamsundarR confirm on the same.

@Nikhil-Ladha Nikhil-Ladha force-pushed the add-state-condition branch from 3de7136 to b02620d Compare June 30, 2025 09:25
Copy link
Collaborator

@nixpanic nixpanic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a few nits, otherwise good to go

update csi-addons/spec pkg to include latest GetVolumeReplicationInfo
RPC updates

Signed-off-by: Nikhil-Ladha <nikhilladha1999@gmail.com>
update GetVolumeReplicationInfo RPC proto definition as per the
csi-addons/spec changes to include Status and StatusMessage fields
in the response

Signed-off-by: Nikhil-Ladha <nikhilladha1999@gmail.com>
@Nikhil-Ladha Nikhil-Ladha force-pushed the add-state-condition branch from b02620d to b162e14 Compare July 1, 2025 12:25
@Nikhil-Ladha Nikhil-Ladha requested a review from nixpanic July 1, 2025 12:26
@Nikhil-Ladha Nikhil-Ladha force-pushed the add-state-condition branch from b162e14 to 8330c00 Compare July 3, 2025 04:29
@Nikhil-Ladha
Copy link
Contributor Author

Updated the sidecar code to return the new status fields in the response.

Test results:

    - lastTransitionTime: "2025-07-03T05:00:17Z"
      message: 'volume is replicating: local image is primary'
      observedGeneration: 2
      reason: Replicating
      status: "True"
      type: Replicating

@Nikhil-Ladha
Copy link
Contributor Author

If everything looks can we get this PR merged, so that ramen PR can also be reviewed and tested?

add replication status condition to VR Status
which helps in understanding the current replication
state of the image/group

Signed-off-by: Nikhil-Ladha <nikhilladha1999@gmail.com>
@Nikhil-Ladha Nikhil-Ladha force-pushed the add-state-condition branch from 8330c00 to 2769399 Compare July 8, 2025 06:33
@Nikhil-Ladha Nikhil-Ladha requested a review from nixpanic July 9, 2025 13:09
mergify bot added a commit that referenced this pull request Jul 10, 2025
@mergify mergify bot merged commit 5af0733 into csi-addons:main Jul 10, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Change to the API, requires extra care vendor Pull requests that update vendored dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants