Skip to content

Conversation

shubham-pampattiwar
Copy link
Collaborator

@shubham-pampattiwar shubham-pampattiwar commented Feb 10, 2022

Thank you for contributing to Velero!

Please add a summary of your change

The PR follows the approach number 1 specified in the ExistingResourcePolicy design and does the following:

  • adds a new spec to the Velero restore API called existingResourcePolicy
  • updates the Velero restore CRD with the addition of existingResourcePolicy` spec option
  • adds the option existing-resource-policy in velero CLI which accepts only none and update as values
  • updates the logic in velero restore workflow to respect the existingResourcePolicy option when set by the user

Does your change fix a particular issue?

Fixes #4842

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

@shubham-pampattiwar shubham-pampattiwar force-pushed the add-restore-policy branch 3 times, most recently from f52cc6f to 3231f7c Compare March 17, 2022 06:45
@shubham-pampattiwar shubham-pampattiwar marked this pull request as ready for review March 17, 2022 06:52
@github-actions github-actions bot requested review from dsu-igeek and sseago March 17, 2022 06:52
@eleanor-millman eleanor-millman removed the request for review from dsu-igeek March 23, 2022 00:39
@eleanor-millman
Copy link
Contributor

@reasonerjt

@reasonerjt reasonerjt requested a review from Lyndon-Li March 23, 2022 00:42
Copy link
Collaborator

@sseago sseago left a comment

Choose a reason for hiding this comment

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

In addition to the change to make sure that service accounts get the updated labels, we need to make sure that this is tested for both SAs and other resources, including the "patch only because resource update failed" case. In all cases we need to make sure that we get the right velero labels applied on the first restore attempt (i.e. no prior labels) and that the labels are right on the second restore attempt (i.e. prior labels overwritten)

sseago
sseago previously approved these changes Apr 28, 2022
@shubham-pampattiwar
Copy link
Collaborator Author

Can one more maintainer please take a look at this PR ?

@eleanor-millman
Copy link
Contributor

cc @reasonerjt

@shubham-pampattiwar
Copy link
Collaborator Author

@Lyndon-Li @sseago Updated the PR with respect to feedback, made the changes modular as well as added unit tests, please take another look. Thank you !

@codecov-commenter
Copy link

codecov-commenter commented May 9, 2022

Codecov Report

Merging #4628 (e3106f3) into main (db99b6e) will increase coverage by 0.01%.
The diff coverage is 45.13%.

@@            Coverage Diff             @@
##             main    #4628      +/-   ##
==========================================
+ Coverage   41.70%   41.71%   +0.01%     
==========================================
  Files         204      204              
  Lines       18040    18150     +110     
==========================================
+ Hits         7523     7572      +49     
- Misses       9956    10007      +51     
- Partials      561      571      +10     
Impacted Files Coverage Δ
pkg/builder/restore_builder.go 0.00% <0.00%> (ø)
pkg/cmd/util/output/restore_describer.go 0.00% <0.00%> (ø)
pkg/restore/restore.go 65.35% <49.51%> (-1.32%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db99b6e...e3106f3. Read the comment docs.

@shubham-pampattiwar shubham-pampattiwar force-pushed the add-restore-policy branch 2 times, most recently from 4b10666 to dcfe39d Compare May 9, 2022 17:34
add updateall policy option

fix updating labels

dump updateAll policy option

remove updateAll policy refs

Signed-off-by: Shubham Pampattiwar <[email protected]>

add changelog file

Signed-off-by: Shubham Pampattiwar <[email protected]>

update docs

Signed-off-by: Shubham Pampattiwar <[email protected]>

patch labels for sa if policy is update

Signed-off-by: Shubham Pampattiwar <[email protected]>

fix existingResourcePolicy for serviceaccounts

modularize changes and add unit tests

Signed-off-by: Shubham Pampattiwar <[email protected]>

fix conflicts and update crds.go

Signed-off-by: Shubham Pampattiwar <[email protected]>

change log level from info to error

Signed-off-by: Shubham Pampattiwar <[email protected]>

update crds.go

Signed-off-by: Shubham Pampattiwar <[email protected]>
@shubham-pampattiwar
Copy link
Collaborator Author

@Lyndon-Li updated the log levels.

@reasonerjt reasonerjt merged commit e51865e into vmware-tanzu:main May 10, 2022
sseago added a commit to sseago/velero that referenced this pull request May 19, 2022
Shubham Pampattiwar has made several contributions to Velero,
most recently in designing and implementing two v1.9
features, including the following:
- [Add design for enabling multiple label support[(vmware-tanzu#4619)
- [Add multiple label selector support to Velero Backup and Restore APIs](vmware-tanzu#4650)
- [Add design for enabling support for ExistingResourcePolicy to restore API](vmware-tanzu#4613)
- [Add existingResourcePolicy to Restore API](vmware-tanzu#4628)

Signed-off-by: Scott Seago <[email protected]>
sseago added a commit to sseago/velero that referenced this pull request May 19, 2022
Shubham Pampattiwar has made several contributions to Velero,
most recently in designing and implementing two v1.9
features, including the following:
- [Add design for enabling multiple label support](vmware-tanzu#4619)
- [Add multiple label selector support to Velero Backup and Restore APIs](vmware-tanzu#4650)
- [Add design for enabling support for ExistingResourcePolicy to restore API](vmware-tanzu#4613)
- [Add existingResourcePolicy to Restore API](vmware-tanzu#4628)

Signed-off-by: Scott Seago <[email protected]>
sseago added a commit to sseago/velero that referenced this pull request May 19, 2022
Shubham Pampattiwar has made several contributions to Velero,
most recently in designing and implementing two v1.9
features, including the following:
- [Add design for enabling multiple label support](vmware-tanzu#4619)
- [Add multiple label selector support to Velero Backup and Restore APIs](vmware-tanzu#4650)
- [Add design for enabling support for ExistingResourcePolicy to restore API](vmware-tanzu#4613)
- [Add existingResourcePolicy to Restore API](vmware-tanzu#4628)

Shubham has also been driving forward the data mover requirements and design discussions for velero 1.10:
- [Add datamover design](vmware-tanzu#4768)

Signed-off-by: Scott Seago <[email protected]>
Lyndon-Li pushed a commit to Lyndon-Li/velero that referenced this pull request May 27, 2022
Shubham Pampattiwar has made several contributions to Velero,
most recently in designing and implementing two v1.9
features, including the following:
- [Add design for enabling multiple label support](vmware-tanzu#4619)
- [Add multiple label selector support to Velero Backup and Restore APIs](vmware-tanzu#4650)
- [Add design for enabling support for ExistingResourcePolicy to restore API](vmware-tanzu#4613)
- [Add existingResourcePolicy to Restore API](vmware-tanzu#4628)

Shubham has also been driving forward the data mover requirements and design discussions for velero 1.10:
- [Add datamover design](vmware-tanzu#4768)

Signed-off-by: Scott Seago <[email protected]>
danfengliu pushed a commit to danfengliu/velero that referenced this pull request Sep 13, 2022
Shubham Pampattiwar has made several contributions to Velero,
most recently in designing and implementing two v1.9
features, including the following:
- [Add design for enabling multiple label support](vmware-tanzu#4619)
- [Add multiple label selector support to Velero Backup and Restore APIs](vmware-tanzu#4650)
- [Add design for enabling support for ExistingResourcePolicy to restore API](vmware-tanzu#4613)
- [Add existingResourcePolicy to Restore API](vmware-tanzu#4628)

Shubham has also been driving forward the data mover requirements and design discussions for velero 1.10:
- [Add datamover design](vmware-tanzu#4768)

Signed-off-by: Scott Seago <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ExistingResourcePolicy support to restore API
6 participants