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.

Sign up for GitHub

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

Jump to bottom

Only skip PVC creation if --namespace-mapping flag is not used #565

Merged
merged 2 commits into from
Feb 22, 2024
Merged

Only skip PVC creation if --namespace-mapping flag is not used #565

merged 2 commits into from
Feb 22, 2024

Conversation

varunsrinivasan2
Copy link
Contributor

@varunsrinivasan2 varunsrinivasan2 commented Jan 23, 2024 •
edited
Loading

What this PR does / why we need it:
When a Velero user uses the --namespace-mapping flag, PVC's should be recreated in the target namespace. Velero vSphere Plugin currently skips PVC creation if the PVC already exists in the cluster. The intended behavior of Velero restore is to recreate the PVC in the target namespace even if the PVC already exists in the cluster. This PR will check the Restore CRD and will only skip PVC creation if the user does not restore with the --namespace-mapping flag.

Testing:

Case 1: Logs when namespace mapping flag is used:

time="2024-02-22T00:12:36Z" level=info msg="Updating target PVC namespace based on the namespace mapping option in the Restore Spec" cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/restore_pvc_action_plugin.go:135" pluginName=velero-plugin-for-vsphere restore=velero/finalmaptest-20240222001233
time="2024-02-22T00:12:36Z" level=info msg="Updated the target PVC namespace from nsmlog to newnsmlog based on the namespace mapping in the Restore Spec" cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/restore_pvc_action_plugin.go:146" pluginName=velero-plugin-for-vsphere restore=velero/finalmaptest-20240222001233
time="2024-02-22T00:12:36Z" level=info msg="VSphere PVCRestoreItemAction for PVC newnsmlog/nginx-logs started" cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/restore_pvc_action_plugin.go:150" pluginName=velero-plugin-for-vsphere restore=velero/finalmaptest-20240222001233
time="2024-02-22T00:12:36Z" level=info msg="Check if PVC newnsmlog/nginx-logs creation should be skipped" cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/util/util.go:410" pluginName=velero-plugin-for-vsphere restore=velero/finalmaptest-20240222001233
time="2024-02-22T00:12:36Z" level=info msg="Creating PVC as it does not exist newnsmlog/nginx-logs" cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/util/util.go:431" pluginName=velero-plugin-for-vsphere restore=velero/finalmaptest-20240222001233

Case 2: Logs when namespace mapping flag is not used and backed-up namespace & PVC are deleted:

time="2024-02-22T00:17:37Z" level=info msg="Updating target PVC namespace based on the namespace mapping option in the Restore Spec" cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/restore_pvc_action_plugin.go:135" pluginName=velero-plugin-for-vsphere restore=velero/finalmaptest2
time="2024-02-22T00:17:37Z" level=info msg="VSphere PVCRestoreItemAction for PVC newnsmlog/nginx-logs started" cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/restore_pvc_action_plugin.go:150" pluginName=velero-plugin-for-vsphere restore=velero/finalmaptest2
time="2024-02-22T00:17:37Z" level=info msg="Check if PVC newnsmlog/nginx-logs creation should be skipped" cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/util/util.go:410" pluginName=velero-plugin-for-vsphere restore=velero/finalmaptest2
time="2024-02-22T00:17:37Z" level=info msg="Creating PVC as it does not exist newnsmlog/nginx-logs" cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/util/util.go:431" pluginName=velero-plugin-for-vsphere restore=velero/finalmaptest2

Case 3: Logs when namespace mapping flag is not used and namespace & PVC exists in cluster:

time="2024-02-21T23:49:25Z" level=info msg="Updating target PVC namespace based on the namespace mapping option in the Restore Spec" cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/restore_pvc_action_plugin.go:135" pluginName=velero-plugin-for-vsphere restore=velero/nodeletecase
time="2024-02-21T23:49:25Z" level=info msg="VSphere PVCRestoreItemAction for PVC nsmlog/nginx-logs started" cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/restore_pvc_action_plugin.go:156" pluginName=velero-plugin-for-vsphere restore=velero/nodeletecase
time="2024-02-21T23:49:25Z" level=info msg="Check if PVC nsmlog/nginx-logs creation should be skipped" cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/util/util.go:410" pluginName=velero-plugin-for-vsphere restore=velero/nodeletecase
time="2024-02-21T23:49:25Z" level=warning msg="Skipping PVC nsmlog/nginx-logs creation since it already exists." cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/util/util.go:424" pluginName=velero-plugin-for-vsphere restore=velero/nodeletecase
time="2024-02-21T23:49:25Z" level=info msg="VSphere PVCRestoreItemAction for PVC nsmlog/nginx-logs completed with err: <nil>" cmd=/plugins/velero-plugin-for-vsphere logSource="/go/src/github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/restore_pvc_action_plugin.go:158" pluginName=velero-plugin-for-vsphere restore=velero/nodeletecase

Which issue(s) this PR fixes:

Fixes #410
vmware-tanzu/velero#7318

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

This change will allow users to restore PVC's in the target namespace when the --namespace-mapping flag is used when creating a restore with Velero.

@xing-yang
Copy link
Contributor

The change looks right. Please add testing results and add release notes.

@varunsrinivasan2 varunsrinivasan2 changed the title [WIP] Only skip PVC creation if --namespace-mapping flag is not used Only skip PVC creation if --namespace-mapping flag is not used Feb 20, 2024
xing-yang
xing-yang approved these changes Feb 22, 2024
View reviewed changes
Copy link
Contributor

@xing-yang xing-yang left a comment

Choose a reason for hiding this comment

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

lgtm

@xing-yang xing-yang merged commit feebe39 into vmware-tanzu:main Feb 22, 2024
2 checks passed
@xing-yang xing-yang mentioned this pull request Feb 28, 2024
Closed
lipingxue pushed a commit to lipingxue/velero-plugin-for-vsphere that referenced this pull request Mar 11, 2024
@varunsrinivasan2 @lipingxue
Only skip PVC creation if --namespace-mapping flag is not used (vmwar…
0b44ef8 
…e-tanzu#565)

* Only skip PVC creation if --namespace-mapping flag is not used

Signed-off-by: Varun Srinivasan <[email protected]>

* Refactor SkipPVCCreation function signature to add target namespace as a parameter

Signed-off-by: Varun Srinivasan <[email protected]>

---------

Signed-off-by: Varun Srinivasan <[email protected]>
@lipingxue lipingxue mentioned this pull request Mar 11, 2024
Merged
lipingxue pushed a commit to lipingxue/velero-plugin-for-vsphere that referenced this pull request Mar 11, 2024
@varunsrinivasan2 @lipingxue
Only skip PVC creation if --namespace-mapping flag is not used (vmwar…
511236e 
…e-tanzu#565)

* Only skip PVC creation if --namespace-mapping flag is not used

Signed-off-by: Varun Srinivasan <[email protected]>

* Refactor SkipPVCCreation function signature to add target namespace as a parameter

Signed-off-by: Varun Srinivasan <[email protected]>

---------

Signed-off-by: Varun Srinivasan <[email protected]>
Signed-off-by: Liping Xue <[email protected]>
lipingxue added a commit that referenced this pull request Mar 11, 2024
@lipingxue @dependabot
@varunsrinivasan2 @xing-yang @deepakkinni
Cherry-pick changes to release-1.5 branch. (#570)
0b9a8de 
* Update support matrix for Vanilla, WCP and GC. (#559)

Signed-off-by: Liping Xue <[email protected]>

* Document change to update known issue. (#560)

* Document change.

Signed-off-by: Liping Xue <[email protected]>

* Address comment from Xing.

Signed-off-by: Liping Xue <[email protected]>

* Remove files that are not needed for this change.

Signed-off-by: Liping Xue <[email protected]>

---------

Signed-off-by: Liping Xue <[email protected]>

* Bump golang.org/x/crypto from 0.14.0 to 0.17.0 (#562)

Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.14.0 to 0.17.0.
- [Commits](golang/crypto@v0.14.0...v0.17.0)

---
updated-dependencies:
- dependency-name: golang.org/x/crypto
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Liping Xue <[email protected]>

* Update document. (#566)

Signed-off-by: Liping Xue <[email protected]>

* Modify parsing behavior of login credentials to handle Go escape characters (#564)

* Signed-off-by: Varun Srinivasan <[email protected]>

Create common config and new function to parse config data

* Always populate "port" key of parameters map after setting default VC port if config port value is empty

Signed-off-by: Varun Srinivasan <[email protected]>

* Clean up test logging and unit tests

Signed-off-by: Varun Srinivasan <[email protected]>

* Keep only necessary config values

Signed-off-by: Varun Srinivasan <[email protected]>

* Refactor ParseConfig to return error for sanitized error handling

Signed-off-by: Varun Srinivasan <[email protected]>

---------

Signed-off-by: Varun Srinivasan <[email protected]>
Signed-off-by: Liping Xue <[email protected]>

* Only skip PVC creation if --namespace-mapping flag is not used (#565)

* Only skip PVC creation if --namespace-mapping flag is not used

Signed-off-by: Varun Srinivasan <[email protected]>

* Refactor SkipPVCCreation function signature to add target namespace as a parameter

Signed-off-by: Varun Srinivasan <[email protected]>

---------

Signed-off-by: Varun Srinivasan <[email protected]>
Signed-off-by: Liping Xue <[email protected]>

* Add doc that wffc is not supported (#567)

Signed-off-by: xing-yang <[email protected]>
Signed-off-by: Liping Xue <[email protected]>

* Fail the Restore if the StorageClass is associated with WaitForFirstConsumer volumeBindingMode (#568)

Signed-off-by: Deepak Kinni <[email protected]>
Co-authored-by: Deepak Kinni <[email protected]>
Signed-off-by: Liping Xue <[email protected]>

---------

Signed-off-by: Liping Xue <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Varun Srinivasan <[email protected]>
Signed-off-by: xing-yang <[email protected]>
Signed-off-by: Deepak Kinni <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Varun Srinivasan <[email protected]>
Co-authored-by: Xing Yang <[email protected]>
Co-authored-by: Deepak Kinni <[email protected]>
Co-authored-by: Deepak Kinni <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deepakkinni deepakkinni deepakkinni approved these changes

@xing-yang xing-yang xing-yang approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

When using --namespace-mappings, cannot restore volume if the original volume still exists
3 participants
@varunsrinivasan2 @xing-yang @deepakkinni