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

Velero restore with include-resources to restore only namespace resource is not working #1970

Closed
Frank51 opened this issue Oct 16, 2019 · 16 comments · Fixed by #7143
Closed
Assignees
Labels
Bug Icebox We see the value, but it is not slated for the next couple releases. Reviewed Q2 2021 Reviewed Q3 2023
Milestone

Comments

@Frank51
Copy link
Contributor

Frank51 commented Oct 16, 2019

What steps did you take and what happened:
Velero user could use '--include-resources' flag to specify which kind of resources need to be restored from backup. But when '--include-resources' only contains 'namespaces', the backup namespace could not be restored successfully to the Kubernetes cluster. If adding 'namespaces' and 'services' into the 'include resources', the backup could be restored correctly at this time.

velero get backups
NAME        STATUS      CREATED                         EXPIRES   STORAGE LOCATION   SELECTOR
yelb-back   Completed   2019-09-20 12:39:34 -0700 PDT   3d        default            <none>

velero describe backup yelb-back
Name:         yelb-back
Namespace:    velero
Labels:       velero.io/storage-location=default
Annotations:  <none>

Phase:  Completed

Namespaces:
  Included:  yelb-ns
  Excluded:  <none>

Resources:
  Included:        *
  Excluded:        <none>
  Cluster-scoped:  auto

Label selector:  <none>

Storage Location:  default

Snapshot PVs:  auto

TTL:  720h0m0s

Hooks:  <none>

Backup Format Version:  1

Started:    2019-09-20 12:39:34 -0700 PDT
Completed:  2019-09-20 12:39:35 -0700 PDT

Expiration:  2019-10-20 12:39:34 -0700 PDT

Persistent Volumes: <none included>


kubectl get ns
NAME              STATUS   AGE
default           Active   5h6m
kube-node-lease   Active   5h6m
kube-public       Active   5h6m
kube-system       Active   5h6m
velero            Active   152m

velero restore create --from-backup yelb-back --include-resources namespaces
Restore request "yelb-back-20191016140424" submitted successfully.
Run `velero restore describe yelb-back-20191016140424` or `velero restore logs yelb-back-20191016140424` for more details.

kubectl get ns
NAME              STATUS   AGE
default           Active   5h7m
kube-node-lease   Active   5h7m
kube-public       Active   5h7m
kube-system       Active   5h7m
velero            Active   152m

# yelb-ns is not restored from the backup.
# However, when I restore both the namespace and service, yelb-ns is restored:

velero restore create --from-backup yelb-back --include-resources namespaces,services
Restore request "yelb-back-20191016140932" submitted successfully.
Run `velero restore describe yelb-back-20191016140932` or `velero restore logs yelb-back-20191016140932` for more details.

kubectl get ns
NAME              STATUS   AGE
default           Active   5h12m
kube-node-lease   Active   5h12m
kube-public       Active   5h12m
kube-system       Active   5h12m
velero            Active   157m
yelb-ns           Active   4s

What did you expect to happen:
When running 'velero restore create --from-backup --include-resources namespaces ', velero should restore only the namespace resource from the backup on the Kubernetes cluster successfully.

The output of the following commands will help us better understand what's going on:
(Pasting long output into a GitHub gist or other pastebin is fine.)

Name:         yelb-back
Namespace:    velero
Labels:       velero.io/storage-location=default
Annotations:  <none>

Phase:  Completed

Namespaces:
  Included:  yelb-ns
  Excluded:  <none>

Resources:
  Included:        *
  Excluded:        <none>
  Cluster-scoped:  auto

Label selector:  <none>

Storage Location:  default

Snapshot PVs:  auto

TTL:  720h0m0s

Hooks:  <none>

Backup Format Version:  1

Started:    2019-09-20 12:39:34 -0700 PDT
Completed:  2019-09-20 12:39:35 -0700 PDT

Expiration:  2019-10-20 12:39:34 -0700 PDT

Persistent Volumes: <none included>

Name:         yelb-back-20191016140424
Namespace:    velero
Labels:       <none>
Annotations:  <none>

Phase:  Completed

Backup:  yelb-back

Namespaces:
  Included:  *
  Excluded:  <none>

Resources:
  Included:        namespaces
  Excluded:        nodes, events, events.events.k8s.io, backups.velero.io, restores.velero.io, resticrepositories.velero.io
  Cluster-scoped:  auto

Namespace mappings:  <none>

Label selector:  <none>

Restore PVs:  auto

Environment:

  • Velero version (use velero version): v1.0.0
  • Velero features (use velero client config get features):
  • Kubernetes version (use kubectl version): (Client Version : v1.16.0), (Server Version :v1.14.5 )
  • Kubernetes installer & version:
  • Cloud provider or hardware configuration: AWS
  • OS (e.g. from /etc/os-release): Ubuntu
@skriss skriss added the Bug label Oct 17, 2019
@skriss
Copy link
Member

skriss commented Oct 17, 2019

Yeah, this is likely being caused by https://github.com/vmware-tanzu/velero/blob/master/pkg/restore/restore.go#L403-L407. This will take a little bit of investigation as I don't remember exactly why we treated namespaces that way, though likely has to do with included/excluded namespaces, namespace mappings, etc.

@Frank51
Copy link
Contributor Author

Frank51 commented Nov 4, 2019

Yeah, this is likely being caused by https://github.com/vmware-tanzu/velero/blob/master/pkg/restore/restore.go#L403-L407. This will take a little bit of investigation as I don't remember exactly why we treated namespaces that way, though likely has to do with included/excluded namespaces, namespace mappings, etc.

Hi, Kriss. Could you give me more detail related to this namespace restore issue? Because I am a little bit confused about that part of the code. Why the namespace will be treated in this way? But when restoring namespace with other resources like PV, PVC, it works.

@skriss
Copy link
Member

skriss commented Nov 4, 2019

@Frank51 sorry, I remember you asked me for this before in Slack :) I'll try to add more context tomorrow - I'll have to jog my own memory as to why it's implemented this way.

@Frank51
Copy link
Contributor Author

Frank51 commented Nov 6, 2019

@Frank51 sorry, I remember you asked me for this before in Slack :) I'll try to add more context tomorrow - I'll have to jog my own memory as to why it's implemented this way.

Hi, Kriss. Sorry to disturb you. Do you have any idea about the implementation of the restore of namespace at that time?

@skriss
Copy link
Member

skriss commented Nov 8, 2019

So I think there are a couple of things to consider:

  • if we're restoring a namespaced resource, we first need to ensure that the namespace itself exists, so the resource can be restored into it
  • when restoring Namespace resources themselves, we need to consider the namespace includes/excludes, as well as the namespace mappings

I believe that the implementation approach of lazily restoring Namespace resources only if we are restoring a resource into it was the easiest way to deal with the above two items. That said, alternate approaches would work fine as long as all of the permutations of the above are properly dealt with.

Hope that helps!

@skriss skriss added this to the v1.3 milestone Nov 14, 2019
@skriss
Copy link
Member

skriss commented Dec 11, 2019

@Frank51 were you interested in working on a fix here?

@Frank51
Copy link
Contributor Author

Frank51 commented Dec 11, 2019

@Frank51 were you interested in working on a fix here?

Sure, I am quite interested in it.

@ashish-amarnath
Copy link
Contributor

/assign @Frank51

@Frank51

This comment has been minimized.

@skriss skriss modified the milestones: v1.3, v1.4 Feb 4, 2020
@nrb nrb self-assigned this Apr 13, 2020
@skriss skriss removed this from the v1.4 milestone Apr 27, 2020
@nrb nrb removed their assignment Oct 22, 2020
@nrb
Copy link
Contributor

nrb commented Nov 2, 2020

This also has implications for users wanting to write RestoreItemActions for namespaces. Right now, since namespaces aren't created as part of the normal restore process but rather directly with a client call, writing a RestoreItemAction plugin for them will not get invoked.

@eleanor-millman eleanor-millman added Reviewed Q2 2021 Icebox We see the value, but it is not slated for the next couple releases. labels May 10, 2021
@stale
Copy link

stale bot commented Nov 14, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the staled label Nov 14, 2021
@stale
Copy link

stale bot commented Nov 29, 2021

Closing the stale issue.

@reasonerjt
Copy link
Contributor

not a stale

@sseago
Copy link
Collaborator

sseago commented Jun 1, 2023

@reasonerjt I wonder whether we want a restore field to control this behavior. Currently, velero doesn't restore empty namespaces. With the change proposed here, empty namespaces would be restored. Since that's a behavior change, would we want to give users an option to preserve prior behavior, or should we consider "velero doesn't restore an empty namespace" to be a bug instead, in which case, we wouldn't want a field, we'd just make the change for all restores.

@pradeepkchaturvedi pradeepkchaturvedi added the 1.13-candidate issue/pr that should be considered to target v1.13 minor release label Aug 4, 2023
@reasonerjt
Copy link
Contributor

We'll double-check to determine if the current behavior "velero doesn't restore an empty namespace" is a bug or not, and may leave it as-is.

@reasonerjt reasonerjt removed the 1.13-candidate issue/pr that should be considered to target v1.13 minor release label Sep 6, 2023
@reasonerjt reasonerjt added this to the v1.13 milestone Sep 6, 2023
reasonerjt added a commit to reasonerjt/velero that referenced this issue Nov 24, 2023
Fixes vmware-tanzu#1970

Signed-off-by: Daniel Jiang <jiangd@vmware.com>
reasonerjt added a commit to reasonerjt/velero that referenced this issue Nov 24, 2023
Fixes vmware-tanzu#1970

Signed-off-by: Daniel Jiang <jiangd@vmware.com>
reasonerjt added a commit to reasonerjt/velero that referenced this issue Nov 24, 2023
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>
@reasonerjt
Copy link
Contributor

@reasonerjt I wonder whether we want a restore field to control this behavior. Currently, velero doesn't restore empty namespaces. With the change proposed here, empty namespaces would be restored. Since that's a behavior change, would we want to give users an option to preserve prior behavior, or should we consider "velero doesn't restore an empty namespace" to be a bug instead, in which case, we wouldn't want a field, we'd just make the change for all restores.

@sseago
IMO "velero doesn't restore an empty namespace" is a bug, it was not documented anywhere

sseago pushed a commit that referenced this issue Nov 28, 2023
Fixes #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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Icebox We see the value, but it is not slated for the next couple releases. Reviewed Q2 2021 Reviewed Q3 2023
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants