Skip to content

Conversation

@UtkarshBhatthere
Copy link
Contributor

@UtkarshBhatthere UtkarshBhatthere commented Oct 3, 2024

Description

Adds implementation for enabling/disabling rbd mirroring (remote replication) to a configured MicroCeph remote cluster.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • CleanCode (Code refactor, test updates, does not introduce functional changes)
  • Documentation update (Contains Doc change)

How Has This Been Tested?

  • CI Tests
  • Unit Tests

Contributor's Checklist

Please check that you have:

  • self-reviewed the code in this PR.
  • added code comments, particularly in hard-to-understand areas.
  • updated the user documentation with corresponding changes.
  • added tests to verify effectiveness of this change.

Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere UtkarshBhatthere force-pushed the feature/newRemoteRbd branch 3 times, most recently from 08aad28 to 4075db3 Compare October 9, 2024 03:32
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere UtkarshBhatthere force-pushed the feature/newRemoteRbd branch 2 times, most recently from e50987c to 7401400 Compare October 9, 2024 07:08
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere UtkarshBhatthere force-pushed the feature/newRemoteRbd branch 6 times, most recently from 5b475cc to ee3100f Compare October 9, 2024 11:18
@UtkarshBhatthere UtkarshBhatthere requested review from gabrielmougard, lmlg, masnax and sabaini and removed request for masnax and sabaini October 10, 2024 08:13
@UtkarshBhatthere UtkarshBhatthere self-assigned this Oct 10, 2024
Copy link
Collaborator

@sabaini sabaini left a comment

Choose a reason for hiding this comment

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

Hey @UtkarshBhatthere quite a bit of work here 🙇‍♂️
As a first pass I've added some doc suggestions and thoughts on API. -- sorry for harping on APIs but as these are cumbersome to change once released I feel spending extra time on them is worthwhile 😊

Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
@UtkarshBhatthere UtkarshBhatthere force-pushed the feature/newRemoteRbd branch 2 times, most recently from 544387e to 1d50bf7 Compare October 11, 2024 12:07
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Copy link
Collaborator

@sabaini sabaini left a comment

Choose a reason for hiding this comment

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

Hi @UtkarshBhatthere thanks for the massive work!

I've added some suggestions, mostly around docs and logging, but otherwise lgtm!

Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Copy link
Collaborator

@sabaini sabaini left a comment

Choose a reason for hiding this comment

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

Hey @UtkarshBhatthere thanks for the additions,

Wrt to log messages, one thing I find valuable for troubleshooting is adding log messages that make it clear where in the code a fault occured. As Go does not log tracebacks automatically, it can be non-obvious where in a program a fault occured. Explicit log messages help with this issue.

cheers!

@UtkarshBhatthere UtkarshBhatthere merged commit f1e0421 into canonical:main Oct 17, 2024
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.

2 participants