mon: run ceph commands to mon with timeout

If the mons are not in quorum yet the commands interacting with mon config store will stale for a very long time. Closes: #8928 Signed-off-by: Sébastien Han <seb@redhat.com>
rook · Oct 8, 2021 · 54006d7 · 54006d7
1 parent 2ddf9c9
commit 54006d7
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 14 deletions.
diff --git a/pkg/operator/ceph/config/monstore.go b/pkg/operator/ceph/config/monstore.go
@@ -23,6 +23,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/rook/rook/pkg/clusterd"
 	"github.com/rook/rook/pkg/daemon/ceph/client"
+	"github.com/rook/rook/pkg/util/exec"
 )
 
 // MonStore provides methods for setting Ceph configurations in the centralized mon
@@ -74,7 +75,7 @@ func (m *MonStore) Set(who, option, value string) error {
 	logger.Infof("setting %q=%q=%q option to the mon configuration database", who, option, value)
 	args := []string{"config", "set", who, normalizeKey(option), value}
 	cephCmd := client.NewCephCommand(m.context, m.clusterInfo, args)
-	out, err := cephCmd.Run()
+	out, err := cephCmd.RunWithTimeout(exec.CephCommandsTimeout)
 	if err != nil {
 		return errors.Wrapf(err, "failed to set ceph config in the centralized mon configuration database; "+
 			"you may need to use the rook-config-override ConfigMap. output: %s", string(out))
@@ -89,7 +90,7 @@ func (m *MonStore) Delete(who, option string) error {
 	logger.Infof("deleting %q option from the mon configuration database", option)
 	args := []string{"config", "rm", who, normalizeKey(option)}
 	cephCmd := client.NewCephCommand(m.context, m.clusterInfo, args)
-	out, err := cephCmd.Run()
+	out, err := cephCmd.RunWithTimeout(exec.CephCommandsTimeout)
 	if err != nil {
 		return errors.Wrapf(err, "failed to delete ceph config in the centralized mon configuration database. output: %s",
 			string(out))
@@ -104,7 +105,7 @@ func (m *MonStore) Delete(who, option string) error {
 func (m *MonStore) Get(who, option string) (string, error) {
 	args := []string{"config", "get", who, normalizeKey(option)}
 	cephCmd := client.NewCephCommand(m.context, m.clusterInfo, args)
-	out, err := cephCmd.Run()
+	out, err := cephCmd.RunWithTimeout(exec.CephCommandsTimeout)
 	if err != nil {
 		return "", errors.Wrapf(err, "failed to get config setting %q for user %q", option, who)
 	}
@@ -115,7 +116,7 @@ func (m *MonStore) Get(who, option string) (string, error) {
 func (m *MonStore) GetDaemon(who string) ([]Option, error) {
 	args := []string{"config", "get", who}
 	cephCmd := client.NewCephCommand(m.context, m.clusterInfo, args)
-	out, err := cephCmd.Run()
+	out, err := cephCmd.RunWithTimeout(exec.CephCommandsTimeout)
 	if err != nil {
 		return []Option{}, errors.Wrapf(err, "failed to get config for daemon %q. output: %s", who, string(out))
 	}

diff --git a/pkg/operator/ceph/config/monstore_test.go b/pkg/operator/ceph/config/monstore_test.go
@@ -20,6 +20,7 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/pkg/errors"
 	"github.com/rook/rook/pkg/clusterd"
@@ -41,8 +42,8 @@ func TestMonStore_Set(t *testing.T) {
 	// us to cause it to return an error when it detects a keyword.
 	execedCmd := ""
 	execInjectErr := false
-	executor.MockExecuteCommandWithOutput =
-		func(command string, args ...string) (string, error) {
+	executor.MockExecuteCommandWithTimeout =
+		func(timeout time.Duration, command string, args ...string) (string, error) {
 			execedCmd = command + " " + strings.Join(args, " ")
 			if execInjectErr {
 				return "output from cmd with error", errors.New("mocked error")
@@ -86,8 +87,8 @@ func TestMonStore_Delete(t *testing.T) {
 	// us to cause it to return an error when it detects a keyword.
 	execedCmd := ""
 	execInjectErr := false
-	executor.MockExecuteCommandWithOutput =
-		func(command string, args ...string) (string, error) {
+	executor.MockExecuteCommandWithTimeout =
+		func(timeout time.Duration, command string, args ...string) (string, error) {
 			execedCmd = command + " " + strings.Join(args, " ")
 			if execInjectErr {
 				return "output from cmd with error", errors.New("mocked error")
@@ -125,8 +126,8 @@ func TestMonStore_GetDaemon(t *testing.T) {
 		"\"rgw_enable_usage_log\":{\"value\":\"true\",\"section\":\"client.rgw.test.a\",\"mask\":{}," +
 		"\"can_update_at_runtime\":true}}"
 	execInjectErr := false
-	executor.MockExecuteCommandWithOutput =
-		func(command string, args ...string) (string, error) {
+	executor.MockExecuteCommandWithTimeout =
+		func(timeout time.Duration, command string, args ...string) (string, error) {
 			execedCmd = command + " " + strings.Join(args, " ")
 			if execInjectErr {
 				return "output from cmd with error", errors.New("mocked error")
@@ -171,8 +172,8 @@ func TestMonStore_DeleteDaemon(t *testing.T) {
 		"\"can_update_at_runtime\":true}," +
 		"\"rgw_enable_usage_log\":{\"value\":\"true\",\"section\":\"client.rgw.test.a\",\"mask\":{}," +
 		"\"can_update_at_runtime\":true}}"
-	executor.MockExecuteCommandWithOutput =
-		func(command string, args ...string) (string, error) {
+	executor.MockExecuteCommandWithTimeout =
+		func(timeout time.Duration, command string, args ...string) (string, error) {
 			execedCmd = command + " " + strings.Join(args, " ")
 			return execReturn, nil
 		}
@@ -197,8 +198,8 @@ func TestMonStore_SetAll(t *testing.T) {
 	// us to cause it to return an error when it detects a keyword.
 	execedCmds := []string{}
 	execInjectErrOnKeyword := "donotinjectanerror"
-	executor.MockExecuteCommandWithOutput =
-		func(command string, args ...string) (string, error) {
+	executor.MockExecuteCommandWithTimeout =
+		func(timeout time.Duration, command string, args ...string) (string, error) {
 			execedCmd := command + " " + strings.Join(args, " ")
 			execedCmds = append(execedCmds, execedCmd)
 			k := execInjectErrOnKeyword