From e1d11e45a57c843692f7740182ba1baf5bc5c934 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Thu, 24 Jan 2019 11:21:33 +0100 Subject: [PATCH 1/2] pkg: timeout ceph commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Ceph CLI has the flag option '--connect-timeout' which controls the timeout of the command when reaching out to a monitor. If the connection cannot be established within 15 seconds, the command will fail. The default value in Ceph is relatively long (300sec IIRC). This is a problem when monitors have trouble reaching out quorum, since this means the command will only fail after 300 sec. Now we fail faster. Resolves: https://github.com/rook/rook/issues/2574 Signed-off-by: Sébastien Han --- pkg/daemon/ceph/client/command.go | 24 ++++-- pkg/daemon/ceph/client/command_test.go | 110 +++++++++++++++++++++++++ pkg/daemon/ceph/client/mon_test.go | 15 ++-- 3 files changed, 137 insertions(+), 12 deletions(-) create mode 100644 pkg/daemon/ceph/client/command_test.go diff --git a/pkg/daemon/ceph/client/command.go b/pkg/daemon/ceph/client/command.go index 20c95074c..dd6ebc8ae 100644 --- a/pkg/daemon/ceph/client/command.go +++ b/pkg/daemon/ceph/client/command.go @@ -28,15 +28,27 @@ import ( var RunAllCephCommandsInToolbox = false const ( - AdminUsername = "client.admin" - CephTool = "ceph" - RBDTool = "rbd" - Kubectl = "kubectl" - CrushTool = "crushtool" - cmdExecuteTimeout = 1 * time.Minute + AdminUsername = "client.admin" + CephTool = "ceph" + RBDTool = "rbd" + Kubectl = "kubectl" + CrushTool = "crushtool" + cmdExecuteTimeout = 1 * time.Minute + cephConnectionTimeout = "15" // in seconds ) 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} diff --git a/pkg/daemon/ceph/client/command_test.go b/pkg/daemon/ceph/client/command_test.go new file mode 100644 index 000000000..122bf0c98 --- /dev/null +++ b/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) +} diff --git a/pkg/daemon/ceph/client/mon_test.go b/pkg/daemon/ceph/client/mon_test.go index 43387db86..3a5b7a487 100644 --- a/pkg/daemon/ceph/client/mon_test.go +++ b/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]) } From 56f218e5a1350d4aca5a60067e1279478261f1ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Tue, 29 Jan 2019 09:37:41 +0100 Subject: [PATCH 2/2] client: ceph - add missing comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following Golang conventions, I'm adding the missing comments on the constants and exported functions. Signed-off-by: Sébastien Han --- pkg/daemon/ceph/client/command.go | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/pkg/daemon/ceph/client/command.go b/pkg/daemon/ceph/client/command.go index dd6ebc8ae..db8962a77 100644 --- a/pkg/daemon/ceph/client/command.go +++ b/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,26 @@ 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" + // 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 @@ -71,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") @@ -97,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