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

Treat namespace as a regular restorable item #7143

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

reasonerjt
Copy link
Contributor

Fixes #1970

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.

Fixes vmware-tanzu#1970

Namespaces will be handled as cluster-scope resource, but for
consistency they will still created via "Ensure namespace" flow for
consistency.

Signed-off-by: Daniel Jiang <jiangd@vmware.com>
Copy link

codecov bot commented Nov 24, 2023

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (b8a5859) 61.17% compared to head (59a8a4b) 61.66%.
Report is 12 commits behind head on main.

Files Patch % Lines
pkg/restore/restore.go 40.00% 9 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7143      +/-   ##
==========================================
+ Coverage   61.17%   61.66%   +0.49%     
==========================================
  Files         258      258              
  Lines       27366    27642     +276     
==========================================
+ Hits        16741    17046     +305     
+ Misses       9441     9401      -40     
- Partials     1184     1195      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sseago
Copy link
Collaborator

sseago commented Nov 27, 2023

@reasonerjt So it looks like with this code, we're still creating the namespace using the EnsureNamespaceExistsAndIsReady func, and then we call restoreItem on the ns, but that exits early because we just restored the namespace. One consequence of this is that item actions on the namespace won't be run -- one of the comments on the issue this is supposed to fix points out that if we treat namespaces as regular items, then item actions will run on creation. I'm wondering whether, in the case where resource type is "Namespace", maybe we should skip the "ensure NS exists" code and instead just go right to restoreItem which will then create the namespace with item actions (if any).

@dou-oscarh
Copy link

@reasonerjt I'm interested in using this change to restore some namespaces and apply a couple of transformations to them, however, when looking at the latest release code (v1.12.3) I don't see those changes in the codebase.

Do you happen to know when will those changes get released?

@blackpiglet
Copy link
Contributor

@dou-oscarh The change is in the main branch. You can try the change with the main release Velero image.
The on-going v1.13.0 release will be ready in the next few weeks.

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.

Velero restore with include-resources to restore only namespace resource is not working
4 participants