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

Add "Deleting" phase for restore display #6604

Merged
merged 1 commit into from
Aug 8, 2023
Merged

Conversation

allenxu404
Copy link
Contributor

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #6582

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.

@allenxu404
Copy link
Contributor Author

/kind changelog-not-required

@allenxu404
Copy link
Contributor Author

/kind changelog-not-required

@github-actions github-actions bot added the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Aug 4, 2023
@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2023

Codecov Report

Merging #6604 (cc03b7a) into main (d027a16) will decrease coverage by 0.03%.
Report is 5 commits behind head on main.
The diff coverage is 11.11%.

@@            Coverage Diff             @@
##             main    #6604      +/-   ##
==========================================
- Coverage   60.18%   60.15%   -0.03%     
==========================================
  Files         242      242              
  Lines       25654    25667      +13     
==========================================
+ Hits        15439    15440       +1     
- Misses       9139     9150      +11     
- Partials     1076     1077       +1     
Files Changed Coverage Δ
pkg/cmd/util/output/restore_describer.go 32.17% <0.00%> (-0.57%) ⬇️
pkg/cmd/cli/restore/get.go 72.09% <25.00%> (-4.84%) ⬇️

... and 1 file with indirect coverage changes

blackpiglet
blackpiglet previously approved these changes Aug 4, 2023
@ywk253100
Copy link
Contributor

This will cause inconsistent between the Velero and kubectl when getting the restores, @sseago is there any way to specify the status in CRD according to different conditions?

@sseago
Copy link
Collaborator

sseago commented Aug 4, 2023

@ywk253100 I don't think we have an issue here. kubectl doesn't show status information for restores, unlike velero CLI:

$ kubectl get restore -n openshift-adp
NAME                           AGE
mysql-datamover4-1             9d
mysql-datamover5-1             9d
mysql-datamover5-2             9d

@sseago
Copy link
Collaborator

sseago commented Aug 4, 2023

And if you're looking at yaml, then you want to see the real Phase value, but again the deletion timestamp tells you it's terminating. As long as the velero CLI summary shows deleting/terminating and any code that differs for terminating restores checks deletion timestamp, this should be good.

@sseago
Copy link
Collaborator

sseago commented Aug 4, 2023

You probably want to check deletion timestamp on the logs command, as was covered in the original change-phase PR:

https://github.com/vmware-tanzu/velero/pull/6599/files#diff-aa4ed12cfdf5527ebf5441e9ea40d8cb40f9f4d446a5742d12e48e6bd281189e

pkg/cmd/cli/restore/describe.go Outdated Show resolved Hide resolved
pkg/cmd/cli/restore/get.go Show resolved Hide resolved
Signed-off-by: allenxu404 <qix2@vmware.com>
@sseago sseago merged commit 5f463c5 into vmware-tanzu:main Aug 8, 2023
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the status of Restore before removing the storage resources to indicate the actual status
5 participants