Merge pull request #7719 from cybozu-go/ceph-simplify-cmd-timeout

ceph: unity timeout values of commands

pkg/daemon/ceph/client/command.go

@@ -19,6 +19,7 @@ package client
 import (
 	"fmt"
 	"path"
+	"strconv"
 	"time"
 
 	"github.com/pkg/errors"
@@ -40,9 +41,8 @@ const (
 	// 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
+	CrushTool          = "crushtool"
+	CephCommandTimeout = 15 * time.Second
 	// DefaultPGCount will cause Ceph to use the internal default PG count
 	DefaultPGCount = "0"
 )
@@ -62,13 +62,15 @@ func FinalizeCephCommandArgs(command string, clusterInfo *ClusterInfo, args []st
 
 	// 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
+	timeout := strconv.Itoa(int(CephCommandTimeout.Seconds()))
 	if command != "rbd" && command != "crushtool" && command != "radosgw-admin" {
-		args = append(args, "--connect-timeout="+CephConnectionTimeout)
+		args = append(args, "--connect-timeout="+timeout)
 	}
 
 	// If the command should be run inside the toolbox pod, include the kubectl args to call the toolbox
 	if RunAllCephCommandsInToolboxPod != "" {
-		toolArgs := []string{"exec", "-i", RunAllCephCommandsInToolboxPod, "-n", clusterInfo.Namespace, "--", "timeout", "15", command}
+		toolArgs := []string{"exec", "-i", RunAllCephCommandsInToolboxPod, "-n", clusterInfo.Namespace,
+			"--", "timeout", timeout, command}
 		return Kubectl, append(toolArgs, args...)
 	}
 
@@ -172,7 +174,7 @@ func (c *CephToolCommand) RunWithTimeout(timeout time.Duration) ([]byte, error)
 // configured its arguments. It is future work to integrate this case into the
 // generalization.
 func ExecuteRBDCommandWithTimeout(context *clusterd.Context, args []string) (string, error) {
-	output, err := context.Executor.ExecuteCommandWithTimeout(CmdExecuteTimeout, RBDTool, args...)
+	output, err := context.Executor.ExecuteCommandWithTimeout(CephCommandTimeout, RBDTool, args...)
 	return output, err
 }
 

pkg/daemon/ceph/client/command_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package client
 
 import (
+	"strconv"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -29,7 +30,7 @@ func TestFinalizeCephCommandArgs(t *testing.T) {
 	args := []string{"quorum_status"}
 	expectedArgs := []string{
 		"quorum_status",
-		"--connect-timeout=15",
+		"--connect-timeout=" + strconv.Itoa(int(CephCommandTimeout.Seconds())),
 		"--cluster=rook",
 		"--conf=/var/lib/rook/rook-ceph/rook/rook.config",
 		"--name=client.admin",

pkg/operator/ceph/cluster/mgr/dashboard.go

@@ -175,7 +175,7 @@ func (c *Cluster) createSelfSignedCert() (bool, error) {
 
 	// retry a few times in the case that the mgr module is not ready to accept commands
 	for i := 0; i < 5; i++ {
-		_, err := client.NewCephCommand(c.context, c.clusterInfo, args).RunWithTimeout(client.CmdExecuteTimeout)
+		_, err := client.NewCephCommand(c.context, c.clusterInfo, args).RunWithTimeout(client.CephCommandTimeout)
 		if err == context.DeadlineExceeded {
 			logger.Warning("cert creation timed out. trying again")
 			continue
@@ -249,8 +249,7 @@ func (c *Cluster) setLoginCredentials(password string) error {
 	}
 
 	_, err := client.ExecuteCephCommandWithRetry(func() (string, []byte, error) {
-		cmd := client.NewCephCommand(c.context, c.clusterInfo, args)
-		output, err := cmd.RunWithTimeout(client.CmdExecuteTimeout)
+		output, err := client.NewCephCommand(c.context, c.clusterInfo, args).RunWithTimeout(client.CephCommandTimeout)
 		return "set dashboard creds", output, err
 	}, c.exitCode, 5, invalidArgErrorCode, dashboardInitWaitTime)
 	if err != nil {

pkg/operator/ceph/cluster/mgr/orchestrator.go

@@ -53,7 +53,7 @@ func (c *Cluster) setRookOrchestratorBackend() error {
 	// retry a few times in the case that the mgr module is not ready to accept commands
 	_, err := client.ExecuteCephCommandWithRetry(func() (string, []byte, error) {
 		args := []string{orchestratorCLIName, "set", "backend", "rook"}
-		output, err := client.NewCephCommand(c.context, c.clusterInfo, args).RunWithTimeout(client.CmdExecuteTimeout)
+		output, err := client.NewCephCommand(c.context, c.clusterInfo, args).RunWithTimeout(client.CephCommandTimeout)
 		return "set rook backend", output, err
 	}, c.exitCode, 5, invalidArgErrorCode, orchestratorInitWaitTime)
 	if err != nil {

pkg/operator/ceph/object/admin.go

@@ -19,7 +19,6 @@ package object
 import (
 	"fmt"
 	"regexp"
-	"time"
 
 	"github.com/pkg/errors"
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
@@ -74,13 +73,8 @@ func extractJSON(output string) (string, error) {
 func RunAdminCommandNoMultisite(c *Context, expectJSON bool, args ...string) (string, error) {
 	command, args := client.FinalizeCephCommandArgs("radosgw-admin", c.clusterInfo, args, c.Context.ConfigDir)
 
-	timeout, err := time.ParseDuration(fmt.Sprintf("%ss", client.CephConnectionTimeout))
-	if err != nil {
-		return "", errors.Wrap(err, "failed to parse CephConnectionTimeout")
-	}
-
 	// start the rgw admin command
-	output, err := c.Context.Executor.ExecuteCommandWithTimeout(timeout, command, args...)
+	output, err := c.Context.Executor.ExecuteCommandWithTimeout(client.CephCommandTimeout, command, args...)
 	if err != nil {
 		return output, err
 	}

pkg/operator/ceph/object/objectstore.go

@@ -24,11 +24,11 @@ import (
 	"strconv"
 	"strings"
 	"syscall"
-	"time"
 
 	"github.com/pkg/errors"
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
 	"github.com/rook/rook/pkg/clusterd"
+	"github.com/rook/rook/pkg/daemon/ceph/client"
 	ceph "github.com/rook/rook/pkg/daemon/ceph/client"
 	"github.com/rook/rook/pkg/operator/ceph/cluster/mgr"
 	"github.com/rook/rook/pkg/operator/ceph/config"
@@ -48,9 +48,6 @@ const (
 	AccessKeyName         = "access-key"
 	SecretKeyName         = "secret-key"
 	svcDNSSuffix          = "svc"
-
-	// Timeout for setting the dashboard access key
-	applyDashboardKeyTimeout = 15 * time.Second
 )
 
 var (
@@ -801,7 +798,7 @@ func enableRGWDashboard(context *Context) error {
 		// starting in ceph v15.2.8. We run it in a goroutine until the fix
 		// is found. We expect the ceph command to timeout so at least the goroutine exits.
 		logger.Info("setting the dashboard api secret key")
-		_, err = cephCmd.RunWithTimeout(applyDashboardKeyTimeout)
+		_, err = cephCmd.RunWithTimeout(client.CephCommandTimeout)
 		if err != nil {
 			logger.Errorf("failed to set user %q secretkey. %v", DashboardUser, err)
 		}
@@ -833,14 +830,14 @@ func disableRGWDashboard(context *Context) {
 
 	args := []string{"dashboard", "reset-rgw-api-access-key"}
 	cephCmd := ceph.NewCephCommand(context.Context, context.clusterInfo, args)
-	_, err = cephCmd.RunWithTimeout(applyDashboardKeyTimeout)
+	_, err = cephCmd.RunWithTimeout(client.CephCommandTimeout)
 	if err != nil {
 		logger.Warningf("failed to reset user accesskey for user %q. %v", DashboardUser, err)
 	}
 
 	args = []string{"dashboard", "reset-rgw-api-secret-key"}
 	cephCmd = ceph.NewCephCommand(context.Context, context.clusterInfo, args)
-	_, err = cephCmd.RunWithTimeout(applyDashboardKeyTimeout)
+	_, err = cephCmd.RunWithTimeout(client.CephCommandTimeout)
 	if err != nil {
 		logger.Warningf("failed to reset user secretkey for user %q. %v", DashboardUser, err)
 	}