Skip to content

Commit d192526

Browse files
plaffittplaffitt
committed
add a timeout on iscsciadm commands
1 parent aac01d4 commit d192526

File tree

4 files changed

+85
-90
lines changed

4 files changed

+85
-90
lines changed

‎iscsi/iscsi.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,14 @@ func (c *Connector) connectTarget(target *TargetInfo, iFace string, iscsiTranspo
318318
// to avoid establishing additional sessions to the same target.
319319
if _, err := iscsiCmd(append(baseArgs, []string{"-R"}...)...); err != nil {
320320
debug.Printf("Failed to rescan session, err: %v", err)
321+
if os.IsTimeout(err) {
322+
debug.Printf("iscsiadm timeouted, logging out")
323+
cmd := execCommand("iscsiadm", append(baseArgs, []string{"-u"}...)...)
324+
out, err := cmd.CombinedOutput()
325+
if err != nil {
326+
return "", fmt.Errorf("could not logout from target: %s", out)
327+
}
328+
}
321329
}
322330

323331
// create our devicePath that we'll be looking for based on the transport being used

‎iscsi/iscsi_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ func Test_extractTransportName(t *testing.T) {
248248

249249
func Test_sessionExists(t *testing.T) {
250250
fakeOutput := "tcp: [4] 192.168.1.107:3260,1 iqn.2010-10.org.openstack:volume-eb393993-73d0-4e39-9ef4-b5841e244ced (non-flash)\n"
251-
defer gostub.Stub(&execCommand, makeFakeExecCommand(0, fakeOutput)).Reset()
251+
defer gostub.Stub(&execWithTimeout, makeFakeExecWithTimeout(false, []byte(fakeOutput), nil)).Reset()
252252

253253
type args struct {
254254
tgtPortal string

‎iscsi/iscsiadm.go

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
package iscsi
22

33
import (
4-
"bytes"
54
"fmt"
65
"strings"
6+
"time"
77
)
88

99
// Secrets provides optional iscsi security credentials (CHAP settings)
@@ -21,26 +21,12 @@ type Secrets struct {
2121
}
2222

2323
func iscsiCmd(args ...string) (string, error) {
24-
cmd := execCommand("iscsiadm", args...)
24+
stdout, err := execWithTimeout("iscsiadm", args, time.Second*3)
25+
2526
debug.Printf("Run iscsiadm command: %s", strings.Join(append([]string{"iscsiadm"}, args...), " "))
26-
var stdout bytes.Buffer
27-
var iscsiadmError error
28-
cmd.Stdout = &stdout
29-
cmd.Stderr = &stdout
30-
defer stdout.Reset()
31-
32-
// we're using Start and Wait because we want to grab exit codes
33-
err := cmd.Start()
34-
if err == nil {
35-
err = cmd.Wait()
36-
}
37-
if err != nil {
38-
formattedOutput := strings.Replace(string(stdout.Bytes()), "\n", "", -1)
39-
iscsiadmError = fmt.Errorf("iscsiadm error: %s (%s)", formattedOutput, err.Error())
40-
}
27+
iscsiadmDebug(string(stdout), err)
4128

42-
iscsiadmDebug(string(stdout.Bytes()), iscsiadmError)
43-
return string(stdout.Bytes()), iscsiadmError
29+
return string(stdout), err
4430
}
4531

4632
func iscsiadmDebug(output string, cmdError error) {

‎iscsi/iscsiadm_test.go

Lines changed: 71 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package iscsi
22

33
import (
4+
"os/exec"
45
"testing"
56

67
"github.com/prashantv/gostub"
@@ -68,20 +69,20 @@ iface.discovery_logout = <empty>
6869

6970
func TestDiscovery(t *testing.T) {
7071
tests := map[string]struct {
71-
tgtPortal string
72-
iface string
73-
discoverySecret Secrets
74-
chapDiscovery bool
75-
wantErr bool
76-
mockedStdout string
77-
mockedExitStatus int
72+
tgtPortal string
73+
iface string
74+
discoverySecret Secrets
75+
chapDiscovery bool
76+
wantErr bool
77+
mockedStdout string
78+
mockedCmdError error
7879
}{
7980
"DiscoverySuccess": {
80-
tgtPortal: "172.18.0.2:3260",
81-
iface: "default",
82-
chapDiscovery: false,
83-
mockedStdout: "172.18.0.2:3260,1 iqn.2016-09.com.openebs.jiva:store1\n",
84-
mockedExitStatus: 0,
81+
tgtPortal: "172.18.0.2:3260",
82+
iface: "default",
83+
chapDiscovery: false,
84+
mockedStdout: "172.18.0.2:3260,1 iqn.2016-09.com.openebs.jiva:store1\n",
85+
mockedCmdError: nil,
8586
},
8687

8788
"ConnectionFailure": {
@@ -92,8 +93,8 @@ func TestDiscovery(t *testing.T) {
9293
iscsiadm: cannot make connection to 172.18.0.2: Connection refused
9394
iscsiadm: connection login retries (reopen_max) 5 exceeded
9495
iscsiadm: Could not perform SendTargets discovery: encountered connection failure\n`,
95-
mockedExitStatus: 4,
96-
wantErr: true,
96+
mockedCmdError: exec.Command("exit", "4").Run(),
97+
wantErr: true,
9798
},
9899

99100
"ChapEntrySuccess": {
@@ -104,8 +105,8 @@ iscsiadm: Could not perform SendTargets discovery: encountered connection failur
104105
UserNameIn: "dummyuser",
105106
PasswordIn: "dummypass",
106107
},
107-
mockedStdout: "172.18.0.2:3260,1 iqn.2016-09.com.openebs.jiva:store1\n",
108-
mockedExitStatus: 0,
108+
mockedStdout: "172.18.0.2:3260,1 iqn.2016-09.com.openebs.jiva:store1\n",
109+
mockedCmdError: nil,
109110
},
110111

111112
"ChapEntryFailure": {
@@ -119,14 +120,14 @@ iscsiadm: Could not perform SendTargets discovery: encountered connection failur
119120
mockedStdout: `iscsiadm: Login failed to authenticate with target
120121
iscsiadm: discovery login to 172.18.0.2 rejected: initiator error (02/01), non-retryable, giving up
121122
iscsiadm: Could not perform SendTargets discovery.\n`,
122-
mockedExitStatus: 4,
123-
wantErr: true,
123+
mockedCmdError: exec.Command("exit", "4").Run(),
124+
wantErr: true,
124125
},
125126
}
126127

127128
for name, tt := range tests {
128129
t.Run(name, func(t *testing.T) {
129-
defer gostub.Stub(&execCommand, makeFakeExecCommand(tt.mockedExitStatus, tt.mockedStdout)).Reset()
130+
defer gostub.Stub(&execWithTimeout, makeFakeExecWithTimeout(false, []byte(tt.mockedStdout), tt.mockedCmdError)).Reset()
130131
err := Discoverydb(tt.tgtPortal, tt.iface, tt.discoverySecret, tt.chapDiscovery)
131132
if (err != nil) != tt.wantErr {
132133
t.Errorf("Discoverydb() error = %v, wantErr %v", err, tt.wantErr)
@@ -138,14 +139,14 @@ iscsiadm: Could not perform SendTargets discovery.\n`,
138139

139140
func TestCreateDBEntry(t *testing.T) {
140141
tests := map[string]struct {
141-
tgtPortal string
142-
tgtIQN string
143-
iface string
144-
discoverySecret Secrets
145-
sessionSecret Secrets
146-
wantErr bool
147-
mockedStdout string
148-
mockedExitStatus int
142+
tgtPortal string
143+
tgtIQN string
144+
iface string
145+
discoverySecret Secrets
146+
sessionSecret Secrets
147+
wantErr bool
148+
mockedStdout string
149+
mockedCmdError error
149150
}{
150151
"CreateDBEntryWithChapDiscoverySuccess": {
151152
tgtPortal: "192.168.1.107:3260",
@@ -161,22 +162,22 @@ func TestCreateDBEntry(t *testing.T) {
161162
PasswordIn: "dummypass",
162163
SecretsType: "chap",
163164
},
164-
mockedStdout: nodeDB,
165-
mockedExitStatus: 0,
165+
mockedStdout: nodeDB,
166+
mockedCmdError: nil,
166167
},
167168
"CreateDBEntryWithChapDiscoveryFailure": {
168-
tgtPortal: "172.18.0.2:3260",
169-
tgtIQN: "iqn.2016-09.com.openebs.jiva:store1",
170-
iface: "default",
171-
mockedStdout: "iscsiadm: No records found\n",
172-
mockedExitStatus: 21,
173-
wantErr: true,
169+
tgtPortal: "172.18.0.2:3260",
170+
tgtIQN: "iqn.2016-09.com.openebs.jiva:store1",
171+
iface: "default",
172+
mockedStdout: "iscsiadm: No records found\n",
173+
mockedCmdError: exec.Command("exit", "21").Run(),
174+
wantErr: true,
174175
},
175176
}
176177

177178
for name, tt := range tests {
178179
t.Run(name, func(t *testing.T) {
179-
defer gostub.Stub(&execCommand, makeFakeExecCommand(tt.mockedExitStatus, tt.mockedStdout)).Reset()
180+
defer gostub.Stub(&execWithTimeout, makeFakeExecWithTimeout(false, []byte(tt.mockedStdout), tt.mockedCmdError)).Reset()
180181
err := CreateDBEntry(tt.tgtIQN, tt.tgtPortal, tt.iface, tt.discoverySecret, tt.sessionSecret)
181182
if (err != nil) != tt.wantErr {
182183
t.Errorf("CreateDBEntry() error = %v, wantErr %v", err, tt.wantErr)
@@ -189,41 +190,41 @@ func TestCreateDBEntry(t *testing.T) {
189190

190191
func TestListInterfaces(t *testing.T) {
191192
tests := map[string]struct {
192-
mockedStdout string
193-
mockedExitStatus int
194-
interfaces []string
195-
wantErr bool
193+
mockedStdout string
194+
mockedCmdError error
195+
interfaces []string
196+
wantErr bool
196197
}{
197198
"EmptyOutput": {
198-
mockedStdout: "",
199-
mockedExitStatus: 0,
200-
interfaces: []string{""},
201-
wantErr: false,
199+
mockedStdout: "",
200+
mockedCmdError: nil,
201+
interfaces: []string{""},
202+
wantErr: false,
202203
},
203204
"DefaultInterface": {
204-
mockedStdout: "default",
205-
mockedExitStatus: 0,
206-
interfaces: []string{"default"},
207-
wantErr: false,
205+
mockedStdout: "default",
206+
mockedCmdError: nil,
207+
interfaces: []string{"default"},
208+
wantErr: false,
208209
},
209210
"TwoInterface": {
210-
mockedStdout: "default\ntest",
211-
mockedExitStatus: 0,
212-
interfaces: []string{"default", "test"},
213-
wantErr: false,
211+
mockedStdout: "default\ntest",
212+
mockedCmdError: nil,
213+
interfaces: []string{"default", "test"},
214+
wantErr: false,
214215
},
215216
"HasError": {
216-
mockedStdout: "",
217-
mockedExitStatus: 1,
218-
interfaces: []string{},
219-
wantErr: true,
217+
mockedStdout: "",
218+
mockedCmdError: exec.Command("exit", "1").Run(),
219+
interfaces: []string{},
220+
wantErr: true,
220221
},
221222
}
222223

223224
for name, tt := range tests {
224225
t.Run(name, func(t *testing.T) {
225226
assert := assert.New(t)
226-
defer gostub.Stub(&execCommand, makeFakeExecCommand(tt.mockedExitStatus, tt.mockedStdout)).Reset()
227+
defer gostub.Stub(&execWithTimeout, makeFakeExecWithTimeout(false, []byte(tt.mockedStdout), tt.mockedCmdError)).Reset()
227228
interfaces, err := ListInterfaces()
228229

229230
if tt.wantErr {
@@ -238,29 +239,29 @@ func TestListInterfaces(t *testing.T) {
238239

239240
func TestShowInterface(t *testing.T) {
240241
tests := map[string]struct {
241-
mockedStdout string
242-
mockedExitStatus int
243-
iFace string
244-
wantErr bool
242+
mockedStdout string
243+
mockedCmdError error
244+
iFace string
245+
wantErr bool
245246
}{
246247
"DefaultInterface": {
247-
mockedStdout: defaultInterface,
248-
mockedExitStatus: 0,
249-
iFace: defaultInterface,
250-
wantErr: false,
248+
mockedStdout: defaultInterface,
249+
mockedCmdError: nil,
250+
iFace: defaultInterface,
251+
wantErr: false,
251252
},
252253
"HasError": {
253-
mockedStdout: "",
254-
mockedExitStatus: 1,
255-
iFace: "",
256-
wantErr: true,
254+
mockedStdout: "",
255+
mockedCmdError: exec.Command("exit", "1").Run(),
256+
iFace: "",
257+
wantErr: true,
257258
},
258259
}
259260

260261
for name, tt := range tests {
261262
t.Run(name, func(t *testing.T) {
262263
assert := assert.New(t)
263-
defer gostub.Stub(&execCommand, makeFakeExecCommand(tt.mockedExitStatus, tt.mockedStdout)).Reset()
264+
defer gostub.Stub(&execWithTimeout, makeFakeExecWithTimeout(false, []byte(tt.mockedStdout), tt.mockedCmdError)).Reset()
264265
interfaces, err := ShowInterface("default")
265266

266267
if tt.wantErr {

0 commit comments

Comments
 (0)