do not attempt to run hooks on Pods that aren't running

Skip Pod hooks when the Pod's phase isn't Running.

Fixes #5137

Signed-off-by: Sebastian Hasler <sebastian.hasler@sec.uni-stuttgart.de>

changelogs/unreleased/6811-haslersn

@@ -0,0 +1 @@
+Do not run preHook/postHook on Succeeded/Pending/Failed pods and log error for Pending/Failed/Unknown pods with hooks.

pkg/podexec/pod_command_executor.go

@@ -123,6 +123,19 @@ func (e *defaultPodCommandExecutor) ExecutePodCommand(log logrus.FieldLogger, it
 			"hookTimeout":   localHook.Timeout,
 		},
 	)
+
+	if pod.Status.Phase == corev1api.PodUnknown {
+		hookLog.Error("Pod phase is Unknown, trying to run hook anyway")
+	} else if pod.Status.Phase == corev1api.PodPending || pod.Status.Phase == corev1api.PodFailed {
+		// Taking a backup of a Pending or Failed Pod that has a hook defined is probably something
+		// which the user didn't intend. We return an error, so this doesn't happen silently.
+		// Possible improvement in the future: When Pod is Pending, defer the backup a bit.
+		return errors.Errorf("Pod phase is %s", pod.Status.Phase)
+	} else if pod.Status.Phase == corev1api.PodSucceeded {
+		hookLog.Info("Skipping hook because Pod phase is Succeeded")
+		return nil
+	}
+
 	hookLog.Info("running exec hook")
 
 	req := e.restClient.Post().

pkg/podexec/pod_command_executor_test.go

@@ -262,6 +262,37 @@ func TestEnsureContainerExists(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestPodCompleted(t *testing.T) {
+	pod := &corev1api.Pod{
+		Spec: corev1api.PodSpec{
+			Containers: []corev1api.Container{
+				{
+					Name: "foo",
+				},
+			},
+		},
+		Status: corev1api.PodStatus{
+			Phase: corev1api.PodSucceeded,
+		},
+	}
+
+	obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod)
+	require.NoError(t, err)
+
+	clientConfig := &rest.Config{}
+	poster := &mockPoster{}
+	defer poster.AssertExpectations(t)
+	podCommandExecutor := NewPodCommandExecutor(clientConfig, poster).(*defaultPodCommandExecutor)
+
+	hook := v1.ExecHook{
+		Container: "foo",
+		Command:   []string{"some", "command"},
+	}
+
+	err = podCommandExecutor.ExecutePodCommand(velerotest.NewLogger(), obj, "namespace", "name", "hookName", &hook)
+	require.NoError(t, err)
+}
+
 type mockStreamExecutorFactory struct {
 	mock.Mock
 }

site/content/docs/main/backup-hooks.md

@@ -8,7 +8,9 @@ Velero supports executing commands in containers in pods during a backup.
 ## Backup Hooks
 
 When performing a backup, you can specify one or more commands to execute in a container in a pod
-when that pod is being backed up. The commands can be configured to run *before* any custom action
+when that pod is being backed up.
+Such hooks do _not_ run when the pod is Pending, Failed or Succeeded.
+The commands can be configured to run *before* any custom action
 processing ("pre" hooks), or after all custom actions have been completed and any additional items
 specified by custom action have been backed up ("post" hooks). Note that hooks are _not_ executed within a shell
 on the containers.