From fe14a2c93433eda7ef28912c4e7fd2a6f5fc000a Mon Sep 17 00:00:00 2001 From: Scott Seago Date: Wed, 5 Mar 2025 09:29:05 -0500 Subject: [PATCH] Move pvc annotation removal from CSI RIA to regular PVC RIA Combine existing PVC non-CSI RIAs and move annotation removal out of the CSI plugin to fix issues with CSI volumes when using fs-backup Signed-off-by: Scott Seago --- changelogs/unreleased/8755-sseago | 1 + pkg/cmd/server/plugin/plugin.go | 16 +- pkg/restore/actions/add_pv_from_pvc_action.go | 71 ------ .../actions/add_pv_from_pvc_action_test.go | 103 --------- .../actions/change_pvc_node_selector.go | 167 -------------- pkg/restore/actions/csi/pvc_action.go | 36 +-- pkg/restore/actions/csi/pvc_action_test.go | 92 +------- pkg/restore/actions/pvc_action.go | 213 ++++++++++++++++++ ...de_selector_test.go => pvc_action_test.go} | 164 +++++++++++++- 9 files changed, 383 insertions(+), 480 deletions(-) create mode 100644 changelogs/unreleased/8755-sseago delete mode 100644 pkg/restore/actions/add_pv_from_pvc_action.go delete mode 100644 pkg/restore/actions/add_pv_from_pvc_action_test.go delete mode 100644 pkg/restore/actions/change_pvc_node_selector.go create mode 100644 pkg/restore/actions/pvc_action.go rename pkg/restore/actions/{change_pvc_node_selector_test.go => pvc_action_test.go} (61%) diff --git a/changelogs/unreleased/8755-sseago b/changelogs/unreleased/8755-sseago new file mode 100644 index 000000000..3d0245ab6 --- /dev/null +++ b/changelogs/unreleased/8755-sseago @@ -0,0 +1 @@ +Move pvc annotation removal from CSI RIA to regular PVC RIA diff --git a/pkg/cmd/server/plugin/plugin.go b/pkg/cmd/server/plugin/plugin.go index d25fb54f8..071f0bbe5 100644 --- a/pkg/cmd/server/plugin/plugin.go +++ b/pkg/cmd/server/plugin/plugin.go @@ -90,10 +90,6 @@ func NewCommand(f client.Factory) *cobra.Command { "velero.io/add-pvc-from-pod", newAddPVCFromPodRestoreItemAction, ). - RegisterRestoreItemAction( - "velero.io/add-pv-from-pvc", - newAddPVFromPVCRestoreItemAction, - ). RegisterRestoreItemAction( "velero.io/change-storage-class", newChangeStorageClassRestoreItemAction(f), @@ -115,8 +111,8 @@ func NewCommand(f client.Factory) *cobra.Command { newCRDV1PreserveUnknownFieldsItemAction, ). RegisterRestoreItemAction( - "velero.io/change-pvc-node-selector", - newChangePVCNodeSelectorItemAction(f), + "velero.io/pvc", + newPVCRestoreItemAction(f), ). RegisterRestoreItemAction( "velero.io/apiservice", @@ -308,10 +304,6 @@ func newAddPVCFromPodRestoreItemAction(logger logrus.FieldLogger) (any, error) { return ria.NewAddPVCFromPodAction(logger), nil } -func newAddPVFromPVCRestoreItemAction(logger logrus.FieldLogger) (any, error) { - return ria.NewAddPVFromPVCAction(logger), nil -} - func newCRDV1PreserveUnknownFieldsItemAction(logger logrus.FieldLogger) (any, error) { return ria.NewCRDV1PreserveUnknownFieldsAction(logger), nil } @@ -352,14 +344,14 @@ func newClusterRoleBindingItemAction(logger logrus.FieldLogger) (any, error) { return ria.NewClusterRoleBindingAction(logger), nil } -func newChangePVCNodeSelectorItemAction(f client.Factory) plugincommon.HandlerInitializer { +func newPVCRestoreItemAction(f client.Factory) plugincommon.HandlerInitializer { return func(logger logrus.FieldLogger) (any, error) { client, err := f.KubeClient() if err != nil { return nil, err } - return ria.NewChangePVCNodeSelectorAction( + return ria.NewPVCAction( logger, client.CoreV1().ConfigMaps(f.Namespace()), client.CoreV1().Nodes(), diff --git a/pkg/restore/actions/add_pv_from_pvc_action.go b/pkg/restore/actions/add_pv_from_pvc_action.go deleted file mode 100644 index d597a33fd..000000000 --- a/pkg/restore/actions/add_pv_from_pvc_action.go +++ /dev/null @@ -1,71 +0,0 @@ -/* -Copyright 2019 the Velero contributors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package actions - -import ( - "github.com/pkg/errors" - "github.com/sirupsen/logrus" - corev1api "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" - - "github.com/vmware-tanzu/velero/pkg/kuberesource" - "github.com/vmware-tanzu/velero/pkg/plugin/velero" -) - -type AddPVFromPVCAction struct { - logger logrus.FieldLogger -} - -func NewAddPVFromPVCAction(logger logrus.FieldLogger) *AddPVFromPVCAction { - return &AddPVFromPVCAction{logger: logger} -} - -func (a *AddPVFromPVCAction) AppliesTo() (velero.ResourceSelector, error) { - return velero.ResourceSelector{ - IncludedResources: []string{"persistentvolumeclaims"}, - }, nil -} - -func (a *AddPVFromPVCAction) Execute(input *velero.RestoreItemActionExecuteInput) (*velero.RestoreItemActionExecuteOutput, error) { - a.logger.Info("Executing AddPVFromPVCAction") - - // use input.ItemFromBackup because we need to look at status fields, which have already been - // removed from input.Item - var pvc corev1api.PersistentVolumeClaim - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(input.ItemFromBackup.UnstructuredContent(), &pvc); err != nil { - return nil, errors.Wrap(err, "unable to convert unstructured item to persistent volume claim") - } - - // TODO: consolidate this logic in a helper function to share with backup_pv_action.go - if pvc.Status.Phase != corev1api.ClaimBound || pvc.Spec.VolumeName == "" { - a.logger.Info("PVC is not bound or its volume name is empty") - return &velero.RestoreItemActionExecuteOutput{ - UpdatedItem: input.Item, - }, nil - } - - pv := velero.ResourceIdentifier{ - GroupResource: kuberesource.PersistentVolumes, - Name: pvc.Spec.VolumeName, - } - - a.logger.Infof("Adding PV %s as an additional item to restore", pvc.Spec.VolumeName) - return &velero.RestoreItemActionExecuteOutput{ - UpdatedItem: input.Item, - AdditionalItems: []velero.ResourceIdentifier{pv}, - }, nil -} diff --git a/pkg/restore/actions/add_pv_from_pvc_action_test.go b/pkg/restore/actions/add_pv_from_pvc_action_test.go deleted file mode 100644 index 625b35dbd..000000000 --- a/pkg/restore/actions/add_pv_from_pvc_action_test.go +++ /dev/null @@ -1,103 +0,0 @@ -/* -Copyright 2019 the Velero contributors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package actions - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - - "github.com/vmware-tanzu/velero/pkg/kuberesource" - "github.com/vmware-tanzu/velero/pkg/plugin/velero" - velerotest "github.com/vmware-tanzu/velero/pkg/test" -) - -func TestAddPVFromPVCActionExecute(t *testing.T) { - tests := []struct { - name string - itemFromBackup *v1.PersistentVolumeClaim - want []velero.ResourceIdentifier - }{ - { - name: "bound PVC with volume name returns associated PV", - itemFromBackup: &v1.PersistentVolumeClaim{ - Spec: v1.PersistentVolumeClaimSpec{ - VolumeName: "bound-pv", - }, - Status: v1.PersistentVolumeClaimStatus{ - Phase: v1.ClaimBound, - }, - }, - want: []velero.ResourceIdentifier{ - { - GroupResource: kuberesource.PersistentVolumes, - Name: "bound-pv", - }, - }, - }, - { - name: "unbound PVC with volume name does not return any additional items", - itemFromBackup: &v1.PersistentVolumeClaim{ - Spec: v1.PersistentVolumeClaimSpec{ - VolumeName: "pending-pv", - }, - Status: v1.PersistentVolumeClaimStatus{ - Phase: v1.ClaimPending, - }, - }, - want: nil, - }, - { - name: "bound PVC without volume name does not return any additional items", - itemFromBackup: &v1.PersistentVolumeClaim{ - Spec: v1.PersistentVolumeClaimSpec{}, - Status: v1.PersistentVolumeClaimStatus{ - Phase: v1.ClaimBound, - }, - }, - want: nil, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - itemFromBackupData, err := runtime.DefaultUnstructuredConverter.ToUnstructured(test.itemFromBackup) - require.NoError(t, err) - - itemData, err := runtime.DefaultUnstructuredConverter.ToUnstructured(test.itemFromBackup) - require.NoError(t, err) - // item should have no status - delete(itemData, "status") - - action := &AddPVFromPVCAction{logger: velerotest.NewLogger()} - - input := &velero.RestoreItemActionExecuteInput{ - Item: &unstructured.Unstructured{Object: itemData}, - ItemFromBackup: &unstructured.Unstructured{Object: itemFromBackupData}, - } - - res, err := action.Execute(input) - require.NoError(t, err) - - assert.Equal(t, test.want, res.AdditionalItems) - }) - } -} diff --git a/pkg/restore/actions/change_pvc_node_selector.go b/pkg/restore/actions/change_pvc_node_selector.go deleted file mode 100644 index 64d5ceb3f..000000000 --- a/pkg/restore/actions/change_pvc_node_selector.go +++ /dev/null @@ -1,167 +0,0 @@ -/* -Copyright 2020 the Velero contributors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package actions - -import ( - "context" - - "github.com/pkg/errors" - "github.com/sirupsen/logrus" - k8serrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - corev1client "k8s.io/client-go/kubernetes/typed/core/v1" - - "github.com/vmware-tanzu/velero/pkg/plugin/framework/common" - "github.com/vmware-tanzu/velero/pkg/plugin/velero" -) - -// ChangePVCNodeSelectorAction updates/reset PVC's node selector -// if a mapping is found in the plugin's config map. -type ChangePVCNodeSelectorAction struct { - logger logrus.FieldLogger - configMapClient corev1client.ConfigMapInterface - nodeClient corev1client.NodeInterface -} - -// NewChangePVCNodeSelectorAction is the constructor for ChangePVCNodeSelectorAction. -func NewChangePVCNodeSelectorAction( - logger logrus.FieldLogger, - configMapClient corev1client.ConfigMapInterface, - nodeClient corev1client.NodeInterface, -) *ChangePVCNodeSelectorAction { - return &ChangePVCNodeSelectorAction{ - logger: logger, - configMapClient: configMapClient, - nodeClient: nodeClient, - } -} - -// AppliesTo returns the resources that ChangePVCNodeSelectorAction should be run for -func (p *ChangePVCNodeSelectorAction) AppliesTo() (velero.ResourceSelector, error) { - return velero.ResourceSelector{ - IncludedResources: []string{"persistentvolumeclaims"}, - }, nil -} - -// Execute updates the pvc's selected-node annotation: -// -// a) if node mapping found in the config map for the plugin -// b) if node mentioned in annotation doesn't exist -func (p *ChangePVCNodeSelectorAction) Execute(input *velero.RestoreItemActionExecuteInput) (*velero.RestoreItemActionExecuteOutput, error) { - p.logger.Info("Executing ChangePVCNodeSelectorAction") - defer p.logger.Info("Done executing ChangePVCNodeSelectorAction") - - typeAcc, err := meta.TypeAccessor(input.Item) - if err != nil { - return &velero.RestoreItemActionExecuteOutput{}, err - } - - metadata, err := meta.Accessor(input.Item) - if err != nil { - return &velero.RestoreItemActionExecuteOutput{}, err - } - - annotations := metadata.GetAnnotations() - if annotations == nil { - return velero.NewRestoreItemActionExecuteOutput(input.Item), nil - } - - log := p.logger.WithFields(map[string]any{ - "kind": typeAcc.GetKind(), - "namespace": metadata.GetNamespace(), - "name": metadata.GetName(), - }) - - // let's check if PVC has annotation of the selected node - node, ok := annotations["volume.kubernetes.io/selected-node"] - if !ok { - log.Debug("PVC doesn't have node selector") - return velero.NewRestoreItemActionExecuteOutput(input.Item), nil - } - - // fetch node mapping from configMap - newNode, err := getNewNodeFromConfigMap(p.configMapClient, node) - if err != nil { - return nil, err - } - - if len(newNode) != 0 { - // Check whether the mapped node exists first. - exists, err := isNodeExist(p.nodeClient, newNode) - if err != nil { - return nil, errors.Wrapf(err, "error checking %s's mapped node %s existence", node, newNode) - } - if !exists { - log.Warnf("Selected-node's mapped node doesn't exist: source: %s, dest: %s. Please check the ConfigMap with label velero.io/change-pvc-node-selector.", node, newNode) - } - - // set node selector - // We assume that node exist for node-mapping - annotations["volume.kubernetes.io/selected-node"] = newNode - metadata.SetAnnotations(annotations) - log.Infof("Updating selected-node to %s from %s", newNode, node) - return velero.NewRestoreItemActionExecuteOutput(input.Item), nil - } - - // configMap doesn't have node-mapping - // Let's check if node exists or not - exists, err := isNodeExist(p.nodeClient, node) - if err != nil { - return nil, errors.Wrapf(err, "error checking node %s existence", node) - } - - if !exists { - log.Infof("Clearing selected-node because node named %s does not exist", node) - delete(annotations, "volume.kubernetes.io/selected-node") - if len(annotations) == 0 { - metadata.SetAnnotations(nil) - } else { - metadata.SetAnnotations(annotations) - } - } - - return velero.NewRestoreItemActionExecuteOutput(input.Item), nil -} - -func getNewNodeFromConfigMap(client corev1client.ConfigMapInterface, node string) (string, error) { - // fetch node mapping from configMap - config, err := common.GetPluginConfig(common.PluginKindRestoreItemAction, "velero.io/change-pvc-node-selector", client) - if err != nil { - return "", err - } - - if config == nil { - // there is no node mapping defined for change-pvc-node - // so we will return empty new node - return "", nil - } - - return config.Data[node], nil -} - -// isNodeExist check if node resource exist or not -func isNodeExist(nodeClient corev1client.NodeInterface, name string) (bool, error) { - _, err := nodeClient.Get(context.TODO(), name, metav1.GetOptions{}) - if err != nil { - if k8serrors.IsNotFound(err) { - return false, nil - } - return false, err - } - return true, nil -} diff --git a/pkg/restore/actions/csi/pvc_action.go b/pkg/restore/actions/csi/pvc_action.go index 9efd3649c..dd7893ff0 100644 --- a/pkg/restore/actions/csi/pvc_action.go +++ b/pkg/restore/actions/csi/pvc_action.go @@ -46,14 +46,7 @@ import ( ) const ( - AnnBindCompleted = "pv.kubernetes.io/bind-completed" - AnnBoundByController = "pv.kubernetes.io/bound-by-controller" - AnnStorageProvisioner = "volume.kubernetes.io/storage-provisioner" - AnnBetaStorageProvisioner = "volume.beta.kubernetes.io/storage-provisioner" - AnnSelectedNode = "volume.kubernetes.io/selected-node" -) - -const ( + AnnSelectedNode = "volume.kubernetes.io/selected-node" GenerateNameRandomLength = 5 ) @@ -72,18 +65,6 @@ func (p *pvcRestoreItemAction) AppliesTo() (velero.ResourceSelector, error) { }, nil } -func removePVCAnnotations(pvc *corev1api.PersistentVolumeClaim, remove []string) { - if pvc.Annotations == nil { - pvc.Annotations = make(map[string]string) - return - } - for k := range pvc.Annotations { - if util.Contains(remove, k) { - delete(pvc.Annotations, k) - } - } -} - func resetPVCSpec(pvc *corev1api.PersistentVolumeClaim, vsName string) { // Restore operation for the PVC will use the VolumeSnapshot as the data source. // So clear out the volume name, which is a ref to the PV @@ -148,21 +129,6 @@ func (p *pvcRestoreItemAction) Execute( }, nil } - // remove the VolumeSnapshot name annotation as well - // clean the DataUploadNameLabel for snapshot data mover case. - removePVCAnnotations( - &pvc, - []string{ - AnnBindCompleted, - AnnBoundByController, - AnnStorageProvisioner, - AnnBetaStorageProvisioner, - AnnSelectedNode, - velerov1api.VolumeSnapshotLabel, - velerov1api.DataUploadNameAnnotation, - }, - ) - // If cross-namespace restore is configured, change the namespace // for PVC object to be restored newNamespace, ok := input.Restore.Spec.NamespaceMapping[pvc.GetNamespace()] diff --git a/pkg/restore/actions/csi/pvc_action_test.go b/pkg/restore/actions/csi/pvc_action_test.go index a3b78a064..224f18859 100644 --- a/pkg/restore/actions/csi/pvc_action_test.go +++ b/pkg/restore/actions/csi/pvc_action_test.go @@ -49,92 +49,6 @@ import ( "github.com/vmware-tanzu/velero/pkg/util/boolptr" ) -func TestRemovePVCAnnotations(t *testing.T) { - testCases := []struct { - name string - pvc corev1api.PersistentVolumeClaim - removeAnnotations []string - expectedAnnotations map[string]string - }{ - { - name: "should create empty annotation map", - pvc: corev1api.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: nil, - }, - }, - removeAnnotations: []string{"foo"}, - expectedAnnotations: map[string]string{}, - }, - { - name: "should preserve all existing annotations", - pvc: corev1api.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - "ann1": "ann1-val", - "ann2": "ann2-val", - "ann3": "ann3-val", - "ann4": "ann4-val", - }, - }, - }, - removeAnnotations: []string{}, - expectedAnnotations: map[string]string{ - "ann1": "ann1-val", - "ann2": "ann2-val", - "ann3": "ann3-val", - "ann4": "ann4-val", - }, - }, - { - name: "should remove all existing annotations", - pvc: corev1api.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - "ann1": "ann1-val", - "ann2": "ann2-val", - "ann3": "ann3-val", - "ann4": "ann4-val", - }, - }, - }, - removeAnnotations: []string{"ann1", "ann2", "ann3", "ann4"}, - expectedAnnotations: map[string]string{}, - }, - { - name: "should preserve some existing annotations", - pvc: corev1api.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - "ann1": "ann1-val", - "ann2": "ann2-val", - "ann3": "ann3-val", - "ann4": "ann4-val", - "ann5": "ann5-val", - "ann6": "ann6-val", - "ann7": "ann7-val", - "ann8": "ann8-val", - }, - }, - }, - removeAnnotations: []string{"ann1", "ann2", "ann3", "ann4"}, - expectedAnnotations: map[string]string{ - "ann5": "ann5-val", - "ann6": "ann6-val", - "ann7": "ann7-val", - "ann8": "ann8-val", - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - removePVCAnnotations(&tc.pvc, tc.removeAnnotations) - assert.Equal(t, tc.expectedAnnotations, tc.pvc.Annotations) - }) - } -} - func TestResetPVCSpec(t *testing.T) { fileMode := corev1api.PersistentVolumeFilesystem blockMode := corev1api.PersistentVolumeBlock @@ -591,7 +505,7 @@ func TestExecute(t *testing.T) { vs: builder.ForVolumeSnapshot("velero", vsName).ObjectMeta( builder.WithAnnotations(velerov1api.VolumeSnapshotRestoreSize, "10Gi"), ).Result(), - expectedPVC: builder.ForPersistentVolumeClaim("velero", "testPVC").Result(), + expectedPVC: builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(velerov1api.VolumeSnapshotLabel, "vsName")).Result(), }, { name: "Restore from VolumeSnapshot without volume-snapshot-name annotation", @@ -615,7 +529,7 @@ func TestExecute(t *testing.T) { restore: builder.ForRestore("velero", "testRestore").Backup("testBackup").ObjectMeta(builder.WithUID("uid")).Result(), pvc: builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(velerov1api.VolumeSnapshotRestoreSize, "10Gi", velerov1api.DataUploadNameAnnotation, "velero/")).Result(), dataUploadResult: builder.ForConfigMap("velero", "testCM").Data("uid", "{}").ObjectMeta(builder.WithLabels(velerov1api.RestoreUIDLabel, "uid", velerov1api.PVCNamespaceNameLabel, "velero.testPVC", velerov1api.ResourceUsageLabel, label.GetValidName(string(velerov1api.VeleroResourceUsageDataUploadResult)))).Result(), - expectedPVC: builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations("velero.io/csi-volumesnapshot-restore-size", "10Gi")).Result(), + expectedPVC: builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations("velero.io/csi-volumesnapshot-restore-size", "10Gi", velerov1api.DataUploadNameAnnotation, "velero/")).Result(), expectedDataDownload: builder.ForDataDownload("velero", "name").TargetVolume(velerov2alpha1.TargetVolumeSpec{PVC: "testPVC", Namespace: "velero"}). ObjectMeta(builder.WithOwnerReference([]metav1.OwnerReference{{APIVersion: velerov1api.SchemeGroupVersion.String(), Kind: "Restore", Name: "testRestore", UID: "uid", Controller: boolptr.True()}}), builder.WithLabelsMap(map[string]string{velerov1api.AsyncOperationIDLabel: "dd-uid.", velerov1api.RestoreNameLabel: "testRestore", velerov1api.RestoreUIDLabel: "uid"}), @@ -627,7 +541,7 @@ func TestExecute(t *testing.T) { restore: builder.ForRestore("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "testRestore").Backup("testBackup").ObjectMeta(builder.WithUID("uid")).Result(), pvc: builder.ForPersistentVolumeClaim("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "kibishii-data-kibishii-deployment-0").ObjectMeta(builder.WithAnnotations(velerov1api.VolumeSnapshotRestoreSize, "10Gi", velerov1api.DataUploadNameAnnotation, "velero/")).Result(), dataUploadResult: builder.ForConfigMap("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "testCM").Data("uid", "{}").ObjectMeta(builder.WithLabels(velerov1api.RestoreUIDLabel, "uid", velerov1api.PVCNamespaceNameLabel, "migre209d0da-49c7-45ba-8d5a-3e59fd591ec1.kibishii-data-ki152333", velerov1api.ResourceUsageLabel, label.GetValidName(string(velerov1api.VeleroResourceUsageDataUploadResult)))).Result(), - expectedPVC: builder.ForPersistentVolumeClaim("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "kibishii-data-kibishii-deployment-0").ObjectMeta(builder.WithAnnotations("velero.io/csi-volumesnapshot-restore-size", "10Gi")).Result(), + expectedPVC: builder.ForPersistentVolumeClaim("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "kibishii-data-kibishii-deployment-0").ObjectMeta(builder.WithAnnotations("velero.io/csi-volumesnapshot-restore-size", "10Gi", velerov1api.DataUploadNameAnnotation, "velero/")).Result(), }, { name: "PVC had no DataUploadNameLabel annotation", diff --git a/pkg/restore/actions/pvc_action.go b/pkg/restore/actions/pvc_action.go new file mode 100644 index 000000000..9bf1af2b5 --- /dev/null +++ b/pkg/restore/actions/pvc_action.go @@ -0,0 +1,213 @@ +/* +Copyright the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package actions + +import ( + "context" + + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + corev1api "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" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/kuberesource" + "github.com/vmware-tanzu/velero/pkg/plugin/framework/common" + "github.com/vmware-tanzu/velero/pkg/plugin/velero" + "github.com/vmware-tanzu/velero/pkg/util" +) + +const ( + AnnBindCompleted = "pv.kubernetes.io/bind-completed" + AnnBoundByController = "pv.kubernetes.io/bound-by-controller" + AnnStorageProvisioner = "volume.kubernetes.io/storage-provisioner" + AnnBetaStorageProvisioner = "volume.beta.kubernetes.io/storage-provisioner" + AnnSelectedNode = "volume.kubernetes.io/selected-node" +) + +// PVCAction updates/reset PVC's node selector +// if a mapping is found in the plugin's config map. +type PVCAction struct { + logger logrus.FieldLogger + configMapClient corev1client.ConfigMapInterface + nodeClient corev1client.NodeInterface +} + +// NewPVCAction is the constructor for PVCAction. +func NewPVCAction( + logger logrus.FieldLogger, + configMapClient corev1client.ConfigMapInterface, + nodeClient corev1client.NodeInterface, +) *PVCAction { + return &PVCAction{ + logger: logger, + configMapClient: configMapClient, + nodeClient: nodeClient, + } +} + +// AppliesTo returns the resources that PVCAction should be run for +func (p *PVCAction) AppliesTo() (velero.ResourceSelector, error) { + return velero.ResourceSelector{ + IncludedResources: []string{"persistentvolumeclaims"}, + }, nil +} + +// PVC actions for restore: +// 1. updates the pvc's selected-node annotation: +// a) if node mapping found in the config map for the plugin +// b) if node mentioned in annotation doesn't exist +// 2. removes some additional annotations +// 3. returns bound PV as an additional item +func (p *PVCAction) Execute(input *velero.RestoreItemActionExecuteInput) (*velero.RestoreItemActionExecuteOutput, error) { + p.logger.Info("Executing PVCAction") + defer p.logger.Info("Done executing PVCAction") + + var pvc, pvcFromBackup corev1api.PersistentVolumeClaim + if err := runtime.DefaultUnstructuredConverter.FromUnstructured( + input.Item.UnstructuredContent(), &pvc); err != nil { + return nil, errors.WithStack(err) + } + if err := runtime.DefaultUnstructuredConverter.FromUnstructured( + input.ItemFromBackup.UnstructuredContent(), &pvcFromBackup); err != nil { + return nil, errors.WithStack(err) + } + + if pvc.Annotations == nil { + pvc.Annotations = make(map[string]string) + } + + log := p.logger.WithFields(map[string]any{ + "kind": pvc.Kind, + "namespace": pvc.Namespace, + "name": pvc.Name, + }) + + // Handle selected node annotation + node, ok := pvc.Annotations[AnnSelectedNode] + if ok { + // fetch node mapping from configMap + newNode, err := getNewNodeFromConfigMap(p.configMapClient, node) + if err != nil { + return nil, err + } + + if len(newNode) != 0 { + // Check whether the mapped node exists first. + exists, err := isNodeExist(p.nodeClient, newNode) + if err != nil { + return nil, errors.Wrapf(err, "error checking %s's mapped node %s existence", node, newNode) + } + if !exists { + log.Warnf("Selected-node's mapped node doesn't exist: source: %s, dest: %s. Please check the ConfigMap with label velero.io/change-pvc-node-selector.", node, newNode) + } + + // set node selector + // We assume that node exist for node-mapping + pvc.Annotations[AnnSelectedNode] = newNode + log.Infof("Updating selected-node to %s from %s", newNode, node) + } else { + // configMap doesn't have node-mapping + // Let's check if node exists or not + exists, err := isNodeExist(p.nodeClient, node) + if err != nil { + return nil, errors.Wrapf(err, "error checking node %s existence", node) + } + + if !exists { + log.Infof("Clearing selected-node because node named %s does not exist", node) + delete(pvc.Annotations, AnnSelectedNode) + } + } + } + + // Remove other annotations + removePVCAnnotations( + &pvc, + []string{ + AnnBindCompleted, + AnnBoundByController, + AnnStorageProvisioner, + AnnBetaStorageProvisioner, + velerov1api.VolumeSnapshotLabel, + velerov1api.DataUploadNameAnnotation, + }, + ) + + pvcMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&pvc) + if err != nil { + return nil, errors.WithStack(err) + } + output := &velero.RestoreItemActionExecuteOutput{ + UpdatedItem: &unstructured.Unstructured{Object: pvcMap}, + } + + // Add PV as additional item if bound + // use pvcFromBackup because we need to look at status fields, which have been removed from pvc + if pvcFromBackup.Status.Phase != corev1api.ClaimBound || pvcFromBackup.Spec.VolumeName == "" { + log.Info("PVC is not bound or its volume name is empty") + } else { + log.Infof("Adding PV %s as an additional item to restore", pvcFromBackup.Spec.VolumeName) + output.AdditionalItems = []velero.ResourceIdentifier{ + { + GroupResource: kuberesource.PersistentVolumes, + Name: pvcFromBackup.Spec.VolumeName, + }, + } + } + return output, nil +} + +func getNewNodeFromConfigMap(client corev1client.ConfigMapInterface, node string) (string, error) { + // fetch node mapping from configMap + config, err := common.GetPluginConfig(common.PluginKindRestoreItemAction, "velero.io/change-pvc-node-selector", client) + if err != nil { + return "", err + } + + if config == nil { + // there is no node mapping defined for change-pvc-node + // so we will return empty new node + return "", nil + } + + return config.Data[node], nil +} + +// isNodeExist check if node resource exist or not +func isNodeExist(nodeClient corev1client.NodeInterface, name string) (bool, error) { + _, err := nodeClient.Get(context.TODO(), name, metav1.GetOptions{}) + if err != nil { + if k8serrors.IsNotFound(err) { + return false, nil + } + return false, err + } + return true, nil +} + +func removePVCAnnotations(pvc *corev1api.PersistentVolumeClaim, remove []string) { + for k := range pvc.Annotations { + if util.Contains(remove, k) { + delete(pvc.Annotations, k) + } + } +} diff --git a/pkg/restore/actions/change_pvc_node_selector_test.go b/pkg/restore/actions/pvc_action_test.go similarity index 61% rename from pkg/restore/actions/change_pvc_node_selector_test.go rename to pkg/restore/actions/pvc_action_test.go index eee2bca09..b5928cbcd 100644 --- a/pkg/restore/actions/change_pvc_node_selector_test.go +++ b/pkg/restore/actions/pvc_action_test.go @@ -32,14 +32,16 @@ import ( "k8s.io/client-go/kubernetes/fake" "github.com/vmware-tanzu/velero/pkg/builder" + "github.com/vmware-tanzu/velero/pkg/kuberesource" "github.com/vmware-tanzu/velero/pkg/plugin/velero" + velerotest "github.com/vmware-tanzu/velero/pkg/test" ) -// TestChangePVCNodeSelectorActionExecute runs the ChangePVCNodeSelectorAction's Execute +// TestPVCActionExecute runs the PVCAction's Execute // method and validates that the item's PVC is modified (or not) as expected. // Validation is done by comparing the result of the Execute method to the test case's // desired result. -func TestChangePVCNodeSelectorActionExecute(t *testing.T) { +func TestPVCActionExecute(t *testing.T) { tests := []struct { name string pvc *corev1api.PersistentVolumeClaim @@ -133,7 +135,7 @@ func TestChangePVCNodeSelectorActionExecute(t *testing.T) { logger := logrus.StandardLogger() buf := bytes.Buffer{} logrus.SetOutput(&buf) - a := NewChangePVCNodeSelectorAction( + a := NewPVCAction( logger, clientset.CoreV1().ConfigMaps("velero"), clientset.CoreV1().Nodes(), @@ -160,6 +162,9 @@ func TestChangePVCNodeSelectorActionExecute(t *testing.T) { Item: &unstructured.Unstructured{ Object: unstructuredMap, }, + ItemFromBackup: &unstructured.Unstructured{ + Object: unstructuredMap, + }, } // execute method under test @@ -186,3 +191,156 @@ func TestChangePVCNodeSelectorActionExecute(t *testing.T) { }) } } + +func TestAddPVFromPVCActionExecute(t *testing.T) { + tests := []struct { + name string + itemFromBackup *corev1api.PersistentVolumeClaim + want []velero.ResourceIdentifier + }{ + { + name: "bound PVC with volume name returns associated PV", + itemFromBackup: &corev1api.PersistentVolumeClaim{ + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeName: "bound-pv", + }, + Status: corev1api.PersistentVolumeClaimStatus{ + Phase: corev1api.ClaimBound, + }, + }, + want: []velero.ResourceIdentifier{ + { + GroupResource: kuberesource.PersistentVolumes, + Name: "bound-pv", + }, + }, + }, + { + name: "unbound PVC with volume name does not return any additional items", + itemFromBackup: &corev1api.PersistentVolumeClaim{ + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeName: "pending-pv", + }, + Status: corev1api.PersistentVolumeClaimStatus{ + Phase: corev1api.ClaimPending, + }, + }, + want: nil, + }, + { + name: "bound PVC without volume name does not return any additional items", + itemFromBackup: &corev1api.PersistentVolumeClaim{ + Spec: corev1api.PersistentVolumeClaimSpec{}, + Status: corev1api.PersistentVolumeClaimStatus{ + Phase: corev1api.ClaimBound, + }, + }, + want: nil, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + itemFromBackupData, err := runtime.DefaultUnstructuredConverter.ToUnstructured(test.itemFromBackup) + require.NoError(t, err) + + itemData, err := runtime.DefaultUnstructuredConverter.ToUnstructured(test.itemFromBackup) + require.NoError(t, err) + // item should have no status + delete(itemData, "status") + + clientset := fake.NewSimpleClientset() + action := NewPVCAction( + velerotest.NewLogger(), + clientset.CoreV1().ConfigMaps("velero"), + clientset.CoreV1().Nodes(), + ) + + input := &velero.RestoreItemActionExecuteInput{ + Item: &unstructured.Unstructured{Object: itemData}, + ItemFromBackup: &unstructured.Unstructured{Object: itemFromBackupData}, + } + + res, err := action.Execute(input) + require.NoError(t, err) + + assert.Equal(t, test.want, res.AdditionalItems) + }) + } +} + +func TestRemovePVCAnnotations(t *testing.T) { + testCases := []struct { + name string + pvc corev1api.PersistentVolumeClaim + removeAnnotations []string + expectedAnnotations map[string]string + }{ + { + name: "should preserve all existing annotations", + pvc: corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "ann1": "ann1-val", + "ann2": "ann2-val", + "ann3": "ann3-val", + "ann4": "ann4-val", + }, + }, + }, + removeAnnotations: []string{}, + expectedAnnotations: map[string]string{ + "ann1": "ann1-val", + "ann2": "ann2-val", + "ann3": "ann3-val", + "ann4": "ann4-val", + }, + }, + { + name: "should remove all existing annotations", + pvc: corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "ann1": "ann1-val", + "ann2": "ann2-val", + "ann3": "ann3-val", + "ann4": "ann4-val", + }, + }, + }, + removeAnnotations: []string{"ann1", "ann2", "ann3", "ann4"}, + expectedAnnotations: map[string]string{}, + }, + { + name: "should preserve some existing annotations", + pvc: corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "ann1": "ann1-val", + "ann2": "ann2-val", + "ann3": "ann3-val", + "ann4": "ann4-val", + "ann5": "ann5-val", + "ann6": "ann6-val", + "ann7": "ann7-val", + "ann8": "ann8-val", + }, + }, + }, + removeAnnotations: []string{"ann1", "ann2", "ann3", "ann4"}, + expectedAnnotations: map[string]string{ + "ann5": "ann5-val", + "ann6": "ann6-val", + "ann7": "ann7-val", + "ann8": "ann8-val", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + removePVCAnnotations(&tc.pvc, tc.removeAnnotations) + assert.Equal(t, tc.expectedAnnotations, tc.pvc.Annotations) + }) + } +}