Open phoenix-bjoern opened 3 days ago
What's the version of Velero you are using?
@blackpiglet Velero v1.14.0 as mentioned in the title ;-)
I apologize, I was wrong in regards to the volume policy (maybe I was misguided by the documentation here):
The problem is actually in the shouldIncludeVolumeInBackup
where volume types like hostPath, Secrets and ConfigMaps get excluded, but not emptyDir: https://github.com/vmware-tanzu/velero/blame/c827fd0c6b55aa0068c6d99024c6ca5eca78ffa3/internal/volumehelper/volume_policy_helper.go#L214
The volume policy for the types does not apply as the util method GetPVCForPodVolume
instantly returns an error ( https://github.com/vmware-tanzu/velero/blame/c827fd0c6b55aa0068c6d99024c6ca5eca78ffa3/internal/volumehelper/volume_policy_helper.go#L146) and the method exits.
So, IMHO this is a bug as an emptyDir will never have a PVC. The bug can be resolved by extending shouldIncludeVolumeInBackup
and add smth like this:
// cannot backup emptyDir volumes as they are not mounted into /var/lib/kubelet/pods
// and therefore not accessible to the node agent daemon set.
if vol.EmptyDir != nil {
includeVolumeInBackup = false
}
Also seeing the same errors on 0.14.0 when using a resource policy.
What steps did you take and what happened: Backing up a StatefulSet with an emptyDir attached reports an error during backup:
This is the (shortend) resource YAML:
What did you expect to happen:
The code to extend the volume policies seems to expect a PVC: https://github.com/vmware-tanzu/velero/blob/c827fd0c6b55aa0068c6d99024c6ca5eca78ffa3/pkg/util/kube/pvc_pv.go#L392
Reporting a missing PVC for an emptyDir doesn't make sense. Instead emptyDirs should be skipped by the code.
The following information will help us better understand what's going on:
The new Volume Policy example provides a snippet for emptyDirs: https://velero.io/docs/main/resource-filtering/#yaml-template
So adding this to a Volume Policy resolves the issue:
Maybe it is intended to make this mandatory. But I can not imagine a situation where it makes sense to actually backup an emptyDir. And adding "ConfigMap" is non-sense, as ConfigMaps are resources which are backed up by Velero anyway (if not excluded explicitly).
If the core team decides that every user should add this condition to their VolumePolicies, I'm fine with closing this issue. But maybe this mandatory change should be mentioned in the release notes then. However, I would propose to not throw an error for emptyDir volumes as such scratch volumes are never be expected to be backed up.