-
Notifications
You must be signed in to change notification settings - Fork 57
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
Refactor vrg-kubeobject functions #1712
Refactor vrg-kubeobject functions #1712
Conversation
71e670d
to
ffce5e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
8c8c4d0
to
395bd2a
Compare
@BenamarMk This PR is ready for review. I have rebased it on top of #1709 so the e2e failures should be gone. |
if v.kubeObjectProtectionDisabled("recovery") { | ||
return nil | ||
} | ||
func (v *VRGInstance) getVRGFromS3Profile(s3ProfileName string) (*ramen.VolumeReplicationGroup, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have this function: GetLastKnownVRGPrimaryFromS3
. Can we have a common function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BenamarMk that is a function in DRPC and this is a function in VRG as a method on VRGInstance, can I take up the refactoring in a later PR?
- created methods for getting capture and recover requests - removed function parameters that can be obtained using v.instance. Co-Authored-by: Annaraya Narasagond <[email protected]> Signed-off-by: Raghavendra Talur <[email protected]>
- Remove `sourceVrgNamespaceName` and `sourceVrgName` as parameters. - Derive `sourceVrgNamespaceName` and `sourceVrgName` directly from the instance. Co-Authored-by: Annaraya Narasagond <[email protected]> Signed-off-by: Raghavendra Talur <[email protected]>
- Removed unnecessary local variable `localS3StoreAccessor` Co-Authored-by: Annaraya Narasagond <[email protected]> Signed-off-by: Raghavendra Talur <[email protected]>
- `getVRGFromS3Profile` gets the source vrg - call `getVRGFromS3Profile` from `kubeObjectsRecover` Co-Authored-by: Annaraya Narasagond <[email protected]> Signed-off-by: Raghavendra Talur <[email protected]>
Co-Authored-by: Annaraya Narasagond <[email protected]> Signed-off-by: Raghavendra Talur <[email protected]>
Co-Authored-by: Annaraya Narasagond <[email protected]> Signed-off-by: Raghavendra Talur <[email protected]>
…ore retrival - Updated the `findS3StoreAccessor` method to accept `s3ProfileName` directly - Removed unnecessary intermediate variable `localS3StoreAccessor` Co-Authored-by: Annaraya Narasagond <[email protected]> Signed-off-by: Raghavendra Talur <[email protected]>
395bd2a
to
5d5170d
Compare
We were passing veleroNamespaceName through many functions where it can be obtained from the v object. Don't pass it as a parameter until the last call. Functions: - `getRecoverOrProtectRequest` - `kubeObjectsRecoveryStartOrResume` - `executeRecoverGroup` by removing the `veleroNamespaceName` parameter. Co-Authored-by: Annaraya Narasagond <[email protected]> Signed-off-by: Raghavendra Talur <[email protected]>
5d5170d
to
c1cb4e1
Compare
Co-Authored-by: Annaraya Narasagond <[email protected]> Signed-off-by: Raghavendra Talur <[email protected]>
d5e747a
to
f9d4955
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the changes! Code is much better now.
Refactored the following functions