Merge pull request #2575 from leseb/ceph-cmd-timeout

pkg: timeout ceph commands

pkg/daemon/ceph/client/command.go

@@ -13,6 +13,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 */
+
 package client
 
 import (
@@ -23,20 +24,38 @@ import (
 	"github.com/rook/rook/pkg/clusterd"
 )
 
-// When running the e2e tests, all ceph commands need to be run in the toolbox.
+// RunAllCephCommandsInToolbox - when running the e2e tests, all ceph commands need to be run in the toolbox.
 // Everywhere else, the ceph tools are assumed to be in the container where we can shell out.
 var RunAllCephCommandsInToolbox = false
 
 const (
-	AdminUsername     = "client.admin"
-	CephTool          = "ceph"
-	RBDTool           = "rbd"
-	Kubectl           = "kubectl"
-	CrushTool         = "crushtool"
-	cmdExecuteTimeout = 1 * time.Minute
+	// AdminUsername is the name of the admin user
+	AdminUsername = "client.admin"
+	// CephTool is the name of the CLI tool for 'ceph'
+	CephTool = "ceph"
+	// RBDTool is the name of the CLI tool for 'rbd'
+	RBDTool = "rbd"
+	// Kubectl is the name of the CLI tool for 'kubectl'
+	Kubectl = "kubectl"
+	// CrushTool is the name of the CLI tool for 'crushtool'
+	CrushTool             = "crushtool"
+	cmdExecuteTimeout     = 1 * time.Minute
+	cephConnectionTimeout = "15" // in seconds
 )
 
+// FinalizeCephCommandArgs builds the command line to be called
 func FinalizeCephCommandArgs(command string, args []string, configDir, clusterName string) (string, []string) {
+	// the rbd client tool does not support the '--connect-timeout' option
+	// so we only use it for the 'ceph' command
+	// Also, there is no point of adding that option to 'crushtool' since that CLI does not connect to anything
+	// 'crushtool' is a utility that lets you create, compile, decompile and test CRUSH map files.
+
+	// we could use a slice and iterate over it but since we have only 3 elements
+	// I don't think this is worth a loop
+	if command != "rbd" && command != "crushtool" && command != "radosgw-admin" {
+		args = append(args, "--connect-timeout="+cephConnectionTimeout)
+	}
+
 	// If the command should be run inside the toolbox pod, include the kubectl args to call the toolbox
 	if RunAllCephCommandsInToolbox {
 		toolArgs := []string{"-it", "exec", "rook-ceph-tools", "-n", clusterName, "--", command}
@@ -59,20 +78,25 @@ func FinalizeCephCommandArgs(command string, args []string, configDir, clusterNa
 	return command, append(args, configArgs...)
 }
 
+// ExecuteCephCommandDebugLog executes the 'ceph' command with 'debug' logs instead of 'info' logs
 func ExecuteCephCommandDebugLog(context *clusterd.Context, clusterName string, args []string) ([]byte, error) {
 	return executeCephCommandWithOutputFile(context, clusterName, true, args)
 }
 
+// ExecuteCephCommand executes the 'ceph' command
 func ExecuteCephCommand(context *clusterd.Context, clusterName string, args []string) ([]byte, error) {
 	return executeCephCommandWithOutputFile(context, clusterName, false, args)
 }
 
+// ExecuteCephCommandPlain executes the 'ceph' command and returns stdout in PLAIN format instead of JSON
 func ExecuteCephCommandPlain(context *clusterd.Context, clusterName string, args []string) ([]byte, error) {
 	command, args := FinalizeCephCommandArgs(CephTool, args, context.ConfigDir, clusterName)
 	args = append(args, "--format", "plain")
 	return executeCommandWithOutputFile(context, false, command, args)
 }
 
+// ExecuteCephCommandPlainNoOutputFile executes the 'ceph' command and returns stdout in PLAIN format instead of JSON
+// with no output file, suppresses '--out-file' option
 func ExecuteCephCommandPlainNoOutputFile(context *clusterd.Context, clusterName string, args []string) ([]byte, error) {
 	command, args := FinalizeCephCommandArgs(CephTool, args, context.ConfigDir, clusterName)
 	args = append(args, "--format", "plain")
@@ -85,17 +109,20 @@ func executeCephCommandWithOutputFile(context *clusterd.Context, clusterName str
 	return executeCommandWithOutputFile(context, debug, command, args)
 }
 
+// ExecuteRBDCommand executes the 'rbd' command
 func ExecuteRBDCommand(context *clusterd.Context, clusterName string, args []string) ([]byte, error) {
 	command, args := FinalizeCephCommandArgs(RBDTool, args, context.ConfigDir, clusterName)
 	args = append(args, "--format", "json")
 	return executeCommand(context, command, args)
 }
 
+// ExecuteRBDCommandNoFormat executes the 'rbd' command and returns stdout in PLAIN format
 func ExecuteRBDCommandNoFormat(context *clusterd.Context, clusterName string, args []string) ([]byte, error) {
 	command, args := FinalizeCephCommandArgs(RBDTool, args, context.ConfigDir, clusterName)
 	return executeCommand(context, command, args)
 }
 
+// ExecuteRBDCommandWithTimeout executes the 'rbd' command with a timeout of 1 minute
 func ExecuteRBDCommandWithTimeout(context *clusterd.Context, clusterName string, args []string) (string, error) {
 	output, err := context.Executor.ExecuteCommandWithTimeout(false, cmdExecuteTimeout, "", RBDTool, args...)
 	return output, err

pkg/daemon/ceph/client/command_test.go

@@ -0,0 +1,110 @@
+/*
+Copyright 2019 The Rook Authors. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package client
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFinalizeCephCommandArgs(t *testing.T) {
+	RunAllCephCommandsInToolbox = false
+	clusterName := "rook"
+	configDir := "/var/lib/rook/rook-ceph"
+	expectedCommand := "ceph"
+	args := []string{"mon_status"}
+	expectedArgs := []string{
+		"mon_status",
+		"--connect-timeout=15",
+		"--cluster=rook",
+		"--conf=/var/lib/rook/rook-ceph/rook/rook.config",
+		"--keyring=/var/lib/rook/rook-ceph/rook/client.admin.keyring",
+	}
+
+	cmd, args := FinalizeCephCommandArgs(expectedCommand, args, configDir, clusterName)
+	assert.Exactly(t, expectedCommand, cmd)
+	assert.Exactly(t, expectedArgs, args)
+}
+
+func TestFinalizeRadosGWAdminCommandArgs(t *testing.T) {
+	RunAllCephCommandsInToolbox = false
+	clusterName := "rook"
+	configDir := "/var/lib/rook/rook-ceph"
+	expectedCommand := "radosgw-admin"
+	args := []string{
+		"realm",
+		"create",
+		"--default",
+		"--rgw-realm=default-rook",
+		"--rgw-zonegroup=default-rook",
+	}
+
+	expectedArgs := []string{
+		"realm",
+		"create",
+		"--default",
+		"--rgw-realm=default-rook",
+		"--rgw-zonegroup=default-rook",
+		"--cluster=rook",
+		"--conf=/var/lib/rook/rook-ceph/rook/rook.config",
+		"--keyring=/var/lib/rook/rook-ceph/rook/client.admin.keyring",
+	}
+
+	cmd, args := FinalizeCephCommandArgs(expectedCommand, args, configDir, clusterName)
+	assert.Exactly(t, expectedCommand, cmd)
+	assert.Exactly(t, expectedArgs, args)
+}
+
+func TestFinalizeCephCommandArgsToolBox(t *testing.T) {
+	RunAllCephCommandsInToolbox = true
+	clusterName := "rook"
+	configDir := "/var/lib/rook/rook-ceph"
+	expectedCommand := "ceph"
+	args := []string{"health"}
+	expectedArgs := []string{
+		"-it",
+		"exec",
+		"rook-ceph-tools",
+		"-n",
+		"rook",
+		"--",
+		"ceph",
+		"health",
+		"--connect-timeout=15",
+	}
+
+	cmd, args := FinalizeCephCommandArgs(expectedCommand, args, configDir, clusterName)
+	assert.Exactly(t, "kubectl", cmd)
+	assert.Exactly(t, expectedArgs, args)
+}
+
+func TestFinalizeCephCommandArgsClusterDefaultName(t *testing.T) {
+	RunAllCephCommandsInToolbox = false
+	clusterName := "ceph"
+	configDir := "/etc"
+	expectedCommand := "ceph"
+	args := []string{"mon_status"}
+	expectedArgs := []string{
+		"mon_status",
+		"--connect-timeout=15",
+	}
+
+	cmd, args := FinalizeCephCommandArgs(expectedCommand, args, configDir, clusterName)
+	assert.Exactly(t, expectedCommand, cmd)
+	assert.Exactly(t, expectedArgs, args)
+}

pkg/daemon/ceph/client/mon_test.go

@@ -27,16 +27,18 @@ func TestCephArgs(t *testing.T) {
 	args := []string{}
 	command, args := FinalizeCephCommandArgs(CephTool, args, "/etc", "a")
 	assert.Equal(t, CephTool, command)
-	assert.Equal(t, 3, len(args))
-	assert.Equal(t, "--cluster=a", args[0])
-	assert.Equal(t, "--conf=/etc/a/a.config", args[1])
-	assert.Equal(t, "--keyring=/etc/a/client.admin.keyring", args[2])
+	assert.Equal(t, 4, len(args))
+	assert.Equal(t, 4, len(args))
+	assert.Equal(t, "--connect-timeout=15", args[0])
+	assert.Equal(t, "--cluster=a", args[1])
+	assert.Equal(t, "--conf=/etc/a/a.config", args[2])
+	assert.Equal(t, "--keyring=/etc/a/client.admin.keyring", args[3])
 
 	RunAllCephCommandsInToolbox = true
 	args = []string{}
 	command, args = FinalizeCephCommandArgs(CephTool, args, "/etc", "a")
 	assert.Equal(t, Kubectl, command)
-	assert.Equal(t, 7, len(args), fmt.Sprintf("%+v", args))
+	assert.Equal(t, 8, len(args), fmt.Sprintf("%+v", args))
 	assert.Equal(t, "-it", args[0])
 	assert.Equal(t, "exec", args[1])
 	assert.Equal(t, "rook-ceph-tools", args[2])
@@ -44,6 +46,7 @@ func TestCephArgs(t *testing.T) {
 	assert.Equal(t, "a", args[4])
 	assert.Equal(t, "--", args[5])
 	assert.Equal(t, CephTool, args[6])
+	assert.Equal(t, "--connect-timeout=15", args[7])
 	RunAllCephCommandsInToolbox = false
 
 	// cluster under /var/lib/rook
@@ -60,6 +63,6 @@ func TestCephArgs(t *testing.T) {
 	args = []string{"myarg"}
 	command, args = FinalizeCephCommandArgs(CephTool, args, "/etc", "ceph")
 	assert.Equal(t, CephTool, command)
-	assert.Equal(t, 1, len(args))
+	assert.Equal(t, 2, len(args))
 	assert.Equal(t, "myarg", args[0])
 }