From 218fd76411839be4317c897448f14536f9c345bd Mon Sep 17 00:00:00 2001
From: Xun Jiang/Bruce Jiang <59276555+blackpiglet@users.noreply.github.com>
Date: Mon, 6 Feb 2023 17:03:35 +0800
Subject: [PATCH] Add mapped selected-node existence check. (#5806)

Signed-off-by: Xun Jiang <blackpiglet@gmail.com>
---
 changelogs/unreleased/5806-blackpiglet       |  1 +
 pkg/restore/change_pvc_node_selector.go      |  9 +++++++++
 pkg/restore/change_pvc_node_selector_test.go | 17 ++++++++++++++++-
 3 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/5806-blackpiglet

diff --git a/changelogs/unreleased/5806-blackpiglet b/changelogs/unreleased/5806-blackpiglet
new file mode 100644
index 000000000..8831cb640
--- /dev/null
+++ b/changelogs/unreleased/5806-blackpiglet
@@ -0,0 +1 @@
+Add mapped selected-node existence check
\ No newline at end of file
diff --git a/pkg/restore/change_pvc_node_selector.go b/pkg/restore/change_pvc_node_selector.go
index d281d318f..80219d63b 100644
--- a/pkg/restore/change_pvc_node_selector.go
+++ b/pkg/restore/change_pvc_node_selector.go
@@ -100,6 +100,15 @@ func (p *ChangePVCNodeSelectorAction) Execute(input *velero.RestoreItemActionExe
 	}
 
 	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
diff --git a/pkg/restore/change_pvc_node_selector_test.go b/pkg/restore/change_pvc_node_selector_test.go
index 8be3051ba..a39a9b15e 100644
--- a/pkg/restore/change_pvc_node_selector_test.go
+++ b/pkg/restore/change_pvc_node_selector_test.go
@@ -17,8 +17,10 @@ limitations under the License.
 package restore
 
 import (
+	"bytes"
 	"context"
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/sirupsen/logrus"
@@ -44,6 +46,7 @@ func TestChangePVCNodeSelectorActionExecute(t *testing.T) {
 		pvc       *corev1api.PersistentVolumeClaim
 		configMap *corev1api.ConfigMap
 		node      *corev1api.Node
+		newNode   *corev1api.Node
 		want      *corev1api.PersistentVolumeClaim
 		wantErr   error
 	}{
@@ -57,6 +60,7 @@ func TestChangePVCNodeSelectorActionExecute(t *testing.T) {
 				ObjectMeta(builder.WithLabels("velero.io/plugin-config", "true", "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"),
@@ -127,8 +131,11 @@ func TestChangePVCNodeSelectorActionExecute(t *testing.T) {
 	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 := NewChangePVCNodeSelectorAction(
-				logrus.StandardLogger(),
+				logger,
 				clientset.CoreV1().ConfigMaps("velero"),
 				clientset.CoreV1().Nodes(),
 			)
@@ -143,6 +150,10 @@ func TestChangePVCNodeSelectorActionExecute(t *testing.T) {
 				_, err := clientset.CoreV1().Nodes().Create(context.TODO(), tc.node, metav1.CreateOptions{})
 				require.NoError(t, err)
 			}
+			if tc.newNode != nil {
+				_, err := clientset.CoreV1().Nodes().Create(context.TODO(), tc.newNode, metav1.CreateOptions{})
+				require.NoError(t, err)
+			}
 			unstructuredMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(tc.pvc)
 			require.NoError(t, err)
 
@@ -155,6 +166,10 @@ func TestChangePVCNodeSelectorActionExecute(t *testing.T) {
 			// execute method under test
 			res, err := a.Execute(input)
 
+			// Make sure mapped selected-node exists.
+			log_output := buf.String()
+			assert.Equal(t, strings.Contains(log_output, "Selected-node's mapped node doesn't exist"), false)
+
 			// validate for both error and non-error cases
 			switch {
 			case tc.wantErr != nil: