ns: refresh process namespace on stale nsenter (e.g. iscsid PID change)

ns/executor.go

@@ -2,6 +2,7 @@ package ns
 
 import (
 	"path/filepath"
+	"strings"
 	"time"
 
 	"github.com/cockroachdb/errors"
@@ -16,6 +17,10 @@ import (
 type Executor struct {
 	namespaces  []types.Namespace // The namespaces to enter.
 	nsDirectory string            // The directory of the namespace.
+	// processName and procDirectory are set when the executor is created and used
+	// to re-resolve the namespace path after a stale nsenter (e.g. iscsid PID change).
+	processName   string
+	procDirectory string
 
 	executor exec.ExecuteInterface // An interface for executing commands. This allows mocking for unit tests.
 }
@@ -33,9 +38,11 @@ func NewNamespaceExecutor(processName, procDirectory string, namespaces []types.
 	}
 
 	NamespaceExecutor := &Executor{
-		namespaces:  namespaces,
-		nsDirectory: nsDir,
-		executor:    exec.NewExecutor(),
+		namespaces:     namespaces,
+		nsDirectory:    nsDir,
+		processName:    processName,
+		procDirectory:  procDirectory,
+		executor:       exec.NewExecutor(),
 	}
 
 	if _, err := NamespaceExecutor.executor.Execute(nil, types.NsBinary, []string{"-V"}, types.ExecuteDefaultTimeout); err != nil {
@@ -68,20 +75,76 @@ func (nsexec *Executor) prepareCommandArgs(binary string, args, envs []string) [
 	return append(cmdArgs, args...)
 }
 
+func (nsexec *Executor) refreshNamespaceDirectory() error {
+	nsDir, err := proc.GetProcessNamespaceDirectory(nsexec.processName, nsexec.procDirectory)
+	if err != nil {
+		return err
+	}
+	nsexec.nsDirectory = nsDir
+	return nil
+}
+
+// isLikelyStaleNamespaceError is true when nsenter failed because the process
+// namespace path no longer exists (e.g. iscsid restarted and got a new PID).
+func isLikelyStaleNamespaceError(err error) bool {
+	if err == nil {
+		return false
+	}
+	msg := err.Error()
+	if !strings.Contains(msg, types.NsBinary) {
+		return false
+	}
+	if !strings.Contains(msg, "/ns/") {
+		return false
+	}
+	// e.g. "nsenter: cannot open /host/proc/9134/ns/mnt: No such file or directory"
+	return strings.Contains(msg, "No such file or directory")
+}
+
 // Execute executes the command in the namespace. If NsDirectory is empty,
 // it will execute the command in the current namespace.
 func (nsexec *Executor) Execute(envs []string, binary string, args []string, timeout time.Duration) (string, error) {
+	out, err := nsexec.executor.Execute(nil, types.NsBinary, nsexec.prepareCommandArgs(binary, args, envs), timeout)
+	if err == nil {
+		return out, nil
+	}
+	if !isLikelyStaleNamespaceError(err) {
+		return out, err
+	}
+	if refreshErr := nsexec.refreshNamespaceDirectory(); refreshErr != nil {
+		return out, err
+	}
 	return nsexec.executor.Execute(nil, types.NsBinary, nsexec.prepareCommandArgs(binary, args, envs), timeout)
 }
 
 // ExecuteWithStdin executes the command in the namespace with stdin.
 // If NsDirectory is empty, it will execute the command in the current namespace.
 func (nsexec *Executor) ExecuteWithStdin(envs []string, binary string, args []string, stdinString string, timeout time.Duration) (string, error) {
+	out, err := nsexec.executor.ExecuteWithStdin(types.NsBinary, nsexec.prepareCommandArgs(binary, args, envs), stdinString, timeout)
+	if err == nil {
+		return out, nil
+	}
+	if !isLikelyStaleNamespaceError(err) {
+		return out, err
+	}
+	if refreshErr := nsexec.refreshNamespaceDirectory(); refreshErr != nil {
+		return out, err
+	}
 	return nsexec.executor.ExecuteWithStdin(types.NsBinary, nsexec.prepareCommandArgs(binary, args, envs), stdinString, timeout)
 }
 
 // ExecuteWithStdinPipe executes the command in the namespace with stdin pipe.
 // If NsDirectory is empty, it will execute the command in the current namespace.
 func (nsexec *Executor) ExecuteWithStdinPipe(envs []string, binary string, args []string, stdinString string, timeout time.Duration) (string, error) {
+	out, err := nsexec.executor.ExecuteWithStdinPipe(types.NsBinary, nsexec.prepareCommandArgs(binary, args, envs), stdinString, timeout)
+	if err == nil {
+		return out, nil
+	}
+	if !isLikelyStaleNamespaceError(err) {
+		return out, err
+	}
+	if refreshErr := nsexec.refreshNamespaceDirectory(); refreshErr != nil {
+		return out, err
+	}
 	return nsexec.executor.ExecuteWithStdinPipe(types.NsBinary, nsexec.prepareCommandArgs(binary, args, envs), stdinString, timeout)
 }

ns/executor_test.go

@@ -1,6 +1,7 @@
 package ns
 
 import (
+	"errors"
 	"strings"
 	"testing"
 	"time"
@@ -11,6 +12,68 @@ import (
 	"github.com/longhorn/go-common-libs/types"
 )
 
+func TestIsLikelyStaleNamespaceError(t *testing.T) {
+	examples := []struct {
+		name  string
+		err   error
+		want  bool
+	}{
+		{"nil", nil, false},
+		{"unrelated", errors.New("failed to execute: iscsiadm"), false},
+		{
+			"stale mnt from nsenter",
+			errors.New(`failed to execute: /usr/bin/nsenter [nsenter --mount=/host/proc/9134/ns/mnt --net=/host/proc/9134/ns/net iscsiadm --version], output , stderr nsenter: cannot open /host/proc/9134/ns/mnt: No such file or directory: exit status 1`),
+			true,
+		},
+		{
+			"non-stale iscsi error",
+			errors.New("failed to execute: /usr/bin/nsenter iscsiadm: iSCSI error: exit status 12"),
+			false,
+		},
+	}
+	for _, e := range examples {
+		t.Run(e.name, func(t *testing.T) {
+			assert.Equal(t, e.want, isLikelyStaleNamespaceError(e.err), e.name)
+		})
+	}
+}
+
+// retryingStubExecutor returns a stale error on the first run and "ok" on the second, for retry behavior.
+type retryingStubExecutor struct{ calls int }
+
+func (e *retryingStubExecutor) Execute(_ []string, _ string, _ []string, _ time.Duration) (string, error) {
+	e.calls++
+	if e.calls == 1 {
+		return "", errors.New(`failed to execute: /usr/bin/nsenter [nsenter --mount=/host/proc/9134/ns/mnt], output , stderr nsenter: cannot open /host/proc/9134/ns/mnt: No such file or directory: exit status 1`)
+	}
+	return "ok", nil
+}
+func (e *retryingStubExecutor) ExecuteWithStdin(_ string, _ []string, _ string, _ time.Duration) (string, error) {
+	return "", errors.New("not used")
+}
+func (e *retryingStubExecutor) ExecuteWithStdinPipe(_ string, _ []string, _ string, _ time.Duration) (string, error) {
+	return "", errors.New("not used")
+}
+
+// TestExecuteRefreshesNamespaceOnStaleError checks one retry re-runs the inner command after
+// a stale path error, when namespace re-resolution succeeds.
+func TestExecuteRefreshesNamespaceOnStaleError(t *testing.T) {
+	inner, err := NewNamespaceExecutor(types.ProcessNone, types.HostProcDirectory, []types.Namespace{types.NamespaceMnt})
+	if err != nil {
+		t.Skipf("no namespace executor (host /proc not suitable): %v", err)
+	}
+	nsexec := &Executor{
+		namespaces:     inner.namespaces,
+		nsDirectory:    inner.nsDirectory,
+		processName:    types.ProcessNone,
+		procDirectory:  types.HostProcDirectory,
+		executor:       &retryingStubExecutor{},
+	}
+	out, err := nsexec.Execute(nil, "true", nil, types.ExecuteDefaultTimeout)
+	assert.NoError(t, err)
+	assert.Equal(t, "ok", out)
+}
+
 func TestExecute(t *testing.T) {
 	type testCase struct {
 		nsDirectory string