Skip to content

Commit d44583d

Browse files
committed
Fix iscsi refcounter in the case of no Block iscsi volumes
1 parent 45f6270 commit d44583d

File tree

3 files changed

+69
-38
lines changed

3 files changed

+69
-38
lines changed

pkg/volume/iscsi/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ go_test(
4242
],
4343
embed = [":go_default_library"],
4444
deps = [
45+
"//pkg/kubelet/config:go_default_library",
4546
"//pkg/volume:go_default_library",
4647
"//pkg/volume/testing:go_default_library",
4748
"//staging/src/k8s.io/api/core/v1:go_default_library",

pkg/volume/iscsi/iscsi_util.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -926,9 +926,13 @@ func isSessionBusy(host volume.VolumeHost, portal, iqn string) bool {
926926
// getVolCount returns the number of volumes in use by the kubelet.
927927
// It does so by counting the number of directories prefixed by the given portal and IQN.
928928
func getVolCount(dir, portal, iqn string) (int, error) {
929-
// The topmost dirs are named after the ifaces, e.g., iface-default or iface-127.0.0.1:3260:pv0
929+
// For FileSystem volumes, the topmost dirs are named after the ifaces, e.g., iface-default or iface-127.0.0.1:3260:pv0.
930+
// For Block volumes, the default topmost dir is volumeDevices.
930931
contents, err := ioutil.ReadDir(dir)
931932
if err != nil {
933+
if os.IsNotExist(err) {
934+
return 0, nil
935+
}
932936
return 0, err
933937
}
934938

pkg/volume/iscsi/iscsi_util_test.go

Lines changed: 63 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@ import (
2323
"reflect"
2424
"testing"
2525

26-
"k8s.io/utils/exec/testing"
26+
testingexec "k8s.io/utils/exec/testing"
2727

28+
"k8s.io/kubernetes/pkg/kubelet/config"
2829
"k8s.io/kubernetes/pkg/volume"
2930
volumetest "k8s.io/kubernetes/pkg/volume/testing"
3031
)
@@ -350,55 +351,78 @@ func TestClonedIfaceUpdateError(t *testing.T) {
350351
}
351352

352353
func TestGetVolCount(t *testing.T) {
354+
// This will create a dir structure like this:
355+
// /tmp/refcounter555814673
356+
// ├── iface-127.0.0.1:3260:pv1
357+
// │   └── 127.0.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-3
358+
// └── iface-127.0.0.1:3260:pv2
359+
// │ ├── 127.0.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-2
360+
// │ └── 192.168.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-1
361+
// └── volumeDevices
362+
// └── 192.168.0.2:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-4
363+
// └── 192.168.0.3:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-5
364+
365+
baseDir, err := createFakePluginDirs()
366+
if err != nil {
367+
t.Errorf("error creating fake plugin dir: %v", err)
368+
}
369+
370+
defer os.RemoveAll(baseDir)
371+
353372
testCases := []struct {
354-
name string
355-
portal string
356-
iqn string
357-
count int
373+
name string
374+
baseDir string
375+
portal string
376+
iqn string
377+
count int
358378
}{
359379
{
360-
name: "wrong portal, no volumes",
361-
portal: "192.168.0.2:3260", // incorrect IP address
362-
iqn: "iqn.2003-01.io.k8s:e2e.volume-1",
363-
count: 0,
380+
name: "wrong portal, no volumes",
381+
baseDir: baseDir,
382+
portal: "192.168.0.2:3260", // incorrect IP address
383+
iqn: "iqn.2003-01.io.k8s:e2e.volume-1",
384+
count: 0,
364385
},
365386
{
366-
name: "wrong iqn, no volumes",
367-
portal: "127.0.0.1:3260",
368-
iqn: "iqn.2003-01.io.k8s:e2e.volume-3", // incorrect volume
369-
count: 0,
387+
name: "wrong iqn, no volumes",
388+
baseDir: baseDir,
389+
portal: "127.0.0.1:3260",
390+
iqn: "iqn.2003-01.io.k8s:e2e.volume-3", // incorrect volume
391+
count: 0,
370392
},
371393
{
372-
name: "single volume",
373-
portal: "192.168.0.1:3260",
374-
iqn: "iqn.2003-01.io.k8s:e2e.volume-1",
375-
count: 1,
394+
name: "single volume",
395+
baseDir: baseDir,
396+
portal: "192.168.0.1:3260",
397+
iqn: "iqn.2003-01.io.k8s:e2e.volume-1",
398+
count: 1,
376399
},
377400
{
378-
name: "two volumes",
379-
portal: "127.0.0.1:3260",
380-
iqn: "iqn.2003-01.io.k8s:e2e.volume-1",
381-
count: 2,
401+
name: "two volumes",
402+
baseDir: baseDir,
403+
portal: "127.0.0.1:3260",
404+
iqn: "iqn.2003-01.io.k8s:e2e.volume-1",
405+
count: 2,
406+
},
407+
{
408+
name: "volumeDevices (block) volume",
409+
baseDir: filepath.Join(baseDir, config.DefaultKubeletVolumeDevicesDirName),
410+
portal: "192.168.0.2:3260",
411+
iqn: "iqn.2003-01.io.k8s:e2e.volume-1-lun-4",
412+
count: 1,
413+
},
414+
{
415+
name: "nonexistent path",
416+
baseDir: filepath.Join(baseDir, "this_path_should_not_exist"),
417+
portal: "127.0.0.1:3260",
418+
iqn: "iqn.2003-01.io.k8s:e2e.unknown",
419+
count: 0,
382420
},
383421
}
384422

385-
// This will create a dir structure like this:
386-
// /tmp/refcounter555814673
387-
// ├── iface-127.0.0.1:3260:pv1
388-
// │   └── 127.0.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-3
389-
// └── iface-127.0.0.1:3260:pv2
390-
// ├── 127.0.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-2
391-
// └── 192.168.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-1
392-
393-
baseDir, err := createFakePluginDir()
394-
if err != nil {
395-
t.Errorf("error creating fake plugin dir: %v", err)
396-
}
397-
defer os.RemoveAll(baseDir)
398-
399423
for _, tc := range testCases {
400424
t.Run(tc.name, func(t *testing.T) {
401-
count, err := getVolCount(baseDir, tc.portal, tc.iqn)
425+
count, err := getVolCount(tc.baseDir, tc.portal, tc.iqn)
402426
if err != nil {
403427
t.Errorf("expected no error, got %v", err)
404428
}
@@ -409,7 +433,7 @@ func TestGetVolCount(t *testing.T) {
409433
}
410434
}
411435

412-
func createFakePluginDir() (string, error) {
436+
func createFakePluginDirs() (string, error) {
413437
dir, err := ioutil.TempDir("", "refcounter")
414438
if err != nil {
415439
return "", err
@@ -419,6 +443,8 @@ func createFakePluginDir() (string, error) {
419443
"iface-127.0.0.1:3260:pv1/127.0.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-3",
420444
"iface-127.0.0.1:3260:pv2/127.0.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-2",
421445
"iface-127.0.0.1:3260:pv2/192.168.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-1",
446+
filepath.Join(config.DefaultKubeletVolumeDevicesDirName, "iface-127.0.0.1:3260/192.168.0.2:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-4"),
447+
filepath.Join(config.DefaultKubeletVolumeDevicesDirName, "iface-127.0.0.1:3260/192.168.0.3:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-5"),
422448
}
423449

424450
for _, d := range subdirs {

0 commit comments

Comments
 (0)