Skip to content

Commit

Permalink
mon: run ceph commands to mon with timeout
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@leseb
leseb committed Oct 8, 2021
1 parent 0e40d6d commit 8da68bf
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 20 deletions.
2 changes: 1 addition & 1 deletion pkg/operator/ceph/cluster/cephstatus_test.go
View file
Expand Up @@ -163,7 +163,7 @@ func TestConfigureHealthSettings(t *testing.T) {
}
setGlobalIDReclaim := false
c.context.Executor = &exectest.MockExecutor{
MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) {
MockExecuteCommandWithTimeout: func(timeout time.Duration, command string, args ...string) (string, error) {
logger.Infof("Command: %s %v", command, args)
if args[0] == "config" && args[3] == "auth_allow_insecure_global_id_reclaim" {
if args[1] == "set" {
Expand Down
10 changes: 7 additions & 3 deletions pkg/operator/ceph/cluster/mgr/mgr_test.go
View file
Expand Up @@ -22,6 +22,7 @@ import (
"io/ioutil"
"os"
"testing"
"time"

cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
"github.com/rook/rook/pkg/client/clientset/versioned/scheme"
Expand Down Expand Up @@ -267,12 +268,15 @@ func TestConfigureModules(t *testing.T) {
}
lastModuleConfigured = args[3]
}
if args[0] == "config" && args[1] == "set" && args[2] == "global" {
configSettings[args[3]] = args[4]
}
}
return "", nil //return "{\"key\":\"mysecurekey\"}", nil
},
MockExecuteCommandWithTimeout: func(timeout time.Duration, command string, args ...string) (string, error) {
if args[0] == "config" && args[1] == "set" && args[2] == "global" {
configSettings[args[3]] = args[4]
}
return "", nil
},
}

clientset := testop.New(t, 3)
Expand Down
9 changes: 5 additions & 4 deletions pkg/operator/ceph/config/monstore.go
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -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)
}
Expand All @@ -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))
}
Expand Down
21 changes: 11 additions & 10 deletions pkg/operator/ceph/config/monstore_test.go
View file
Expand Up @@ -20,6 +20,7 @@ import (
"reflect"
"strings"
"testing"
"time"

"github.com/pkg/errors"
"github.com/rook/rook/pkg/clusterd"
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down
5 changes: 3 additions & 2 deletions pkg/operator/ceph/pool/controller_test.go
View file
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"os"
"testing"
"time"

"github.com/coreos/pkg/capnslog"
"github.com/pkg/errors"
Expand Down Expand Up @@ -476,7 +477,7 @@ func TestConfigureRBDStats(t *testing.T) {
)

executor := &exectest.MockExecutor{
MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) {
MockExecuteCommandWithTimeout: func(timeout time.Duration, command string, args ...string) (string, error) {
logger.Infof("Command: %s %v", command, args)
if args[0] == "config" && args[2] == "mgr." && args[3] == "mgr/prometheus/rbd_stats_pools" {
if args[1] == "set" && args[4] != "" {
Expand Down Expand Up @@ -543,7 +544,7 @@ func TestConfigureRBDStats(t *testing.T) {
// Case 5: Two CephBlockPools with EnableRBDStats:false & EnableRBDStats:true.
// SetConfig returns an error
context.Executor = &exectest.MockExecutor{
MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) {
MockExecuteCommandWithTimeout: func(timeout time.Duration, command string, args ...string) (string, error) {
logger.Infof("Command: %s %v", command, args)
return "", errors.New("mock error to simulate failure of mon store Set() function")
},
Expand Down

0 comments on commit 8da68bf

Please sign in to comment.