From b23b4966ad34bf2783dcf2fcebc7479f52e92f6f Mon Sep 17 00:00:00 2001 From: Varun Srinivasan Date: Mon, 22 Jan 2024 18:08:57 -0800 Subject: [PATCH 1/2] Only skip PVC creation if --namespace-mapping flag is not used Signed-off-by: Varun Srinivasan --- pkg/plugin/restore_pvc_action_plugin.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/plugin/restore_pvc_action_plugin.go b/pkg/plugin/restore_pvc_action_plugin.go index b429ddf6..a3676142 100644 --- a/pkg/plugin/restore_pvc_action_plugin.go +++ b/pkg/plugin/restore_pvc_action_plugin.go @@ -3,22 +3,23 @@ package plugin import ( "context" "fmt" + "os" + "github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/cmd" "k8s.io/client-go/kubernetes" - "os" "github.com/pkg/errors" "github.com/sirupsen/logrus" backupdriverv1 "github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/apis/backupdriver/v1alpha1" "github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/backuprepository" "github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/constants" - k8serrors "k8s.io/apimachinery/pkg/api/errors" backupdriverTypedV1 "github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/generated/clientset/versioned/typed/backupdriver/v1alpha1" pluginItem "github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/plugin/util" "github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/snapshotUtils" "github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/utils" "github.com/vmware-tanzu/velero/pkg/plugin/velero" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -176,9 +177,10 @@ func (p *NewPVCRestoreItemAction) Execute(input *velero.RestoreItemActionExecute bSkipPVC, err := pluginItem.SkipPVCCreation(ctx, restConfig, &pvc, p.Log) if err != nil { return nil, errors.WithStack(err) - } else if bSkipPVC { + } else if bSkipPVC && input.Restore.Spec.NamespaceMapping == nil { // Skip PVCRestoreItemAction for PVC creation // as it already exists + // and Restore is not remapping namespace return &velero.RestoreItemActionExecuteOutput{ UpdatedItem: item, }, nil From fc37c68b35e106318f2171419d0dfc445c965b82 Mon Sep 17 00:00:00 2001 From: Varun Srinivasan Date: Wed, 21 Feb 2024 16:32:36 -0800 Subject: [PATCH 2/2] Refactor SkipPVCCreation function signature to add target namespace as a parameter Signed-off-by: Varun Srinivasan --- pkg/plugin/restore_pvc_action_plugin.go | 5 +++-- pkg/plugin/util/util.go | 22 ++++++++++++---------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/pkg/plugin/restore_pvc_action_plugin.go b/pkg/plugin/restore_pvc_action_plugin.go index a3676142..5ff0fc7a 100644 --- a/pkg/plugin/restore_pvc_action_plugin.go +++ b/pkg/plugin/restore_pvc_action_plugin.go @@ -174,10 +174,11 @@ func (p *NewPVCRestoreItemAction) Execute(input *velero.RestoreItemActionExecute return nil, errors.WithStack(err) } - bSkipPVC, err := pluginItem.SkipPVCCreation(ctx, restConfig, &pvc, p.Log) + bSkipPVC, err := pluginItem.SkipPVCCreation(ctx, restConfig, &pvc, targetNamespace, p.Log) if err != nil { return nil, errors.WithStack(err) - } else if bSkipPVC && input.Restore.Spec.NamespaceMapping == nil { + } + if bSkipPVC { // Skip PVCRestoreItemAction for PVC creation // as it already exists // and Restore is not remapping namespace diff --git a/pkg/plugin/util/util.go b/pkg/plugin/util/util.go index b5d96afa..796e411b 100644 --- a/pkg/plugin/util/util.go +++ b/pkg/plugin/util/util.go @@ -5,16 +5,19 @@ import ( "encoding/base64" "encoding/json" "fmt" + "strconv" + "strings" + "github.com/pkg/errors" "github.com/sirupsen/logrus" backupdriverv1 "github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/apis/backupdriver/v1alpha1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" "github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/cmd" "github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/constants" "github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/utils" "github.com/vmware-tanzu/velero/pkg/podvolume" corev1 "k8s.io/api/core/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -22,8 +25,6 @@ import ( "k8s.io/client-go/kubernetes" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" - "strconv" - "strings" ) func GetSnapshotFromPVCAnnotation(snapshotAnnotation string, itemSnapshot interface{}) error { @@ -400,33 +401,34 @@ func IsPVCBackedUpByFsBackup(pvcNamespace, pvcName string, podClient corev1clien // If PVC already exists, it returns true and nil. // If PVC is not found, it returns false and nil to indicate // PVC should be created. -func SkipPVCCreation(ctx context.Context, config *rest.Config, pvc *corev1.PersistentVolumeClaim, logger logrus.FieldLogger) (bool, error) { +func SkipPVCCreation(ctx context.Context, config *rest.Config, pvc *corev1.PersistentVolumeClaim, targetNamespace string, logger logrus.FieldLogger) (bool, error) { if pvc == nil { errMsg := "Input PVC cannot be nil" logger.Error(errMsg) return true, errors.New(errMsg) } - logger.Infof("Check if PVC %s/%s creation should be skipped", pvc.Namespace, pvc.Name) + logger.Infof("Check if PVC %s/%s creation should be skipped", targetNamespace, pvc.Name) kubeClient, err := GetKubeClient(config, logger) if err != nil { logger.Error("Failed to get clientset from given config") return true, err } - getPvc, err := kubeClient.CoreV1().PersistentVolumeClaims(pvc.Namespace).Get(ctx, pvc.Name, metav1.GetOptions{}) + getPvc, err := kubeClient.CoreV1().PersistentVolumeClaims(targetNamespace).Get(ctx, pvc.Name, metav1.GetOptions{}) if err != nil && !apierrs.IsNotFound(err) { - logger.Errorf("Error occurred when trying to get PVC %s/%s: %v", pvc.Namespace, pvc.Name, err) + logger.Errorf("Error occurred when trying to get PVC %s/%s: %v", targetNamespace, pvc.Name, err) return true, err - } else if getPvc != nil && getPvc.Namespace == pvc.Namespace && getPvc.Name == pvc.Name { + } else if getPvc != nil && getPvc.Namespace == targetNamespace && getPvc.Name == pvc.Name { // NOTE: If PVC does not exist, "Get" may return an empty PersistentVolumeClaim object, not nil; so we need to check Namespace/Name match here // NOTE: Need to skip it here in the plugin to avoid hanging as Velero skips restoring already existing resources - logger.Warnf("Skipping PVC %s/%s creation since it already exists.", pvc.Namespace, pvc.Name) + logger.Warnf("Skipping PVC %s/%s creation since it already exists.", targetNamespace, pvc.Name) return true, nil } else if err != nil && apierrs.IsNotFound(err) { - logger.Debugf("PVC %s/%s is not found. Create a new one.", pvc.Namespace, pvc.Name) + logger.Debugf("PVC %s/%s is not found. Create a new one.", targetNamespace, pvc.Name) } // Create a new PVC + logger.Infof("Creating PVC as it does not exist %s/%s", targetNamespace, pvc.Name) return false, nil }