From 1582e7a2e08c012018ae79292ea4d8beadec0636 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Han?= <seb@redhat.com>
Date: Tue, 28 Sep 2021 18:50:23 +0200
Subject: [PATCH] multus: do not build all the args to exec commands
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When proxying commands to the cmd-proxy container we don't need to build
the command line with the same flags as the operator. The cmd-proxy
container does not use any ceph config file and just relies on the
CEPH_ARGS environment variable in the container. So passing the same
args as the operator causes to fail since we don't have a ceph config
file in `/var/lib/rook/openshift-storage/openshift-storage.config` thus
the remote exec fails with:

```
global_init: unable to open config file from search list ...
```

Signed-off-by: Sébastien Han <seb@redhat.com>
---
 pkg/daemon/ceph/client/command.go      | 21 +++++++++++++++++++--
 pkg/daemon/ceph/client/command_test.go |  2 ++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/pkg/daemon/ceph/client/command.go b/pkg/daemon/ceph/client/command.go
index a9cf1272e58c3..ae44c4f662196 100644
--- a/pkg/daemon/ceph/client/command.go
+++ b/pkg/daemon/ceph/client/command.go
@@ -126,10 +126,27 @@ func NewRBDCommand(context *clusterd.Context, clusterInfo *ClusterInfo, args []s
 }
 
 func (c *CephToolCommand) run() ([]byte, error) {
-	command, args := FinalizeCephCommandArgs(c.tool, c.clusterInfo, c.args, c.context.ConfigDir)
+	// Return if the context has been canceled
 	if c.clusterInfo.Context.Err() != nil {
 		return nil, c.clusterInfo.Context.Err()
 	}
+
+	// Initalize the command and args
+	command := c.tool
+	args := c.args
+
+	// If this is a remote execution, we don't want to build the full set of args. For instance all
+	// these args are not needed since those paths don't exist inside the cmd-proxy container:
+	//      --cluster=openshift-storage
+	//		--conf=/var/lib/rook/openshift-storage/openshift-storage.config
+	//		--name=client.admin
+	//		--keyring=/var/lib/rook/openshift-storage/client.admin.keyring
+	//
+	// The cmd-proxy container will take care of the rest with the help of the env CEPH_ARGS
+	if !c.RemoteExecution {
+		command, args = FinalizeCephCommandArgs(c.tool, c.clusterInfo, c.args, c.context.ConfigDir)
+	}
+
 	if c.JsonOutput {
 		args = append(args, "--format", "json")
 	} else {
@@ -147,7 +164,7 @@ func (c *CephToolCommand) run() ([]byte, error) {
 	if command == RBDTool {
 		if c.RemoteExecution {
 			output, stderr, err = c.context.RemoteExecutor.ExecCommandInContainerWithFullOutputWithTimeout(ProxyAppLabel, CommandProxyInitContainerName, c.clusterInfo.Namespace, append([]string{command}, args...)...)
-			output = fmt.Sprintf("%s.%s", output, stderr)
+			output = fmt.Sprintf("%s. %s", output, stderr)
 		} else if c.timeout == 0 {
 			output, err = c.context.Executor.ExecuteCommandWithOutput(command, args...)
 		} else {
diff --git a/pkg/daemon/ceph/client/command_test.go b/pkg/daemon/ceph/client/command_test.go
index 574047b9ff34f..6476e45053cc2 100644
--- a/pkg/daemon/ceph/client/command_test.go
+++ b/pkg/daemon/ceph/client/command_test.go
@@ -116,6 +116,7 @@ func TestNewRBDCommand(t *testing.T) {
 		executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) {
 			switch {
 			case command == "rbd" && args[0] == "create":
+				assert.Len(t, args, 8)
 				return "success", nil
 			}
 			return "", errors.Errorf("unexpected ceph command %q", args)
@@ -137,6 +138,7 @@ func TestNewRBDCommand(t *testing.T) {
 		assert.True(t, cmd.RemoteExecution)
 		_, err := cmd.Run()
 		assert.Error(t, err)
+		assert.Len(t, cmd.args, 4)
 		// This is not the best but it shows we go through the right codepath
 		assert.EqualError(t, err, "no pods found with selector \"rook-ceph-mgr\"")
 	})