diff --git a/cmd/rook/ceph/ceph.go b/cmd/rook/ceph/ceph.go index 2ef90ddfe..28fd28b23 100644 --- a/cmd/rook/ceph/ceph.go +++ b/cmd/rook/ceph/ceph.go @@ -24,7 +24,6 @@ import ( "github.com/rook/rook/pkg/clusterd" cephconfig "github.com/rook/rook/pkg/daemon/ceph/config" osdconfig "github.com/rook/rook/pkg/operator/ceph/cluster/osd/config" - "github.com/rook/rook/pkg/util/flags" ) // Cmd is the main command for operator and daemons. @@ -83,16 +82,6 @@ func addCephFlags(command *cobra.Command) { // TODO: remove these legacy flags in the future command.Flags().StringVar(&cfg.networkInfo.PublicAddrIPv4, "public-ipv4", "", "public IPv4 address for this machine") command.Flags().StringVar(&cfg.networkInfo.ClusterAddrIPv4, "private-ipv4", "", "private IPv4 address for this machine") - command.Flags().MarkDeprecated("public-ipv4", "Use --public-ip instead. Will be removed in a future version.") - command.Flags().MarkDeprecated("private-ipv4", "Use --private-ip instead. Will be removed in a future version.") -} - -func verifyRenamedFlags(cmd *cobra.Command) error { - renamed := []flags.RenamedFlag{ - {NewFlagName: "public-ip", OldFlagName: "public-ipv4"}, - {NewFlagName: "private-ip", OldFlagName: "private-ipv4"}, - } - return flags.VerifyRenamedFlags(cmd, renamed) } func (c *config) NetworkInfo() clusterd.NetworkInfo { diff --git a/cmd/rook/ceph/osd.go b/cmd/rook/ceph/osd.go index a52212c6e..2def022f8 100644 --- a/cmd/rook/ceph/osd.go +++ b/cmd/rook/ceph/osd.go @@ -175,10 +175,6 @@ func prepareOSD(cmd *cobra.Command, args []string) error { return err } - if err := verifyRenamedFlags(osdCmd); err != nil { - return err - } - var dataDevices []osddaemon.DesiredDevice if osdDataDeviceFilter != "" { if cfg.devices != "" || osdDataDevicePathFilter != "" { diff --git a/cmd/rook/edgefs/start.go b/cmd/rook/edgefs/start.go index 151dae87f..42f3bf378 100644 --- a/cmd/rook/edgefs/start.go +++ b/cmd/rook/edgefs/start.go @@ -25,7 +25,6 @@ import ( const ( edgefStartCmdPath = "/opt/nedge/sbin/edgefs-start.sh" - logSectionName = "edgefs-internal" ) var startCmd = &cobra.Command{ @@ -44,7 +43,7 @@ func start(cmd *cobra.Command, args []string) error { rook.SetLogLevel() rook.LogStartupInfo(operatorCmd.Flags()) executor := &exec.CommandExecutor{} - err := executor.ExecuteCommand(true, logSectionName, edgefStartCmdPath, args...) + err := executor.ExecuteCommand(edgefStartCmdPath, args...) logger.Infof("Start script %s exited: %+v", edgefStartCmdPath, err) return nil diff --git a/cmd/rook/rook/rook.go b/cmd/rook/rook/rook.go index acf47cd46..1fded786a 100644 --- a/cmd/rook/rook/rook.go +++ b/cmd/rook/rook/rook.go @@ -125,8 +125,6 @@ func NewContext() *clusterd.Context { context.Executor = &exec.TranslateCommandExecutor{ Executor: context.Executor, Translator: func( - debug bool, - actionName string, command string, arg ...string, ) (string, []string) { diff --git a/go.mod b/go.mod index ad0ece99b..2bcd5e946 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,6 @@ require ( github.com/google/go-cmp v0.3.1 github.com/google/uuid v1.1.1 github.com/icrowley/fake v0.0.0-20180203215853-4178557ae428 - github.com/jbw976/go-ps v0.0.0-20170713234100-82859aed1b5d github.com/kube-object-storage/lib-bucket-provisioner v0.0.0-20200107223247-51020689f1fb github.com/openshift/cluster-api v0.0.0-20191129101638-b09907ac6668 github.com/openshift/machine-api-operator v0.2.1-0.20190903202259-474e14e4965a @@ -30,6 +29,7 @@ require ( k8s.io/apimachinery v0.17.2 k8s.io/apiserver v0.17.2 k8s.io/client-go v12.0.0+incompatible + k8s.io/code-generator v0.17.2 k8s.io/kube-controller-manager v0.0.0 k8s.io/kubernetes v1.17.2 k8s.io/utils v0.0.0-20191114200735-6ca3b61696b6 diff --git a/go.sum b/go.sum index 0dc5e36b2..fe6e94318 100644 --- a/go.sum +++ b/go.sum @@ -112,6 +112,7 @@ github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7 github.com/coreos/pkg v0.0.0-20160727233714-3ac0863d7acf/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/coreos/pkg v0.0.0-20180108230652-97fdf19511ea h1:n2Ltr3SrfQlf/9nOna1DoGKxLx3qTSI8Ttl6Xrqp6mw= github.com/coreos/pkg v0.0.0-20180108230652-97fdf19511ea/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= +github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f h1:lBNOc5arjvs8E5mO2tbpBpLoyyu8B6e44T7hJy6potg= github.com/coreos/prometheus-operator v0.34.0 h1:TF9qaydNeUamLKs0hniaapa4FBz8U8TIlRRtJX987A4= github.com/coreos/prometheus-operator v0.34.0/go.mod h1:Li6rMllG/hYIyXfMuvUwhyC+hqwJVHdsDdP21hypT1M= github.com/corpix/uarand v0.1.1 h1:RMr1TWc9F4n5jiPDzFHtmaUXLKLNUFK0SgCLo4BhX/U= @@ -376,8 +377,6 @@ github.com/imdario/mergo v0.3.7/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJ github.com/improbable-eng/thanos v0.3.2/go.mod h1:GZewVGILKuJVPNRn7L4Zw+7X96qzFOwj63b22xYGXBE= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= -github.com/jbw976/go-ps v0.0.0-20170713234100-82859aed1b5d h1:Eu2H80hepnodkFlP5JSPijRi3uD5RnGUjCEJBYLL1Qg= -github.com/jbw976/go-ps v0.0.0-20170713234100-82859aed1b5d/go.mod h1:jNoCLbavHxJDd+Zl3/DAnYwbD8Ml5PxMMf0JOphQQKo= github.com/jellevandenhooff/dkim v0.0.0-20150330215556-f50fe3d243e1/go.mod h1:E0B/fFc00Y+Rasa88328GlI/XbtyysCtTHZS8h7IrBU= github.com/jimstudt/http-authentication v0.0.0-20140401203705-3eca13d6893a/go.mod h1:wK6yTYYcgjHE1Z1QtXACPDjcFJyBskHEdagmnq3vsP8= github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af h1:pmfjZENx5imkbgOkpRUYLnmbU7UEFbjtDA2hxJ1ichM= @@ -697,6 +696,7 @@ golang.org/x/crypto v0.0.0-20191119213627-4f8c1d86b1ba h1:9bFeDpN3gTqNanMVqNcoR/ golang.org/x/crypto v0.0.0-20191119213627-4f8c1d86b1ba/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= +golang.org/x/exp v0.0.0-20190312203227-4b39c73a6495 h1:I6A9Ag9FpEKOjcKrRNjQkPHawoXIhKyTGfvvjFAiiAk= golang.org/x/exp v0.0.0-20190312203227-4b39c73a6495/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= @@ -824,13 +824,16 @@ golang.org/x/tools v0.0.0-20190614205625-5aca471b1d59/go.mod h1:/rFqwRUd4F7ZHNgw golang.org/x/tools v0.0.0-20190617190820-da514acc4774/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190621195816-6e04913cbbac/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190909030654-5b82db07426d/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.0.0-20190920225731-5eefd052ad72 h1:bw9doJza/SFBEweII/rHQh338oozWyiFsBRHtrflcws= golang.org/x/tools v0.0.0-20190920225731-5eefd052ad72/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gomodules.xyz/jsonpatch/v2 v2.0.1 h1:xyiBuvkD2g5n7cYzx6u2sxQvsAy4QJsZFCzGVdzOXZ0= gomodules.xyz/jsonpatch/v2 v2.0.1/go.mod h1:IhYNNY4jnS53ZnfE4PAmpKtDpTCj1JFXc+3mwe7XcUU= +gonum.org/v1/gonum v0.0.0-20190331200053-3d26580ed485 h1:OB/uP/Puiu5vS5QMRPrXCDWUPb+kt8f1KW8oQzFejQw= gonum.org/v1/gonum v0.0.0-20190331200053-3d26580ed485/go.mod h1:2ltnJ7xHfj0zHS40VVPYEAAMTa3ZGguvHGBSJeRWqE0= gonum.org/v1/netlib v0.0.0-20190313105609-8cb42192e0e0/go.mod h1:wa6Ws7BG/ESfp6dHfk7C6KdzKA7wR7u/rKwOGE66zvw= +gonum.org/v1/netlib v0.0.0-20190331212654-76723241ea4e h1:jRyg0XfpwWlhEV8mDfdNGBeSJM2fuyh9Yjrnd8kF2Ts= gonum.org/v1/netlib v0.0.0-20190331212654-76723241ea4e/go.mod h1:kS+toOQn6AQKjmKJ7gzohV1XkqsFehRA2FbsbkopSuQ= google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE= google.golang.org/api v0.6.1-0.20190607001116-5213b8090861/go.mod h1:btoxGiFvQNVUZQ8W08zLtrVS08CNpINPEfxXxgJL1Q4= @@ -908,12 +911,14 @@ k8s.io/client-go v0.17.2/go.mod h1:QAzRgsa0C2xl4/eVpeVAZMvikCn8Nm81yqVx3Kk9XYI= k8s.io/cloud-provider v0.17.2 h1:5JVAaJ3JIlaXlOqAX/LkdnXPl1UlcjHMPbepo8LFcyo= k8s.io/cloud-provider v0.17.2/go.mod h1:9rEcGqEsUFHxC83oMUGBcsXTBpRVNVPX/U+nyQJTvHU= k8s.io/cluster-bootstrap v0.17.2/go.mod h1:qiazpAM05fjAc+PEkrY8HSUhKlJSMBuLnVUSO6nvZL4= +k8s.io/code-generator v0.17.2 h1:pTwl3rLB1fUyxmvEzmVPMM0tBSdUehd7z+bDzpj4lPE= k8s.io/code-generator v0.17.2/go.mod h1:DVmfPQgxQENqDIzVR2ddLXMH34qeszkKSdH/N+s+38s= k8s.io/component-base v0.17.2 h1:0XHf+cerTvL9I5Xwn9v+0jmqzGAZI7zNydv4tL6Cw6A= k8s.io/component-base v0.17.2/go.mod h1:zMPW3g5aH7cHJpKYQ/ZsGMcgbsA/VyhEugF3QT1awLs= k8s.io/cri-api v0.17.2/go.mod h1:BzAkbBHHp81d+aXzbiIcUbilLkbXa40B8mUHOk6EX3s= k8s.io/csi-translation-lib v0.17.2/go.mod h1:NrhnhXJg/V6cHRTdPbmxvBuV3rJSqXsdLBE5JSRzcVI= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= +k8s.io/gengo v0.0.0-20190822140433-26a664648505 h1:ZY6yclUKVbZ+SdWnkfY+Je5vrMpKOxmGeKRbsXVmqYM= k8s.io/gengo v0.0.0-20190822140433-26a664648505/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/heapster v1.2.0-beta.1/go.mod h1:h1uhptVXMwC8xtZBYsPXKVi8fpdlYkTs6k949KozGrM= k8s.io/klog v0.0.0-20181102134211-b9b56d5dfc92/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= diff --git a/pkg/clusterd/disk_test.go b/pkg/clusterd/disk_test.go index d9e07c042..c1ec2e404 100644 --- a/pkg/clusterd/disk_test.go +++ b/pkg/clusterd/disk_test.go @@ -24,12 +24,12 @@ import ( func TestDiscoverDevices(t *testing.T) { executor := &exectest.MockExecutor{ - MockExecuteCommand: func(debug bool, actionName string, command string, arg ...string) error { - logger.Infof("mock execute. %s. %s", actionName, command) + MockExecuteCommand: func(command string, arg ...string) error { + logger.Infof("mock execute. %s", command) return nil }, - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, arg ...string) (string, error) { - logger.Infof("mock execute with output. %s. %s", actionName, command) + MockExecuteCommandWithOutput: func(command string, arg ...string) (string, error) { + logger.Infof("mock execute with output. %s", command) return "", nil }, } diff --git a/pkg/daemon/ceph/agent/flexvolume/manager/ceph/manager_test.go b/pkg/daemon/ceph/agent/flexvolume/manager/ceph/manager_test.go index 085e922e2..6172177ad 100644 --- a/pkg/daemon/ceph/agent/flexvolume/manager/ceph/manager_test.go +++ b/pkg/daemon/ceph/agent/flexvolume/manager/ceph/manager_test.go @@ -50,13 +50,13 @@ func TestInitLoadRBDModSingleMajor(t *testing.T) { modprobeCalled := false executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { assert.Equal(t, "modinfo", command) assert.Equal(t, "rbd", args[2]) modInfoCalled = true return "single_major:Use a single major number for all rbd devices (default: false) (bool)", nil }, - MockExecuteCommand: func(debug bool, actionName string, command string, args ...string) error { + MockExecuteCommand: func(command string, args ...string) error { assert.Equal(t, "modprobe", command) assert.Equal(t, "rbd", args[0]) assert.Equal(t, "single_major=Y", args[1]) @@ -78,13 +78,13 @@ func TestInitLoadRBDModNoSingleMajor(t *testing.T) { modprobeCalled := false executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { assert.Equal(t, "modinfo", command) assert.Equal(t, "rbd", args[2]) modInfoCalled = true return "", nil }, - MockExecuteCommand: func(debug bool, actionName string, command string, args ...string) error { + MockExecuteCommand: func(command string, args ...string) error { assert.Equal(t, "modprobe", command) assert.Equal(t, 1, len(args)) assert.Equal(t, "rbd", args[0]) @@ -117,14 +117,14 @@ func TestAttach(t *testing.T) { runCount := 1 executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { if strings.Contains(command, "ceph-authtool") { err := cephtest.CreateConfigDir(path.Join(configDir, clusterNamespace)) assert.Nil(t, err) } return "", nil }, - MockExecuteCommandWithTimeout: func(debug bool, timeout time.Duration, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithTimeout: func(timeout time.Duration, command string, args ...string) (string, error) { assert.Equal(t, "rbd", command) assert.Equal(t, "map", args[0]) assert.Equal(t, fmt.Sprintf("testpool/image%d", runCount), args[1]) @@ -201,14 +201,14 @@ func TestDetach(t *testing.T) { clientset.CoreV1().ConfigMaps(clusterNamespace).Create(cm) executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { if strings.Contains(command, "ceph-authtool") { err := cephtest.CreateConfigDir(path.Join(configDir, clusterNamespace)) assert.Nil(t, err) } return "", nil }, - MockExecuteCommandWithTimeout: func(debug bool, timeout time.Duration, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithTimeout: func(timeout time.Duration, command string, args ...string) (string, error) { assert.Equal(t, "rbd", command) assert.Equal(t, "unmap", args[0]) assert.Equal(t, "testpool/image1", args[1]) @@ -253,14 +253,14 @@ func TestDetachCustomKeyring(t *testing.T) { clientset.CoreV1().ConfigMaps(clusterNamespace).Create(cm) executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { if strings.Contains(command, "ceph-authtool") { err := cephtest.CreateConfigDir(path.Join(configDir, clusterNamespace)) assert.Nil(t, err) } return "", nil }, - MockExecuteCommandWithTimeout: func(debug bool, timeout time.Duration, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithTimeout: func(timeout time.Duration, command string, args ...string) (string, error) { assert.Equal(t, "rbd", command) assert.Equal(t, "unmap", args[0]) assert.Equal(t, "testpool/image1", args[1]) diff --git a/pkg/daemon/ceph/client/auth.go b/pkg/daemon/ceph/client/auth.go index 5564f1c2e..c3e0e0261 100644 --- a/pkg/daemon/ceph/client/auth.go +++ b/pkg/daemon/ceph/client/auth.go @@ -22,23 +22,11 @@ import ( "github.com/rook/rook/pkg/clusterd" ) -// AuthAdd will create a new user with the given capabilities and using the already generated keyring -// found at the given keyring path. This should not be used when the user may already exist. -func AuthAdd(context *clusterd.Context, clusterName, name, keyringPath string, caps []string) error { - args := append([]string{"auth", "add", name, "-i", keyringPath}, caps...) - _, err := NewCephCommand(context, clusterName, args).Run() - if err != nil { - return errors.Wrapf(err, "failed to auth add for %s", name) - } - - return nil -} - // AuthGetOrCreate will either get or create a user with the given capabilities. The keyring for the // user will be written to the given keyring path. func AuthGetOrCreate(context *clusterd.Context, clusterName, name, keyringPath string, caps []string) error { + logger.Infof("getting or creating ceph auth %q", name) args := append([]string{"auth", "get-or-create", name, "-o", keyringPath}, caps...) - cmd := NewCephCommand(context, clusterName, args) cmd.JsonOutput = false cmd.OutputFile = false @@ -52,6 +40,7 @@ func AuthGetOrCreate(context *clusterd.Context, clusterName, name, keyringPath s // AuthGetKey gets the key for the given user. func AuthGetKey(context *clusterd.Context, clusterName, name string) (string, error) { + logger.Infof("getting ceph auth key %q", name) args := []string{"auth", "get-key", name} buf, err := NewCephCommand(context, clusterName, args).Run() if err != nil { @@ -63,6 +52,7 @@ func AuthGetKey(context *clusterd.Context, clusterName, name string) (string, er // AuthGetOrCreateKey gets or creates the key for the given user. func AuthGetOrCreateKey(context *clusterd.Context, clusterName, name string, caps []string) (string, error) { + logger.Infof("getting or creating ceph auth key %q", name) args := append([]string{"auth", "get-or-create-key", name}, caps...) buf, err := NewCephCommand(context, clusterName, args).Run() if err != nil { @@ -74,6 +64,7 @@ func AuthGetOrCreateKey(context *clusterd.Context, clusterName, name string, cap // AuthUpdateCaps updates the capabilities for the given user. func AuthUpdateCaps(context *clusterd.Context, clusterName, name string, caps []string) error { + logger.Infof("updating ceph auth caps %q to %v", name, caps) args := append([]string{"auth", "caps", name}, caps...) _, err := NewCephCommand(context, clusterName, args).Run() if err != nil { @@ -84,6 +75,7 @@ func AuthUpdateCaps(context *clusterd.Context, clusterName, name string, caps [] // AuthGetCaps gets the capabilities for the given user. func AuthGetCaps(context *clusterd.Context, clusterName, name string) (caps map[string]string, error error) { + logger.Infof("getting ceph auth caps for %q", name) args := append([]string{"auth", "get", name}) output, err := NewCephCommand(context, clusterName, args).Run() if err != nil { @@ -115,6 +107,7 @@ func AuthGetCaps(context *clusterd.Context, clusterName, name string) (caps map[ // AuthDelete will delete the given user. func AuthDelete(context *clusterd.Context, clusterName, name string) error { + logger.Infof("deleting ceph auth %q", name) args := []string{"auth", "del", name} _, err := NewCephCommand(context, clusterName, args).Run() if err != nil { diff --git a/pkg/daemon/ceph/client/command.go b/pkg/daemon/ceph/client/command.go index 42b7e4958..e0f5d1ff0 100644 --- a/pkg/daemon/ceph/client/command.go +++ b/pkg/daemon/ceph/client/command.go @@ -18,7 +18,6 @@ package client import ( "fmt" - "os" "path" "time" @@ -91,43 +90,27 @@ type CephToolCommand struct { clusterName string args []string timeout time.Duration - Debug bool JsonOutput bool OutputFile bool } -func newCephToolCommand(tool string, context *clusterd.Context, clusterName string, args []string, debug bool) *CephToolCommand { +func newCephToolCommand(tool string, context *clusterd.Context, clusterName string, args []string) *CephToolCommand { return &CephToolCommand{ context: context, tool: tool, clusterName: clusterName, args: args, - Debug: debug, JsonOutput: true, OutputFile: true, } } -// TODO: make this work even if the operator config comes from the CM? -// IsDebugLevel detects operator log level -func IsDebugLevel() bool { - logLevel := os.Getenv("ROOK_LOG_LEVEL") - switch logLevel { - case "INFO": - return true - case "DEBUG": - return false - default: - return true - } -} - func NewCephCommand(context *clusterd.Context, clusterName string, args []string) *CephToolCommand { - return newCephToolCommand(CephTool, context, clusterName, args, IsDebugLevel()) + return newCephToolCommand(CephTool, context, clusterName, args) } func NewRBDCommand(context *clusterd.Context, clusterName string, args []string) *CephToolCommand { - cmd := newCephToolCommand(RBDTool, context, clusterName, args, IsDebugLevel()) + cmd := newCephToolCommand(RBDTool, context, clusterName, args) cmd.JsonOutput = false cmd.OutputFile = false return cmd @@ -153,22 +136,22 @@ func (c *CephToolCommand) run() ([]byte, error) { // file in the wrong place, so we will instead capture the output // from stdout for the tests if c.timeout == 0 { - output, err = c.context.Executor.ExecuteCommandWithOutput(c.Debug, "", command, args...) + output, err = c.context.Executor.ExecuteCommandWithOutput(command, args...) } else { - output, err = c.context.Executor.ExecuteCommandWithTimeout(c.Debug, c.timeout, "", command, args...) + output, err = c.context.Executor.ExecuteCommandWithTimeout(c.timeout, command, args...) } } else { if c.timeout == 0 { - output, err = c.context.Executor.ExecuteCommandWithOutputFile(c.Debug, "", command, "--out-file", args...) + output, err = c.context.Executor.ExecuteCommandWithOutputFile(command, "--out-file", args...) } else { - output, err = c.context.Executor.ExecuteCommandWithOutputFileTimeout(c.Debug, c.timeout, "", command, "--out-file", args...) + output, err = c.context.Executor.ExecuteCommandWithOutputFileTimeout(c.timeout, command, "--out-file", args...) } } } else { if c.timeout == 0 { - output, err = c.context.Executor.ExecuteCommandWithOutput(c.Debug, "", command, args...) + output, err = c.context.Executor.ExecuteCommandWithOutput(command, args...) } else { - output, err = c.context.Executor.ExecuteCommandWithTimeout(c.Debug, c.timeout, "", command, args...) + output, err = c.context.Executor.ExecuteCommandWithTimeout(c.timeout, command, args...) } } @@ -190,32 +173,32 @@ 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, clusterName string, args []string) (string, error) { - output, err := context.Executor.ExecuteCommandWithTimeout(false, CmdExecuteTimeout, "", RBDTool, args...) + output, err := context.Executor.ExecuteCommandWithTimeout(CmdExecuteTimeout, RBDTool, args...) return output, err } func ExecuteCephCommandWithRetry( - cmd func() ([]byte, error), + cmd func() (string, []byte, error), getExitCode func(err error) (int, bool), retries int, retryOnExitCode int, waitTime time.Duration, ) ([]byte, error) { for i := 0; i < retries; i++ { - data, err := cmd() + action, data, err := cmd() if err != nil { exitCode, parsed := getExitCode(err) if parsed { if exitCode == retryOnExitCode { - logger.Infof("command failed. trying again...") + logger.Infof("command failed for %s. trying again...", action) time.Sleep(waitTime) continue } } - return nil, errors.Wrapf(err, "failed to complete command") + return nil, errors.Wrapf(err, "failed to complete command for %s", action) } if i > 0 { - logger.Infof("command succeeded on attempt %d", i) + logger.Infof("action %s succeeded on attempt %d", action, i) } return data, nil } diff --git a/pkg/daemon/ceph/client/crush_test.go b/pkg/daemon/ceph/client/crush_test.go index 3843f12e9..f0c0d8510 100644 --- a/pkg/daemon/ceph/client/crush_test.go +++ b/pkg/daemon/ceph/client/crush_test.go @@ -229,7 +229,7 @@ const testCrushMap = `{ func TestGetCrushMap(t *testing.T) { executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[1] == "crush" && args[2] == "dump" { return testCrushMap, nil diff --git a/pkg/daemon/ceph/client/erasure-code-profile_test.go b/pkg/daemon/ceph/client/erasure-code-profile_test.go index 901902247..5d220b965 100644 --- a/pkg/daemon/ceph/client/erasure-code-profile_test.go +++ b/pkg/daemon/ceph/client/erasure-code-profile_test.go @@ -44,7 +44,7 @@ func testCreateProfile(t *testing.T, failureDomain, crushRoot, deviceClass strin executor := &exectest.MockExecutor{} context := &clusterd.Context{Executor: executor} - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[1] == "erasure-code-profile" { if args[2] == "get" { diff --git a/pkg/daemon/ceph/client/filesystem.go b/pkg/daemon/ceph/client/filesystem.go index 82a4c6f3a..c27030f9d 100644 --- a/pkg/daemon/ceph/client/filesystem.go +++ b/pkg/daemon/ceph/client/filesystem.go @@ -112,6 +112,7 @@ func GetFilesystem(context *clusterd.Context, clusterName string, fsName string) // AllowStandbyReplay gets detailed status information about a Ceph filesystem. func AllowStandbyReplay(context *clusterd.Context, clusterName string, fsName string, allowStandbyReplay bool) error { + logger.Infof("setting allow_standby_replay for filesystem %q", fsName) args := []string{"fs", "set", fsName, "allow_standby_replay", strconv.FormatBool(allowStandbyReplay)} _, err := NewCephCommand(context, clusterName, args).Run() if err != nil { @@ -127,6 +128,7 @@ func CreateFilesystem(context *clusterd.Context, clusterName, name, metadataPool return errors.New("at least one data pool is required") } + logger.Infof("creating filesystem %q with metadata pool %q and data pools %v", name, metadataPool, dataPools) args := []string{} var err error diff --git a/pkg/daemon/ceph/client/filesystem_test.go b/pkg/daemon/ceph/client/filesystem_test.go index 6f22ea871..2a5eddaf7 100644 --- a/pkg/daemon/ceph/client/filesystem_test.go +++ b/pkg/daemon/ceph/client/filesystem_test.go @@ -103,7 +103,7 @@ func TestFilesystemRemove(t *testing.T) { DataPools: []int{1}, }, } - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[0] == "fs" { if args[1] == "get" { @@ -149,7 +149,7 @@ func TestFilesystemRemove(t *testing.T) { } return "", errors.Errorf("unexpected ceph command %q", args) } - executor.MockExecuteCommandWithOutput = func(debug bool, actionName string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { emptyPool := "{\"images\":{\"count\":0,\"provisioned_bytes\":0,\"snap_count\":0},\"trash\":{\"count\":1,\"provisioned_bytes\":2048,\"snap_count\":0}}" if args[0] == "pool" { diff --git a/pkg/daemon/ceph/client/image.go b/pkg/daemon/ceph/client/image.go index 2784aecb0..81f0d24a7 100644 --- a/pkg/daemon/ceph/client/image.go +++ b/pkg/daemon/ceph/client/image.go @@ -92,11 +92,11 @@ func CreateImage(context *clusterd.Context, clusterName, name, poolName, dataPoo if dataPoolName != "" { args = append(args, fmt.Sprintf("--data-pool=%s", dataPoolName)) } + logger.Infof("creating rbd image %q with size %dMB in pool %q", imageSpec, sizeMB, dataPoolName) buf, err := NewRBDCommand(context, clusterName, args).Run() if err != nil { - cmdErr, ok := err.(*exec.CommandError) - if ok && cmdErr.ExitStatus() == int(syscall.EEXIST) { + if code, ok := exec.ExitStatus(err); ok && code == int(syscall.EEXIST) { // Image with the same name already exists in the given rbd pool. Continuing with the link to PV. logger.Warningf("Requested image %s exists in pool %s. Continuing", name, poolName) } else { @@ -117,6 +117,7 @@ func CreateImage(context *clusterd.Context, clusterName, name, poolName, dataPoo } func DeleteImage(context *clusterd.Context, clusterName, name, poolName string) error { + logger.Infof("deleting rbd image %q from pool %q", name, poolName) imageSpec := getImageSpec(name, poolName) args := []string{"rm", imageSpec} buf, err := NewRBDCommand(context, clusterName, args).Run() @@ -129,6 +130,7 @@ func DeleteImage(context *clusterd.Context, clusterName, name, poolName string) } func ExpandImage(context *clusterd.Context, clusterName, name, poolName, monitors, keyring string, size uint64) error { + logger.Infof("expanding rbd image %q in pool %q to size %dMB", name, poolName, display.BToMb(size)) imageSpec := getImageSpec(name, poolName) args := []string{ "resize", diff --git a/pkg/daemon/ceph/client/image_test.go b/pkg/daemon/ceph/client/image_test.go index 13ad06750..dc0ded3c2 100644 --- a/pkg/daemon/ceph/client/image_test.go +++ b/pkg/daemon/ceph/client/image_test.go @@ -38,7 +38,7 @@ func TestCreateImage(t *testing.T) { // mock an error during the create image call. rbd tool returns error information to the output stream, // separate from the error object, so verify that information also makes it back to us (because it is useful). - executor.MockExecuteCommandWithOutput = func(debug bool, actionName string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { switch { case command == "rbd" && args[0] == "create": return "mocked detailed ceph error output stream", errors.New("some mocked error") @@ -53,7 +53,7 @@ func TestCreateImage(t *testing.T) { // (except for 0, that's OK) createCalled := false expectedSizeArg := "" - executor.MockExecuteCommandWithOutput = func(debug bool, actionName string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { switch { case command == "rbd" && args[0] == "create": createCalled = true @@ -146,7 +146,7 @@ func TestCreateImage(t *testing.T) { func TestExpandImage(t *testing.T) { executor := &exectest.MockExecutor{} context := &clusterd.Context{Executor: executor} - executor.MockExecuteCommandWithTimeout = func(debug bool, timeout time.Duration, actionName string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithTimeout = func(timeout time.Duration, command string, args ...string) (string, error) { switch { case args[1] != "kube/some-image": return "", errors.Errorf("no image %s", args[1]) @@ -171,7 +171,7 @@ func TestListImageLogLevelInfo(t *testing.T) { var err error listCalled := false emptyListResult := false - executor.MockExecuteCommandWithOutput = func(debug bool, actionName string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { switch { case command == "rbd" && args[0] == "ls" && args[1] == "-l": listCalled = true @@ -235,7 +235,7 @@ func TestListImageLogLevelDebug(t *testing.T) { listCalled := false emptyListResult := false - executor.MockExecuteCommandWithOutput = func(debug bool, actionName string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { switch { case command == "rbd" && args[0] == "ls" && args[1] == "-l": listCalled = true diff --git a/pkg/daemon/ceph/client/mgr_test.go b/pkg/daemon/ceph/client/mgr_test.go index 0a1560b60..87eaf3a98 100644 --- a/pkg/daemon/ceph/client/mgr_test.go +++ b/pkg/daemon/ceph/client/mgr_test.go @@ -29,7 +29,7 @@ func TestEnableModuleRetries(t *testing.T) { moduleEnableRetries := 0 moduleEnableWaitTime = 0 executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) switch { case args[0] == "balancer" && args[1] == "on": @@ -66,7 +66,7 @@ func TestEnableModuleRetries(t *testing.T) { func TestEnableModule(t *testing.T) { executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) switch { case args[0] == "mgr" && args[1] == "module" && args[2] == "enable": @@ -98,7 +98,7 @@ func TestEnableModule(t *testing.T) { func TestEnableDisableBalancerModule(t *testing.T) { executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) switch { case args[0] == "balancer" && args[1] == "on": @@ -121,7 +121,7 @@ func TestEnableDisableBalancerModule(t *testing.T) { func TestSetBalancerMode(t *testing.T) { executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[0] == "balancer" && args[1] == "mode" && args[2] == "upmap" { return "", nil diff --git a/pkg/daemon/ceph/client/mon.go b/pkg/daemon/ceph/client/mon.go index a46ac36e6..40a38f076 100644 --- a/pkg/daemon/ceph/client/mon.go +++ b/pkg/daemon/ceph/client/mon.go @@ -31,17 +31,6 @@ type MonStatusResponse struct { } `json:"monmap"` } -// request to simplify deserialization of a test request -type MonStatusRequest struct { - Prefix string `json:"prefix"` - Format string `json:"format"` - ID int `json:"id"` - Weight float32 `json:"weight"` - Pool string `json:"pool"` - Var string `json:"var"` - Args []string `json:"args"` -} - // MonMapEntry represents an entry in the monitor map type MonMapEntry struct { Name string `json:"name"` @@ -61,10 +50,9 @@ type AddrvecEntry struct { } // GetMonQuorumStatus calls quorum_status mon_command -func GetMonQuorumStatus(context *clusterd.Context, clusterName string, debug bool) (MonStatusResponse, error) { +func GetMonQuorumStatus(context *clusterd.Context, clusterName string) (MonStatusResponse, error) { args := []string{"quorum_status"} cmd := NewCephCommand(context, clusterName, args) - cmd.Debug = debug buf, err := cmd.Run() if err != nil { return MonStatusResponse{}, errors.Wrapf(err, "mon quorum status failed") @@ -78,33 +66,3 @@ func GetMonQuorumStatus(context *clusterd.Context, clusterName string, debug boo return resp, nil } - -type MonTimeStatus struct { - Skew map[string]MonTimeSkewStatus `json:"time_skew_status"` - Checks struct { - Epoch int `json:"epoch"` - Round int `json:"round"` - RoundStatus string `json:"round_status"` - } `json:"timechecks"` -} - -type MonTimeSkewStatus struct { - Skew json.Number `json:"skew"` - Latency json.Number `json:"latency"` - Health string `json:"health"` -} - -func GetMonTimeStatus(context *clusterd.Context, clusterName string) (*MonTimeStatus, error) { - args := []string{"time-sync-status"} - buf, err := NewCephCommand(context, clusterName, args).Run() - if err != nil { - return nil, errors.Wrapf(err, "failed to get time sync status") - } - - var timeStatus MonTimeStatus - if err := json.Unmarshal(buf, &timeStatus); err != nil { - return nil, errors.Wrapf(err, "failed to unmarshal time sync status response") - } - - return &timeStatus, nil -} diff --git a/pkg/daemon/ceph/client/osd.go b/pkg/daemon/ceph/client/osd.go index 51783b66f..d46f212a3 100644 --- a/pkg/daemon/ceph/client/osd.go +++ b/pkg/daemon/ceph/client/osd.go @@ -234,7 +234,6 @@ func GetOSDPerfStats(context *clusterd.Context, clusterName string) (*OSDPerfSta func GetOSDDump(context *clusterd.Context, clusterName string) (*OSDDump, error) { args := []string{"osd", "dump"} cmd := NewCephCommand(context, clusterName, args) - cmd.Debug = true buf, err := cmd.Run() if err != nil { return nil, errors.Wrapf(err, "failed to get osd dump") @@ -254,12 +253,6 @@ func OSDOut(context *clusterd.Context, clusterName string, osdID int) (string, e return string(buf), err } -func OSDRemove(context *clusterd.Context, clusterName string, osdID int) (string, error) { - args := []string{"osd", "rm", strconv.Itoa(osdID)} - buf, err := NewCephCommand(context, clusterName, args).Run() - return string(buf), err -} - func OsdSafeToDestroy(context *clusterd.Context, clusterName string, osdID int, cephVersion cephver.CephVersion) (bool, error) { args := []string{"osd", "safe-to-destroy", strconv.Itoa(osdID)} cmd := NewCephCommand(context, clusterName, args) @@ -278,16 +271,6 @@ func OsdSafeToDestroy(context *clusterd.Context, clusterName string, osdID int, return false, nil } -func (usage *OSDUsage) ByID(osdID int) *OSDNodeUsage { - for i := range usage.OSDNodes { - if usage.OSDNodes[i].ID == osdID { - return &usage.OSDNodes[i] - } - } - - return nil -} - // HostTree returns the osd tree func HostTree(context *clusterd.Context, clusterName string) (OsdTree, error) { var output OsdTree diff --git a/pkg/daemon/ceph/client/osd_test.go b/pkg/daemon/ceph/client/osd_test.go index 4bf99069a..3819deca0 100644 --- a/pkg/daemon/ceph/client/osd_test.go +++ b/pkg/daemon/ceph/client/osd_test.go @@ -61,7 +61,7 @@ var ( func TestHostTree(t *testing.T) { executor := &exectest.MockExecutor{} emptyTreeResult := false - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) switch { case args[0] == "osd" && args[1] == "tree": @@ -89,7 +89,7 @@ func TestHostTree(t *testing.T) { func TestOsdListNum(t *testing.T) { executor := &exectest.MockExecutor{} emptyOsdListNumResult := false - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) switch { case args[0] == "osd" && args[1] == "ls": diff --git a/pkg/daemon/ceph/client/pg.go b/pkg/daemon/ceph/client/pg.go deleted file mode 100644 index d31acbd2a..000000000 --- a/pkg/daemon/ceph/client/pg.go +++ /dev/null @@ -1,62 +0,0 @@ -/* -Copyright 2018 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 ( - "encoding/json" - - "github.com/pkg/errors" - "github.com/rook/rook/pkg/clusterd" -) - -type PGDumpBrief struct { - PgStats []PgStats `json:"pg_stats"` -} - -type PgStats struct { - ID string `json:"pgid"` - State string `json:"state"` - UpOsdIDs []int `json:"up"` - UpPrimaryID int `json:"up_primary"` - ActingOsdIDs []int `json:"acting"` - ActingPrimaryID int `json:"acting_primary"` -} - -func GetPGDumpBrief(context *clusterd.Context, clusterName string, isNautilusOrNewer bool) (*PGDumpBrief, error) { - var pgDump PGDumpBrief - var pgStats []PgStats - args := []string{"pg", "dump", "pgs_brief"} - buf, err := NewCephCommand(context, clusterName, args).Run() - if err != nil { - return nil, errors.Wrapf(err, "failed to get pg dump") - } - - if !isNautilusOrNewer { - if err := json.Unmarshal(buf, &pgStats); err != nil { - return nil, errors.Wrapf(err, "failed to unmarshal pg dump response") - } - pgDump = PGDumpBrief{ - PgStats: pgStats, - } - return &pgDump, nil - } - - if err := json.Unmarshal(buf, &pgDump); err != nil { - return nil, errors.Wrapf(err, "failed to unmarshal pg dump response") - } - - return &pgDump, nil -} diff --git a/pkg/daemon/ceph/client/pool_test.go b/pkg/daemon/ceph/client/pool_test.go index e714b0c10..5a85a27c9 100644 --- a/pkg/daemon/ceph/client/pool_test.go +++ b/pkg/daemon/ceph/client/pool_test.go @@ -32,7 +32,7 @@ func TestCreateECPoolWithOverwrites(t *testing.T) { p := CephStoragePoolDetails{Name: "mypool", Size: 12345, ErasureCodeProfile: "myecprofile", FailureDomain: "host"} executor := &exectest.MockExecutor{} context := &clusterd.Context{Executor: executor} - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[1] == "pool" { if args[2] == "create" { @@ -66,7 +66,7 @@ func TestCreateECPoolWithoutOverwrites(t *testing.T) { p := CephStoragePoolDetails{Name: "mypool", Size: 12345, ErasureCodeProfile: "myecprofile", FailureDomain: "host"} executor := &exectest.MockExecutor{} context := &clusterd.Context{Executor: executor} - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[1] == "pool" { if args[2] == "create" { @@ -109,7 +109,7 @@ func testCreateReplicaPool(t *testing.T, failureDomain, crushRoot, deviceClass s crushRuleCreated := false executor := &exectest.MockExecutor{} context := &clusterd.Context{Executor: executor} - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[1] == "pool" { if args[2] == "create" { @@ -180,7 +180,7 @@ func TestGetPoolStatistics(t *testing.T) { p.Trash.SnapCount = 0 executor := &exectest.MockExecutor{} context := &clusterd.Context{Executor: executor} - executor.MockExecuteCommandWithOutput = func(debug bool, actionName string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { a := "{\"images\":{\"count\":1,\"provisioned_bytes\":1024,\"snap_count\":1},\"trash\":{\"count\":1,\"provisioned_bytes\":2048,\"snap_count\":0}}" logger.Infof("Command: %s %v", command, args) @@ -210,7 +210,7 @@ func TestSetPoolReplicatedSizeProperty(t *testing.T) { p := CephStoragePoolDetails{Name: "mypool", Size: 12345} executor := &exectest.MockExecutor{} context := &clusterd.Context{Executor: executor} - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[2] == "set" { @@ -228,7 +228,7 @@ func TestSetPoolReplicatedSizeProperty(t *testing.T) { assert.NoError(t, err) // TEST POOL SIZE 1 AND RequireSafeReplicaSize True - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[2] == "set" { diff --git a/pkg/daemon/ceph/client/status.go b/pkg/daemon/ceph/client/status.go index f4da83c0c..9d0fb4843 100644 --- a/pkg/daemon/ceph/client/status.go +++ b/pkg/daemon/ceph/client/status.go @@ -142,10 +142,9 @@ type Fsmap struct { UpStandby int `json:"up:standby"` } -func Status(context *clusterd.Context, clusterName string, debug bool) (CephStatus, error) { +func Status(context *clusterd.Context, clusterName string) (CephStatus, error) { args := []string{"status"} cmd := NewCephCommand(context, clusterName, args) - cmd.Debug = debug buf, err := cmd.Run() if err != nil { return CephStatus{}, errors.Wrapf(err, "failed to get status. %s", string(buf)) @@ -164,7 +163,7 @@ func Status(context *clusterd.Context, clusterName string, debug bool) (CephStat // clean is true if the cluster is clean // err is not nil if getting the status failed. func IsClusterClean(context *clusterd.Context, clusterName string) (string, bool, error) { - status, err := Status(context, clusterName, true) + status, err := Status(context, clusterName) if err != nil { return "unable to get PG health", false, err } @@ -234,7 +233,7 @@ func getMDSRank(status CephStatus, clusterName, fsName string) (int, error) { // MdsActiveOrStandbyReplay returns wether a given MDS is active or in standby func MdsActiveOrStandbyReplay(context *clusterd.Context, clusterName, fsName string) error { - status, err := Status(context, clusterName, false) + status, err := Status(context, clusterName) if err != nil { return errors.Wrapf(err, "failed to get ceph status") } @@ -261,7 +260,7 @@ func MdsActiveOrStandbyReplay(context *clusterd.Context, clusterName, fsName str // IsCephHealthy verifies Ceph is healthy, useful when performing an upgrade // check if it's a minor or major upgrade... too! func IsCephHealthy(context *clusterd.Context, clusterName string) bool { - cephStatus, err := Status(context, clusterName, false) + cephStatus, err := Status(context, clusterName) if err != nil { logger.Errorf("failed to detect if Ceph is healthy. failed to get ceph status. %v", err) return false diff --git a/pkg/daemon/ceph/client/upgrade.go b/pkg/daemon/ceph/client/upgrade.go index 23f84c9c5..be5747316 100644 --- a/pkg/daemon/ceph/client/upgrade.go +++ b/pkg/daemon/ceph/client/upgrade.go @@ -366,7 +366,7 @@ func buildHostListFromTree(tree OsdTree) (OsdTree, error) { return osdList, nil } -// osdDoNothing determines wether we should perfom upgrade pre-check and post-checks for the OSD daemon +// osdDoNothing determines wether we should perform upgrade pre-check and post-checks for the OSD daemon // it checks for various cluster info like number of OSD and their placement // it returns 'true' if we need to do nothing and false and we should pre-check/post-check func osdDoNothing(context *clusterd.Context, clusterName string) bool { diff --git a/pkg/daemon/ceph/client/upgrade_test.go b/pkg/daemon/ceph/client/upgrade_test.go index bcf114c67..34591a258 100644 --- a/pkg/daemon/ceph/client/upgrade_test.go +++ b/pkg/daemon/ceph/client/upgrade_test.go @@ -28,7 +28,7 @@ import ( func TestGetCephMonVersionString(t *testing.T) { executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutput = func(debug bool, name string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { assert.Equal(t, "version", args[0]) return "", nil } @@ -40,7 +40,7 @@ func TestGetCephMonVersionString(t *testing.T) { func TestGetCephMonVersionsString(t *testing.T) { executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutput = func(debug bool, name string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { assert.Equal(t, "versions", args[0]) return "", nil } @@ -52,7 +52,7 @@ func TestGetCephMonVersionsString(t *testing.T) { func TestEnableMessenger2(t *testing.T) { executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutput = func(debug bool, name string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { assert.Equal(t, "mon", args[0]) assert.Equal(t, "enable-msgr2", args[1]) return "", nil @@ -65,7 +65,7 @@ func TestEnableMessenger2(t *testing.T) { func TestEnableReleaseOSDFunctionality(t *testing.T) { executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutput = func(debug bool, name string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { assert.Equal(t, "osd", args[0]) assert.Equal(t, "require-osd-release", args[1]) assert.Equal(t, 3, len(args)) @@ -80,7 +80,7 @@ func TestEnableReleaseOSDFunctionality(t *testing.T) { func TestOkToStopDaemon(t *testing.T) { // First test executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutput = func(debug bool, name string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { switch { case args[0] == "mon" && args[1] == "ok-to-stop" && args[2] == "a": return "", nil @@ -94,7 +94,7 @@ func TestOkToStopDaemon(t *testing.T) { assert.NoError(t, err) // Second test - executor.MockExecuteCommandWithOutput = func(debug bool, name string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { assert.Equal(t, "mgr", args[0]) assert.Equal(t, "ok-to-stop", args[1]) assert.Equal(t, "a", args[2]) @@ -107,7 +107,7 @@ func TestOkToStopDaemon(t *testing.T) { assert.NoError(t, err) // Third test - executor.MockExecuteCommandWithOutput = func(debug bool, name string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { assert.Equal(t, "dummy", args[0]) assert.Equal(t, "ok-to-stop", args[1]) assert.Equal(t, "a", args[2]) diff --git a/pkg/daemon/ceph/client/usage.go b/pkg/daemon/ceph/client/usage.go deleted file mode 100644 index a887501c1..000000000 --- a/pkg/daemon/ceph/client/usage.go +++ /dev/null @@ -1,47 +0,0 @@ -/* -Copyright 2016 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 ( - "encoding/json" - - "github.com/pkg/errors" - "github.com/rook/rook/pkg/clusterd" -) - -type CephUsage struct { - Stats struct { - TotalBytes json.Number `json:"total_bytes"` - TotalUsedBytes json.Number `json:"total_used_bytes"` - TotalAvailBytes json.Number `json:"total_avail_bytes"` - TotalObjects json.Number `json:"total_objects"` - } `json:"stats"` -} - -func Usage(context *clusterd.Context, clusterName string) (*CephUsage, error) { - args := []string{"df", "detail"} - buf, err := NewCephCommand(context, clusterName, args).Run() - if err != nil { - return nil, errors.Wrapf(err, "failed to get usage") - } - - var usage CephUsage - if err := json.Unmarshal(buf, &usage); err != nil { - return nil, errors.Wrapf(err, "failed to unmarshal usage response") - } - - return &usage, nil -} diff --git a/pkg/daemon/ceph/osd/agent.go b/pkg/daemon/ceph/osd/agent.go index ab4ca8fd5..3e0336712 100644 --- a/pkg/daemon/ceph/osd/agent.go +++ b/pkg/daemon/ceph/osd/agent.go @@ -17,13 +17,10 @@ limitations under the License. package osd import ( - "fmt" - "github.com/rook/rook/pkg/clusterd" cephconfig "github.com/rook/rook/pkg/daemon/ceph/config" "github.com/rook/rook/pkg/operator/ceph/cluster/osd/config" "github.com/rook/rook/pkg/operator/k8sutil" - "github.com/rook/rook/pkg/util/proc" ) const ( @@ -35,10 +32,8 @@ type OsdAgent struct { cluster *cephconfig.ClusterInfo nodeName string forceFormat bool - osdProc map[int]*proc.MonitoredProc devices []DesiredDevice metadataDevice string - procMan *proc.ProcManager storeConfig config.StoreConfig kv *k8sutil.ConfigMapKVStore pvcBacked bool @@ -64,14 +59,11 @@ func NewAgent(context *clusterd.Context, devices []DesiredDevice, metadataDevice nodeName: nodeName, kv: kv, pvcBacked: pvcBacked, - procMan: proc.New(context.Executor), - osdProc: make(map[int]*proc.MonitoredProc), } } func getDeviceLVPath(context *clusterd.Context, deviceName string) string { - cmd := fmt.Sprintf("get logical volume path for device") - output, err := context.Executor.ExecuteCommandWithOutput(false, cmd, "pvdisplay", "-C", "-o", "lvpath", "--noheadings", deviceName) + output, err := context.Executor.ExecuteCommandWithOutput("pvdisplay", "-C", "-o", "lvpath", "--noheadings", deviceName) if err != nil { logger.Warningf("failed to retrieve logical volume path for %q. %v", deviceName, err) return "" diff --git a/pkg/daemon/ceph/osd/daemon.go b/pkg/daemon/ceph/osd/daemon.go index 184e516ed..a16a2b855 100644 --- a/pkg/daemon/ceph/osd/daemon.go +++ b/pkg/daemon/ceph/osd/daemon.go @@ -68,23 +68,23 @@ func StartOSD(context *clusterd.Context, osdType, osdID, osdUUID, lvPath string, go handleTerminate(context, lvPath, volumeGroupName) - if err := context.Executor.ExecuteCommand(false, "", "vgchange", "-an", volumeGroupName); err != nil { + if err := context.Executor.ExecuteCommand("vgchange", "-an", volumeGroupName); err != nil { return errors.Wrapf(err, "failed to deactivate volume group for lv %q", lvPath) } - if err := context.Executor.ExecuteCommand(false, "", "vgchange", "-ay", volumeGroupName); err != nil { + if err := context.Executor.ExecuteCommand("vgchange", "-ay", volumeGroupName); err != nil { return errors.Wrapf(err, "failed to activate volume group for lv %q", lvPath) } } // activate the osd with ceph-volume storeFlag := "--" + osdType - if err := context.Executor.ExecuteCommand(false, "", "stdbuf", "-oL", "ceph-volume", "lvm", "activate", "--no-systemd", storeFlag, osdID, osdUUID); err != nil { + if err := context.Executor.ExecuteCommand("stdbuf", "-oL", "ceph-volume", "lvm", "activate", "--no-systemd", storeFlag, osdID, osdUUID); err != nil { return errors.Wrapf(err, "failed to activate osd") } // run the ceph-osd daemon - if err := context.Executor.ExecuteCommand(false, "", "ceph-osd", cephArgs...); err != nil { + if err := context.Executor.ExecuteCommand("ceph-osd", cephArgs...); err != nil { // Instead of returning, we want to allow the lvm release to happen below, so we just log the err logger.Errorf("failed to start osd or shutting down. %v", err) } @@ -116,7 +116,7 @@ func handleTerminate(context *clusterd.Context, lvPath, volumeGroupName string) func killCephOSDProcess(context *clusterd.Context, lvPath string) error { - pid, err := context.Executor.ExecuteCommandWithOutput(false, "", "fuser", "-a", lvPath) + pid, err := context.Executor.ExecuteCommandWithOutput("fuser", "-a", lvPath) if err != nil { return errors.Wrapf(err, "failed to retrieve process ID for %q", lvPath) } @@ -131,7 +131,7 @@ func killCephOSDProcess(context *clusterd.Context, lvPath string) error { // been testing with kill -9 so this is expected to be safe. There is a fix upstream Ceph that will // improve the shutdown time of the OSD. For cleanliness we should consider removing the -9 // once it is backported to Nautilus: https://github.com/ceph/ceph/pull/31677. - if err := context.Executor.ExecuteCommand(false, "", "kill", "-9", pid); err != nil { + if err := context.Executor.ExecuteCommand("kill", "-9", pid); err != nil { return errors.Wrapf(err, "failed to kill ceph-osd process") } } @@ -429,7 +429,7 @@ func getAvailableDevices(context *clusterd.Context, desiredDevices []DesiredDevi // releaseLVMDevice deactivates the LV to release the device. func releaseLVMDevice(context *clusterd.Context, volumeGroupName string) error { - if err := context.Executor.ExecuteCommand(false, "", "lvchange", "-an", volumeGroupName); err != nil { + if err := context.Executor.ExecuteCommand("lvchange", "-an", volumeGroupName); err != nil { return errors.Wrapf(err, "failed to deactivate LVM %s", volumeGroupName) } logger.Info("Successfully released device from lvm") diff --git a/pkg/daemon/ceph/osd/daemon_test.go b/pkg/daemon/ceph/osd/daemon_test.go index 41a09d01d..8751226b5 100644 --- a/pkg/daemon/ceph/osd/daemon_test.go +++ b/pkg/daemon/ceph/osd/daemon_test.go @@ -137,23 +137,23 @@ USEC_INITIALIZED=15981915740802 func TestAvailableDevices(t *testing.T) { executor := &exectest.MockExecutor{} // set up a mock function to return "rook owned" partitions on the device and it does not have a filesystem - executor.MockExecuteCommandWithOutput = func(debug bool, name string, command string, args ...string) (string, error) { - logger.Infof("OUTPUT for %s. %s %+v", name, command, args) + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { + logger.Infof("OUTPUT for %s %v", command, args) if command == "lsblk" { - if strings.Index(name, "sdb") != -1 { + if strings.Index(args[3], "sdb") != -1 { // /dev/sdb has a partition return `NAME="sdb" SIZE="65" TYPE="disk" PKNAME="" NAME="sdb1" SIZE="30" TYPE="part" PKNAME="sdb"`, nil } return "", nil } else if command == "blkid" { - if strings.Index(name, "sdb1") != -1 { + if strings.Index(args[3], "sdb1") != -1 { // partition sdb1 has a label MY-PART return "MY-PART", nil } } else if command == "udevadm" { - if strings.Index(name, "sdc") != -1 { + if strings.Index(args[3], "sdc") != -1 { // /dev/sdc has a file system return udevFSOutput, nil } diff --git a/pkg/daemon/ceph/osd/device.go b/pkg/daemon/ceph/osd/device.go index 916a2df99..c8fbda9c5 100644 --- a/pkg/daemon/ceph/osd/device.go +++ b/pkg/daemon/ceph/osd/device.go @@ -19,7 +19,6 @@ package osd import ( "encoding/json" "fmt" - "time" "github.com/google/uuid" @@ -29,7 +28,6 @@ import ( cephconfig "github.com/rook/rook/pkg/daemon/ceph/config" "github.com/rook/rook/pkg/operator/ceph/cluster/osd/config" "github.com/rook/rook/pkg/operator/k8sutil" - "github.com/rook/rook/pkg/util/exec" ) const ( @@ -101,24 +99,6 @@ func (m *DeviceOsdMapping) String() string { return string(b) } -func waitForPath(path string, executor exec.Executor) error { - retryCount := 0 - retryMax := 25 - sleepTime := 250 - for { - _, err := executor.ExecuteStat(path) - if err == nil { - return nil - } - - retryCount++ - if retryCount > retryMax { - return err - } - <-time.After(time.Duration(sleepTime) * time.Millisecond) - } -} - func initializeOSD(config *osdConfig, context *clusterd.Context, cluster *cephconfig.ClusterInfo) error { // add auth privileges for the OSD, the bootstrap-osd privileges were very limited if err := addOSDAuth(context, cluster.Name, config.id, config.rootPath); err != nil { diff --git a/pkg/daemon/ceph/osd/device_test.go b/pkg/daemon/ceph/osd/device_test.go index 173b20114..8e2572163 100644 --- a/pkg/daemon/ceph/osd/device_test.go +++ b/pkg/daemon/ceph/osd/device_test.go @@ -33,7 +33,7 @@ func TestOSDBootstrap(t *testing.T) { defer os.RemoveAll(configDir) executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { return "{\"key\":\"mysecurekey\"}", nil }, } diff --git a/pkg/daemon/ceph/osd/volume.go b/pkg/daemon/ceph/osd/volume.go index 9d5830c11..539579f66 100644 --- a/pkg/daemon/ceph/osd/volume.go +++ b/pkg/daemon/ceph/osd/volume.go @@ -294,7 +294,7 @@ func (a *OsdAgent) initializeBlockPVC(context *clusterd.Context, devices *Device } // execute ceph-volume with the device - op, err := context.Executor.ExecuteCommandWithCombinedOutput(false, "", baseCommand, immediateExecuteArgs...) + op, err := context.Executor.ExecuteCommandWithCombinedOutput(baseCommand, immediateExecuteArgs...) if err != nil { cvLogFilePath := path.Join(cvLogDir, "ceph-volume.log") @@ -341,12 +341,11 @@ func getLVPath(op string) string { } func getLVNameFromDevicePath(context *clusterd.Context, devicePath string) (string, error) { - devInfo, err := context.Executor.ExecuteCommandWithOutput(true, "", - "dmsetup", "info", "-c", "--noheadings", "-o", "name", devicePath) + devInfo, err := context.Executor.ExecuteCommandWithOutput("dmsetup", "info", "-c", "--noheadings", "-o", "name", devicePath) if err != nil { return "", errors.Wrapf(err, "failed dmsetup info. output: %q", devInfo) } - out, err := context.Executor.ExecuteCommandWithOutput(true, "", "dmsetup", "splitname", devInfo, "--noheadings") + out, err := context.Executor.ExecuteCommandWithOutput("dmsetup", "splitname", devInfo, "--noheadings") if err != nil { return "", errors.Wrapf(err, "failed dmsetup splitname %q", devInfo) } @@ -487,12 +486,12 @@ func (a *OsdAgent) initializeDevices(context *clusterd.Context, devices *DeviceO logger.Infof("Base command - %+v", baseCommand) logger.Infof("immediateReportArgs - %+v", baseCommand) logger.Infof("immediateExecuteArgs - %+v", immediateExecuteArgs) - if err := context.Executor.ExecuteCommand(false, "", baseCommand, immediateReportArgs...); err != nil { + if err := context.Executor.ExecuteCommand(baseCommand, immediateReportArgs...); err != nil { return errors.Wrapf(err, "failed ceph-volume report") // fail return here as validation provided by ceph-volume } // execute ceph-volume immediately with the device-specific setting instead of batching up multiple devices together - if err := context.Executor.ExecuteCommand(false, "", baseCommand, immediateExecuteArgs...); err != nil { + if err := context.Executor.ExecuteCommand(baseCommand, immediateExecuteArgs...); err != nil { return errors.Wrapf(err, "failed ceph-volume") } @@ -535,7 +534,7 @@ func (a *OsdAgent) initializeDevices(context *clusterd.Context, devices *DeviceO "--report", }...) - if err := context.Executor.ExecuteCommand(false, "", baseCommand, reportArgs...); err != nil { + if err := context.Executor.ExecuteCommand(baseCommand, reportArgs...); err != nil { return errors.Wrapf(err, "failed ceph-volume report") // fail return here as validation provided by ceph-volume } @@ -544,7 +543,7 @@ func (a *OsdAgent) initializeDevices(context *clusterd.Context, devices *DeviceO "json", }...) - cvOut, err := context.Executor.ExecuteCommandWithCombinedOutput(false, "", baseCommand, reportArgs...) + cvOut, err := context.Executor.ExecuteCommandWithCombinedOutput(baseCommand, reportArgs...) if err != nil { return errors.Wrapf(err, "failed ceph-volume json report: %s", cvOut) // fail return here as validation provided by ceph-volume } @@ -561,7 +560,7 @@ func (a *OsdAgent) initializeDevices(context *clusterd.Context, devices *DeviceO } // execute ceph-volume batching up multiple devices - if err := context.Executor.ExecuteCommand(false, "", baseCommand, mdArgs...); err != nil { + if err := context.Executor.ExecuteCommand(baseCommand, mdArgs...); err != nil { return errors.Wrapf(err, "failed ceph-volume") // fail return here as validation provided by ceph-volume } } @@ -587,11 +586,10 @@ func getCephVolumeLVMOSDs(context *clusterd.Context, clusterName string, cephfsi // lv can be a block device if raw mode is used cvMode := "lvm" - result, err := context.Executor.ExecuteCommandWithOutput(false, "", cephVolumeCmd, cvMode, "list", lv, "--format", "json") + result, err := context.Executor.ExecuteCommandWithOutput(cephVolumeCmd, cvMode, "list", lv, "--format", "json") if err != nil { return nil, errors.Wrapf(err, "failed to retrieve ceph-volume %s list results", cvMode) } - logger.Debugf("%v", result) var osds []oposd.OSDInfo var cephVolumeResult map[string][]osdInfo @@ -664,11 +662,10 @@ func getCephVolumeRawOSDs(context *clusterd.Context, clusterName string, cephfsi // lv can be a block device if raw mode is used cvMode := "raw" - result, err := context.Executor.ExecuteCommandWithOutput(false, "", cephVolumeCmd, cvMode, "list", block, "--format", "json") + result, err := context.Executor.ExecuteCommandWithOutput(cephVolumeCmd, cvMode, "list", block, "--format", "json") if err != nil { return nil, errors.Wrapf(err, "failed to retrieve ceph-volume %s list results", cvMode) } - logger.Debugf("%v", result) var osds []oposd.OSDInfo var cephVolumeResult map[string]osdInfoBlock diff --git a/pkg/daemon/ceph/osd/volume_test.go b/pkg/daemon/ceph/osd/volume_test.go index 47758ca75..9aea3996c 100644 --- a/pkg/daemon/ceph/osd/volume_test.go +++ b/pkg/daemon/ceph/osd/volume_test.go @@ -262,8 +262,8 @@ var cephVolumeRAWTestResult = `{ func TestParseCephVolumeLVMResult(t *testing.T) { executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutput = func(debug bool, name string, command string, args ...string) (string, error) { - logger.Infof("%s %+v", command, args) + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { + logger.Infof("%s %v", command, args) if command == "ceph-volume" { return cephVolumeLVMTestResult, nil @@ -281,8 +281,8 @@ func TestParseCephVolumeLVMResult(t *testing.T) { func TestParseCephVolumeRawResult(t *testing.T) { executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutput = func(debug bool, name string, command string, args ...string) (string, error) { - logger.Infof("%s %+v", command, args) + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { + logger.Infof("%s %v", command, args) if command == "ceph-volume" { return cephVolumeRAWTestResult, nil @@ -301,8 +301,8 @@ func TestParseCephVolumeRawResult(t *testing.T) { func TestCephVolumeResultMultiClusterSingleOSD(t *testing.T) { executor := &exectest.MockExecutor{} // set up a mock function to return "rook owned" partitions on the device and it does not have a filesystem - executor.MockExecuteCommandWithOutput = func(debug bool, name string, command string, args ...string) (string, error) { - logger.Infof("%s %+v", command, args) + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { + logger.Infof("%s %v", command, args) if command == "ceph-volume" { return cephVolumeTestResultMultiCluster, nil @@ -322,8 +322,8 @@ func TestCephVolumeResultMultiClusterSingleOSD(t *testing.T) { func TestCephVolumeResultMultiClusterMultiOSD(t *testing.T) { executor := &exectest.MockExecutor{} // set up a mock function to return "rook owned" partitions on the device and it does not have a filesystem - executor.MockExecuteCommandWithOutput = func(debug bool, name string, command string, args ...string) (string, error) { - logger.Infof("%s %+v", command, args) + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { + logger.Infof("%s %v", command, args) if command == "ceph-volume" { return cephVolumeTestResultMultiClusterMultiOSD, nil diff --git a/pkg/daemon/discover/discover.go b/pkg/daemon/discover/discover.go index 6f17ca4af..8b43942b3 100644 --- a/pkg/daemon/discover/discover.go +++ b/pkg/daemon/discover/discover.go @@ -460,7 +460,7 @@ func probeDevices(context *clusterd.Context) ([]sys.LocalDisk, error) { // getCephVolumeInventory: Return a map of strings indexed by device with the // information about the device returned by the command func getCephVolumeInventory(context *clusterd.Context) (*map[string]string, error) { - inventory, err := context.Executor.ExecuteCommandWithOutput(false, "", "ceph-volume", "inventory", "--format", "json") + inventory, err := context.Executor.ExecuteCommandWithOutput("ceph-volume", "inventory", "--format", "json") if err != nil { return nil, fmt.Errorf("failed to execute ceph-volume inventory. %+v", err) } diff --git a/pkg/daemon/discover/discover_test.go b/pkg/daemon/discover/discover_test.go index 55f636994..b0752afcf 100644 --- a/pkg/daemon/discover/discover_test.go +++ b/pkg/daemon/discover/discover_test.go @@ -71,31 +71,16 @@ Partition table holds up to 128 entries func TestProbeDevices(t *testing.T) { // set up mock execute so we can verify the partitioning happens on sda executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutput = func(debug bool, name string, command string, args ...string) (string, error) { - logger.Infof("RUN Command for '%s'. %s arg %+v", name, command, args) + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { + logger.Infof("RUN Command %s %v", command, args) output := "" - switch name { - case "lsblk all": + if args[0] == "--all" { output = "testa" - case "lsblk /dev/testa": + } else if args[0] == "/dev/testa" { output = `SIZE="249510756352" ROTA="1" RO="0" TYPE="disk" PKNAME=""` - case "get filesystem type for /dev/testa": - output = udevOutput - case "get parent for device testa": - output = ` testa -testa testa1 -testa testa2 -testa2 centos_host13-root -testa2 centos_host13-swap -testa2 centos_host13-home -` - case "get disk /dev/testa uuid": - output = sgdiskOutput - - case "get disk testa fs serial": + } else if args[0] == "info" && args[1] == "--query=property" { output = udevOutput } - return output, nil } @@ -105,7 +90,6 @@ testa2 centos_host13-home assert.Nil(t, err) assert.Equal(t, 1, len(devices)) assert.Equal(t, "ext2", devices[0].Filesystem) - } func TestMatchUdevMonitorFiltering(t *testing.T) { @@ -354,9 +338,9 @@ func TestDeviceListsEqual(t *testing.T) { func TestGetCephVolumeInventory(t *testing.T) { run := 0 executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, arg ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, arg ...string) (string, error) { run++ - logger.Infof("run %d action %s command %s", run, actionName, command) + logger.Infof("run %d command %s", run, command) switch { case run == 1: return `[{"available": true, "rejected_reasons": [], "sys_api": {"scheduler_mode": "noop", diff --git a/pkg/operator/ceph/client/controller_test.go b/pkg/operator/ceph/client/controller_test.go index 57641a296..230d740e0 100644 --- a/pkg/operator/ceph/client/controller_test.go +++ b/pkg/operator/ceph/client/controller_test.go @@ -102,7 +102,7 @@ func TestGenerateClient(t *testing.T) { func TestCreateClient(t *testing.T) { clientset := testop.New(t, 1) executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command, outfileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command, outfileArg string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if command == "ceph" && args[1] == "get-or-create-key" { return `{"key":"AQC7ilJdAPijOBAABp+YAzg2QupRAWdnIh7w/Q=="}`, nil @@ -146,7 +146,7 @@ func TestCreateClient(t *testing.T) { func TestUpdateClient(t *testing.T) { clientset := testop.New(t, 1) executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command, outfileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command, outfileArg string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) return "", nil }, @@ -180,7 +180,7 @@ func TestUpdateClient(t *testing.T) { func TestDeleteClient(t *testing.T) { clientset := testop.New(t, 2) executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command, outfileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command, outfileArg string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if command == "ceph" && args[1] == "get-key" && args[2] == "client1" { return `{"key":"AQC7ilJdAPijOBAABp+YAzg2QupRAWdnIh7w/Q=="}`, nil diff --git a/pkg/operator/ceph/cluster/cephstatus.go b/pkg/operator/ceph/cluster/cephstatus.go index 706219d31..45c578e7a 100644 --- a/pkg/operator/ceph/cluster/cephstatus.go +++ b/pkg/operator/ceph/cluster/cephstatus.go @@ -81,7 +81,7 @@ func (c *cephStatusChecker) checkCephStatus(stopCh chan struct{}) { // checkStatus queries the status of ceph health then updates the CR status func (c *cephStatusChecker) checkStatus() { logger.Debugf("checking health of cluster") - status, err := client.Status(c.context, c.namespace, true) + status, err := client.Status(c.context, c.namespace) if err != nil { logger.Errorf("failed to get ceph status. %v", err) return diff --git a/pkg/operator/ceph/cluster/crash/add.go b/pkg/operator/ceph/cluster/crash/add.go index f437a2af1..38dfb7c46 100644 --- a/pkg/operator/ceph/cluster/crash/add.go +++ b/pkg/operator/ceph/cluster/crash/add.go @@ -118,7 +118,6 @@ func Add(mgr manager.Manager, context *clusterd.Context) error { req := reconcile.Request{NamespacedName: types.NamespacedName{Name: nodeName}} return []reconcile.Request{req} } - logger.Debugf("%q is not a ceph pod", pod.Name) return []reconcile.Request{} }), }, diff --git a/pkg/operator/ceph/cluster/mgr/dashboard.go b/pkg/operator/ceph/cluster/mgr/dashboard.go index c47d151f4..2ccbabda6 100755 --- a/pkg/operator/ceph/cluster/mgr/dashboard.go +++ b/pkg/operator/ceph/cluster/mgr/dashboard.go @@ -20,7 +20,6 @@ package mgr import ( "context" "crypto/rand" - "os/exec" "strconv" "syscall" "time" @@ -210,29 +209,22 @@ func (c *Cluster) createSelfSignedCert() (bool, error) { return false, nil } -// Get the return code from the process -func getExitCode(err error) (int, bool) { - if exiterr, ok := err.(*exec.ExitError); ok { - if status, ok := exiterr.Sys().(syscall.WaitStatus); ok { - return status.ExitStatus(), true - } - } - return 0, false -} - func (c *Cluster) setLoginCredentials(password string) error { // Set the login credentials. Write the command/args to the debug log so we don't write the password by default to the log. - logger.Infof("Running command: ceph dashboard set-login-credentials admin *******") + logger.Infof("setting ceph dashboard %q login creds", dashboardUsername) + // retry a few times in the case that the mgr module is not ready to accept commands - _, err := client.ExecuteCephCommandWithRetry(func() ([]byte, error) { + _, err := client.ExecuteCephCommandWithRetry(func() (string, []byte, error) { args := []string{"dashboard", "set-login-credentials", dashboardUsername, password} cmd := client.NewCephCommand(c.context, c.Namespace, args) - cmd.Debug = true - return cmd.RunWithTimeout(client.CmdExecuteTimeout) + output, err := cmd.RunWithTimeout(client.CmdExecuteTimeout) + return "set dashboard creds", output, err }, c.exitCode, 5, invalidArgErrorCode, dashboardInitWaitTime) if err != nil { return errors.Wrapf(err, "failed to set login creds on mgr") } + + logger.Infof("successfully set ceph dashboard creds") return nil } diff --git a/pkg/operator/ceph/cluster/mgr/dashboard_test.go b/pkg/operator/ceph/cluster/mgr/dashboard_test.go index f80f351c7..97dc8eb11 100644 --- a/pkg/operator/ceph/cluster/mgr/dashboard_test.go +++ b/pkg/operator/ceph/cluster/mgr/dashboard_test.go @@ -78,7 +78,7 @@ func TestStartSecureDashboard(t *testing.T) { exitCodeResponse := 0 clientset := test.New(t, 3) executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { logger.Infof("command: %s %v", command, args) exitCodeResponse = 0 if args[1] == "module" { @@ -99,8 +99,8 @@ func TestStartSecureDashboard(t *testing.T) { return "", nil }, } - executor.MockExecuteCommandWithOutputFileTimeout = func(debug bool, timeout time.Duration, actionName string, command, outfileArg string, arg ...string) (string, error) { - return executor.MockExecuteCommandWithOutputFile(debug, actionName, command, outfileArg, arg...) + executor.MockExecuteCommandWithOutputFileTimeout = func(timeout time.Duration, command, outfileArg string, arg ...string) (string, error) { + return executor.MockExecuteCommandWithOutputFile(command, outfileArg, arg...) } clusterInfo := &cephconfig.ClusterInfo{ diff --git a/pkg/operator/ceph/cluster/mgr/mgr.go b/pkg/operator/ceph/cluster/mgr/mgr.go index 5b0ae8ee3..828982bcd 100644 --- a/pkg/operator/ceph/cluster/mgr/mgr.go +++ b/pkg/operator/ceph/cluster/mgr/mgr.go @@ -37,6 +37,7 @@ import ( "github.com/rook/rook/pkg/operator/ceph/controller" cephver "github.com/rook/rook/pkg/operator/ceph/version" "github.com/rook/rook/pkg/operator/k8sutil" + "github.com/rook/rook/pkg/util/exec" v1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -121,7 +122,7 @@ func New( resources: resources, priorityClassName: priorityClassName, ownerRef: ownerRef, - exitCode: getExitCode, + exitCode: exec.ExitStatus, dataDirHostPath: dataDirHostPath, skipUpgradeChecks: skipUpgradeChecks, } @@ -167,7 +168,6 @@ func (c *Cluster) Start() error { // start the deployment d := c.makeDeployment(mgrConfig) - logger.Debugf("starting mgr deployment: %+v", d) // Set the deployment hash as an annotation err = patch.DefaultAnnotator.SetLastAppliedAnnotation(d) diff --git a/pkg/operator/ceph/cluster/mgr/mgr_test.go b/pkg/operator/ceph/cluster/mgr/mgr_test.go index 54d711da0..456527898 100755 --- a/pkg/operator/ceph/cluster/mgr/mgr_test.go +++ b/pkg/operator/ceph/cluster/mgr/mgr_test.go @@ -43,7 +43,7 @@ func TestStartMGR(t *testing.T) { updateDeploymentAndWait, deploymentsUpdated = testopk8s.UpdateDeploymentAndWaitStub() executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { return "{\"key\":\"mysecurekey\"}", nil }, } @@ -139,7 +139,7 @@ func TestConfigureModules(t *testing.T) { configSettings := map[string]string{} lastModuleConfigured := "" executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if command == "ceph" && len(args) > 3 { if args[0] == "mgr" && args[1] == "module" { diff --git a/pkg/operator/ceph/cluster/mgr/orchestrator.go b/pkg/operator/ceph/cluster/mgr/orchestrator.go index 1069a52a6..534cbe300 100644 --- a/pkg/operator/ceph/cluster/mgr/orchestrator.go +++ b/pkg/operator/ceph/cluster/mgr/orchestrator.go @@ -55,9 +55,10 @@ func (c *Cluster) setRookOrchestratorBackend() error { orchestratorCLIName = orchestratorNewCLIName } // retry a few times in the case that the mgr module is not ready to accept commands - _, err := client.ExecuteCephCommandWithRetry(func() ([]byte, error) { + _, err := client.ExecuteCephCommandWithRetry(func() (string, []byte, error) { args := []string{orchestratorCLIName, "set", "backend", "rook"} - return client.NewCephCommand(c.context, c.Namespace, args).RunWithTimeout(client.CmdExecuteTimeout) + output, err := client.NewCephCommand(c.context, c.Namespace, args).RunWithTimeout(client.CmdExecuteTimeout) + return "set rook backend", output, err }, c.exitCode, 5, invalidArgErrorCode, orchestratorInitWaitTime) if err != nil { return errors.Wrapf(err, "failed to set rook as the orchestrator backend") diff --git a/pkg/operator/ceph/cluster/mgr/orchestrator_test.go b/pkg/operator/ceph/cluster/mgr/orchestrator_test.go index 1cd66387e..2b11fd072 100644 --- a/pkg/operator/ceph/cluster/mgr/orchestrator_test.go +++ b/pkg/operator/ceph/cluster/mgr/orchestrator_test.go @@ -34,7 +34,7 @@ func TestOrchestratorModules(t *testing.T) { rookModuleEnabled := false rookBackendSet := false backendErrorCount := 0 - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[0] == "mgr" && args[1] == "module" && args[2] == "enable" { if args[3] == "orchestrator_cli" { @@ -48,7 +48,7 @@ func TestOrchestratorModules(t *testing.T) { } return "", errors.Errorf("unexpected ceph command '%v'", args) } - executor.MockExecuteCommandWithOutputFileTimeout = func(debug bool, timeout time.Duration, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFileTimeout = func(timeout time.Duration, command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[0] == "orchestrator" && args[1] == "set" && args[2] == "backend" && args[3] == "rook" { if backendErrorCount < 5 { @@ -93,7 +93,7 @@ func TestOrchestratorModules(t *testing.T) { c.clusterInfo.CephVersion = cephver.Octopus err = c.setRookOrchestratorBackend() assert.Error(t, err) - executor.MockExecuteCommandWithOutputFileTimeout = func(debug bool, timeout time.Duration, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFileTimeout = func(timeout time.Duration, command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[0] == "orch" && args[1] == "set" && args[2] == "backend" && args[3] == "rook" { if backendErrorCount < 5 { diff --git a/pkg/operator/ceph/cluster/mon/config.go b/pkg/operator/ceph/cluster/mon/config.go index b5a497d4c..3a1715a4b 100644 --- a/pkg/operator/ceph/cluster/mon/config.go +++ b/pkg/operator/ceph/cluster/mon/config.go @@ -249,7 +249,7 @@ func genSecret(executor exec.Executor, configDir, name string, args []string) (s "-n", name, } args = append(base, args...) - _, err := executor.ExecuteCommandWithOutput(false, "gen secret", "ceph-authtool", args...) + _, err := executor.ExecuteCommandWithOutput("ceph-authtool", args...) if err != nil { return "", errors.Wrapf(err, "failed to gen secret") } diff --git a/pkg/operator/ceph/cluster/mon/config_test.go b/pkg/operator/ceph/cluster/mon/config_test.go index 4c73e9916..e613fda01 100644 --- a/pkg/operator/ceph/cluster/mon/config_test.go +++ b/pkg/operator/ceph/cluster/mon/config_test.go @@ -38,7 +38,7 @@ func TestCreateClusterSecrets(t *testing.T) { defer os.RemoveAll(configDir) adminSecret := "AQDkLIBd9vLGJxAAnXsIKPrwvUXAmY+D1g0X1Q==" executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { logger.Infof("COMMAND: %s %v", command, args) if command == "ceph-authtool" && args[0] == "--create-keyring" { filename := args[1] diff --git a/pkg/operator/ceph/cluster/mon/health.go b/pkg/operator/ceph/cluster/mon/health.go index cafa64b4c..903d9c06d 100644 --- a/pkg/operator/ceph/cluster/mon/health.go +++ b/pkg/operator/ceph/cluster/mon/health.go @@ -83,7 +83,7 @@ func (c *Cluster) checkHealth() error { // connect to the mons // get the status and check for quorum - quorumStatus, err := client.GetMonQuorumStatus(c.context, c.ClusterInfo.Name, client.IsDebugLevel()) + quorumStatus, err := client.GetMonQuorumStatus(c.context, c.ClusterInfo.Name) if err != nil { return errors.Wrapf(err, "failed to get mon quorum status") } diff --git a/pkg/operator/ceph/cluster/mon/health_test.go b/pkg/operator/ceph/cluster/mon/health_test.go index cbc18552e..8073d48a1 100644 --- a/pkg/operator/ceph/cluster/mon/health_test.go +++ b/pkg/operator/ceph/cluster/mon/health_test.go @@ -42,7 +42,7 @@ func TestCheckHealth(t *testing.T) { updateDeploymentAndWait, deploymentsUpdated = testopk8s.UpdateDeploymentAndWaitStub() executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { return clienttest.MonInQuorumResponse(), nil }, } @@ -97,7 +97,7 @@ func TestCheckHealthNotFound(t *testing.T) { updateDeploymentAndWait, deploymentsUpdated = testopk8s.UpdateDeploymentAndWaitStub() executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { return clienttest.MonInQuorumResponse(), nil }, } @@ -153,7 +153,7 @@ func TestAddRemoveMons(t *testing.T) { monQuorumResponse := clienttest.MonInQuorumResponse() executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { return monQuorumResponse, nil }, } diff --git a/pkg/operator/ceph/cluster/mon/mon.go b/pkg/operator/ceph/cluster/mon/mon.go index eb8304108..6eb671f26 100644 --- a/pkg/operator/ceph/cluster/mon/mon.go +++ b/pkg/operator/ceph/cluster/mon/mon.go @@ -974,7 +974,7 @@ func waitForQuorumWithMons(context *clusterd.Context, clusterName string, mons [ // get the quorum_status response that contains info about all monitors in the mon map and // their quorum status - monQuorumStatusResp, err := client.GetMonQuorumStatus(context, clusterName, client.IsDebugLevel()) + monQuorumStatusResp, err := client.GetMonQuorumStatus(context, clusterName) if err != nil { logger.Debugf("failed to get quorum_status. %v", err) continue diff --git a/pkg/operator/ceph/cluster/mon/mon_test.go b/pkg/operator/ceph/cluster/mon/mon_test.go index c78ddaf3b..87c8200a3 100644 --- a/pkg/operator/ceph/cluster/mon/mon_test.go +++ b/pkg/operator/ceph/cluster/mon/mon_test.go @@ -84,14 +84,14 @@ func newTestStartClusterWithQuorumResponse(t *testing.T, namespace string, monRe configDir, _ := ioutil.TempDir("", "") defer os.RemoveAll(configDir) executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { if strings.Contains(command, "ceph-authtool") { err := cephtest.CreateConfigDir(path.Join(configDir, namespace)) return "", errors.Wrapf(err, "failed testing of start cluster without quorum response") } return "", nil }, - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { // mock quorum health check because a second `Start()` triggers a health check return monResponse() }, diff --git a/pkg/operator/ceph/cluster/osd/health_test.go b/pkg/operator/ceph/cluster/osd/health_test.go index 369ded981..7ee797389 100644 --- a/pkg/operator/ceph/cluster/osd/health_test.go +++ b/pkg/operator/ceph/cluster/osd/health_test.go @@ -36,11 +36,11 @@ func TestOSDStatus(t *testing.T) { var execCount = 0 executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { return "{\"key\":\"mysecurekey\", \"osdid\":3.0}", nil }, } - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command string, outFileArg string, args ...string) (string, error) { logger.Infof("ExecuteCommandWithOutputFile: %s %v", command, args) execCount++ if args[1] == "dump" { @@ -53,7 +53,7 @@ func TestOSDStatus(t *testing.T) { return "", nil } - executor.MockExecuteCommandWithOutput = func(debug bool, actionName string, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { logger.Infof("ExecuteCommandWithOutput: %s %v", command, args) return "", nil } diff --git a/pkg/operator/ceph/cluster/osd/osd_test.go b/pkg/operator/ceph/cluster/osd/osd_test.go index b0ad1ba0b..9f1242f22 100644 --- a/pkg/operator/ceph/cluster/osd/osd_test.go +++ b/pkg/operator/ceph/cluster/osd/osd_test.go @@ -124,7 +124,7 @@ func TestAddRemoveNode(t *testing.T) { } generateKey := "expected key" executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { return "{\"key\": \"" + generateKey + "\"}", nil }, } @@ -158,7 +158,7 @@ func TestAddRemoveNode(t *testing.T) { // mock the ceph calls that will be called during remove node mockExec := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[0] == "status" { return `{"pgmap":{"num_pgs":100,"pgs_by_state":[{"state_name":"active+clean","count":100}]}}`, nil diff --git a/pkg/operator/ceph/cluster/osd/spec.go b/pkg/operator/ceph/cluster/osd/spec.go index 6a06b7fb2..a2a295aec 100644 --- a/pkg/operator/ceph/cluster/osd/spec.go +++ b/pkg/operator/ceph/cluster/osd/spec.go @@ -823,6 +823,9 @@ func (c *Cluster) provisionOSDContainer(osdProps osdProperties, copyBinariesMoun osdOnPVCMetadata := osdProps.metadataPVC.ClaimName != "" envVars := c.getConfigEnvVars(osdProps, k8sutil.DataDir) + // enable debug logging in the prepare job + envVars = append(envVars, setDebugLogLevelEnvVar(true)) + // only 1 of device list, device filter, device path filter and use all devices can be specified. We prioritize in that order. if len(osdProps.devices) > 0 { deviceNames := make([]string, len(osdProps.devices)) @@ -1010,6 +1013,14 @@ func pvcBackedOSDEnvVar(pvcBacked string) v1.EnvVar { return v1.EnvVar{Name: pvcBackedOSDVarName, Value: pvcBacked} } +func setDebugLogLevelEnvVar(debug bool) v1.EnvVar { + level := "INFO" + if debug { + level = "DEBUG" + } + return v1.EnvVar{Name: "ROOK_LOG_LEVEL", Value: level} +} + func blockPathEnvVariable(lvPath string) v1.EnvVar { return v1.EnvVar{Name: blockPathVarName, Value: lvPath} } diff --git a/pkg/operator/ceph/cluster/rbd/mirror_test.go b/pkg/operator/ceph/cluster/rbd/mirror_test.go index 9680a5614..76a2f09e3 100644 --- a/pkg/operator/ceph/cluster/rbd/mirror_test.go +++ b/pkg/operator/ceph/cluster/rbd/mirror_test.go @@ -34,7 +34,7 @@ func TestRBDMirror(t *testing.T) { clientset := testop.New(t, 1) keysCreated := map[string]bool{} executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command string, outFileArg string, args ...string) (string, error) { logger.Infof("%s %+v", command, args) if args[0] == "auth" && args[1] == "get-or-create-key" { daemonName := args[2] diff --git a/pkg/operator/ceph/config/keyring/store_test.go b/pkg/operator/ceph/config/keyring/store_test.go index 4575c8cac..29aad5f6c 100644 --- a/pkg/operator/ceph/config/keyring/store_test.go +++ b/pkg/operator/ceph/config/keyring/store_test.go @@ -35,7 +35,7 @@ func TestGenerateKey(t *testing.T) { var generateKey = "" var failGenerateKey = false executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { if failGenerateKey { return "", errors.New("test error") } diff --git a/pkg/operator/ceph/config/monstore_test.go b/pkg/operator/ceph/config/monstore_test.go index 8117dcdb7..246aea6f1 100644 --- a/pkg/operator/ceph/config/monstore_test.go +++ b/pkg/operator/ceph/config/monstore_test.go @@ -40,7 +40,7 @@ func TestMonStore_Set(t *testing.T) { execedCmd := "" execInjectErr := false executor.MockExecuteCommandWithOutputFile = - func(debug bool, actionName string, command string, outfile string, args ...string) (string, error) { + func(command string, outfile string, args ...string) (string, error) { execedCmd = command + " " + strings.Join(args, " ") if execInjectErr { return "output from cmd with error", errors.New("mocked error") @@ -85,7 +85,7 @@ func TestMonStore_SetAll(t *testing.T) { execedCmds := []string{} execInjectErrOnKeyword := "donotinjectanerror" executor.MockExecuteCommandWithOutputFile = - func(debug bool, actionName string, command string, outfile string, args ...string) (string, error) { + func(command string, outfile string, args ...string) (string, error) { execedCmd := command + " " + strings.Join(args, " ") execedCmds = append(execedCmds, execedCmd) k := execInjectErrOnKeyword diff --git a/pkg/operator/ceph/controller/controller_utils.go b/pkg/operator/ceph/controller/controller_utils.go index 5708c3373..1926fcfee 100644 --- a/pkg/operator/ceph/controller/controller_utils.go +++ b/pkg/operator/ceph/controller/controller_utils.go @@ -69,7 +69,7 @@ func IsReadyToReconcile(client client.Client, clustercontext *clusterd.Context, if cephCluster.Status.Phase == k8sutil.ReadyStatus { // Test a Ceph command to verify the Operator is ready // This is done to silence errors when the operator just started and cannot reconcile yet - _, err = cephclient.Status(clustercontext, namespacedName.Namespace, true) + _, err = cephclient.Status(clustercontext, namespacedName.Namespace) if err != nil { if strings.Contains(err.Error(), "error calling conf_read_file") { logger.Info("operator is not ready to run ceph command, cannot reconcile yet.") diff --git a/pkg/operator/ceph/file/filesystem_test.go b/pkg/operator/ceph/file/filesystem_test.go index 92f3a5ce9..354ede00e 100644 --- a/pkg/operator/ceph/file/filesystem_test.go +++ b/pkg/operator/ceph/file/filesystem_test.go @@ -77,10 +77,10 @@ func TestCreateFilesystem(t *testing.T) { fses := `[{"name":"myfs","metadata_pool":"myfs-metadata","metadata_pool_id":1,"data_pool_ids":[2],"data_pools":["myfs-data0"]}]` executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { return "{\"key\":\"mysecurekey\"}", nil }, - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { if strings.Contains(command, "ceph-authtool") { err := cephtest.CreateConfigDir(path.Join(configDir, "ns")) assert.Nil(t, err) @@ -131,7 +131,7 @@ func TestCreateFilesystem(t *testing.T) { // Test multiple filesystem creation executor = &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { if contains(args, "ls") { return fses, nil } @@ -155,10 +155,10 @@ func TestCreateNopoolFilesystem(t *testing.T) { fses := `[{"name":"myfs"}]` executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { return "{\"key\":\"mysecurekey\"}", nil }, - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { if strings.Contains(command, "ceph-authtool") { err := cephtest.CreateConfigDir(path.Join(configDir, "ns")) assert.Nil(t, err) @@ -194,7 +194,7 @@ func TestCreateNopoolFilesystem(t *testing.T) { // Test multiple filesystem creation executor = &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { if contains(args, "ls") { return fses, nil } diff --git a/pkg/operator/ceph/nfs/nfs.go b/pkg/operator/ceph/nfs/nfs.go index 8447f6ee2..54c2afbcb 100644 --- a/pkg/operator/ceph/nfs/nfs.go +++ b/pkg/operator/ceph/nfs/nfs.go @@ -127,15 +127,13 @@ func (c *CephNFSController) addRADOSConfigFile(n cephv1.CephNFS, name string) er "--namespace", n.Spec.RADOS.Namespace, "--conf", cephclient.CephConfFilePath(c.context.ConfigDir, c.namespace), } - moniker := "rados stat " + config - err := c.context.Executor.ExecuteCommand(false, moniker, cmd, append(args, "stat", config)...) + err := c.context.Executor.ExecuteCommand(cmd, append(args, "stat", config)...) if err == nil { // If stat works then we assume it's present already return nil } // try to create it - moniker = "rados create " + config - return c.context.Executor.ExecuteCommand(false, moniker, cmd, append(args, "create", config)...) + return c.context.Executor.ExecuteCommand(cmd, append(args, "create", config)...) } func (c *CephNFSController) addServerToDatabase(nfs cephv1.CephNFS, name string) { @@ -173,7 +171,7 @@ stderr: %s`, moniker, x.Stdout, x.Stderr) } return nil // The below can be used when/if `ganesha-rados-grace` supports a `--cephconf` or similar option - // return c.context.Executor.ExecuteCommand(false, moniker, cmd, args...) + // return c.context.Executor.ExecuteCommand(cmd, args...) } func (c *CephNFSController) generateConfig(n cephv1.CephNFS, name string) (string, error) { diff --git a/pkg/operator/ceph/object/admin.go b/pkg/operator/ceph/object/admin.go index c4797d813..bf633f4bc 100644 --- a/pkg/operator/ceph/object/admin.go +++ b/pkg/operator/ceph/object/admin.go @@ -40,7 +40,7 @@ func runAdminCommandNoRealm(c *Context, args ...string) (string, error) { command, args := client.FinalizeCephCommandArgs("radosgw-admin", args, c.Context.ConfigDir, c.ClusterName) // start the rgw admin command - output, err := c.Context.Executor.ExecuteCommandWithOutput(client.IsDebugLevel(), "", command, args...) + output, err := c.Context.Executor.ExecuteCommandWithOutput(command, args...) if err != nil { return "", errors.Wrapf(err, "failed to run radosgw-admin") } diff --git a/pkg/operator/ceph/object/bucket.go b/pkg/operator/ceph/object/bucket.go index 4de69ec51..94a9f2be6 100644 --- a/pkg/operator/ceph/object/bucket.go +++ b/pkg/operator/ceph/object/bucket.go @@ -78,7 +78,7 @@ func GetBucketStats(c *Context, bucketName string) (*ObjectBucketStats, bool, er "--bucket", bucketName) if err != nil { - if strings.Contains(err.Error(), "No such file or directory") { + if strings.Contains(err.Error(), "exit status 2") { return nil, true, errors.New("not found") } else { return nil, false, errors.Wrapf(err, "failed to get bucket stats") diff --git a/pkg/operator/ceph/object/controller_test.go b/pkg/operator/ceph/object/controller_test.go index 15534b631..dce63a5a2 100644 --- a/pkg/operator/ceph/object/controller_test.go +++ b/pkg/operator/ceph/object/controller_test.go @@ -166,7 +166,7 @@ func TestCephObjectStoreController(t *testing.T) { } executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName, command, outfile string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command, outfile string, args ...string) (string, error) { if args[0] == "status" { return `{"pgmap":{"num_pgs":100,"pgs_by_state":[{"state_name":"active+clean","count":100}]}}`, nil } @@ -175,7 +175,7 @@ func TestCephObjectStoreController(t *testing.T) { } return "", nil }, - MockExecuteCommandWithOutput: func(debug bool, actionName, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { if args[0] == "realm" && args[1] == "list" { return realmListJSON, nil } diff --git a/pkg/operator/ceph/object/objectstore_test.go b/pkg/operator/ceph/object/objectstore_test.go index c2535955a..d5a11050f 100644 --- a/pkg/operator/ceph/object/objectstore_test.go +++ b/pkg/operator/ceph/object/objectstore_test.go @@ -28,7 +28,7 @@ import ( func TestReconcileRealm(t *testing.T) { defaultStore := true - executorFunc := func(debug bool, actionName string, command string, args ...string) (string, error) { + executorFunc := func(command string, args ...string) (string, error) { idResponse := `{"id":"test-id"}` logger.Infof("Execute: %s %v", command, args) if args[1] == "get" { @@ -81,7 +81,7 @@ func deleteStore(t *testing.T, name string, existingStores string, expectedDelet executor := &exectest.MockExecutor{} deletedRootPool := false deletedErasureCodeProfile := false - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { //logger.Infof("command: %s %v", command, args) if args[0] == "osd" { if args[1] == "pool" { @@ -118,7 +118,7 @@ func deleteStore(t *testing.T, name string, existingStores string, expectedDelet } return "", errors.Errorf("unexpected ceph command %q", args) } - executorFunc := func(debug bool, actionName, command string, args ...string) (string, error) { + executorFunc := func(command string, args ...string) (string, error) { //logger.Infof("Command: %s %v", command, args) if args[0] == "realm" { if args[1] == "delete" { diff --git a/pkg/operator/ceph/object/rgw_test.go b/pkg/operator/ceph/object/rgw_test.go index f029cc48e..f51a8e215 100644 --- a/pkg/operator/ceph/object/rgw_test.go +++ b/pkg/operator/ceph/object/rgw_test.go @@ -38,10 +38,10 @@ import ( func TestStartRGW(t *testing.T) { clientset := testop.New(t, 3) executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { return `{"key":"mysecurekey"}`, nil }, - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { return `{"id":"test-id"}`, nil }, } @@ -76,13 +76,13 @@ func validateStart(t *testing.T, c *clusterConfig, clientset *fclient.Clientset) } func TestCreateObjectStore(t *testing.T) { - commandWithOutputFunc := func(debug bool, actionName, command string, args ...string) (string, error) { + commandWithOutputFunc := func(command string, args ...string) (string, error) { return `{"realms": []}`, nil } executor := &exectest.MockExecutor{ MockExecuteCommandWithCombinedOutput: commandWithOutputFunc, MockExecuteCommandWithOutput: commandWithOutputFunc, - MockExecuteCommandWithOutputFile: func(debug bool, actionName, command, outfile string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command, outfile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if command == "ceph" { if args[1] == "erasure-code-profile" { diff --git a/pkg/operator/ceph/object/user/controller_test.go b/pkg/operator/ceph/object/user/controller_test.go index 739359db1..0feb70839 100644 --- a/pkg/operator/ceph/object/user/controller_test.go +++ b/pkg/operator/ceph/object/user/controller_test.go @@ -116,13 +116,13 @@ func TestCephObjectStoreUserController(t *testing.T) { } executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName, command, outfile string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command, outfile string, args ...string) (string, error) { if args[0] == "status" { return `{"pgmap":{"num_pgs":100,"pgs_by_state":[{"state_name":"active+clean","count":100}]}}`, nil } return "", nil }, - MockExecuteCommandWithOutput: func(debug bool, actionName, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { if args[0] == "user" { return userCreateJSON, nil } diff --git a/pkg/operator/ceph/pool/controller_test.go b/pkg/operator/ceph/pool/controller_test.go index cabb1e710..6c66b05d2 100644 --- a/pkg/operator/ceph/pool/controller_test.go +++ b/pkg/operator/ceph/pool/controller_test.go @@ -89,7 +89,7 @@ func TestValidatePool(t *testing.T) { func TestValidateCrushProperties(t *testing.T) { executor := &exectest.MockExecutor{} context := &clusterd.Context{Executor: executor} - executor.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutputFile = func(command, outputFile string, args ...string) (string, error) { logger.Infof("Command: %s %v", command, args) if args[1] == "crush" && args[2] == "dump" { return `{"types":[{"type_id": 0,"name": "osd"}],"buckets":[{"id": -1,"name":"default"},{"id": -2,"name":"good"}]}`, nil @@ -127,7 +127,7 @@ func TestValidateCrushProperties(t *testing.T) { func TestCreatePool(t *testing.T) { executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName, command, outfile string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command, outfile string, args ...string) (string, error) { if command == "ceph" && args[1] == "erasure-code-profile" { return `{"k":"2","m":"1","plugin":"jerasure","technique":"reed_sol_van"}`, nil } @@ -152,7 +152,7 @@ func TestCreatePool(t *testing.T) { func TestDeletePool(t *testing.T) { failOnDelete := false executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName, command, outfile string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command, outfile string, args ...string) (string, error) { if command == "ceph" && args[1] == "lspools" { return `[{"poolnum":1,"poolname":"mypool"}]`, nil } else if command == "ceph" && args[1] == "pool" && args[2] == "get" { @@ -161,7 +161,7 @@ func TestDeletePool(t *testing.T) { return "", nil }, - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { emptyPool := "{\"images\":{\"count\":0,\"provisioned_bytes\":0,\"snap_count\":0},\"trash\":{\"count\":1,\"provisioned_bytes\":2048,\"snap_count\":0}}" p := "{\"images\":{\"count\":1,\"provisioned_bytes\":1024,\"snap_count\":0},\"trash\":{\"count\":1,\"provisioned_bytes\":2048,\"snap_count\":0}}" logger.Infof("Command: %s %v", command, args) @@ -236,7 +236,7 @@ func TestCephBlockPoolController(t *testing.T) { } executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName, command, outfile string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command, outfile string, args ...string) (string, error) { if args[0] == "status" { return `{"pgmap":{"num_pgs":100,"pgs_by_state":[{"state_name":"active+clean","count":100}]}}`, nil } diff --git a/pkg/operator/ceph/provisioner/provisioner_test.go b/pkg/operator/ceph/provisioner/provisioner_test.go index ff91dada2..5e97987d9 100644 --- a/pkg/operator/ceph/provisioner/provisioner_test.go +++ b/pkg/operator/ceph/provisioner/provisioner_test.go @@ -46,7 +46,7 @@ func TestProvisionImage(t *testing.T) { defer os.Setenv("POD_NAMESPACE", "") defer os.RemoveAll(configDir) executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { if strings.Contains(command, "ceph-authtool") { err := cephtest.CreateConfigDir(path.Join(configDir, namespace)) assert.Nil(t, err) @@ -113,7 +113,7 @@ func TestReclaimPolicyForProvisionedImages(t *testing.T) { defer os.Setenv("POD_NAMESPACE", "") defer os.RemoveAll(configDir) executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { if strings.Contains(command, "ceph-authtool") { err := cephtest.CreateConfigDir(path.Join(configDir, namespace)) assert.Nil(t, err) diff --git a/pkg/operator/cockroachdb/controller.go b/pkg/operator/cockroachdb/controller.go index afdc5f5aa..56dde82af 100644 --- a/pkg/operator/cockroachdb/controller.go +++ b/pkg/operator/cockroachdb/controller.go @@ -486,8 +486,7 @@ func (c *ClusterController) initCluster(cluster *cluster) error { } hostFlag := fmt.Sprintf("--host=%s", createQualifiedReplicaServiceName(0, cluster.namespace)) - out, err := c.context.Executor.ExecuteCommandWithCombinedOutput(false, "cockroachdb init", - "/cockroach/cockroach", "init", "--insecure", hostFlag) + out, err := c.context.Executor.ExecuteCommandWithCombinedOutput("/cockroach/cockroach", "init", "--insecure", hostFlag) if err != nil { return fmt.Errorf("cluster init failed for namespace %s: %+v. %s", cluster.namespace, err, out) } diff --git a/pkg/operator/cockroachdb/controller_test.go b/pkg/operator/cockroachdb/controller_test.go index 599cf9cc8..02a8cbce9 100644 --- a/pkg/operator/cockroachdb/controller_test.go +++ b/pkg/operator/cockroachdb/controller_test.go @@ -151,7 +151,7 @@ func TestOnAdd(t *testing.T) { // keep track of if the cockroachdb init command was called initCalled := false executor := &exectest.MockExecutor{ - MockExecuteCommandWithCombinedOutput: func(debug bool, actionName string, command string, arg ...string) (string, error) { + MockExecuteCommandWithCombinedOutput: func(command string, arg ...string) (string, error) { if strings.Contains(command, "cockroach") && arg[0] == "init" { initCalled = true } diff --git a/pkg/operator/edgefs/cluster/mgr/mgr_test.go b/pkg/operator/edgefs/cluster/mgr/mgr_test.go index 310a4b379..5a298d1a8 100644 --- a/pkg/operator/edgefs/cluster/mgr/mgr_test.go +++ b/pkg/operator/edgefs/cluster/mgr/mgr_test.go @@ -34,7 +34,7 @@ import ( func TestStartMGR(t *testing.T) { executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) { return "{\"key\":\"mysecurekey\"}", nil }, } diff --git a/pkg/operator/k8sutil/k8sutil.go b/pkg/operator/k8sutil/k8sutil.go index 790d5f794..da4e8a858 100644 --- a/pkg/operator/k8sutil/k8sutil.go +++ b/pkg/operator/k8sutil/k8sutil.go @@ -122,7 +122,7 @@ func deleteResourceAndWait(namespace, name, resourceType string, // wait for the resource to be deleted sleepTime := 2 * time.Second - for i := 0; i < 30; i++ { + for i := 0; i < 45; i++ { // check for the existence of the resource err = getAction() if err != nil { @@ -133,7 +133,10 @@ func deleteResourceAndWait(namespace, name, resourceType string, return fmt.Errorf("failed to get %s. %+v", name, err) } - logger.Infof("%s still found. waiting...", name) + if i%5 == 0 { + // occasionally print a message + logger.Infof("%q still found. waiting...", name) + } time.Sleep(sleepTime) } diff --git a/pkg/util/display/numbers.go b/pkg/util/display/numbers.go deleted file mode 100644 index b92a9e608..000000000 --- a/pkg/util/display/numbers.go +++ /dev/null @@ -1,26 +0,0 @@ -/* -Copyright 2016 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 display - -import "strconv" - -func NumToStrOmitEmpty(num uint) string { - if num == 0 { - return "" - } - - return strconv.FormatUint(uint64(num), 10) -} diff --git a/pkg/util/display/numbers_test.go b/pkg/util/display/numbers_test.go deleted file mode 100644 index 8c34eb561..000000000 --- a/pkg/util/display/numbers_test.go +++ /dev/null @@ -1,30 +0,0 @@ -/* -Copyright 2016 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 display - -import ( - "math" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestNumToStrOmitEmpty(t *testing.T) { - assert.Equal(t, "", NumToStrOmitEmpty(0)) - assert.Equal(t, "1", NumToStrOmitEmpty(1)) - assert.Equal(t, "9999", NumToStrOmitEmpty(9999)) - assert.Equal(t, "18446744073709551615", NumToStrOmitEmpty(math.MaxUint64)) -} diff --git a/pkg/util/exec/error.go b/pkg/util/exec/error.go index 69a7abea0..19cf2155b 100644 --- a/pkg/util/exec/error.go +++ b/pkg/util/exec/error.go @@ -16,38 +16,17 @@ limitations under the License. package exec import ( - "fmt" "os/exec" "syscall" ) -type CommandError struct { - ActionName string - Err error -} - -func (e *CommandError) Error() string { - var exitErrMsg string - if exitErr, ok := e.Err.(*exec.ExitError); ok { - // the error is an ExitError, include the stderr output in the final error output - exitErrMsg = string(exitErr.Stderr) - } - - return fmt.Sprintf("Failed to complete '%s': %+v. %s", e.ActionName, e.Err, exitErrMsg) -} - -func (e *CommandError) ExitStatus() int { - exitStatus := -1 - exitErr, ok := e.Err.(*exec.ExitError) +func ExitStatus(err error) (int, bool) { + exitErr, ok := err.(*exec.ExitError) if ok { waitStatus, ok := exitErr.ProcessState.Sys().(syscall.WaitStatus) if ok { - exitStatus = waitStatus.ExitStatus() + return waitStatus.ExitStatus(), true } } - return exitStatus -} - -func createCommandError(err error, actionName string) error { - return &CommandError{ActionName: actionName, Err: err} + return 0, false } diff --git a/pkg/util/exec/exec.go b/pkg/util/exec/exec.go index d99151b93..f6a9bec04 100644 --- a/pkg/util/exec/exec.go +++ b/pkg/util/exec/exec.go @@ -31,50 +31,36 @@ import ( ) type Executor interface { - StartExecuteCommand(debug bool, actionName string, command string, arg ...string) (*exec.Cmd, error) - ExecuteCommand(debug bool, actionName string, command string, arg ...string) error - ExecuteCommandWithOutput(debug bool, actionName string, command string, arg ...string) (string, error) - ExecuteCommandWithCombinedOutput(debug bool, actionName string, command string, arg ...string) (string, error) - ExecuteCommandWithOutputFile(debug bool, actionName, command, outfileArg string, arg ...string) (string, error) - ExecuteCommandWithOutputFileTimeout(debug bool, timeout time.Duration, actionName, command, outfileArg string, arg ...string) (string, error) - ExecuteCommandWithTimeout(debug bool, timeout time.Duration, actionName string, command string, arg ...string) (string, error) - ExecuteStat(name string) (os.FileInfo, error) + ExecuteCommand(command string, arg ...string) error + ExecuteCommandWithOutput(command string, arg ...string) (string, error) + ExecuteCommandWithCombinedOutput(command string, arg ...string) (string, error) + ExecuteCommandWithOutputFile(command, outfileArg string, arg ...string) (string, error) + ExecuteCommandWithOutputFileTimeout(timeout time.Duration, command, outfileArg string, arg ...string) (string, error) + ExecuteCommandWithTimeout(timeout time.Duration, command string, arg ...string) (string, error) } type CommandExecutor struct { } -// Start a process and return immediately -func (*CommandExecutor) StartExecuteCommand(debug bool, actionName string, command string, arg ...string) (*exec.Cmd, error) { - cmd, stdout, stderr, err := startCommand(debug, command, arg...) - if err != nil { - return cmd, createCommandError(err, actionName) - } - - go logOutput(actionName, stdout, stderr) - - return cmd, nil -} - // Start a process and wait for its completion -func (*CommandExecutor) ExecuteCommand(debug bool, actionName string, command string, arg ...string) error { - cmd, stdout, stderr, err := startCommand(debug, command, arg...) +func (*CommandExecutor) ExecuteCommand(command string, arg ...string) error { + cmd, stdout, stderr, err := startCommand(command, arg...) if err != nil { - return createCommandError(err, actionName) + return err } - logOutput(actionName, stdout, stderr) + logOutput(stdout, stderr) if err := cmd.Wait(); err != nil { - return createCommandError(err, actionName) + return err } return nil } // ExecuteCommandWithTimeout starts a process and wait for its completion with timeout. -func (*CommandExecutor) ExecuteCommandWithTimeout(debug bool, timeout time.Duration, actionName string, command string, arg ...string) (string, error) { - logCommand(debug, command, arg...) +func (*CommandExecutor) ExecuteCommandWithTimeout(timeout time.Duration, command string, arg ...string) (string, error) { + logCommand(command, arg...) cmd := exec.Command(command, arg...) var b bytes.Buffer @@ -82,7 +68,7 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(debug bool, timeout time.Durat cmd.Stderr = &b if err := cmd.Start(); err != nil { - return "", createCommandError(err, actionName) + return "", err } done := make(chan error, 1) @@ -90,55 +76,54 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(debug bool, timeout time.Durat done <- cmd.Wait() }() - interrupSent := false + interruptSent := false for { select { case <-time.After(timeout): - if interrupSent { - logger.Infof("Timeout waiting for process %s to return after interrupt signal was sent. Sending kill signal to the process", command) + if interruptSent { + logger.Infof("timeout waiting for process %s to return after interrupt signal was sent. Sending kill signal to the process", command) var e error if err := cmd.Process.Kill(); err != nil { - logger.Errorf("Failed to kill process %s: %+v", command, err) - e = fmt.Errorf("Timeout waiting for the command %s to return after interrupt signal was sent. Tried to kill the process but that failed: %+v", command, err) + logger.Errorf("Failed to kill process %s: %v", command, err) + e = fmt.Errorf("timeout waiting for the command %s to return after interrupt signal was sent. Tried to kill the process but that failed: %v", command, err) } else { - e = fmt.Errorf("Timeout waiting for the command %s to return", command) + e = fmt.Errorf("timeout waiting for the command %s to return", command) } - return strings.TrimSpace(b.String()), createCommandError(e, command) + return strings.TrimSpace(b.String()), e } - logger.Infof("Timeout waiting for process %s to return. Sending interrupt signal to the process", command) + logger.Infof("timeout waiting for process %s to return. Sending interrupt signal to the process", command) if err := cmd.Process.Signal(os.Interrupt); err != nil { - logger.Errorf("Failed to send interrupt signal to process %s: %+v", command, err) + logger.Errorf("Failed to send interrupt signal to process %s: %v", command, err) // kill signal will be sent next loop } - interrupSent = true + interruptSent = true case err := <-done: if err != nil { - return strings.TrimSpace(b.String()), createCommandError(err, command) + return strings.TrimSpace(b.String()), err } - if interrupSent { - e := fmt.Errorf("Timeout waiting for the command %s to return", command) - return strings.TrimSpace(b.String()), createCommandError(e, command) + if interruptSent { + return strings.TrimSpace(b.String()), fmt.Errorf("timeout waiting for the command %s to return", command) } return strings.TrimSpace(b.String()), nil } } } -func (*CommandExecutor) ExecuteCommandWithOutput(debug bool, actionName string, command string, arg ...string) (string, error) { - logCommand(debug, command, arg...) +func (*CommandExecutor) ExecuteCommandWithOutput(command string, arg ...string) (string, error) { + logCommand(command, arg...) cmd := exec.Command(command, arg...) - return runCommandWithOutput(actionName, cmd, false) + return runCommandWithOutput(cmd, false) } -func (*CommandExecutor) ExecuteCommandWithCombinedOutput(debug bool, actionName string, command string, arg ...string) (string, error) { - logCommand(debug, command, arg...) +func (*CommandExecutor) ExecuteCommandWithCombinedOutput(command string, arg ...string) (string, error) { + logCommand(command, arg...) cmd := exec.Command(command, arg...) - return runCommandWithOutput(actionName, cmd, true) + return runCommandWithOutput(cmd, true) } // Same as ExecuteCommandWithOutputFile but with a timeout limit. -func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(debug bool, timeout time.Duration, actionName string, +func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(timeout time.Duration, command, outfileArg string, arg ...string) (string, error) { outFile, err := ioutil.TempFile("", "") @@ -149,7 +134,7 @@ func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(debug bool, timeout defer os.Remove(outFile.Name()) arg = append(arg, outfileArg, outFile.Name()) - logCommand(debug, command, arg...) + logCommand(command, arg...) ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() @@ -160,7 +145,7 @@ func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(debug bool, timeout // if there was anything that went to stdout/stderr then log it, even before // we return an error if string(cmdOut) != "" { - logger.Info(string(cmdOut)) + logger.Debug(string(cmdOut)) } if ctx.Err() == context.DeadlineExceeded { @@ -175,7 +160,7 @@ func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(debug bool, timeout return string(fileOut), err } -func (*CommandExecutor) ExecuteCommandWithOutputFile(debug bool, actionName string, command, outfileArg string, arg ...string) (string, error) { +func (*CommandExecutor) ExecuteCommandWithOutputFile(command, outfileArg string, arg ...string) (string, error) { // create a temporary file to serve as the output file for the command to be run and ensure // it is cleaned up after this function is done @@ -189,7 +174,7 @@ func (*CommandExecutor) ExecuteCommandWithOutputFile(debug bool, actionName stri // append the output file argument to the list or args arg = append(arg, outfileArg, outFile.Name()) - logCommand(debug, command, arg...) + logCommand(command, arg...) cmd := exec.Command(command, arg...) cmdOut, err := cmd.CombinedOutput() // if there was anything that went to stdout/stderr then log it, even before we return an error @@ -205,8 +190,8 @@ func (*CommandExecutor) ExecuteCommandWithOutputFile(debug bool, actionName stri return string(fileOut), err } -func startCommand(debug bool, command string, arg ...string) (*exec.Cmd, io.ReadCloser, io.ReadCloser, error) { - logCommand(debug, command, arg...) +func startCommand(command string, arg ...string) (*exec.Cmd, io.ReadCloser, io.ReadCloser, error) { + logCommand(command, arg...) cmd := exec.Command(command, arg...) stdout, err := cmd.StdoutPipe() @@ -223,21 +208,17 @@ func startCommand(debug bool, command string, arg ...string) (*exec.Cmd, io.Read return cmd, stdout, stderr, err } -func (*CommandExecutor) ExecuteStat(name string) (os.FileInfo, error) { - return os.Stat(name) -} - // read from reader line by line and write it to the log func logFromReader(logger *capnslog.PackageLogger, reader io.ReadCloser) { in := bufio.NewScanner(reader) lastLine := "" for in.Scan() { lastLine = in.Text() - logger.Info(lastLine) + logger.Debug(lastLine) } } -func logOutput(name string, stdout, stderr io.ReadCloser) { +func logOutput(stdout, stderr io.ReadCloser) { if stdout == nil || stderr == nil { logger.Warningf("failed to collect stdout and stderr") return @@ -246,11 +227,11 @@ func logOutput(name string, stdout, stderr io.ReadCloser) { // The child processes should appropriately be outputting at the desired global level. Therefore, // we always log at INFO level here, so that log statements from child procs at higher levels // (e.g., WARNING) will still be displayed. We are relying on the child procs to output appropriately. - childLogger := capnslog.NewPackageLogger("github.com/rook/rook", name) + childLogger := capnslog.NewPackageLogger("github.com/rook/rook", "exec") if !childLogger.LevelAt(capnslog.INFO) { rl, err := capnslog.GetRepoLogger("github.com/rook/rook") if err == nil { - rl.SetLogLevel(map[string]capnslog.LogLevel{name: capnslog.INFO}) + rl.SetLogLevel(map[string]capnslog.LogLevel{"exec": capnslog.INFO}) } } @@ -258,7 +239,7 @@ func logOutput(name string, stdout, stderr io.ReadCloser) { logFromReader(childLogger, stdout) } -func runCommandWithOutput(actionName string, cmd *exec.Cmd, combinedOutput bool) (string, error) { +func runCommandWithOutput(cmd *exec.Cmd, combinedOutput bool) (string, error) { var output []byte var err error @@ -271,17 +252,12 @@ func runCommandWithOutput(actionName string, cmd *exec.Cmd, combinedOutput bool) out := strings.TrimSpace(string(output)) if err != nil { - return out, createCommandError(err, actionName) + return out, err } return out, nil } -func logCommand(debug bool, command string, arg ...string) { - msg := fmt.Sprintf("Running command: %s %s", command, strings.Join(arg, " ")) - if debug { - logger.Debug(msg) - } else { - logger.Info(msg) - } +func logCommand(command string, arg ...string) { + logger.Debugf("Running command: %s %s", command, strings.Join(arg, " ")) } diff --git a/pkg/util/exec/test/mockexec.go b/pkg/util/exec/test/mockexec.go index 6ade83894..0d90937a3 100644 --- a/pkg/util/exec/test/mockexec.go +++ b/pkg/util/exec/test/mockexec.go @@ -16,85 +16,66 @@ limitations under the License. package test import ( - "os" "os/exec" "time" ) // ******************** MockExecutor ******************** type MockExecutor struct { - MockExecuteCommand func(debug bool, actionName string, command string, arg ...string) error - MockStartExecuteCommand func(debug bool, actionName string, command string, arg ...string) (*exec.Cmd, error) - MockExecuteCommandWithOutput func(debug bool, actionName string, command string, arg ...string) (string, error) - MockExecuteCommandWithCombinedOutput func(debug bool, actionName string, command string, arg ...string) (string, error) - MockExecuteCommandWithOutputFile func(debug bool, actionName string, command, outfileArg string, arg ...string) (string, error) - MockExecuteCommandWithOutputFileTimeout func(debug bool, timeout time.Duration, actionName string, command, outfileArg string, arg ...string) (string, error) - MockExecuteCommandWithTimeout func(debug bool, timeout time.Duration, actionName string, command string, arg ...string) (string, error) - MockExecuteStat func(name string) (os.FileInfo, error) + MockExecuteCommand func(command string, arg ...string) error + MockStartExecuteCommand func(command string, arg ...string) (*exec.Cmd, error) + MockExecuteCommandWithOutput func(command string, arg ...string) (string, error) + MockExecuteCommandWithCombinedOutput func(command string, arg ...string) (string, error) + MockExecuteCommandWithOutputFile func(command, outfileArg string, arg ...string) (string, error) + MockExecuteCommandWithOutputFileTimeout func(timeout time.Duration, command, outfileArg string, arg ...string) (string, error) + MockExecuteCommandWithTimeout func(timeout time.Duration, command string, arg ...string) (string, error) } -func (e *MockExecutor) ExecuteCommand(debug bool, actionName string, command string, arg ...string) error { +func (e *MockExecutor) ExecuteCommand(command string, arg ...string) error { if e.MockExecuteCommand != nil { - return e.MockExecuteCommand(debug, actionName, command, arg...) + return e.MockExecuteCommand(command, arg...) } return nil } -func (e *MockExecutor) StartExecuteCommand(debug bool, actionName string, command string, arg ...string) (*exec.Cmd, error) { - if e.MockStartExecuteCommand != nil { - return e.MockStartExecuteCommand(debug, actionName, command, arg...) - } - - args := []string{command} - return &exec.Cmd{Args: append(args, arg...)}, nil -} - -func (e *MockExecutor) ExecuteCommandWithOutput(debug bool, actionName string, command string, arg ...string) (string, error) { +func (e *MockExecutor) ExecuteCommandWithOutput(command string, arg ...string) (string, error) { if e.MockExecuteCommandWithOutput != nil { - return e.MockExecuteCommandWithOutput(debug, actionName, command, arg...) + return e.MockExecuteCommandWithOutput(command, arg...) } return "", nil } -func (e *MockExecutor) ExecuteCommandWithTimeout(debug bool, timeout time.Duration, actionName string, command string, arg ...string) (string, error) { +func (e *MockExecutor) ExecuteCommandWithTimeout(timeout time.Duration, command string, arg ...string) (string, error) { if e.MockExecuteCommandWithTimeout != nil { - return e.MockExecuteCommandWithTimeout(debug, time.Second, actionName, command, arg...) + return e.MockExecuteCommandWithTimeout(time.Second, command, arg...) } return "", nil } -func (e *MockExecutor) ExecuteCommandWithCombinedOutput(debug bool, actionName string, command string, arg ...string) (string, error) { +func (e *MockExecutor) ExecuteCommandWithCombinedOutput(command string, arg ...string) (string, error) { if e.MockExecuteCommandWithCombinedOutput != nil { - return e.MockExecuteCommandWithCombinedOutput(debug, actionName, command, arg...) + return e.MockExecuteCommandWithCombinedOutput(command, arg...) } return "", nil } -func (e *MockExecutor) ExecuteCommandWithOutputFile(debug bool, actionName string, command, outfileArg string, arg ...string) (string, error) { +func (e *MockExecutor) ExecuteCommandWithOutputFile(command, outfileArg string, arg ...string) (string, error) { if e.MockExecuteCommandWithOutputFile != nil { - return e.MockExecuteCommandWithOutputFile(debug, actionName, command, outfileArg, arg...) + return e.MockExecuteCommandWithOutputFile(command, outfileArg, arg...) } return "", nil } -func (e *MockExecutor) ExecuteCommandWithOutputFileTimeout(debug bool, timeout time.Duration, actionName string, command, outfileArg string, arg ...string) (string, error) { +func (e *MockExecutor) ExecuteCommandWithOutputFileTimeout(timeout time.Duration, command, outfileArg string, arg ...string) (string, error) { if e.MockExecuteCommandWithOutputFileTimeout != nil { - return e.MockExecuteCommandWithOutputFileTimeout(debug, timeout, actionName, command, outfileArg, arg...) + return e.MockExecuteCommandWithOutputFileTimeout(timeout, command, outfileArg, arg...) } return "", nil } - -func (e *MockExecutor) ExecuteStat(name string) (os.FileInfo, error) { - if e.MockExecuteStat != nil { - return e.MockExecuteStat(name) - } - - return nil, nil -} diff --git a/pkg/util/exec/translate_exec.go b/pkg/util/exec/translate_exec.go index 06ae9a09b..e9492804f 100644 --- a/pkg/util/exec/translate_exec.go +++ b/pkg/util/exec/translate_exec.go @@ -16,9 +16,6 @@ limitations under the License. package exec import ( - "fmt" - "os" - "os/exec" "time" ) @@ -31,54 +28,43 @@ type TranslateCommandExecutor struct { Executor Executor // Translator translates every command before running it - Translator func(debug bool, actionName string, command string, arg ...string) (string, []string) -} - -// StartExecuteCommand starts a process and return immediately -func (e *TranslateCommandExecutor) StartExecuteCommand(debug bool, actionName string, command string, arg ...string) (*exec.Cmd, error) { - transCommand, transArgs := e.Translator(debug, actionName, command, arg...) - return e.Executor.StartExecuteCommand(debug, actionName, transCommand, transArgs...) + Translator func(command string, arg ...string) (string, []string) } // ExecuteCommand starts a process and wait for its completion -func (e *TranslateCommandExecutor) ExecuteCommand(debug bool, actionName string, command string, arg ...string) error { - transCommand, transArgs := e.Translator(debug, actionName, command, arg...) - return e.Executor.ExecuteCommand(debug, actionName, transCommand, transArgs...) +func (e *TranslateCommandExecutor) ExecuteCommand(command string, arg ...string) error { + transCommand, transArgs := e.Translator(command, arg...) + return e.Executor.ExecuteCommand(transCommand, transArgs...) } // ExecuteCommandWithOutput starts a process and wait for its completion -func (e *TranslateCommandExecutor) ExecuteCommandWithOutput(debug bool, actionName string, command string, arg ...string) (string, error) { - transCommand, transArgs := e.Translator(debug, actionName, command, arg...) - return e.Executor.ExecuteCommandWithOutput(debug, actionName, transCommand, transArgs...) +func (e *TranslateCommandExecutor) ExecuteCommandWithOutput(command string, arg ...string) (string, error) { + transCommand, transArgs := e.Translator(command, arg...) + return e.Executor.ExecuteCommandWithOutput(transCommand, transArgs...) } // ExecuteCommandWithCombinedOutput starts a process and returns its stdout and stderr combined. -func (e *TranslateCommandExecutor) ExecuteCommandWithCombinedOutput(debug bool, actionName string, command string, arg ...string) (string, error) { - transCommand, transArgs := e.Translator(debug, actionName, command, arg...) - return e.Executor.ExecuteCommandWithCombinedOutput(debug, actionName, transCommand, transArgs...) +func (e *TranslateCommandExecutor) ExecuteCommandWithCombinedOutput(command string, arg ...string) (string, error) { + transCommand, transArgs := e.Translator(command, arg...) + return e.Executor.ExecuteCommandWithCombinedOutput(transCommand, transArgs...) } // ExecuteCommandWithOutputFile starts a process and saves output to file -func (e *TranslateCommandExecutor) ExecuteCommandWithOutputFile(debug bool, actionName string, command, outfileArg string, arg ...string) (string, error) { - transCommand, transArgs := e.Translator(debug, actionName, command, arg...) - return e.Executor.ExecuteCommandWithOutputFile(debug, actionName, transCommand, outfileArg, transArgs...) +func (e *TranslateCommandExecutor) ExecuteCommandWithOutputFile(command, outfileArg string, arg ...string) (string, error) { + transCommand, transArgs := e.Translator(command, arg...) + return e.Executor.ExecuteCommandWithOutputFile(transCommand, outfileArg, transArgs...) } // ExecuteCommandWithOutputFileTimeout is the same as ExecuteCommandWithOutputFile but with a timeout limit. func (e *TranslateCommandExecutor) ExecuteCommandWithOutputFileTimeout( - debug bool, timeout time.Duration, actionName string, + timeout time.Duration, command, outfileArg string, arg ...string) (string, error) { - transCommand, transArgs := e.Translator(debug, actionName, command, arg...) - return e.Executor.ExecuteCommandWithOutputFileTimeout(debug, timeout, actionName, transCommand, outfileArg, transArgs...) + transCommand, transArgs := e.Translator(command, arg...) + return e.Executor.ExecuteCommandWithOutputFileTimeout(timeout, transCommand, outfileArg, transArgs...) } // ExecuteCommandWithTimeout starts a process and wait for its completion with timeout. -func (e *TranslateCommandExecutor) ExecuteCommandWithTimeout(debug bool, timeout time.Duration, actionName string, command string, arg ...string) (string, error) { - transCommand, transArgs := e.Translator(debug, actionName, command, arg...) - return e.Executor.ExecuteCommandWithTimeout(debug, timeout, actionName, transCommand, transArgs...) -} - -// ExecuteStat returns a file stat -func (e *TranslateCommandExecutor) ExecuteStat(name string) (os.FileInfo, error) { - return nil, fmt.Errorf("TODO: TranslateCommandExecutor.ExecuteStat() not implemented ... is it needed?") +func (e *TranslateCommandExecutor) ExecuteCommandWithTimeout(timeout time.Duration, command string, arg ...string) (string, error) { + transCommand, transArgs := e.Translator(command, arg...) + return e.Executor.ExecuteCommandWithTimeout(timeout, transCommand, transArgs...) } diff --git a/pkg/util/flags/flags.go b/pkg/util/flags/flags.go index 7a98a5a90..3fb9759aa 100644 --- a/pkg/util/flags/flags.go +++ b/pkg/util/flags/flags.go @@ -38,38 +38,6 @@ func VerifyRequiredFlags(cmd *cobra.Command, requiredFlags []string) error { return createRequiredFlagError(cmd.Name(), missingFlags) } -type RenamedFlag struct { - NewFlagName string - OldFlagName string -} - -func VerifyRenamedFlags(cmd *cobra.Command, renamedFlags []RenamedFlag) error { - var missingFlags []string - for _, renamedFlag := range renamedFlags { - val, err := cmd.Flags().GetString(renamedFlag.NewFlagName) - if err != nil || val == "" { - val, err := cmd.Flags().GetString(renamedFlag.OldFlagName) - if err != nil || val == "" { - missingFlags = append(missingFlags, renamedFlag.NewFlagName) - } - } - } - - return createRequiredFlagError(cmd.Name(), missingFlags) -} - -func VerifyRequiredUint64Flags(cmd *cobra.Command, requiredFlags []string) error { - var missingFlags []string - for _, reqFlag := range requiredFlags { - val, err := cmd.Flags().GetUint64(reqFlag) - if err != nil || val == 0 { - missingFlags = append(missingFlags, reqFlag) - } - } - - return createRequiredFlagError(cmd.Name(), missingFlags) -} - func createRequiredFlagError(name string, flags []string) error { if len(flags) == 0 { return nil diff --git a/pkg/util/flags/flags_test.go b/pkg/util/flags/flags_test.go index c978b5234..b116e0c16 100644 --- a/pkg/util/flags/flags_test.go +++ b/pkg/util/flags/flags_test.go @@ -48,64 +48,6 @@ func TestStringFlags(t *testing.T) { assert.Nil(t, err) } -func TestRenamedFlags(t *testing.T) { - cmd := &cobra.Command{ - Use: "test", - Short: "Creates a test arg", - } - - var arg1 string - var arg2 string - cmd.Flags().StringVar(&arg1, "foo", "", "test 1") - cmd.Flags().StringVar(&arg2, "bar", "", "test 2") - - // both arguments are missing - err := VerifyRenamedFlags(cmd, []RenamedFlag{{"foo", "bar"}}) - assert.Equal(t, "foo is required for test", err.Error()) - - // old argument is missing - cmd.Flags().Set("foo", "fooval") - err = VerifyRenamedFlags(cmd, []RenamedFlag{{"foo", "bar"}}) - assert.Nil(t, err) - - // new argument is missing - cmd.Flags().Set("foo", "") - cmd.Flags().Set("bar", "barval") - err = VerifyRenamedFlags(cmd, []RenamedFlag{{"foo", "bar"}}) - assert.Nil(t, err) - - // no arguments are missing - cmd.Flags().Set("foo", "fooval") - err = VerifyRenamedFlags(cmd, []RenamedFlag{{"foo", "bar"}}) - assert.Nil(t, err) -} - -func TestUintFlags(t *testing.T) { - cmd := &cobra.Command{ - Use: "test", - Short: "Creates a test arg", - } - - var arg1 uint64 - var arg2 uint64 - cmd.Flags().Uint64Var(&arg1, "foo", 0, "test 1") - cmd.Flags().Uint64Var(&arg2, "bar", 0, "test 2") - - // both arguments are missing - err := VerifyRequiredUint64Flags(cmd, []string{"foo", "bar"}) - assert.Equal(t, "foo,bar are required for test", err.Error()) - - // one argument is missing - cmd.Flags().Set("foo", "1234") - err = VerifyRequiredUint64Flags(cmd, []string{"foo", "bar"}) - assert.Equal(t, "bar is required for test", err.Error()) - - // no arguments are missing - cmd.Flags().Set("bar", "5432") - err = VerifyRequiredUint64Flags(cmd, []string{"foo", "bar"}) - assert.Nil(t, err) -} - func TestGetFlagsAndValues(t *testing.T) { cmd := &cobra.Command{ Use: "test", diff --git a/pkg/util/proc/log.go b/pkg/util/proc/log.go deleted file mode 100644 index 8ac6f6da3..000000000 --- a/pkg/util/proc/log.go +++ /dev/null @@ -1,20 +0,0 @@ -/* -Copyright 2016 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 proc - -import "github.com/coreos/pkg/capnslog" - -var logger = capnslog.NewPackageLogger("github.com/rook/rook", "proc") diff --git a/pkg/util/proc/monitoredproc.go b/pkg/util/proc/monitoredproc.go deleted file mode 100644 index ca9b034f0..000000000 --- a/pkg/util/proc/monitoredproc.go +++ /dev/null @@ -1,152 +0,0 @@ -/* -Copyright 2016 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 proc - -import ( - "math" - "os/exec" - "syscall" - "time" -) - -const ( - maxDelaySeconds = 30 -) - -type MonitoredProc struct { - parent *ProcManager - cmd *exec.Cmd - monitor bool - retries int - totalRetries int - retrySecondsExponentBase float64 - waitForExit func() -} - -func newMonitoredProc(p *ProcManager, cmd *exec.Cmd) *MonitoredProc { - m := &MonitoredProc{ - parent: p, - cmd: cmd, - retrySecondsExponentBase: 2, - } - m.waitForExit = m.waitForProcessExit - return m -} - -func (p *MonitoredProc) Monitor(logName string) { - p.monitor = true - var err error - var lastRetryCheck time.Time - var lastStartTime time.Time - - for { - // wait for the given process to complete, unless the last retry had failed immediately - if err == nil { - p.waitForExit() - } - - if !p.monitor { - logger.Infof("done monitoring process %v", p.cmd.Args) - break - } - - // calculate the delay - delay := calcRetryDelay(p.retrySecondsExponentBase, p.retries, time.Now(), lastStartTime, lastRetryCheck) - lastRetryCheck = time.Now() - - logger.Infof("starting process %v again after %.1f seconds", p.cmd.Args, delay) - <-time.After(time.Second * time.Duration(delay)) - - if !p.monitor { - logger.Infof("done monitoring process %v", p.cmd.Args) - break - } - - // start the process - p.cmd, err = p.parent.executor.StartExecuteCommand(false, logName, p.cmd.Args[0], p.cmd.Args[1:]...) - if err != nil { - logger.Warningf("retry %d (total %d): process %v failed to restart. %v", p.retries, p.totalRetries, p.cmd.Args, err) - p.retries++ - } else { - logger.Infof("retry (total %d). started process %v", p.totalRetries, p.cmd.Args) - lastStartTime = time.Now() - p.retries = 0 - } - - p.totalRetries++ - } -} - -func (p *MonitoredProc) waitForProcessExit() { - state, err := p.cmd.Process.Wait() - if err != nil { - logger.Errorf("waiting for process %d had an error: %+v", p.cmd.Process.Pid, err) - return - } - - // check the wait status of the process which has all the exit information - waitStatus, ok := state.Sys().(syscall.WaitStatus) - if !ok { - logger.Errorf("unknown waitStatus for process %d: %+v", p.cmd.Process.Pid, state.Sys()) - return - } - - logger.Infof("process %d completed. Exited: %t, ExitStatus: %d, Signaled: %t, Signal: %d, %+v", - p.cmd.Process.Pid, waitStatus.Exited(), waitStatus.ExitStatus(), waitStatus.Signaled(), waitStatus.Signal(), p.cmd) -} - -func (p *MonitoredProc) Stop(mon bool) error { - p.monitor = mon - if p.cmd == nil || p.cmd.Process == nil { - return nil - } - - pid := p.cmd.Process.Pid - logger.Infof("stopping child process %d\n", pid) - if err := p.cmd.Process.Kill(); err != nil { - logger.Errorf("failed to stop child process %d: %+v\n", pid, err) - return err - } - - logger.Infof("child process %d stopped successfully\n", pid) - return nil -} - -// determines the time to delay in seconds before retrying again -func calcRetryDelay(base float64, retries int, now, lastStartTime, lastRetryCheck time.Time) float64 { - if base == 0 { - return 0 - } - - power := 0.0 - if retries > 0 { - power = float64(retries) - } else if !lastRetryCheck.IsZero() && !lastStartTime.IsZero() && now.Sub(lastStartTime).Seconds() < maxDelaySeconds { - // the process was running for shorter than the max retry delay, so it wasn't running for too long before - // it exited again. retry count is 0, so the process did start "successfully" last time, but in order to prevent - // rapid retry loops where the process starts successfully but crashes right after, let's use the number of - // seconds since the last retry check as a proxy for retry count. this will give us exponential back off - // for a process that crashes immediately after starting. - power = now.Sub(lastRetryCheck).Seconds() - } - - delay := math.Pow(base, power) - if delay > float64(maxDelaySeconds) { - return float64(maxDelaySeconds) - } - - return delay -} diff --git a/pkg/util/proc/monitoredproc_test.go b/pkg/util/proc/monitoredproc_test.go deleted file mode 100644 index ec44765cd..000000000 --- a/pkg/util/proc/monitoredproc_test.go +++ /dev/null @@ -1,110 +0,0 @@ -/* -Copyright 2016 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 proc - -import ( - "errors" - "os/exec" - "testing" - "time" - - "github.com/rook/rook/pkg/util/exec/test" - "github.com/stretchr/testify/assert" -) - -func TestRestartDelay(t *testing.T) { - var zeroTime time.Time - - // test cases for when last retry check time is not available - assert.Equal(t, float64(0), calcRetryDelay(0, 0, zeroTime, zeroTime, zeroTime)) - assert.Equal(t, float64(0), calcRetryDelay(0, 2, zeroTime, zeroTime, zeroTime)) - assert.Equal(t, float64(1), calcRetryDelay(2, 0, zeroTime, zeroTime, zeroTime)) - assert.Equal(t, float64(2), calcRetryDelay(2, 1, zeroTime, zeroTime, zeroTime)) - assert.Equal(t, float64(4), calcRetryDelay(2, 2, zeroTime, zeroTime, zeroTime)) - assert.Equal(t, float64(30), calcRetryDelay(2, 5, zeroTime, zeroTime, zeroTime)) - assert.Equal(t, float64(30), calcRetryDelay(2, 100, zeroTime, zeroTime, zeroTime)) - - // test cases for when last retry check is available and retry count is 0 and the process hasn't been - // running long at all. the time delay should be 2^(number of secs since last retry), with a max limit of 30. - now := time.Now() - lastStartTime := now - lastRetryCheck := now - assert.Equal(t, float64(1), calcRetryDelay(2, 0, now, lastStartTime, lastRetryCheck)) - - lastRetryCheck = now.Add(-1 * time.Second) - assert.Equal(t, float64(2), calcRetryDelay(2, 0, now, lastStartTime, lastRetryCheck)) - - lastRetryCheck = now.Add(-2 * time.Second) - assert.Equal(t, float64(4), calcRetryDelay(2, 0, now, lastStartTime, lastRetryCheck)) - - lastRetryCheck = now.Add(-4 * time.Second) - assert.Equal(t, float64(16), calcRetryDelay(2, 0, now, lastStartTime, lastRetryCheck)) - - lastRetryCheck = now.Add(-5 * time.Second) - assert.Equal(t, float64(30), calcRetryDelay(2, 0, now, lastStartTime, lastRetryCheck)) - - lastRetryCheck = now.Add(-999999999 * time.Second) - assert.Equal(t, float64(30), calcRetryDelay(2, 0, now, lastStartTime, lastRetryCheck)) - - // test case for when the process has been running for quite awhile. we should not delay for very long since we - // are clearly not in a rapid retry loop. - lastStartTime = now.Add(-100 * time.Second) - assert.Equal(t, float64(1), calcRetryDelay(2, 0, now, lastStartTime, lastRetryCheck)) - - // test cases for when both retry count and last retry check are available, retry count should be favored - lastRetryCheck = now.Add(-4 * time.Second) - assert.Equal(t, float64(2), calcRetryDelay(2, 1, now, lastStartTime, lastRetryCheck)) -} - -func TestMonitoredRestart(t *testing.T) { - executor := &test.MockExecutor{} - procMgr := New(executor) - cmd := &exec.Cmd{Args: []string{"/my/path", "1", "2", "3"}} - proc := newMonitoredProc(procMgr, cmd) - proc.retrySecondsExponentBase = 0.0 - - commands := 0 - executor.MockStartExecuteCommand = func(debug bool, name string, command string, args ...string) (*exec.Cmd, error) { - commands++ - cmd := &exec.Cmd{Args: append([]string{command}, args...)} - switch { - case commands == 1: - return cmd, errors.New("test failure") - case commands == 2: - assert.Equal(t, 1, proc.retries) - } - return cmd, nil - } - - iter := 0 - proc.waitForExit = func() { - logger.Debugf("waited for process") - assert.True(t, proc.monitor) - switch { - case iter == 0: - case iter == 1: - assert.True(t, proc.monitor) - logger.Debugf("stop monitoring") - proc.monitor = false - } - iter++ - } - - proc.Monitor("testproc") - assert.False(t, proc.monitor) - assert.Equal(t, proc.retries, 0) - assert.Equal(t, proc.totalRetries, 2) -} diff --git a/pkg/util/proc/procmanager.go b/pkg/util/proc/procmanager.go deleted file mode 100644 index 886640c31..000000000 --- a/pkg/util/proc/procmanager.go +++ /dev/null @@ -1,239 +0,0 @@ -/* -Copyright 2016 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 proc - -import ( - "fmt" - "os" - "regexp" - "strings" - "sync" - "syscall" - - ps "github.com/jbw976/go-ps" - "github.com/rook/rook/pkg/util/exec" -) - -type ProcStartPolicy int - -const ( - StartAction = "start" - RunAction = "run" - StopAction = "stop" - - RestartExisting ProcStartPolicy = iota - ReuseExisting -) - -type ProcManager struct { - sync.RWMutex - procs []*MonitoredProc - executor exec.Executor -} - -// Create a new proc manager -func New(executor exec.Executor) *ProcManager { - return &ProcManager{executor: executor} -} - -// Start a child process and wait for its completion -func (p *ProcManager) RunWithOutput(logName, command string, args ...string) (string, error) { - - logger.Infof("Running process %s with args: %v", command, args) - output, err := p.executor.ExecuteCommandWithOutput(false, logName, command, args...) - if err != nil { - return "", fmt.Errorf("failed to run %s: %+v", command, err) - } - - return output, nil -} - -// Start a child process and wait for its completion -func (p *ProcManager) RunWithCombinedOutput(logName, command string, args ...string) (string, error) { - - logger.Infof("Running process %s with args: %v", command, args) - output, err := p.executor.ExecuteCommandWithCombinedOutput(false, logName, command, args...) - if err != nil { - return "", fmt.Errorf("failed to run %s: %+v", command, err) - } - - return output, nil -} - -// Start a child process and wait for its completion -func (p *ProcManager) Run(logName, command string, args ...string) error { - - logger.Infof("Running process %s with args: %v", command, args) - err := p.executor.ExecuteCommand(false, logName, command, args...) - if err != nil { - return fmt.Errorf("failed to run %s: %+v", command, err) - } - - return nil -} - -// Start the given process with the provided arguments. Handling of any matching existing process will be in accordance -// with the given ProcStartPolicy. The search pattern will be used to search through the cmdline args of existing -// processes to find any matching existing process. Therefore, it should be a regex pattern that can uniquely -// identify the process (e.g., --id=1) -func (p *ProcManager) Start(name, command, procSearchPattern string, policy ProcStartPolicy, args ...string) (*MonitoredProc, error) { - // look for an existing process first - shouldStart, err := p.checkProcessExists(command, procSearchPattern, policy) - if err != nil { - return nil, err - } - - if !shouldStart { - // based on the presence of an existing process and the given policy, we should not start a process, bail out. - return nil, nil - } - - logger.Infof("Starting process %s with args: %v", name, args) - cmd, err := p.executor.StartExecuteCommand(false, name, command, args...) - if err != nil { - return nil, fmt.Errorf("failed to start process %s: %+v", name, err) - } - - proc := newMonitoredProc(p, cmd) - - // monitor the process if it was not mocked - if cmd != nil && cmd.Process != nil { - go proc.Monitor(name) - } - - p.Lock() - p.procs = append(p.procs, proc) - p.Unlock() - return proc, nil -} - -func (p *ProcManager) Shutdown() error { - p.RLock() - for _, proc := range p.procs { - if err := proc.Stop(false); err != nil { - return fmt.Errorf("error while shutting down. %v", err) - } - } - p.procs = nil - p.RUnlock() - return nil -} - -// Checks if a process exists. If the restart policy indicates the process should be restarted, -// the process will be stopped here. -func (p *ProcManager) checkProcessExists(binary, procSearchPattern string, policy ProcStartPolicy) (shouldStart bool, err error) { - // check if this process is already being monitored - p.RLock() - defer p.RUnlock() - if index, proc := p.findMonitoredProc(binary, procSearchPattern); proc != nil { - if policy == ReuseExisting { - // no need to start the process since the process manager is already watching to ensure it is running - logger.Infof("Process is already being managed and will not be started again. %s", procSearchPattern) - return false, nil - } else if policy == RestartExisting { - // stop the process and the process manager so a new process manager can be started. - // the caller is responsible to start a new process - logger.Infof("Policy is 'restart' for %s. Stopping its monitor.", procSearchPattern) - p.purgeManagedProc(index, proc) - return true, nil - } - } - - // check if this process is currently running even though not being managed - existingProc, err := ps.FindProcessByCmdline(binary, procSearchPattern) - if err != nil { - return false, fmt.Errorf("failed to search for process %s with pattern '%s': %+v", binary, procSearchPattern, err) - } - - if existingProc == nil { - logger.Infof("no existing process found for binary %s and pattern '%s'", binary, procSearchPattern) - return true, nil - } - - logger.Infof("existing process found for binary %s with pid %d, cmdline '%s'.", binary, existingProc.Pid(), existingProc.Cmdline()) - if policy == ReuseExisting { - logger.Infof("Policy is 'reuse', reusing existing process.") - return false, nil - } - - logger.Infof("Policy is 'restart', restarting existing process.") - - // double check if the process is managed and stop it - index, proc := p.findMonitoredProcByPID(existingProc.Pid()) - if proc != nil { - p.purgeManagedProc(index, proc) - return true, nil - } - - // we couldn't stop the existing process through our own managed process set, try to stop the process - // via a direct signal to its PID - if err := syscall.Kill(existingProc.Pid(), syscall.SIGKILL); err != nil { - return false, fmt.Errorf("failed to stop child process %d: %v", existingProc.Pid(), err) - } - - return true, nil -} - -// Stop and remove a process from the list of managed. -// Assumes we are inside the process lock. -func (p *ProcManager) purgeManagedProc(index int, proc *MonitoredProc) { - err := proc.Stop(false) - if err != nil { - logger.Warningf("did not stop process %+v. %+v", proc.cmd, err) - } - - // remove the process from our managed list (if it's in there) - // If the process failed to stop, it is still not being managed so we will go ahead and remove it - if index >= 0 { - p.procs[index] = p.procs[len(p.procs)-1] - p.procs[len(p.procs)-1] = nil - p.procs = p.procs[:len(p.procs)-1] - } -} - -// Find a monitored process. -// Assumes we are inside the process lock. -func (p *ProcManager) findMonitoredProc(binary, searchPattern string) (int, *MonitoredProc) { - // find if the process is being monitored - for index, proc := range p.procs { - if os.Args[0] != binary { - // skip processes that are not owned by rook - continue - } - - cmdline := strings.Join(proc.cmd.Args, " ") - if matched, _ := regexp.MatchString(searchPattern, cmdline); matched { - logger.Infof("found monitored process for %s", searchPattern) - return index, proc - } - - } - - return -1, nil -} - -// Find a monitored process by its pid. -// Assumes we are inside the process lock. -func (p *ProcManager) findMonitoredProcByPID(pid int) (int, *MonitoredProc) { - // find if the process is being monitored - for index, proc := range p.procs { - if proc.cmd.Process != nil && proc.cmd.Process.Pid == pid { - return index, proc - } - } - - return -1, nil -} diff --git a/pkg/util/proc/procmanager_test.go b/pkg/util/proc/procmanager_test.go deleted file mode 100644 index 6b0eddfc3..000000000 --- a/pkg/util/proc/procmanager_test.go +++ /dev/null @@ -1,83 +0,0 @@ -/* -Copyright 2016 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 proc - -import ( - "os" - "os/exec" - "testing" - - exectest "github.com/rook/rook/pkg/util/exec/test" - "github.com/stretchr/testify/assert" -) - -func TestCheckProcessExists(t *testing.T) { - executor := &exectest.MockExecutor{} - p := &ProcManager{executor: executor} - - // no managed process to find - shouldStart, err := p.checkProcessExists(os.Args[0], "bar=2", ReuseExisting) - assert.Nil(t, err) - assert.True(t, shouldStart) - - // create a couple managed processes - cmd1 := exec.Command("mycmd1", []string{os.Args[0], "foo=1", "bar=2"}...) - cmd2 := exec.Command("mycmd2", []string{os.Args[0], "foo=3", "bar=4"}...) - monitored1 := &MonitoredProc{parent: p, cmd: cmd1, waitForExit: func() {}} - monitored2 := &MonitoredProc{parent: p, cmd: cmd2, waitForExit: func() {}} - p.procs = []*MonitoredProc{monitored1, monitored2} - - // find the managed process - shouldStart, err = p.checkProcessExists(os.Args[0], "bar=2", ReuseExisting) - assert.Nil(t, err) - assert.False(t, shouldStart) - - // can't find another process - index, proc := p.findMonitoredProcByPID(0) - assert.Equal(t, -1, index) - assert.Nil(t, proc) - - // purge the managed process - p.purgeManagedProc(0, monitored1) - assert.Equal(t, 1, len(p.procs)) - assert.Equal(t, "mycmd2", p.procs[0].cmd.Args[0]) - - p.Shutdown() - assert.Equal(t, 0, len(p.procs)) -} - -func TestRunProcesses(t *testing.T) { - executor := &exectest.MockExecutor{} - p := &ProcManager{executor: executor} - - proc, err := p.Start("mylog", "mydaemon", "mysearch", ReuseExisting, "arg1", "arg2") - assert.Nil(t, err) - assert.Equal(t, "mydaemon", proc.cmd.Args[0]) - assert.Equal(t, "arg1", proc.cmd.Args[1]) - assert.Equal(t, "arg2", proc.cmd.Args[2]) - - run := false - executor.MockExecuteCommand = func(debug bool, name string, command string, args ...string) error { - assert.Equal(t, "arg1", args[0]) - assert.Equal(t, "arg2", args[1]) - run = true - return nil - } - - err = p.Run("mylog", "mytool", "arg1", "arg2") - assert.Nil(t, err) - assert.True(t, run) -} diff --git a/pkg/util/sys/device.go b/pkg/util/sys/device.go index b238dd8ee..dfca6d5a2 100644 --- a/pkg/util/sys/device.go +++ b/pkg/util/sys/device.go @@ -110,8 +110,7 @@ type LocalDisk struct { // ListDevices list all devices available on a machine func ListDevices(executor exec.Executor) ([]string, error) { - cmd := "lsblk all" - devices, err := executor.ExecuteCommandWithOutput(false, cmd, "lsblk", "--all", "--noheadings", "--list", "--output", "KNAME") + devices, err := executor.ExecuteCommandWithOutput("lsblk", "--all", "--noheadings", "--list", "--output", "KNAME") if err != nil { return nil, fmt.Errorf("failed to list all devices: %+v", err) } @@ -130,8 +129,7 @@ func GetDevicePartitions(device string, executor exec.Executor) (partitions []Pa devicePath = device //use the exact device path (like /mnt/) in case of PVC block device } - cmd := fmt.Sprintf("lsblk %s", devicePath) - output, err := executor.ExecuteCommandWithOutput(false, cmd, "lsblk", devicePath, + output, err := executor.ExecuteCommandWithOutput("lsblk", devicePath, "--bytes", "--pairs", "--output", "NAME,SIZE,TYPE,PKNAME") logger.Infof("Output: %+v", output) if err != nil { @@ -200,8 +198,7 @@ func GetDeviceProperties(device string, executor exec.Executor) (map[string]stri // GetDevicePropertiesFromPath gets a device property from a path func GetDevicePropertiesFromPath(devicePath string, executor exec.Executor) (map[string]string, error) { - cmd := fmt.Sprintf("lsblk %s", devicePath) - output, err := executor.ExecuteCommandWithOutput(false, cmd, "lsblk", devicePath, + output, err := executor.ExecuteCommandWithOutput("lsblk", devicePath, "--bytes", "--nodeps", "--pairs", "--output", "SIZE,ROTA,RO,TYPE,PKNAME,NAME") if err != nil { // The "not a block device" error also returns code 32 so the ExitStatus() check hides this error @@ -210,8 +207,7 @@ func GetDevicePropertiesFromPath(devicePath string, executor exec.Executor) (map } // try to get more information about the command error - cmdErr, ok := err.(*exec.CommandError) - if ok && cmdErr.ExitStatus() == 32 { + if code, ok := exec.ExitStatus(err); ok && code == 32 { // certain device types (such as loop) return exit status 32 when probed further, // ignore and continue without logging return map[string]string{}, nil @@ -238,8 +234,7 @@ func IsLV(devicePath string, executor exec.Executor) (bool, error) { // GetUdevInfo gets udev information func GetUdevInfo(device string, executor exec.Executor) (map[string]string, error) { - cmd := fmt.Sprintf("udevadm info %s", device) - output, err := executor.ExecuteCommandWithOutput(false, cmd, "udevadm", "info", "--query=property", fmt.Sprintf("/dev/%s", device)) + output, err := executor.ExecuteCommandWithOutput("udevadm", "info", "--query=property", fmt.Sprintf("/dev/%s", device)) if err != nil { return nil, err } @@ -253,10 +248,9 @@ func GetDeviceFilesystems(device string, executor exec.Executor) (string, error) if len(devicePath) == 1 { device = fmt.Sprintf("/dev/%s", device) } - cmd := fmt.Sprintf("get filesystem type for %s", device) - output, err := executor.ExecuteCommandWithOutput(false, cmd, "udevadm", "info", "--query=property", device) + output, err := executor.ExecuteCommandWithOutput("udevadm", "info", "--query=property", device) if err != nil { - return "", fmt.Errorf("command %s failed: %+v", cmd, err) + return "", err } return parseFS(output), nil @@ -274,10 +268,7 @@ func GetDiskUUID(device string, executor exec.Executor) (string, error) { device = fmt.Sprintf("/dev/%s", device) } - cmd := fmt.Sprintf("get disk %s uuid", device) - - output, err := executor.ExecuteCommandWithOutput(false, cmd, - sgdisk, "--print", device) + output, err := executor.ExecuteCommandWithOutput(sgdisk, "--print", device) if err != nil { return "", err } @@ -378,7 +369,7 @@ func inventoryDevice(executor exec.Executor, dev string) (CephVolumeInventory, e device := path.Join("/dev", dev) args := []string{"inventory", "--format", "json", device} - inventory, err := executor.ExecuteCommandWithOutput(false, "", "ceph-volume", args...) + inventory, err := executor.ExecuteCommandWithOutput("ceph-volume", args...) if err != nil { return CVInventory, fmt.Errorf("failed to execute ceph-volume inventory on disk %q. %v", err, device) } @@ -394,9 +385,7 @@ func inventoryDevice(executor exec.Executor, dev string) (CephVolumeInventory, e // ListDevicesChild list all child available on a device func ListDevicesChild(executor exec.Executor, device string) ([]string, error) { - cmd := "lsblk for child" - - childListRaw, err := executor.ExecuteCommandWithOutput(false, cmd, "lsblk", "--noheadings", "--pairs", path.Join("/dev", device)) + childListRaw, err := executor.ExecuteCommandWithOutput("lsblk", "--noheadings", "--pairs", path.Join("/dev", device)) if err != nil { return []string{}, fmt.Errorf("failed to list child devices of %q. %v", device, err) } diff --git a/pkg/util/sys/device_test.go b/pkg/util/sys/device_test.go index 02ef3e229..5d561b1b6 100644 --- a/pkg/util/sys/device_test.go +++ b/pkg/util/sys/device_test.go @@ -116,9 +116,9 @@ func TestParseFileSystem(t *testing.T) { func TestGetPartitions(t *testing.T) { run := 0 executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, arg ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, arg ...string) (string, error) { run++ - logger.Infof("run %d action %s command %s", run, actionName, command) + logger.Infof("run %d command %s", run, command) switch { case run == 1: return `NAME="sdc" SIZE="100000" TYPE="disk" PKNAME=""`, nil @@ -185,8 +185,8 @@ func TestParseUdevInfo(t *testing.T) { func TestListDevicesChildListDevicesChild(t *testing.T) { executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, arg ...string) (string, error) { - logger.Infof("action %s command %s", actionName, command) + MockExecuteCommandWithOutput: func(command string, arg ...string) (string, error) { + logger.Infof("command %s", command) return lsblkChildOutput, nil }, } diff --git a/pkg/util/sys/kmod.go b/pkg/util/sys/kmod.go index ac6e70c13..7acd72b1a 100644 --- a/pkg/util/sys/kmod.go +++ b/pkg/util/sys/kmod.go @@ -42,7 +42,7 @@ func IsBuiltinKernelModule(name string, executor pkgexec.Executor) (bool, error) } kv = fmt.Sprintf("/lib/modules/%s/modules.builtin", kv) - out, err := executor.ExecuteCommandWithCombinedOutput(false, "check builtin kmod", "cat", kv) + out, err := executor.ExecuteCommandWithCombinedOutput("cat", kv) if err != nil { return false, fmt.Errorf("failed to cat %s: %+v", kv, err) } @@ -58,7 +58,7 @@ func LoadKernelModule(name string, options []string, executor pkgexec.Executor) args := append([]string{name}, options...) - if err := executor.ExecuteCommand(false, fmt.Sprintf("modprobe %s", name), "modprobe", args[:]...); err != nil { + if err := executor.ExecuteCommand("modprobe", args[:]...); err != nil { return fmt.Errorf("failed to load kernel module %s: %+v", name, err) } @@ -66,7 +66,7 @@ func LoadKernelModule(name string, options []string, executor pkgexec.Executor) } func CheckKernelModuleParam(name, param string, executor pkgexec.Executor) (bool, error) { - out, err := executor.ExecuteCommandWithOutput(false, "check kmod param", "modinfo", "-F", "parm", name) + out, err := executor.ExecuteCommandWithOutput("modinfo", "-F", "parm", name) if err != nil { return false, fmt.Errorf("failed to check for %s module %s param: %+v", name, param, err) } diff --git a/tests/framework/clients/test_client.go b/tests/framework/clients/test_client.go index a364d7685..d2e316ddc 100644 --- a/tests/framework/clients/test_client.go +++ b/tests/framework/clients/test_client.go @@ -63,7 +63,7 @@ func CreateTestClient(k8sHelper *utils.K8sHelper, manifests installer.CephManife // Status returns rook status details func (c TestClient) Status(namespace string) (client.CephStatus, error) { context := c.k8sh.MakeContext() - status, err := client.Status(context, namespace, false) + status, err := client.Status(context, namespace) if err != nil { return client.CephStatus{}, fmt.Errorf("failed to get status: %+v", err) } diff --git a/tests/framework/utils/exec_utils.go b/tests/framework/utils/exec_utils.go index 7ac02784d..ef40ec1c1 100644 --- a/tests/framework/utils/exec_utils.go +++ b/tests/framework/utils/exec_utils.go @@ -20,12 +20,11 @@ import ( "bufio" "bytes" "fmt" - "log" "os/exec" "strings" - "syscall" "github.com/coreos/pkg/capnslog" + utilexec "github.com/rook/rook/pkg/util/exec" ) var logger = capnslog.NewPackageLogger("github.com/rook/rook", "testutil") @@ -111,17 +110,14 @@ func ExecuteCommand(cmdStruct CommandArgs) CommandOut { stdin.Close() } - if err := cmd.Wait(); err != nil { - if exiterr, ok := err.(*exec.ExitError); ok { - // The program has exited with an exit code != 0 - if status, ok := exiterr.Sys().(syscall.WaitStatus); ok { - log.Printf("Exit Status: %d", status.ExitStatus()) - return CommandOut{StdErr: errBuffer.String(), StdOut: outBuffer.String(), ExitCode: status.ExitStatus(), Err: exiterr} - } - } else { - return CommandOut{StdErr: errBuffer.String(), StdOut: outBuffer.String(), Err: nil} + err = cmd.Wait() + out := CommandOut{StdErr: errBuffer.String(), StdOut: outBuffer.String()} + if err != nil { + out.Err = err + if code, ok := utilexec.ExitStatus(err); ok { + out.ExitCode = code } } - return CommandOut{StdErr: errBuffer.String(), StdOut: outBuffer.String(), Err: err} + return out } diff --git a/tests/framework/utils/helm_helper.go b/tests/framework/utils/helm_helper.go index c3fb21228..5cb93d959 100644 --- a/tests/framework/utils/helm_helper.go +++ b/tests/framework/utils/helm_helper.go @@ -39,7 +39,7 @@ func NewHelmHelper(helmPath string) *HelmHelper { // Execute is wrapper for executing helm commands func (h *HelmHelper) Execute(args ...string) (string, error) { - result, err := h.executor.ExecuteCommandWithOutput(false, "", h.HelmPath, args...) + result, err := h.executor.ExecuteCommandWithOutput(h.HelmPath, args...) if err != nil { logger.Errorf("Errors Encountered while executing helm command %v: %v", result, err) return result, fmt.Errorf("Failed to run helm commands on args %v : %v , err -> %v", args, result, err) diff --git a/tests/framework/utils/k8s_helper.go b/tests/framework/utils/k8s_helper.go index c9f87bc60..efb537dec 100644 --- a/tests/framework/utils/k8s_helper.go +++ b/tests/framework/utils/k8s_helper.go @@ -117,7 +117,7 @@ func (k8sh *K8sHelper) GetDockerImage(image string) error { if dockercmd == "" { dockercmd = "docker" } - return k8sh.executor.ExecuteCommand(false, "", dockercmd, "pull", image) + return k8sh.executor.ExecuteCommand(dockercmd, "pull", image) } // SetDeploymentVersion sets the container version on the deployment. It is assumed to be the rook/ceph image. @@ -128,7 +128,7 @@ func (k8sh *K8sHelper) SetDeploymentVersion(namespace, deploymentName, container // Kubectl is wrapper for executing kubectl commands func (k8sh *K8sHelper) Kubectl(args ...string) (string, error) { - result, err := k8sh.executor.ExecuteCommandWithTimeout(false, 15*time.Second, "kubectl", "kubectl", args...) + result, err := k8sh.executor.ExecuteCommandWithTimeout(15*time.Second, "kubectl", args...) if err != nil { k8slogger.Errorf("Failed to execute: kubectl %+v : %+v. %s", args, err, result) if args[0] == "delete" { @@ -162,9 +162,9 @@ func (k8sh *K8sHelper) KubectlWithStdin(stdin string, args ...string) (string, e } func getKubeConfig(executor exec.Executor) (*rest.Config, error) { - context, err := executor.ExecuteCommandWithOutput(false, "", "kubectl", "config", "view", "-o", "json") + context, err := executor.ExecuteCommandWithOutput("kubectl", "config", "view", "-o", "json") if err != nil { - k8slogger.Errorf("Errors Encountered while executing kubectl command : %v", err) + k8slogger.Errorf("failed to execute kubectl command. %v", err) } // Parse the kubectl context to get the settings for client connections diff --git a/tests/framework/utils/k8s_helper_test.go b/tests/framework/utils/k8s_helper_test.go index dc11eb7ae..a56a0225c 100644 --- a/tests/framework/utils/k8s_helper_test.go +++ b/tests/framework/utils/k8s_helper_test.go @@ -96,7 +96,7 @@ var rawContext = `{ func TestLoadMinikubeContext(t *testing.T) { executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, command string, subcommand string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, subcommand string, args ...string) (string, error) { return fmt.Sprintf(rawContext, "minikube"), nil }} config, err := getKubeConfig(executor) @@ -111,7 +111,7 @@ func TestLoadMinikubeContext(t *testing.T) { func TestLoadDindContext(t *testing.T) { executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, command string, subcommand string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, subcommand string, args ...string) (string, error) { return fmt.Sprintf(rawContext, "dind"), nil }} config, err := getKubeConfig(executor) @@ -126,7 +126,7 @@ func TestLoadDindContext(t *testing.T) { func TestLoadVagrantContext(t *testing.T) { executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, command string, subcommand string, args ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, subcommand string, args ...string) (string, error) { return fmt.Sprintf(rawContext, "vagrant-single"), nil }} config, err := getKubeConfig(executor)