diff --git a/pkg/restore/actions/pvc_action.go b/pkg/restore/actions/pvc_action.go index fdc0c26ec..a4a63374d 100644 --- a/pkg/restore/actions/pvc_action.go +++ b/pkg/restore/actions/pvc_action.go @@ -17,20 +17,15 @@ limitations under the License. package actions import ( - "context" - "github.com/pkg/errors" "github.com/sirupsen/logrus" corev1api "k8s.io/api/core/v1" - apierrors "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" ) @@ -91,46 +86,13 @@ func (p *PVCAction) Execute(input *velero.RestoreItemActionExecuteInput) (*veler 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 { - log.Info("Clearing PVC selected-node annotation") - delete(pvc.Annotations, AnnSelectedNode) - } - } - - // Remove other annotations + // Remove PVC annotations removePVCAnnotations( &pvc, []string{ @@ -138,6 +100,7 @@ func (p *PVCAction) Execute(input *velero.RestoreItemActionExecuteInput) (*veler AnnBoundByController, AnnStorageProvisioner, AnnBetaStorageProvisioner, + AnnSelectedNode, velerov1api.VolumeSnapshotLabel, velerov1api.DataUploadNameAnnotation, }, @@ -167,34 +130,6 @@ func (p *PVCAction) Execute(input *velero.RestoreItemActionExecuteInput) (*veler 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 apierrors.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) { diff --git a/pkg/restore/actions/pvc_action_test.go b/pkg/restore/actions/pvc_action_test.go index f00ec9264..aa2ddeeef 100644 --- a/pkg/restore/actions/pvc_action_test.go +++ b/pkg/restore/actions/pvc_action_test.go @@ -17,11 +17,9 @@ limitations under the License. package actions import ( - "bytes" "fmt" "testing" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1api "k8s.io/api/core/v1" @@ -42,105 +40,57 @@ import ( // desired result. func TestPVCActionExecute(t *testing.T) { tests := []struct { - name string - pvc *corev1api.PersistentVolumeClaim - configMap *corev1api.ConfigMap - node *corev1api.Node - newNode *corev1api.Node - want *corev1api.PersistentVolumeClaim - wantErr error + name string + pvc *corev1api.PersistentVolumeClaim + want *corev1api.PersistentVolumeClaim + wantErr error }{ { - name: "a valid mapping for a persistent volume claim is applied correctly", - pvc: builder.ForPersistentVolumeClaim("source-ns", "pvc-1"). - ObjectMeta( - builder.WithAnnotations("volume.kubernetes.io/selected-node", "source-node"), - ).Result(), - configMap: builder.ForConfigMap("velero", "change-pvc-node"). - ObjectMeta(builder.WithLabels("velero.io/plugin-config", "", "velero.io/change-pvc-node-selector", "RestoreItemAction")). - Data("source-node", "dest-node"). - Result(), - newNode: builder.ForNode("dest-node").Result(), - want: builder.ForPersistentVolumeClaim("source-ns", "pvc-1"). - ObjectMeta( - builder.WithAnnotations("volume.kubernetes.io/selected-node", "dest-node"), - ).Result(), - }, - { - name: "when no config map exists for the plugin, the item is returned without node selector", - pvc: builder.ForPersistentVolumeClaim("source-ns", "pvc-1"). - ObjectMeta( - builder.WithAnnotations("volume.kubernetes.io/selected-node", "source-node"), - ).Result(), - configMap: builder.ForConfigMap("velero", "change-pvc-node"). - ObjectMeta(builder.WithLabels("velero.io/plugin-config", "", "velero.io/some-other-plugin", "RestoreItemAction")). - Data("source-node", "dest-node"). - Result(), - node: builder.ForNode("source-node").Result(), - want: builder.ForPersistentVolumeClaim("source-ns", "pvc-1").Result(), - }, - { - name: "when no node-mappings exist in the plugin config map, the item is returned without node selector", - pvc: builder.ForPersistentVolumeClaim("source-ns", "pvc-1"). - ObjectMeta( - builder.WithAnnotations("volume.kubernetes.io/selected-node", "source-node"), - ).Result(), - configMap: builder.ForConfigMap("velero", "change-pvc-node"). - ObjectMeta(builder.WithLabels("velero.io/plugin-config", "", "velero.io/change-pvc-node-selector", "RestoreItemAction")). - Result(), - node: builder.ForNode("source-node").Result(), - want: builder.ForPersistentVolumeClaim("source-ns", "pvc-1").Result(), - }, - { - name: "when persistent volume claim has no node selector, the item is returned as-is", + name: "a persistent volume claim with no annotation", pvc: builder.ForPersistentVolumeClaim("source-ns", "pvc-1").Result(), - configMap: builder.ForConfigMap("velero", "change-pvc-node"). - ObjectMeta(builder.WithLabels("velero.io/plugin-config", "", "velero.io/change-pvc-node-selector", "RestoreItemAction")). - Data("source-node", "dest-node"). - Result(), want: builder.ForPersistentVolumeClaim("source-ns", "pvc-1").Result(), }, { - name: "when persistent volume claim's node-selector has no mapping in the config map, the item is returned without node selector", + name: "a persistent volume claim with selected-node annotation", pvc: builder.ForPersistentVolumeClaim("source-ns", "pvc-1"). ObjectMeta( builder.WithAnnotations("volume.kubernetes.io/selected-node", "source-node"), ).Result(), - configMap: builder.ForConfigMap("velero", "change-pvc-node"). - ObjectMeta(builder.WithLabels("velero.io/plugin-config", "", "velero.io/change-pvc-node-selector", "RestoreItemAction")). - Data("source-node-1", "dest-node"). - Result(), - node: builder.ForNode("source-node").Result(), - want: builder.ForPersistentVolumeClaim("source-ns", "pvc-1").Result(), + want: builder.ForPersistentVolumeClaim("source-ns", "pvc-1").ObjectMeta(builder.WithAnnotationsMap(map[string]string{})).Result(), + }, + { + name: "a persistent volume claim with other annotation", + pvc: builder.ForPersistentVolumeClaim("source-ns", "pvc-1"). + ObjectMeta( + builder.WithAnnotations("other-anno-1", "other-value-1", "other-anno-2", "other-value-2"), + ).Result(), + want: builder.ForPersistentVolumeClaim("source-ns", "pvc-1").ObjectMeta( + builder.WithAnnotations("other-anno-1", "other-value-1", "other-anno-2", "other-value-2"), + ).Result(), + }, + { + name: "a persistent volume claim with other annotation and selected-node annotation", + pvc: builder.ForPersistentVolumeClaim("source-ns", "pvc-1"). + ObjectMeta( + builder.WithAnnotations("other-anno", "other-value", "volume.kubernetes.io/selected-node", "source-node"), + ).Result(), + want: builder.ForPersistentVolumeClaim("source-ns", "pvc-1").ObjectMeta( + builder.WithAnnotations("other-anno", "other-value"), + ).Result(), }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { clientset := fake.NewSimpleClientset() - logger := logrus.StandardLogger() - buf := bytes.Buffer{} - logrus.SetOutput(&buf) + a := NewPVCAction( - logger, + velerotest.NewLogger(), clientset.CoreV1().ConfigMaps("velero"), clientset.CoreV1().Nodes(), ) // set up test data - if tc.configMap != nil { - _, err := clientset.CoreV1().ConfigMaps(tc.configMap.Namespace).Create(t.Context(), tc.configMap, metav1.CreateOptions{}) - require.NoError(t, err) - } - - if tc.node != nil { - _, err := clientset.CoreV1().Nodes().Create(t.Context(), tc.node, metav1.CreateOptions{}) - require.NoError(t, err) - } - if tc.newNode != nil { - _, err := clientset.CoreV1().Nodes().Create(t.Context(), tc.newNode, metav1.CreateOptions{}) - require.NoError(t, err) - } unstructuredMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(tc.pvc) require.NoError(t, err) @@ -156,10 +106,6 @@ func TestPVCActionExecute(t *testing.T) { // execute method under test res, err := a.Execute(input) - // Make sure mapped selected-node exists. - logOutput := buf.String() - assert.NotContains(t, logOutput, "Selected-node's mapped node doesn't exist") - // validate for both error and non-error cases switch { case tc.wantErr != nil: diff --git a/site/content/docs/main/restore-reference.md b/site/content/docs/main/restore-reference.md index cf5418f7b..6c5394cf0 100644 --- a/site/content/docs/main/restore-reference.md +++ b/site/content/docs/main/restore-reference.md @@ -215,37 +215,9 @@ data: ### PVC selected-node -Velero by default removes PVC's `volume.kubernetes.io/selected-node` annotation during restore, so that the restored PVC could be provisioned appropriately according to ```WaitForFirstConsumer``` rules, storage topologies and the restored pod's schedule result, etc. +Velero removes PVC's `volume.kubernetes.io/selected-node` annotation during restore, so that the restored PVC could be provisioned appropriately according to ```WaitForFirstConsumer``` rules, storage topologies and the restored pod's schedule result, etc. -For more information of how this selected-node annotation matters to PVC restore, see issue https://github.com/vmware-tanzu/velero/issues/9053. - -As an expectation, when you provide the selected-node configuration, Velero sets the annotation to the node in the configuration, if the node doesn't exist in the cluster then the annotation will also be removed. -Note: This feature is under deprecation as of Velero 1.15, following Velero deprecation policy. This feature is primarily used to remedy some problems in old Kubernetes versions as described [here](https://github.com/vmware-tanzu/velero/pull/2377). It may not work with the new features of Kubernetes and Velero. For more information, see issue https://github.com/vmware-tanzu/velero/issues/9053 for more information. -To configure a selected-node, create a config map in the Velero namespace like the following: - -```yaml -apiVersion: v1 -kind: ConfigMap -metadata: - # any name can be used; Velero uses the labels (below) - # to identify it rather than the name - name: change-pvc-node-selector-config - # must be in the velero namespace - namespace: velero - # the below labels should be used verbatim in your - # ConfigMap. - labels: - # this value-less label identifies the ConfigMap as - # config for a plugin (i.e. the built-in restore item action plugin) - velero.io/plugin-config: "" - # this label identifies the name and kind of plugin - # that this ConfigMap is for. - velero.io/change-pvc-node-selector: RestoreItemAction -data: - # add 1+ key-value pairs here, where the key is the old - # node name and the value is the new node name. - : -``` +For more information of how this selected-node annotation matters to PVC restore, see issue https://github.com/vmware-tanzu/velero/issues/9053. ## Restoring into a different namespace