From bf553c49c97d956a78890aee862dfe5db84b7670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Tue, 28 Sep 2021 18:50:23 +0200 Subject: [PATCH] ceph: do not build all the args to remote exec cmd 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 (cherry picked from commit 17999bcb3d89b12740c8342c55fbd628b7b3c841) # Conflicts: # pkg/daemon/ceph/client/command.go --- pkg/daemon/ceph/client/command.go | 33 ++++++++++++++++++++++++++ pkg/daemon/ceph/client/command_test.go | 2 ++ 2 files changed, 35 insertions(+) diff --git a/pkg/daemon/ceph/client/command.go b/pkg/daemon/ceph/client/command.go index 168bf1371d460..8f2173453e682 100644 --- a/pkg/daemon/ceph/client/command.go +++ b/pkg/daemon/ceph/client/command.go @@ -129,7 +129,31 @@ func NewRBDCommand(context *clusterd.Context, clusterInfo *ClusterInfo, args []s } func (c *CephToolCommand) run() ([]byte, error) { +<<<<<<< HEAD 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() + } + + // Initialize 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) + } + +>>>>>>> 17999bcb3 (ceph: do not build all the args to remote exec cmd) if c.JsonOutput { args = append(args, "--format", "json") } else { @@ -142,6 +166,7 @@ func (c *CephToolCommand) run() ([]byte, error) { var output, stderr string var err error +<<<<<<< HEAD if c.OutputFile { if command == Kubectl { // Kubectl commands targeting the toolbox container generate a temp @@ -171,6 +196,14 @@ func (c *CephToolCommand) run() ([]byte, error) { } else { output, err = c.context.Executor.ExecuteCommandWithTimeout(c.timeout, command, args...) } +======= + // NewRBDCommand does not use the --out-file option so we only check for remote execution here + // Still forcing the check for the command if the behavior changes in the future + 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) +>>>>>>> 17999bcb3 (ceph: do not build all the args to remote exec cmd) } 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 ffa6744b45300..5bdd0b3759cbd 100644 --- a/pkg/daemon/ceph/client/command_test.go +++ b/pkg/daemon/ceph/client/command_test.go @@ -113,6 +113,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) @@ -134,6 +135,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\"") })