From 0344074c8287d0100ad222f9c16a81acc4425058 Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Tue, 17 Mar 2020 13:46:54 -0600 Subject: [PATCH 1/7] ceph: remove dead code for process management Rook no longer relies on its own process management, now we can rely completely on Kubernetes to manage the pod lifecycle. The code to check for running processes and replacement them hasn't been used for a long while. Signed-off-by: Travis Nielsen --- go.mod | 2 +- go.sum | 8 +- pkg/daemon/ceph/osd/agent.go | 5 - pkg/daemon/ceph/osd/device.go | 20 --- pkg/util/proc/log.go | 20 --- pkg/util/proc/monitoredproc.go | 152 ------------------ pkg/util/proc/monitoredproc_test.go | 110 ------------- pkg/util/proc/procmanager.go | 239 ---------------------------- pkg/util/proc/procmanager_test.go | 83 ---------- 9 files changed, 7 insertions(+), 632 deletions(-) delete mode 100644 pkg/util/proc/log.go delete mode 100644 pkg/util/proc/monitoredproc.go delete mode 100644 pkg/util/proc/monitoredproc_test.go delete mode 100644 pkg/util/proc/procmanager.go delete mode 100644 pkg/util/proc/procmanager_test.go 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..8b0d928ee 100644 --- a/go.sum +++ b/go.sum @@ -376,8 +376,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 +695,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 +823,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 +910,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/daemon/ceph/osd/agent.go b/pkg/daemon/ceph/osd/agent.go index ab4ca8fd5..faa50a40b 100644 --- a/pkg/daemon/ceph/osd/agent.go +++ b/pkg/daemon/ceph/osd/agent.go @@ -23,7 +23,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/proc" ) const ( @@ -35,10 +34,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,8 +61,6 @@ 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), } } 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/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) -} From 7d2811bfb6474e927238858ebd7517e68773f13c Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Tue, 17 Mar 2020 14:22:31 -0600 Subject: [PATCH 2/7] ceph: remove obsolete ipv4 command line flags Long ago the ipv4 flags were renamed to public-ip and private-ip so we can go ahead and remove the obsolete flags. Signed-off-by: Travis Nielsen --- cmd/rook/ceph/ceph.go | 11 ------ cmd/rook/ceph/osd.go | 4 --- pkg/util/display/numbers.go | 26 -------------- pkg/util/display/numbers_test.go | 30 ----------------- pkg/util/flags/flags.go | 32 ------------------ pkg/util/flags/flags_test.go | 58 -------------------------------- 6 files changed, 161 deletions(-) delete mode 100644 pkg/util/display/numbers.go delete mode 100644 pkg/util/display/numbers_test.go 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/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/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", From 18b0e7d2953390678a0c67e48356dd6fc6dda982 Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Tue, 17 Mar 2020 15:57:53 -0600 Subject: [PATCH 3/7] ceph: scrub ceph commands to write actions to the log The ceph commands are now only written to the log in debug mode. For commands that change the system state we now ensure that a useful log entry is written. If all the details of the ceph commands are needed, debug logging should still be enabled. Signed-off-by: Travis Nielsen --- go.sum | 1 + pkg/daemon/ceph/client/auth.go | 19 ++---- pkg/daemon/ceph/client/command.go | 29 +++------ pkg/daemon/ceph/client/filesystem.go | 2 + pkg/daemon/ceph/client/image.go | 3 + pkg/daemon/ceph/client/mon.go | 44 +------------ pkg/daemon/ceph/client/osd.go | 17 ----- pkg/daemon/ceph/client/pg.go | 62 ------------------- pkg/daemon/ceph/client/status.go | 9 ++- pkg/daemon/ceph/client/upgrade.go | 2 +- pkg/daemon/ceph/client/usage.go | 47 -------------- pkg/operator/ceph/cluster/cephstatus.go | 2 +- pkg/operator/ceph/cluster/crash/add.go | 1 - pkg/operator/ceph/cluster/mgr/dashboard.go | 11 ++-- pkg/operator/ceph/cluster/mgr/mgr.go | 1 - pkg/operator/ceph/cluster/mgr/orchestrator.go | 5 +- pkg/operator/ceph/cluster/mon/config.go | 2 +- pkg/operator/ceph/cluster/mon/health.go | 2 +- pkg/operator/ceph/cluster/mon/mon.go | 2 +- .../ceph/controller/controller_utils.go | 2 +- pkg/operator/ceph/object/admin.go | 2 +- pkg/operator/ceph/object/bucket.go | 2 +- pkg/operator/k8sutil/k8sutil.go | 7 ++- tests/framework/clients/test_client.go | 2 +- 24 files changed, 48 insertions(+), 228 deletions(-) delete mode 100644 pkg/daemon/ceph/client/pg.go delete mode 100644 pkg/daemon/ceph/client/usage.go diff --git a/go.sum b/go.sum index 8b0d928ee..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= 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..446146c08 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" @@ -108,26 +107,12 @@ func newCephToolCommand(tool string, context *clusterd.Context, clusterName stri } } -// 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, true) } func NewRBDCommand(context *clusterd.Context, clusterName string, args []string) *CephToolCommand { - cmd := newCephToolCommand(RBDTool, context, clusterName, args, IsDebugLevel()) + cmd := newCephToolCommand(RBDTool, context, clusterName, args, true) cmd.JsonOutput = false cmd.OutputFile = false return cmd @@ -195,27 +180,27 @@ func ExecuteRBDCommandWithTimeout(context *clusterd.Context, clusterName string, } 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/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/image.go b/pkg/daemon/ceph/client/image.go index 2784aecb0..bcc506d8b 100644 --- a/pkg/daemon/ceph/client/image.go +++ b/pkg/daemon/ceph/client/image.go @@ -92,6 +92,7 @@ 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 { @@ -117,6 +118,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 +131,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/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/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/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/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/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..ca402352d 100755 --- a/pkg/operator/ceph/cluster/mgr/dashboard.go +++ b/pkg/operator/ceph/cluster/mgr/dashboard.go @@ -222,17 +222,20 @@ func getExitCode(err error) (int, bool) { 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/mgr.go b/pkg/operator/ceph/cluster/mgr/mgr.go index 5b0ae8ee3..c9ad23876 100644 --- a/pkg/operator/ceph/cluster/mgr/mgr.go +++ b/pkg/operator/ceph/cluster/mgr/mgr.go @@ -167,7 +167,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/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/mon/config.go b/pkg/operator/ceph/cluster/mon/config.go index b5a497d4c..20fec4782 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(true, "gen secret", "ceph-authtool", args...) if err != nil { return "", errors.Wrapf(err, "failed to gen secret") } 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/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/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/object/admin.go b/pkg/operator/ceph/object/admin.go index c4797d813..c21038867 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(true, "", 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/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/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) } From 84b8cdcf75cb1d27a35e44707351a2d23bc073cd Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Tue, 17 Mar 2020 15:58:04 -0600 Subject: [PATCH 4/7] exec: simplify the exec package from unused methods and logging The methods and arguments to the exec methods are not all used anymore. This cleans up the methods to only what is necessary to improve the readability and maintainability. Signed-off-by: Travis Nielsen --- pkg/util/exec/exec.go | 85 +++++++++++++---------------- pkg/util/exec/test/mockexec.go | 9 --- pkg/util/exec/translate_exec.go | 7 --- tests/framework/utils/k8s_helper.go | 6 +- 4 files changed, 41 insertions(+), 66 deletions(-) diff --git a/pkg/util/exec/exec.go b/pkg/util/exec/exec.go index d99151b93..f64548a96 100644 --- a/pkg/util/exec/exec.go +++ b/pkg/util/exec/exec.go @@ -31,39 +31,26 @@ 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) + ExecuteCommand(suppressLogOutput bool, actionName string, command string, arg ...string) error + ExecuteCommandWithOutput(suppressLogCommand bool, actionName string, command string, arg ...string) (string, error) + ExecuteCommandWithCombinedOutput(suppressLogOutput bool, actionName string, command string, arg ...string) (string, error) + ExecuteCommandWithOutputFile(suppressLogOutput bool, actionName, command, outfileArg string, arg ...string) (string, error) + ExecuteCommandWithOutputFileTimeout(suppressLogOutput bool, timeout time.Duration, actionName, command, outfileArg string, arg ...string) (string, error) + ExecuteCommandWithTimeout(suppressLogOutput bool, timeout time.Duration, actionName string, command string, arg ...string) (string, error) ExecuteStat(name string) (os.FileInfo, 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(suppressLogOutput bool, actionName string, command string, arg ...string) error { + cmd, stdout, stderr, err := startCommand(suppressLogOutput, command, arg...) if err != nil { return createCommandError(err, actionName) } - logOutput(actionName, stdout, stderr) + logOutput(suppressLogOutput, actionName, stdout, stderr) if err := cmd.Wait(); err != nil { return createCommandError(err, actionName) @@ -73,8 +60,8 @@ func (*CommandExecutor) ExecuteCommand(debug bool, actionName string, command st } // 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(suppressLogOutput bool, timeout time.Duration, actionName string, command string, arg ...string) (string, error) { + logCommand(suppressLogOutput, command, arg...) cmd := exec.Command(command, arg...) var b bytes.Buffer @@ -90,11 +77,11 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(debug bool, timeout time.Durat done <- cmd.Wait() }() - interrupSent := false + interruptSent := false for { select { case <-time.After(timeout): - if interrupSent { + 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 { @@ -111,12 +98,12 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(debug bool, timeout time.Durat 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) } - if interrupSent { + if interruptSent { e := fmt.Errorf("Timeout waiting for the command %s to return", command) return strings.TrimSpace(b.String()), createCommandError(e, command) } @@ -125,20 +112,20 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(debug bool, timeout time.Durat } } -func (*CommandExecutor) ExecuteCommandWithOutput(debug bool, actionName string, command string, arg ...string) (string, error) { - logCommand(debug, command, arg...) +func (*CommandExecutor) ExecuteCommandWithOutput(suppressLogOutput bool, actionName string, command string, arg ...string) (string, error) { + logCommand(suppressLogOutput, command, arg...) cmd := exec.Command(command, arg...) return runCommandWithOutput(actionName, cmd, false) } -func (*CommandExecutor) ExecuteCommandWithCombinedOutput(debug bool, actionName string, command string, arg ...string) (string, error) { - logCommand(debug, command, arg...) +func (*CommandExecutor) ExecuteCommandWithCombinedOutput(suppressLogOutput bool, actionName string, command string, arg ...string) (string, error) { + logCommand(suppressLogOutput, command, arg...) cmd := exec.Command(command, arg...) return runCommandWithOutput(actionName, cmd, true) } // Same as ExecuteCommandWithOutputFile but with a timeout limit. -func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(debug bool, timeout time.Duration, actionName string, +func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(suppressLogOutput bool, timeout time.Duration, actionName string, command, outfileArg string, arg ...string) (string, error) { outFile, err := ioutil.TempFile("", "") @@ -149,7 +136,7 @@ func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(debug bool, timeout defer os.Remove(outFile.Name()) arg = append(arg, outfileArg, outFile.Name()) - logCommand(debug, command, arg...) + logCommand(suppressLogOutput, command, arg...) ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() @@ -160,7 +147,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)) + writeToLog(suppressLogOutput, string(cmdOut)) } if ctx.Err() == context.DeadlineExceeded { @@ -175,7 +162,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(suppressLogOutput bool, actionName string, 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,12 +176,12 @@ 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(suppressLogOutput, 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 if string(cmdOut) != "" { - logger.Debug(string(cmdOut)) + writeToLog(suppressLogOutput, string(cmdOut)) } if err != nil { return string(cmdOut), err @@ -205,8 +192,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(suppressLogOutput bool, command string, arg ...string) (*exec.Cmd, io.ReadCloser, io.ReadCloser, error) { + logCommand(suppressLogOutput, command, arg...) cmd := exec.Command(command, arg...) stdout, err := cmd.StdoutPipe() @@ -228,16 +215,16 @@ func (*CommandExecutor) ExecuteStat(name string) (os.FileInfo, error) { } // read from reader line by line and write it to the log -func logFromReader(logger *capnslog.PackageLogger, reader io.ReadCloser) { +func logFromReader(suppressLogOutput bool, logger *capnslog.PackageLogger, reader io.ReadCloser) { in := bufio.NewScanner(reader) lastLine := "" for in.Scan() { lastLine = in.Text() - logger.Info(lastLine) + writeToLog(suppressLogOutput, lastLine) } } -func logOutput(name string, stdout, stderr io.ReadCloser) { +func logOutput(suppressLogOutput bool, name string, stdout, stderr io.ReadCloser) { if stdout == nil || stderr == nil { logger.Warningf("failed to collect stdout and stderr") return @@ -254,8 +241,8 @@ func logOutput(name string, stdout, stderr io.ReadCloser) { } } - go logFromReader(childLogger, stderr) - logFromReader(childLogger, stdout) + go logFromReader(suppressLogOutput, childLogger, stderr) + logFromReader(suppressLogOutput, childLogger, stdout) } func runCommandWithOutput(actionName string, cmd *exec.Cmd, combinedOutput bool) (string, error) { @@ -277,9 +264,13 @@ func runCommandWithOutput(actionName string, cmd *exec.Cmd, combinedOutput bool) return out, nil } -func logCommand(debug bool, command string, arg ...string) { +func logCommand(suppressLogOutput bool, command string, arg ...string) { msg := fmt.Sprintf("Running command: %s %s", command, strings.Join(arg, " ")) - if debug { + writeToLog(suppressLogOutput, msg) +} + +func writeToLog(suppressLogOutput bool, msg string) { + if suppressLogOutput { logger.Debug(msg) } else { logger.Info(msg) diff --git a/pkg/util/exec/test/mockexec.go b/pkg/util/exec/test/mockexec.go index 6ade83894..652b525a4 100644 --- a/pkg/util/exec/test/mockexec.go +++ b/pkg/util/exec/test/mockexec.go @@ -41,15 +41,6 @@ func (e *MockExecutor) ExecuteCommand(debug bool, actionName string, command str 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) { if e.MockExecuteCommandWithOutput != nil { return e.MockExecuteCommandWithOutput(debug, actionName, command, arg...) diff --git a/pkg/util/exec/translate_exec.go b/pkg/util/exec/translate_exec.go index 06ae9a09b..68450d367 100644 --- a/pkg/util/exec/translate_exec.go +++ b/pkg/util/exec/translate_exec.go @@ -18,7 +18,6 @@ package exec import ( "fmt" "os" - "os/exec" "time" ) @@ -34,12 +33,6 @@ type TranslateCommandExecutor struct { 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...) -} - // 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...) diff --git a/tests/framework/utils/k8s_helper.go b/tests/framework/utils/k8s_helper.go index c9f87bc60..ee1df1722 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(false, dockercmd, "pull", image) } // SetDeploymentVersion sets the container version on the deployment. It is assumed to be the rook/ceph image. @@ -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(false, "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 From f2ecaa2bda0159af77fd08a0387c49402bf7e687 Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Tue, 17 Mar 2020 18:23:16 -0600 Subject: [PATCH 5/7] exec: remove the unused actionName param The helpers for executing a process have long required an actionName param which is not being used. Now we remove the old param while also cleaning up various other usages of the exec package. Signed-off-by: Travis Nielsen --- cmd/rook/edgefs/start.go | 3 +- cmd/rook/rook/rook.go | 1 - pkg/clusterd/disk_test.go | 8 +-- .../flexvolume/manager/ceph/manager_test.go | 20 +++--- pkg/daemon/ceph/client/command.go | 14 ++-- pkg/daemon/ceph/client/crush_test.go | 2 +- .../ceph/client/erasure-code-profile_test.go | 2 +- pkg/daemon/ceph/client/filesystem_test.go | 4 +- pkg/daemon/ceph/client/image.go | 3 +- pkg/daemon/ceph/client/image_test.go | 10 +-- pkg/daemon/ceph/client/mgr_test.go | 8 +-- pkg/daemon/ceph/client/osd_test.go | 4 +- pkg/daemon/ceph/client/pool_test.go | 12 ++-- pkg/daemon/ceph/client/upgrade_test.go | 14 ++-- pkg/daemon/ceph/osd/daemon.go | 12 ++-- pkg/daemon/ceph/osd/daemon_test.go | 10 +-- pkg/daemon/ceph/osd/device_test.go | 2 +- pkg/daemon/ceph/osd/volume.go | 16 ++--- pkg/daemon/ceph/osd/volume_test.go | 16 ++--- pkg/daemon/discover/discover_test.go | 30 ++------- pkg/operator/ceph/client/controller_test.go | 6 +- pkg/operator/ceph/cluster/mgr/dashboard.go | 11 ---- .../ceph/cluster/mgr/dashboard_test.go | 6 +- pkg/operator/ceph/cluster/mgr/mgr.go | 3 +- pkg/operator/ceph/cluster/mgr/mgr_test.go | 4 +- .../ceph/cluster/mgr/orchestrator_test.go | 6 +- pkg/operator/ceph/cluster/mon/config.go | 2 +- pkg/operator/ceph/cluster/mon/config_test.go | 2 +- pkg/operator/ceph/cluster/mon/health_test.go | 6 +- pkg/operator/ceph/cluster/mon/mon_test.go | 4 +- pkg/operator/ceph/cluster/osd/health_test.go | 6 +- pkg/operator/ceph/cluster/osd/osd_test.go | 4 +- pkg/operator/ceph/cluster/rbd/mirror_test.go | 2 +- .../ceph/config/keyring/store_test.go | 2 +- pkg/operator/ceph/config/monstore_test.go | 4 +- pkg/operator/ceph/file/filesystem_test.go | 12 ++-- pkg/operator/ceph/nfs/nfs.go | 8 +-- pkg/operator/ceph/object/admin.go | 2 +- pkg/operator/ceph/object/controller_test.go | 4 +- pkg/operator/ceph/object/objectstore_test.go | 6 +- pkg/operator/ceph/object/rgw_test.go | 8 +-- .../ceph/object/user/controller_test.go | 4 +- pkg/operator/ceph/pool/controller_test.go | 10 +-- .../ceph/provisioner/provisioner_test.go | 4 +- pkg/operator/cockroachdb/controller.go | 2 +- pkg/operator/cockroachdb/controller_test.go | 2 +- pkg/operator/edgefs/cluster/mgr/mgr_test.go | 2 +- pkg/util/exec/error.go | 29 ++------ pkg/util/exec/exec.go | 66 +++++++++---------- pkg/util/exec/test/mockexec.go | 48 ++++++-------- pkg/util/exec/translate_exec.go | 45 ++++++------- pkg/util/sys/device.go | 31 +++------ pkg/util/sys/device_test.go | 8 +-- pkg/util/sys/kmod.go | 6 +- tests/framework/utils/exec_utils.go | 20 +++--- tests/framework/utils/helm_helper.go | 2 +- tests/framework/utils/k8s_helper.go | 2 +- 57 files changed, 245 insertions(+), 335 deletions(-) diff --git a/cmd/rook/edgefs/start.go b/cmd/rook/edgefs/start.go index 151dae87f..6ff2af64d 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(true, 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..d0c7e027b 100644 --- a/cmd/rook/rook/rook.go +++ b/cmd/rook/rook/rook.go @@ -126,7 +126,6 @@ func NewContext() *clusterd.Context { Executor: context.Executor, Translator: func( debug bool, - actionName string, command string, arg ...string, ) (string, []string) { diff --git a/pkg/clusterd/disk_test.go b/pkg/clusterd/disk_test.go index d9e07c042..2b55ad809 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(debug bool, 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(debug bool, command string, arg ...string) (string, error) { + logger.Infof("mock execute with outputP. %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..f22d5e4f5 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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/command.go b/pkg/daemon/ceph/client/command.go index 446146c08..895f9d935 100644 --- a/pkg/daemon/ceph/client/command.go +++ b/pkg/daemon/ceph/client/command.go @@ -138,22 +138,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(c.Debug, command, args...) } else { - output, err = c.context.Executor.ExecuteCommandWithTimeout(c.Debug, c.timeout, "", command, args...) + output, err = c.context.Executor.ExecuteCommandWithTimeout(c.Debug, 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(c.Debug, 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.Debug, 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(c.Debug, command, args...) } else { - output, err = c.context.Executor.ExecuteCommandWithTimeout(c.Debug, c.timeout, "", command, args...) + output, err = c.context.Executor.ExecuteCommandWithTimeout(c.Debug, c.timeout, command, args...) } } @@ -175,7 +175,7 @@ func (c *CephToolCommand) RunWithTimeout(timeout time.Duration) ([]byte, error) // configured its arguments. It is future work to integrate this case into the // generalization. func ExecuteRBDCommandWithTimeout(context *clusterd.Context, clusterName string, args []string) (string, error) { - output, err := context.Executor.ExecuteCommandWithTimeout(false, CmdExecuteTimeout, "", RBDTool, args...) + output, err := context.Executor.ExecuteCommandWithTimeout(false, CmdExecuteTimeout, RBDTool, args...) return output, err } diff --git a/pkg/daemon/ceph/client/crush_test.go b/pkg/daemon/ceph/client/crush_test.go index 3843f12e9..8af94e131 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(debug bool, 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..b06e3e3ea 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(debug bool, 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_test.go b/pkg/daemon/ceph/client/filesystem_test.go index 6f22ea871..5ab651919 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(debug bool, 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(debug bool, 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 bcc506d8b..81f0d24a7 100644 --- a/pkg/daemon/ceph/client/image.go +++ b/pkg/daemon/ceph/client/image.go @@ -96,8 +96,7 @@ func CreateImage(context *clusterd.Context, clusterName, name, poolName, dataPoo 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 { diff --git a/pkg/daemon/ceph/client/image_test.go b/pkg/daemon/ceph/client/image_test.go index 13ad06750..25174568e 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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..199469fae 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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/osd_test.go b/pkg/daemon/ceph/client/osd_test.go index 4bf99069a..75a7dc0b7 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(debug bool, 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(debug bool, 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/pool_test.go b/pkg/daemon/ceph/client/pool_test.go index e714b0c10..198947baf 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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/upgrade_test.go b/pkg/daemon/ceph/client/upgrade_test.go index bcf114c67..739c4dc23 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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/osd/daemon.go b/pkg/daemon/ceph/osd/daemon.go index 184e516ed..a0726051f 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(false, "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(false, "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(false, "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(false, "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) } @@ -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(false, "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(false, "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..d00942b26 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(debug bool, 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_test.go b/pkg/daemon/ceph/osd/device_test.go index 173b20114..42b2e8c8b 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(debug bool, 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 f9d69f307..529a98d1a 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(false, baseCommand, immediateExecuteArgs...) if err != nil { cvLogFilePath := path.Join(cvLogDir, "ceph-volume.log") @@ -487,12 +487,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(false, 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(false, baseCommand, immediateExecuteArgs...); err != nil { return errors.Wrapf(err, "failed ceph-volume") } @@ -535,7 +535,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(false, baseCommand, reportArgs...); err != nil { return errors.Wrapf(err, "failed ceph-volume report") // fail return here as validation provided by ceph-volume } @@ -544,7 +544,7 @@ func (a *OsdAgent) initializeDevices(context *clusterd.Context, devices *DeviceO "json", }...) - cvOut, err := context.Executor.ExecuteCommandWithCombinedOutput(false, "", baseCommand, reportArgs...) + cvOut, err := context.Executor.ExecuteCommandWithCombinedOutput(false, 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 +561,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(false, baseCommand, mdArgs...); err != nil { return errors.Wrapf(err, "failed ceph-volume") // fail return here as validation provided by ceph-volume } } @@ -587,7 +587,7 @@ 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(false, cephVolumeCmd, cvMode, "list", lv, "--format", "json") if err != nil { return nil, errors.Wrapf(err, "failed to retrieve ceph-volume %s list results", cvMode) } @@ -664,7 +664,7 @@ 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(false, cephVolumeCmd, cvMode, "list", block, "--format", "json") if err != nil { return nil, errors.Wrapf(err, "failed to retrieve ceph-volume %s list results", cvMode) } diff --git a/pkg/daemon/ceph/osd/volume_test.go b/pkg/daemon/ceph/osd/volume_test.go index 47758ca75..37261605c 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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_test.go b/pkg/daemon/discover/discover_test.go index 55f636994..d32b83c93 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(debug bool, 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(debug bool, 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..a56755e1d 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(debug bool, 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(debug bool, 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(debug bool, 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/mgr/dashboard.go b/pkg/operator/ceph/cluster/mgr/dashboard.go index ca402352d..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,16 +209,6 @@ 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("setting ceph dashboard %q login creds", dashboardUsername) diff --git a/pkg/operator/ceph/cluster/mgr/dashboard_test.go b/pkg/operator/ceph/cluster/mgr/dashboard_test.go index f80f351c7..2485b7196 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(debug bool, 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(debug bool, timeout time.Duration, command, outfileArg string, arg ...string) (string, error) { + return executor.MockExecuteCommandWithOutputFile(debug, 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 c9ad23876..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, } diff --git a/pkg/operator/ceph/cluster/mgr/mgr_test.go b/pkg/operator/ceph/cluster/mgr/mgr_test.go index 54d711da0..3d85949d8 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(debug bool, 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(debug bool, 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_test.go b/pkg/operator/ceph/cluster/mgr/orchestrator_test.go index 1cd66387e..866653ebb 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(debug bool, 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(debug bool, 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(debug bool, 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 20fec4782..54cc2c352 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(true, "gen secret", "ceph-authtool", args...) + _, err := executor.ExecuteCommandWithOutput(true, "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..2076131f5 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(debug bool, 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_test.go b/pkg/operator/ceph/cluster/mon/health_test.go index cbc18552e..52e094af3 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(debug bool, 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(debug bool, 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(debug bool, command string, outFileArg string, args ...string) (string, error) { return monQuorumResponse, nil }, } diff --git a/pkg/operator/ceph/cluster/mon/mon_test.go b/pkg/operator/ceph/cluster/mon/mon_test.go index c78ddaf3b..f984cff6a 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(debug bool, 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(debug bool, 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..554cae0b8 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(debug bool, 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(debug bool, 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(debug bool, 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..62265a251 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(debug bool, 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(debug bool, 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/rbd/mirror_test.go b/pkg/operator/ceph/cluster/rbd/mirror_test.go index 9680a5614..7e2973f13 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(debug bool, 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..f5a024137 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(debug bool, 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..f5905e5d7 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(debug bool, 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(debug bool, 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/file/filesystem_test.go b/pkg/operator/ceph/file/filesystem_test.go index 92f3a5ce9..74d0eb4ed 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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..e177b7b0e 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(false, 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(false, 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(false, 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 c21038867..4b61d4fa5 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(true, "", command, args...) + output, err := c.Context.Executor.ExecuteCommandWithOutput(true, command, args...) if err != nil { return "", errors.Wrapf(err, "failed to run radosgw-admin") } diff --git a/pkg/operator/ceph/object/controller_test.go b/pkg/operator/ceph/object/controller_test.go index 15534b631..10310ec10 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(debug bool, 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(debug bool, 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..50ccc0846 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(debug bool, 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(debug bool, 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(debug bool, 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..f5579c89e 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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..bf63f8dc0 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(debug bool, 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(debug bool, 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..c490a725a 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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(debug bool, 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..f9b88b76e 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(debug bool, 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(debug bool, 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..9b2136749 100644 --- a/pkg/operator/cockroachdb/controller.go +++ b/pkg/operator/cockroachdb/controller.go @@ -486,7 +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", + out, err := c.context.Executor.ExecuteCommandWithCombinedOutput(false, "/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..c02e620e5 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(debug bool, 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..7056be700 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(debug bool, command string, outFileArg string, args ...string) (string, error) { return "{\"key\":\"mysecurekey\"}", nil }, } 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 f64548a96..87ed0b3f4 100644 --- a/pkg/util/exec/exec.go +++ b/pkg/util/exec/exec.go @@ -31,36 +31,35 @@ import ( ) type Executor interface { - ExecuteCommand(suppressLogOutput bool, actionName string, command string, arg ...string) error - ExecuteCommandWithOutput(suppressLogCommand bool, actionName string, command string, arg ...string) (string, error) - ExecuteCommandWithCombinedOutput(suppressLogOutput bool, actionName string, command string, arg ...string) (string, error) - ExecuteCommandWithOutputFile(suppressLogOutput bool, actionName, command, outfileArg string, arg ...string) (string, error) - ExecuteCommandWithOutputFileTimeout(suppressLogOutput bool, timeout time.Duration, actionName, command, outfileArg string, arg ...string) (string, error) - ExecuteCommandWithTimeout(suppressLogOutput bool, timeout time.Duration, actionName string, command string, arg ...string) (string, error) - ExecuteStat(name string) (os.FileInfo, error) + ExecuteCommand(suppressLogOutput bool, command string, arg ...string) error + ExecuteCommandWithOutput(suppressLogCommand bool, command string, arg ...string) (string, error) + ExecuteCommandWithCombinedOutput(suppressLogOutput bool, command string, arg ...string) (string, error) + ExecuteCommandWithOutputFile(suppressLogOutput bool, command, outfileArg string, arg ...string) (string, error) + ExecuteCommandWithOutputFileTimeout(suppressLogOutput bool, timeout time.Duration, command, outfileArg string, arg ...string) (string, error) + ExecuteCommandWithTimeout(suppressLogOutput bool, timeout time.Duration, command string, arg ...string) (string, error) } type CommandExecutor struct { } // Start a process and wait for its completion -func (*CommandExecutor) ExecuteCommand(suppressLogOutput bool, actionName string, command string, arg ...string) error { +func (*CommandExecutor) ExecuteCommand(suppressLogOutput bool, command string, arg ...string) error { cmd, stdout, stderr, err := startCommand(suppressLogOutput, command, arg...) if err != nil { - return createCommandError(err, actionName) + return err } - logOutput(suppressLogOutput, actionName, stdout, stderr) + logOutput(suppressLogOutput, 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(suppressLogOutput bool, timeout time.Duration, actionName string, command string, arg ...string) (string, error) { +func (*CommandExecutor) ExecuteCommandWithTimeout(suppressLogOutput bool, timeout time.Duration, command string, arg ...string) (string, error) { logCommand(suppressLogOutput, command, arg...) cmd := exec.Command(command, arg...) @@ -69,7 +68,7 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(suppressLogOutput bool, timeou cmd.Stderr = &b if err := cmd.Start(); err != nil { - return "", createCommandError(err, actionName) + return "", err } done := make(chan error, 1) @@ -82,18 +81,18 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(suppressLogOutput bool, timeou select { case <-time.After(timeout): if interruptSent { - logger.Infof("Timeout waiting for process %s to return after interrupt signal was sent. Sending kill signal to the process", command) + 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) + 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) // kill signal will be sent next loop @@ -101,31 +100,30 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(suppressLogOutput bool, timeou interruptSent = true case err := <-done: if err != nil { - return strings.TrimSpace(b.String()), createCommandError(err, command) + return strings.TrimSpace(b.String()), err } if interruptSent { - 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()), fmt.Errorf("timeout waiting for the command %s to return", command) } return strings.TrimSpace(b.String()), nil } } } -func (*CommandExecutor) ExecuteCommandWithOutput(suppressLogOutput bool, actionName string, command string, arg ...string) (string, error) { +func (*CommandExecutor) ExecuteCommandWithOutput(suppressLogOutput bool, command string, arg ...string) (string, error) { logCommand(suppressLogOutput, command, arg...) cmd := exec.Command(command, arg...) - return runCommandWithOutput(actionName, cmd, false) + return runCommandWithOutput(cmd, false) } -func (*CommandExecutor) ExecuteCommandWithCombinedOutput(suppressLogOutput bool, actionName string, command string, arg ...string) (string, error) { +func (*CommandExecutor) ExecuteCommandWithCombinedOutput(suppressLogOutput bool, command string, arg ...string) (string, error) { logCommand(suppressLogOutput, 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(suppressLogOutput bool, timeout time.Duration, actionName string, +func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(suppressLogOutput bool, timeout time.Duration, command, outfileArg string, arg ...string) (string, error) { outFile, err := ioutil.TempFile("", "") @@ -162,7 +160,7 @@ func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(suppressLogOutput bo return string(fileOut), err } -func (*CommandExecutor) ExecuteCommandWithOutputFile(suppressLogOutput bool, actionName string, command, outfileArg string, arg ...string) (string, error) { +func (*CommandExecutor) ExecuteCommandWithOutputFile(suppressLogOutput bool, 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 @@ -210,10 +208,6 @@ func startCommand(suppressLogOutput bool, command string, arg ...string) (*exec. 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(suppressLogOutput bool, logger *capnslog.PackageLogger, reader io.ReadCloser) { in := bufio.NewScanner(reader) @@ -224,7 +218,7 @@ func logFromReader(suppressLogOutput bool, logger *capnslog.PackageLogger, reade } } -func logOutput(suppressLogOutput bool, name string, stdout, stderr io.ReadCloser) { +func logOutput(suppressLogOutput bool, stdout, stderr io.ReadCloser) { if stdout == nil || stderr == nil { logger.Warningf("failed to collect stdout and stderr") return @@ -233,11 +227,11 @@ func logOutput(suppressLogOutput bool, 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}) } } @@ -245,7 +239,7 @@ func logOutput(suppressLogOutput bool, name string, stdout, stderr io.ReadCloser logFromReader(suppressLogOutput, 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 @@ -258,7 +252,7 @@ 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 diff --git a/pkg/util/exec/test/mockexec.go b/pkg/util/exec/test/mockexec.go index 652b525a4..3988d3ecc 100644 --- a/pkg/util/exec/test/mockexec.go +++ b/pkg/util/exec/test/mockexec.go @@ -16,76 +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(suppressLogOutput bool, command string, arg ...string) error + MockStartExecuteCommand func(suppressLogOutput bool, command string, arg ...string) (*exec.Cmd, error) + MockExecuteCommandWithOutput func(suppressLogOutput bool, command string, arg ...string) (string, error) + MockExecuteCommandWithCombinedOutput func(suppressLogOutput bool, command string, arg ...string) (string, error) + MockExecuteCommandWithOutputFile func(suppressLogOutput bool, command, outfileArg string, arg ...string) (string, error) + MockExecuteCommandWithOutputFileTimeout func(suppressLogOutput bool, timeout time.Duration, command, outfileArg string, arg ...string) (string, error) + MockExecuteCommandWithTimeout func(suppressLogOutput bool, 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(suppressLogOutput bool, command string, arg ...string) error { if e.MockExecuteCommand != nil { - return e.MockExecuteCommand(debug, actionName, command, arg...) + return e.MockExecuteCommand(suppressLogOutput, command, arg...) } return nil } -func (e *MockExecutor) ExecuteCommandWithOutput(debug bool, actionName string, command string, arg ...string) (string, error) { +func (e *MockExecutor) ExecuteCommandWithOutput(suppressLogOutput bool, command string, arg ...string) (string, error) { if e.MockExecuteCommandWithOutput != nil { - return e.MockExecuteCommandWithOutput(debug, actionName, command, arg...) + return e.MockExecuteCommandWithOutput(suppressLogOutput, 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(suppressLogOutput bool, 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(suppressLogOutput, time.Second, command, arg...) } return "", nil } -func (e *MockExecutor) ExecuteCommandWithCombinedOutput(debug bool, actionName string, command string, arg ...string) (string, error) { +func (e *MockExecutor) ExecuteCommandWithCombinedOutput(suppressLogOutput bool, command string, arg ...string) (string, error) { if e.MockExecuteCommandWithCombinedOutput != nil { - return e.MockExecuteCommandWithCombinedOutput(debug, actionName, command, arg...) + return e.MockExecuteCommandWithCombinedOutput(suppressLogOutput, command, arg...) } return "", nil } -func (e *MockExecutor) ExecuteCommandWithOutputFile(debug bool, actionName string, command, outfileArg string, arg ...string) (string, error) { +func (e *MockExecutor) ExecuteCommandWithOutputFile(suppressLogOutput bool, command, outfileArg string, arg ...string) (string, error) { if e.MockExecuteCommandWithOutputFile != nil { - return e.MockExecuteCommandWithOutputFile(debug, actionName, command, outfileArg, arg...) + return e.MockExecuteCommandWithOutputFile(suppressLogOutput, 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(suppressLogOutput bool, 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(suppressLogOutput, 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 68450d367..0b0439674 100644 --- a/pkg/util/exec/translate_exec.go +++ b/pkg/util/exec/translate_exec.go @@ -16,8 +16,6 @@ limitations under the License. package exec import ( - "fmt" - "os" "time" ) @@ -30,48 +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) + Translator func(suppressLogOutput bool, 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(suppressLogOutput bool, command string, arg ...string) error { + transCommand, transArgs := e.Translator(suppressLogOutput, command, arg...) + return e.Executor.ExecuteCommand(suppressLogOutput, 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(suppressLogOutput bool, command string, arg ...string) (string, error) { + transCommand, transArgs := e.Translator(suppressLogOutput, command, arg...) + return e.Executor.ExecuteCommandWithOutput(suppressLogOutput, 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(suppressLogOutput bool, command string, arg ...string) (string, error) { + transCommand, transArgs := e.Translator(suppressLogOutput, command, arg...) + return e.Executor.ExecuteCommandWithCombinedOutput(suppressLogOutput, 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(suppressLogOutput bool, command, outfileArg string, arg ...string) (string, error) { + transCommand, transArgs := e.Translator(suppressLogOutput, command, arg...) + return e.Executor.ExecuteCommandWithOutputFile(suppressLogOutput, 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, + suppressLogOutput bool, 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(suppressLogOutput, command, arg...) + return e.Executor.ExecuteCommandWithOutputFileTimeout(suppressLogOutput, 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(suppressLogOutput bool, timeout time.Duration, command string, arg ...string) (string, error) { + transCommand, transArgs := e.Translator(suppressLogOutput, command, arg...) + return e.Executor.ExecuteCommandWithTimeout(suppressLogOutput, timeout, transCommand, transArgs...) } diff --git a/pkg/util/sys/device.go b/pkg/util/sys/device.go index b238dd8ee..a7026efbf 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(false, "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(false, "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(false, "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(false, "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(false, "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(false, 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(false, "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(false, "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..3b9c20065 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(debug bool, 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(debug bool, 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..4eb6bce2f 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(false, "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(false, "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(false, "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/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..a3fa1edcc 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(false, 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 ee1df1722..ab8f921cb 100644 --- a/tests/framework/utils/k8s_helper.go +++ b/tests/framework/utils/k8s_helper.go @@ -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(false, 15*time.Second, "kubectl", args...) if err != nil { k8slogger.Errorf("Failed to execute: kubectl %+v : %+v. %s", args, err, result) if args[0] == "delete" { From e8f9cfcb714685d93017be279d9ffdfc3847acca Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Wed, 18 Mar 2020 11:53:33 -0600 Subject: [PATCH 6/7] exec: always write commands to debug log Signed-off-by: Travis Nielsen --- cmd/rook/edgefs/start.go | 2 +- cmd/rook/rook/rook.go | 1 - pkg/clusterd/disk_test.go | 6 +- .../flexvolume/manager/ceph/manager_test.go | 20 ++--- pkg/daemon/ceph/client/command.go | 22 +++--- pkg/daemon/ceph/client/crush_test.go | 2 +- .../ceph/client/erasure-code-profile_test.go | 2 +- pkg/daemon/ceph/client/filesystem_test.go | 4 +- pkg/daemon/ceph/client/image_test.go | 10 +-- pkg/daemon/ceph/client/mgr_test.go | 8 +- pkg/daemon/ceph/client/osd_test.go | 4 +- pkg/daemon/ceph/client/pool_test.go | 12 +-- pkg/daemon/ceph/client/upgrade_test.go | 14 ++-- pkg/daemon/ceph/osd/agent.go | 5 +- pkg/daemon/ceph/osd/daemon.go | 14 ++-- pkg/daemon/ceph/osd/daemon_test.go | 2 +- pkg/daemon/ceph/osd/device_test.go | 2 +- pkg/daemon/ceph/osd/volume.go | 23 +++--- pkg/daemon/ceph/osd/volume_test.go | 8 +- pkg/daemon/discover/discover.go | 2 +- pkg/daemon/discover/discover_test.go | 4 +- pkg/operator/ceph/client/controller_test.go | 6 +- .../ceph/cluster/mgr/dashboard_test.go | 6 +- pkg/operator/ceph/cluster/mgr/mgr_test.go | 4 +- .../ceph/cluster/mgr/orchestrator_test.go | 6 +- pkg/operator/ceph/cluster/mon/config.go | 2 +- pkg/operator/ceph/cluster/mon/config_test.go | 2 +- pkg/operator/ceph/cluster/mon/health_test.go | 6 +- pkg/operator/ceph/cluster/mon/mon_test.go | 4 +- pkg/operator/ceph/cluster/osd/health_test.go | 6 +- pkg/operator/ceph/cluster/osd/osd_test.go | 4 +- pkg/operator/ceph/cluster/rbd/mirror_test.go | 2 +- .../ceph/config/keyring/store_test.go | 2 +- pkg/operator/ceph/config/monstore_test.go | 4 +- pkg/operator/ceph/file/filesystem_test.go | 12 +-- pkg/operator/ceph/nfs/nfs.go | 6 +- pkg/operator/ceph/object/admin.go | 2 +- pkg/operator/ceph/object/controller_test.go | 4 +- pkg/operator/ceph/object/objectstore_test.go | 6 +- pkg/operator/ceph/object/rgw_test.go | 8 +- .../ceph/object/user/controller_test.go | 4 +- pkg/operator/ceph/pool/controller_test.go | 10 +-- .../ceph/provisioner/provisioner_test.go | 4 +- pkg/operator/cockroachdb/controller.go | 3 +- pkg/operator/cockroachdb/controller_test.go | 2 +- pkg/operator/edgefs/cluster/mgr/mgr_test.go | 2 +- pkg/util/exec/exec.go | 75 ++++++++----------- pkg/util/exec/test/mockexec.go | 38 +++++----- pkg/util/exec/translate_exec.go | 38 +++++----- pkg/util/sys/device.go | 16 ++-- pkg/util/sys/device_test.go | 4 +- pkg/util/sys/kmod.go | 6 +- tests/framework/utils/helm_helper.go | 2 +- tests/framework/utils/k8s_helper.go | 6 +- tests/framework/utils/k8s_helper_test.go | 6 +- 55 files changed, 228 insertions(+), 247 deletions(-) diff --git a/cmd/rook/edgefs/start.go b/cmd/rook/edgefs/start.go index 6ff2af64d..42f3bf378 100644 --- a/cmd/rook/edgefs/start.go +++ b/cmd/rook/edgefs/start.go @@ -43,7 +43,7 @@ func start(cmd *cobra.Command, args []string) error { rook.SetLogLevel() rook.LogStartupInfo(operatorCmd.Flags()) executor := &exec.CommandExecutor{} - err := executor.ExecuteCommand(true, 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 d0c7e027b..1fded786a 100644 --- a/cmd/rook/rook/rook.go +++ b/cmd/rook/rook/rook.go @@ -125,7 +125,6 @@ func NewContext() *clusterd.Context { context.Executor = &exec.TranslateCommandExecutor{ Executor: context.Executor, Translator: func( - debug bool, command string, arg ...string, ) (string, []string) { diff --git a/pkg/clusterd/disk_test.go b/pkg/clusterd/disk_test.go index 2b55ad809..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, command string, arg ...string) error { + MockExecuteCommand: func(command string, arg ...string) error { logger.Infof("mock execute. %s", command) return nil }, - MockExecuteCommandWithOutput: func(debug bool, command string, arg ...string) (string, error) { - logger.Infof("mock execute with outputP. %s", 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 f22d5e4f5..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, 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, 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, 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, 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, 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, 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, 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, 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, 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, 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/command.go b/pkg/daemon/ceph/client/command.go index 895f9d935..e0f5d1ff0 100644 --- a/pkg/daemon/ceph/client/command.go +++ b/pkg/daemon/ceph/client/command.go @@ -90,29 +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, } } func NewCephCommand(context *clusterd.Context, clusterName string, args []string) *CephToolCommand { - return newCephToolCommand(CephTool, context, clusterName, args, true) + return newCephToolCommand(CephTool, context, clusterName, args) } func NewRBDCommand(context *clusterd.Context, clusterName string, args []string) *CephToolCommand { - cmd := newCephToolCommand(RBDTool, context, clusterName, args, true) + cmd := newCephToolCommand(RBDTool, context, clusterName, args) cmd.JsonOutput = false cmd.OutputFile = false return cmd @@ -138,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...) } } @@ -175,7 +173,7 @@ func (c *CephToolCommand) RunWithTimeout(timeout time.Duration) ([]byte, error) // configured its arguments. It is future work to integrate this case into the // generalization. func ExecuteRBDCommandWithTimeout(context *clusterd.Context, 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 } diff --git a/pkg/daemon/ceph/client/crush_test.go b/pkg/daemon/ceph/client/crush_test.go index 8af94e131..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, 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 b06e3e3ea..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, 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_test.go b/pkg/daemon/ceph/client/filesystem_test.go index 5ab651919..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, 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, 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_test.go b/pkg/daemon/ceph/client/image_test.go index 25174568e..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, 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, 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, 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, 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, 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 199469fae..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, 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, 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, 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, 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/osd_test.go b/pkg/daemon/ceph/client/osd_test.go index 75a7dc0b7..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, 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, 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/pool_test.go b/pkg/daemon/ceph/client/pool_test.go index 198947baf..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, 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, 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, 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, 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, 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, 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/upgrade_test.go b/pkg/daemon/ceph/client/upgrade_test.go index 739c4dc23..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, 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, 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, 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, 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, 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, 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, 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/osd/agent.go b/pkg/daemon/ceph/osd/agent.go index faa50a40b..3e0336712 100644 --- a/pkg/daemon/ceph/osd/agent.go +++ b/pkg/daemon/ceph/osd/agent.go @@ -17,8 +17,6 @@ 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" @@ -65,8 +63,7 @@ func NewAgent(context *clusterd.Context, devices []DesiredDevice, metadataDevice } 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 a0726051f..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 d00942b26..8751226b5 100644 --- a/pkg/daemon/ceph/osd/daemon_test.go +++ b/pkg/daemon/ceph/osd/daemon_test.go @@ -137,7 +137,7 @@ 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, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { logger.Infof("OUTPUT for %s %v", command, args) if command == "lsblk" { diff --git a/pkg/daemon/ceph/osd/device_test.go b/pkg/daemon/ceph/osd/device_test.go index 42b2e8c8b..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, 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 529a98d1a..a57c9ebc3 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 37261605c..9aea3996c 100644 --- a/pkg/daemon/ceph/osd/volume_test.go +++ b/pkg/daemon/ceph/osd/volume_test.go @@ -262,7 +262,7 @@ var cephVolumeRAWTestResult = `{ func TestParseCephVolumeLVMResult(t *testing.T) { executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutput = func(debug bool, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { logger.Infof("%s %v", command, args) if command == "ceph-volume" { @@ -281,7 +281,7 @@ func TestParseCephVolumeLVMResult(t *testing.T) { func TestParseCephVolumeRawResult(t *testing.T) { executor := &exectest.MockExecutor{} - executor.MockExecuteCommandWithOutput = func(debug bool, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { logger.Infof("%s %v", command, args) if command == "ceph-volume" { @@ -301,7 +301,7 @@ 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, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { logger.Infof("%s %v", command, args) if command == "ceph-volume" { @@ -322,7 +322,7 @@ 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, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { logger.Infof("%s %v", command, args) if command == "ceph-volume" { 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 d32b83c93..b0752afcf 100644 --- a/pkg/daemon/discover/discover_test.go +++ b/pkg/daemon/discover/discover_test.go @@ -71,7 +71,7 @@ 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, command string, args ...string) (string, error) { + executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) { logger.Infof("RUN Command %s %v", command, args) output := "" if args[0] == "--all" { @@ -338,7 +338,7 @@ func TestDeviceListsEqual(t *testing.T) { func TestGetCephVolumeInventory(t *testing.T) { run := 0 executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, command string, arg ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, arg ...string) (string, error) { run++ logger.Infof("run %d command %s", run, command) switch { diff --git a/pkg/operator/ceph/client/controller_test.go b/pkg/operator/ceph/client/controller_test.go index a56755e1d..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, 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, 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, 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/mgr/dashboard_test.go b/pkg/operator/ceph/cluster/mgr/dashboard_test.go index 2485b7196..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, 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, command, outfileArg string, arg ...string) (string, error) { - return executor.MockExecuteCommandWithOutputFile(debug, 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_test.go b/pkg/operator/ceph/cluster/mgr/mgr_test.go index 3d85949d8..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, 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, 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_test.go b/pkg/operator/ceph/cluster/mgr/orchestrator_test.go index 866653ebb..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, 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, 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, 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 54cc2c352..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(true, "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 2076131f5..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, 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_test.go b/pkg/operator/ceph/cluster/mon/health_test.go index 52e094af3..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, 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, 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, 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_test.go b/pkg/operator/ceph/cluster/mon/mon_test.go index f984cff6a..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, 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, 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 554cae0b8..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, 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, 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, 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 62265a251..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, 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, 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/rbd/mirror_test.go b/pkg/operator/ceph/cluster/rbd/mirror_test.go index 7e2973f13..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, 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 f5a024137..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, 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 f5905e5d7..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, 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, 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/file/filesystem_test.go b/pkg/operator/ceph/file/filesystem_test.go index 74d0eb4ed..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, 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, 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, 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, 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, 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, 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 e177b7b0e..54c2afbcb 100644 --- a/pkg/operator/ceph/nfs/nfs.go +++ b/pkg/operator/ceph/nfs/nfs.go @@ -127,13 +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), } - err := c.context.Executor.ExecuteCommand(false, 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 - return c.context.Executor.ExecuteCommand(false, cmd, append(args, "create", config)...) + return c.context.Executor.ExecuteCommand(cmd, append(args, "create", config)...) } func (c *CephNFSController) addServerToDatabase(nfs cephv1.CephNFS, name string) { @@ -171,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, 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 4b61d4fa5..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(true, 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/controller_test.go b/pkg/operator/ceph/object/controller_test.go index 10310ec10..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, 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, 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 50ccc0846..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, 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, 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, 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 f5579c89e..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, 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, 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, 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, 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 bf63f8dc0..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, 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, 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 c490a725a..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, 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, 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, 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, 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, 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 f9b88b76e..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, 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, 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 9b2136749..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, - "/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 c02e620e5..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, 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 7056be700..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, 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/util/exec/exec.go b/pkg/util/exec/exec.go index 87ed0b3f4..f6a9bec04 100644 --- a/pkg/util/exec/exec.go +++ b/pkg/util/exec/exec.go @@ -31,25 +31,25 @@ import ( ) type Executor interface { - ExecuteCommand(suppressLogOutput bool, command string, arg ...string) error - ExecuteCommandWithOutput(suppressLogCommand bool, command string, arg ...string) (string, error) - ExecuteCommandWithCombinedOutput(suppressLogOutput bool, command string, arg ...string) (string, error) - ExecuteCommandWithOutputFile(suppressLogOutput bool, command, outfileArg string, arg ...string) (string, error) - ExecuteCommandWithOutputFileTimeout(suppressLogOutput bool, timeout time.Duration, command, outfileArg string, arg ...string) (string, error) - ExecuteCommandWithTimeout(suppressLogOutput bool, timeout time.Duration, command string, arg ...string) (string, 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 wait for its completion -func (*CommandExecutor) ExecuteCommand(suppressLogOutput bool, command string, arg ...string) error { - cmd, stdout, stderr, err := startCommand(suppressLogOutput, command, arg...) +func (*CommandExecutor) ExecuteCommand(command string, arg ...string) error { + cmd, stdout, stderr, err := startCommand(command, arg...) if err != nil { return err } - logOutput(suppressLogOutput, stdout, stderr) + logOutput(stdout, stderr) if err := cmd.Wait(); err != nil { return err @@ -59,8 +59,8 @@ func (*CommandExecutor) ExecuteCommand(suppressLogOutput bool, command string, a } // ExecuteCommandWithTimeout starts a process and wait for its completion with timeout. -func (*CommandExecutor) ExecuteCommandWithTimeout(suppressLogOutput bool, timeout time.Duration, command string, arg ...string) (string, error) { - logCommand(suppressLogOutput, 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 @@ -84,8 +84,8 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(suppressLogOutput bool, timeou 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) } @@ -94,7 +94,7 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(suppressLogOutput bool, timeou 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 } interruptSent = true @@ -110,20 +110,20 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(suppressLogOutput bool, timeou } } -func (*CommandExecutor) ExecuteCommandWithOutput(suppressLogOutput bool, command string, arg ...string) (string, error) { - logCommand(suppressLogOutput, command, arg...) +func (*CommandExecutor) ExecuteCommandWithOutput(command string, arg ...string) (string, error) { + logCommand(command, arg...) cmd := exec.Command(command, arg...) return runCommandWithOutput(cmd, false) } -func (*CommandExecutor) ExecuteCommandWithCombinedOutput(suppressLogOutput bool, command string, arg ...string) (string, error) { - logCommand(suppressLogOutput, command, arg...) +func (*CommandExecutor) ExecuteCommandWithCombinedOutput(command string, arg ...string) (string, error) { + logCommand(command, arg...) cmd := exec.Command(command, arg...) return runCommandWithOutput(cmd, true) } // Same as ExecuteCommandWithOutputFile but with a timeout limit. -func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(suppressLogOutput bool, timeout time.Duration, +func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(timeout time.Duration, command, outfileArg string, arg ...string) (string, error) { outFile, err := ioutil.TempFile("", "") @@ -134,7 +134,7 @@ func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(suppressLogOutput bo defer os.Remove(outFile.Name()) arg = append(arg, outfileArg, outFile.Name()) - logCommand(suppressLogOutput, command, arg...) + logCommand(command, arg...) ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() @@ -145,7 +145,7 @@ func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(suppressLogOutput bo // if there was anything that went to stdout/stderr then log it, even before // we return an error if string(cmdOut) != "" { - writeToLog(suppressLogOutput, string(cmdOut)) + logger.Debug(string(cmdOut)) } if ctx.Err() == context.DeadlineExceeded { @@ -160,7 +160,7 @@ func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(suppressLogOutput bo return string(fileOut), err } -func (*CommandExecutor) ExecuteCommandWithOutputFile(suppressLogOutput bool, 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 @@ -174,12 +174,12 @@ func (*CommandExecutor) ExecuteCommandWithOutputFile(suppressLogOutput bool, com // append the output file argument to the list or args arg = append(arg, outfileArg, outFile.Name()) - logCommand(suppressLogOutput, 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 if string(cmdOut) != "" { - writeToLog(suppressLogOutput, string(cmdOut)) + logger.Debug(string(cmdOut)) } if err != nil { return string(cmdOut), err @@ -190,8 +190,8 @@ func (*CommandExecutor) ExecuteCommandWithOutputFile(suppressLogOutput bool, com return string(fileOut), err } -func startCommand(suppressLogOutput bool, command string, arg ...string) (*exec.Cmd, io.ReadCloser, io.ReadCloser, error) { - logCommand(suppressLogOutput, 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() @@ -209,16 +209,16 @@ func startCommand(suppressLogOutput bool, command string, arg ...string) (*exec. } // read from reader line by line and write it to the log -func logFromReader(suppressLogOutput bool, logger *capnslog.PackageLogger, reader io.ReadCloser) { +func logFromReader(logger *capnslog.PackageLogger, reader io.ReadCloser) { in := bufio.NewScanner(reader) lastLine := "" for in.Scan() { lastLine = in.Text() - writeToLog(suppressLogOutput, lastLine) + logger.Debug(lastLine) } } -func logOutput(suppressLogOutput bool, stdout, stderr io.ReadCloser) { +func logOutput(stdout, stderr io.ReadCloser) { if stdout == nil || stderr == nil { logger.Warningf("failed to collect stdout and stderr") return @@ -235,8 +235,8 @@ func logOutput(suppressLogOutput bool, stdout, stderr io.ReadCloser) { } } - go logFromReader(suppressLogOutput, childLogger, stderr) - logFromReader(suppressLogOutput, childLogger, stdout) + go logFromReader(childLogger, stderr) + logFromReader(childLogger, stdout) } func runCommandWithOutput(cmd *exec.Cmd, combinedOutput bool) (string, error) { @@ -258,15 +258,6 @@ func runCommandWithOutput(cmd *exec.Cmd, combinedOutput bool) (string, error) { return out, nil } -func logCommand(suppressLogOutput bool, command string, arg ...string) { - msg := fmt.Sprintf("Running command: %s %s", command, strings.Join(arg, " ")) - writeToLog(suppressLogOutput, msg) -} - -func writeToLog(suppressLogOutput bool, msg string) { - if suppressLogOutput { - 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 3988d3ecc..0d90937a3 100644 --- a/pkg/util/exec/test/mockexec.go +++ b/pkg/util/exec/test/mockexec.go @@ -22,59 +22,59 @@ import ( // ******************** MockExecutor ******************** type MockExecutor struct { - MockExecuteCommand func(suppressLogOutput bool, command string, arg ...string) error - MockStartExecuteCommand func(suppressLogOutput bool, command string, arg ...string) (*exec.Cmd, error) - MockExecuteCommandWithOutput func(suppressLogOutput bool, command string, arg ...string) (string, error) - MockExecuteCommandWithCombinedOutput func(suppressLogOutput bool, command string, arg ...string) (string, error) - MockExecuteCommandWithOutputFile func(suppressLogOutput bool, command, outfileArg string, arg ...string) (string, error) - MockExecuteCommandWithOutputFileTimeout func(suppressLogOutput bool, timeout time.Duration, command, outfileArg string, arg ...string) (string, error) - MockExecuteCommandWithTimeout func(suppressLogOutput bool, timeout time.Duration, command string, arg ...string) (string, 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(suppressLogOutput bool, command string, arg ...string) error { +func (e *MockExecutor) ExecuteCommand(command string, arg ...string) error { if e.MockExecuteCommand != nil { - return e.MockExecuteCommand(suppressLogOutput, command, arg...) + return e.MockExecuteCommand(command, arg...) } return nil } -func (e *MockExecutor) ExecuteCommandWithOutput(suppressLogOutput bool, command string, arg ...string) (string, error) { +func (e *MockExecutor) ExecuteCommandWithOutput(command string, arg ...string) (string, error) { if e.MockExecuteCommandWithOutput != nil { - return e.MockExecuteCommandWithOutput(suppressLogOutput, command, arg...) + return e.MockExecuteCommandWithOutput(command, arg...) } return "", nil } -func (e *MockExecutor) ExecuteCommandWithTimeout(suppressLogOutput bool, timeout time.Duration, 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(suppressLogOutput, time.Second, command, arg...) + return e.MockExecuteCommandWithTimeout(time.Second, command, arg...) } return "", nil } -func (e *MockExecutor) ExecuteCommandWithCombinedOutput(suppressLogOutput bool, command string, arg ...string) (string, error) { +func (e *MockExecutor) ExecuteCommandWithCombinedOutput(command string, arg ...string) (string, error) { if e.MockExecuteCommandWithCombinedOutput != nil { - return e.MockExecuteCommandWithCombinedOutput(suppressLogOutput, command, arg...) + return e.MockExecuteCommandWithCombinedOutput(command, arg...) } return "", nil } -func (e *MockExecutor) ExecuteCommandWithOutputFile(suppressLogOutput bool, 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(suppressLogOutput, command, outfileArg, arg...) + return e.MockExecuteCommandWithOutputFile(command, outfileArg, arg...) } return "", nil } -func (e *MockExecutor) ExecuteCommandWithOutputFileTimeout(suppressLogOutput bool, timeout time.Duration, 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(suppressLogOutput, timeout, command, outfileArg, arg...) + return e.MockExecuteCommandWithOutputFileTimeout(timeout, command, outfileArg, arg...) } return "", nil diff --git a/pkg/util/exec/translate_exec.go b/pkg/util/exec/translate_exec.go index 0b0439674..e9492804f 100644 --- a/pkg/util/exec/translate_exec.go +++ b/pkg/util/exec/translate_exec.go @@ -28,43 +28,43 @@ type TranslateCommandExecutor struct { Executor Executor // Translator translates every command before running it - Translator func(suppressLogOutput bool, command string, arg ...string) (string, []string) + Translator func(command string, arg ...string) (string, []string) } // ExecuteCommand starts a process and wait for its completion -func (e *TranslateCommandExecutor) ExecuteCommand(suppressLogOutput bool, command string, arg ...string) error { - transCommand, transArgs := e.Translator(suppressLogOutput, command, arg...) - return e.Executor.ExecuteCommand(suppressLogOutput, 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(suppressLogOutput bool, command string, arg ...string) (string, error) { - transCommand, transArgs := e.Translator(suppressLogOutput, command, arg...) - return e.Executor.ExecuteCommandWithOutput(suppressLogOutput, 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(suppressLogOutput bool, command string, arg ...string) (string, error) { - transCommand, transArgs := e.Translator(suppressLogOutput, command, arg...) - return e.Executor.ExecuteCommandWithCombinedOutput(suppressLogOutput, 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(suppressLogOutput bool, command, outfileArg string, arg ...string) (string, error) { - transCommand, transArgs := e.Translator(suppressLogOutput, command, arg...) - return e.Executor.ExecuteCommandWithOutputFile(suppressLogOutput, 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( - suppressLogOutput bool, timeout time.Duration, + timeout time.Duration, command, outfileArg string, arg ...string) (string, error) { - transCommand, transArgs := e.Translator(suppressLogOutput, command, arg...) - return e.Executor.ExecuteCommandWithOutputFileTimeout(suppressLogOutput, timeout, 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(suppressLogOutput bool, timeout time.Duration, command string, arg ...string) (string, error) { - transCommand, transArgs := e.Translator(suppressLogOutput, command, arg...) - return e.Executor.ExecuteCommandWithTimeout(suppressLogOutput, timeout, transCommand, transArgs...) +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/sys/device.go b/pkg/util/sys/device.go index a7026efbf..dfca6d5a2 100644 --- a/pkg/util/sys/device.go +++ b/pkg/util/sys/device.go @@ -110,7 +110,7 @@ type LocalDisk struct { // ListDevices list all devices available on a machine func ListDevices(executor exec.Executor) ([]string, error) { - devices, err := executor.ExecuteCommandWithOutput(false, "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) } @@ -129,7 +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 } - output, err := executor.ExecuteCommandWithOutput(false, "lsblk", devicePath, + output, err := executor.ExecuteCommandWithOutput("lsblk", devicePath, "--bytes", "--pairs", "--output", "NAME,SIZE,TYPE,PKNAME") logger.Infof("Output: %+v", output) if err != nil { @@ -198,7 +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) { - output, err := executor.ExecuteCommandWithOutput(false, "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 @@ -234,7 +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) { - output, err := executor.ExecuteCommandWithOutput(false, "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 } @@ -248,7 +248,7 @@ func GetDeviceFilesystems(device string, executor exec.Executor) (string, error) if len(devicePath) == 1 { device = fmt.Sprintf("/dev/%s", device) } - output, err := executor.ExecuteCommandWithOutput(false, "udevadm", "info", "--query=property", device) + output, err := executor.ExecuteCommandWithOutput("udevadm", "info", "--query=property", device) if err != nil { return "", err } @@ -268,7 +268,7 @@ func GetDiskUUID(device string, executor exec.Executor) (string, error) { device = fmt.Sprintf("/dev/%s", device) } - output, err := executor.ExecuteCommandWithOutput(false, sgdisk, "--print", device) + output, err := executor.ExecuteCommandWithOutput(sgdisk, "--print", device) if err != nil { return "", err } @@ -369,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) } @@ -385,7 +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) { - childListRaw, err := executor.ExecuteCommandWithOutput(false, "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 3b9c20065..5d561b1b6 100644 --- a/pkg/util/sys/device_test.go +++ b/pkg/util/sys/device_test.go @@ -116,7 +116,7 @@ func TestParseFileSystem(t *testing.T) { func TestGetPartitions(t *testing.T) { run := 0 executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, command string, arg ...string) (string, error) { + MockExecuteCommandWithOutput: func(command string, arg ...string) (string, error) { run++ logger.Infof("run %d command %s", run, command) switch { @@ -185,7 +185,7 @@ func TestParseUdevInfo(t *testing.T) { func TestListDevicesChildListDevicesChild(t *testing.T) { executor := &exectest.MockExecutor{ - MockExecuteCommandWithOutput: func(debug bool, command string, arg ...string) (string, error) { + 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 4eb6bce2f..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, "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, "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, "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/utils/helm_helper.go b/tests/framework/utils/helm_helper.go index a3fa1edcc..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 ab8f921cb..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", 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,7 +162,7 @@ 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("failed to execute kubectl command. %v", err) } 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) From 4a2ca1c1ef18ea68979cd88658d73ea03d8223a9 Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Wed, 18 Mar 2020 12:06:56 -0600 Subject: [PATCH 7/7] ceph: enable debug logging in the osd prepare job All processes created with the exec package will now only have debug logging for the commands and their output. The osd prepare job relies heavily on many of these commands so we can enable debug logging for the entire job. Signed-off-by: Travis Nielsen --- pkg/operator/ceph/cluster/osd/spec.go | 11 +++++++++++ 1 file changed, 11 insertions(+) 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} }