Resource is being restored even though custom plugin specifies SkipRestore=true for that resource

[codegold79]

Update (Feb. 23, 2021):

After some investigation (with help from @brito-rafa and @drajshek), we found that this issue is not a bug. Instead, it's a matter of naming and updating documentation. The fix is to include in IncludedResources slice the resource (kind works as well) and groups, and not just resources. As you can see, the naming does not make that obvious. Here is what worked for us:

func (p *RestorePlugin) AppliesTo() (velero.ResourceSelector, error) {
	return velero.ResourceSelector{IncludedResources: []string{
			"ingress.extensions",
			"ingress.k8s.networking.io",
		}
	}, nil
}

To close this issue, I'll update documentation.

What steps did you take and what happened:

For our custom plugin, we specified that SkipRestore be true for the resource we're working on, in our case, ingresses. Here are the relevant places in the plugin code:

func (p *RestorePlugin) AppliesTo() (velero.ResourceSelector, error) {
	return velero.ResourceSelector{IncludedResources: []string{"Ingress"}}, nil
}

return &velero.RestoreItemActionExecuteOutput{
	UpdatedItem:     nil,
	AdditionalItems: nil,
	SkipRestore:     true,
}, nil

Unfortunately, when we do a restore, we find the ingresses resource is restored.

What did you expect to happen:
When I do a restore, I expect that the ingresses resource will NOT be restored. I expect it to be skipped.

Relevant parts of logs. Additional debugging lines were added to the Velero build.:

In velero restore logs <restorename>, note that the ingresses resource appears in two groups: extensions (deprecated) and networking.k8s.io. The ingresses in the former group, extensions, is correctly skipped as expected. However, the latter group, networking.k8s.io, captures no restore.resolvedActions. ResolvedActions is what enables a resource to be skipped, and you can see for ingresses.networking.k8s.io, it is nil.

[...clipped...]

msg="Restoring resource 'ingresses.extensions' into namespace 'cafe'"
msg="Getting client for extensions/v1beta1, Kind=Ingress"
msg="\ngroupResource: ingresses.extensions, namespace: cafe, applicableActions:
[]restore.resolvedAction{restore.resolvedAction{RestoreItemAction:(*clientmgmt.restartableRestoreItemAction)(0xc0007cd2c0), ...
msg="Executing item action for ingresses.extensions"
msg="Skipping restore of Ingress: cafe-ingress because a registered plugin discarded it"

[...clipped...]

msg="Restoring resource 'ingresses.networking.k8s.io' into namespace 'cafe'"
msg="Getting client for networking.k8s.io/v1beta1, Kind=Ingress"
msg="\ngroupResource: ingresses.networking.k8s.io, namespace: cafe, applicableActions: []restore.resolvedAction(nil)" …
msg="Attempting to restore Ingress: cafe-ingress"

[...clipped...]

In kubectl logs deployment/velero -n velero, you can see that discovery helper is unable to retrieve more than one ingresses resource, despite it being in two groups. Disregard line number because I had added lines for more debugging:

msg="===TANZU-MIGRATOR-DEBUG: GVR returned by discovery helper: schema.GroupVersionResource{Group:\"\", Version:\"v1\", Resource:\"nodes\"}" logSource="pkg/discovery/helper.go:118"

Anything else you would like to add:
The reason why discovery helper finds only a single resource when there are two is because of the ResourceFor(...) method in pkg/discovery/helper.go. Although there are multiple resources to be found, only a single one is returned.

func (h *helper) ResourceFor(input schema.GroupVersionResource) (schema.GroupVersionResource, metav1.APIResource, error) {

     [...clipped...]

     gvr, err := h.mapper.ResourceFor(input)
     if err != nil {
           return schema.GroupVersionResource{}, metav1.APIResource{}, err
     }

    [...clipped...]

     h.logger.Infof("===TANZU-MIGRATOR-DEBUG: GVR returned by discovery helper: %#v", gvr)
     return gvr, apiResource, nil
}

I would suggest using k8s.io/apimachinery the plural method, ResourcesFor(...) located in kubernetes/apimachinery instead of the singular version.

I have verified that the plural ResourcesFor(...) method does indeed return multiple resources. Here were the debug lines I added:

func (h *helper) ResourceFor(input schema.GroupVersionResource) (schema.GroupVersionResource, metav1.APIResource, error) {
     [...clipped..]

     h.logger.Infof("===TANZU-MIGRATOR-DEBUG: Discovery helper called for input: %#v", input)
     gvrs, _ := h.mapper.ResourcesFor(input)
     h.logger.Infof("===TANZU-MIGRATOR-DEBUG: GVRs found for input: %#v", gvrs)

     [...clipped..]
}

Here is an excerpt from the deployment logs (disregard line numbers as they have been affected by the additional debugging lines):

msg="===TANZU-MIGRATOR-DEBUG: Discovery helper called for input: schema.GroupVersionResource{Group:\"\", Version:\"\", Resource:\"Ingress\"}" logSource="pkg/discovery/helper.go:109"
msg="===TANZU-MIGRATOR-DEBUG: GVRs found for input: []schema.GroupVersionResource{
     schema.GroupVersionResource{Group:\"extensions\", Version:\"v1beta1\", Resource:\"ingresses\"},
     schema.GroupVersionResource{Group:\"networking.k8s.io\", Version:\"v1beta1\", Resource:\"ingresses\"}
}" logSource="pkg/discovery/helper.go:112"

What baffles me is that ResourceFor(...) (shown below) should have returned an AmbiguousResourceError if the length of the resources slice was greater than 1, which I think, based on the logs, it was for the ingresses resource.

func (m *DefaultRESTMapper) ResourceFor(resource schema.GroupVersionResource) (schema.GroupVersionResource, error) {
	resources, err := m.ResourcesFor(resource)
	if err != nil {
		return schema.GroupVersionResource{}, err
	}
	if len(resources) == 1 {
		return resources[0], nil
	}

	return schema.GroupVersionResource{}, &AmbiguousResourceError{PartialResource: resource, MatchingResources: resources}
}

Environment:

• Velero version (use velero version): 1.5.1
• Velero features (use velero client config get features): features: <NOT SET>
• Kubernetes version (use kubectl version):

Client Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.3", GitCommit:"06ad960bfd03b39c8310aaf92d1e7c12ce618213", GitTreeState:"clean", BuildDate:"2020-02-13T18:08:14Z", GoVersion:"go1.13.8", Compiler:"gc", Platform:"darwin/amd64"}

• Kubernetes installer & version: KinD version v0.9.0
• Cloud provider or hardware configuration: MinIO
• OS (e.g. from /etc/os-release): MacOS Catalina

Vote on this issue!

This is an invitation to the Velero community to vote on issues, you can see the project's top voted issues listed here.
Use the "reaction smiley face" up to the right of this comment to vote.

• 👍 for "I would like to see this bug fixed as soon as possible"
• 👎 for "There are more important bugs to focus on right now"