Skip to content
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

fix(restore): set targetip on CVRs after restore is completed #198

Merged
merged 1 commit into from
Nov 10, 2020
Merged

fix(restore): set targetip on CVRs after restore is completed #198

merged 1 commit into from
Nov 10, 2020

Conversation

zlymeda
Copy link
Contributor

@zlymeda zlymeda commented Nov 4, 2020

Auto-setting of targetip on CVR if openebs.io/restore-completed is found

Pull Request template

Why is this PR required? What issue does it fix?:
Currently, users have to set targetip on CVRs after restore. Corresponding change in openebs/velero-plugin#131 will mark CVRs which needs to update the targetip. Changes in this PR will check the annotation and set the targetip.

What this PR does?:
Sets targetip on CVRs marked with openebs.io/restore-completed annotation.

Does this PR require any upgrade changes?:
No

If the changes in this PR are manually verified, list down the scenarios covered::
make test in velero-plugin repository with changes deployed.

Any additional information for your reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs

related maya PR: openebs-archive/maya#1761
related velero-plugin PR: openebs/velero-plugin#131

Checklist:

Copy link
Contributor

@mynktl mynktl left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @zlymeda!

I've given few minor comments. PTAL.

pkg/volumereplica/volumereplica.go Outdated Show resolved Hide resolved
pkg/volumereplica/volumereplica.go Outdated Show resolved Hide resolved
Copy link
Contributor

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!! Good one 👍

Makefile Show resolved Hide resolved
pkg/controllers/replica-controller/handler.go Outdated Show resolved Hide resolved
pkg/controllers/replica-controller/handler.go Show resolved Hide resolved
pkg/controllers/replica-controller/handler.go Show resolved Hide resolved
pkg/volumereplica/volumereplica.go Outdated Show resolved Hide resolved
@zlymeda
Copy link
Contributor Author

zlymeda commented Nov 5, 2020

@mittachaitu @mynktl I updated the PR, PTAL, thanks

@sonasingh46
Copy link
Contributor

@zlymeda -- Thanks for the PR. Given some comments.
Also, let me know if you need some help in fixing the DCO signoff that is failing.

@zlymeda
Copy link
Contributor Author

zlymeda commented Nov 6, 2020

@sonasingh46 regarding the DCO, I accepted two inline changes and those don't have my signoff in the message. Should I squeeze commits and force push?

Copy link
Contributor

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

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

Changes are good

@mynktl mynktl added this to the 2.4.0 milestone Nov 10, 2020
@zlymeda
Copy link
Contributor Author

zlymeda commented Nov 10, 2020

I squeezed commits and force-pushed to fix the DCO.

@sonasingh46
Copy link
Contributor

@zlymeda -- One last thing -- can you please rebase and then we can go ahead with the merge.

Auto-setting of targetip on CVR if openebs.io/restore-completed is found

Signed-off-by: Ondrej Belusky <[email protected]>
@zlymeda
Copy link
Contributor Author

zlymeda commented Nov 10, 2020

@sonasingh46 rebased

@sonasingh46 sonasingh46 merged commit 27d5cb5 into openebs-archive:master Nov 10, 2020
@zlymeda zlymeda deleted the fix/set-targetip branch November 10, 2020 20:26
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.

5 participants