From d44583d28ae4b4237ed2d94f0f2d614c1c8dddea Mon Sep 17 00:00:00 2001 From: Ryan Roemmich Date: Sun, 24 Nov 2019 22:31:46 -0800 Subject: [PATCH] Fix iscsi refcounter in the case of no Block iscsi volumes --- pkg/volume/iscsi/BUILD | 1 + pkg/volume/iscsi/iscsi_util.go | 6 +- pkg/volume/iscsi/iscsi_util_test.go | 102 +++++++++++++++++----------- 3 files changed, 70 insertions(+), 39 deletions(-) diff --git a/pkg/volume/iscsi/BUILD b/pkg/volume/iscsi/BUILD index 60d8dd0b0e1..c7ac6a79bf9 100644 --- a/pkg/volume/iscsi/BUILD +++ b/pkg/volume/iscsi/BUILD @@ -42,6 +42,7 @@ go_test( ], embed = [":go_default_library"], deps = [ + "//pkg/kubelet/config:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/testing:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", diff --git a/pkg/volume/iscsi/iscsi_util.go b/pkg/volume/iscsi/iscsi_util.go index f8742d9f716..9a4744d31ce 100644 --- a/pkg/volume/iscsi/iscsi_util.go +++ b/pkg/volume/iscsi/iscsi_util.go @@ -926,9 +926,13 @@ func isSessionBusy(host volume.VolumeHost, portal, iqn string) bool { // getVolCount returns the number of volumes in use by the kubelet. // It does so by counting the number of directories prefixed by the given portal and IQN. func getVolCount(dir, portal, iqn string) (int, error) { - // The topmost dirs are named after the ifaces, e.g., iface-default or iface-127.0.0.1:3260:pv0 + // For FileSystem volumes, the topmost dirs are named after the ifaces, e.g., iface-default or iface-127.0.0.1:3260:pv0. + // For Block volumes, the default topmost dir is volumeDevices. contents, err := ioutil.ReadDir(dir) if err != nil { + if os.IsNotExist(err) { + return 0, nil + } return 0, err } diff --git a/pkg/volume/iscsi/iscsi_util_test.go b/pkg/volume/iscsi/iscsi_util_test.go index b6c46713c83..69dad3f1622 100644 --- a/pkg/volume/iscsi/iscsi_util_test.go +++ b/pkg/volume/iscsi/iscsi_util_test.go @@ -23,8 +23,9 @@ import ( "reflect" "testing" - "k8s.io/utils/exec/testing" + testingexec "k8s.io/utils/exec/testing" + "k8s.io/kubernetes/pkg/kubelet/config" "k8s.io/kubernetes/pkg/volume" volumetest "k8s.io/kubernetes/pkg/volume/testing" ) @@ -350,55 +351,78 @@ func TestClonedIfaceUpdateError(t *testing.T) { } func TestGetVolCount(t *testing.T) { - testCases := []struct { - name string - portal string - iqn string - count int - }{ - { - name: "wrong portal, no volumes", - portal: "192.168.0.2:3260", // incorrect IP address - iqn: "iqn.2003-01.io.k8s:e2e.volume-1", - count: 0, - }, - { - name: "wrong iqn, no volumes", - portal: "127.0.0.1:3260", - iqn: "iqn.2003-01.io.k8s:e2e.volume-3", // incorrect volume - count: 0, - }, - { - name: "single volume", - portal: "192.168.0.1:3260", - iqn: "iqn.2003-01.io.k8s:e2e.volume-1", - count: 1, - }, - { - name: "two volumes", - portal: "127.0.0.1:3260", - iqn: "iqn.2003-01.io.k8s:e2e.volume-1", - count: 2, - }, - } - // This will create a dir structure like this: // /tmp/refcounter555814673 // ├── iface-127.0.0.1:3260:pv1 // │   └── 127.0.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-3 // └── iface-127.0.0.1:3260:pv2 - // ├── 127.0.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-2 - // └── 192.168.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-1 + // │ ├── 127.0.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-2 + // │ └── 192.168.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-1 + // └── volumeDevices + // └── 192.168.0.2:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-4 + // └── 192.168.0.3:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-5 - baseDir, err := createFakePluginDir() + baseDir, err := createFakePluginDirs() if err != nil { t.Errorf("error creating fake plugin dir: %v", err) } + defer os.RemoveAll(baseDir) + testCases := []struct { + name string + baseDir string + portal string + iqn string + count int + }{ + { + name: "wrong portal, no volumes", + baseDir: baseDir, + portal: "192.168.0.2:3260", // incorrect IP address + iqn: "iqn.2003-01.io.k8s:e2e.volume-1", + count: 0, + }, + { + name: "wrong iqn, no volumes", + baseDir: baseDir, + portal: "127.0.0.1:3260", + iqn: "iqn.2003-01.io.k8s:e2e.volume-3", // incorrect volume + count: 0, + }, + { + name: "single volume", + baseDir: baseDir, + portal: "192.168.0.1:3260", + iqn: "iqn.2003-01.io.k8s:e2e.volume-1", + count: 1, + }, + { + name: "two volumes", + baseDir: baseDir, + portal: "127.0.0.1:3260", + iqn: "iqn.2003-01.io.k8s:e2e.volume-1", + count: 2, + }, + { + name: "volumeDevices (block) volume", + baseDir: filepath.Join(baseDir, config.DefaultKubeletVolumeDevicesDirName), + portal: "192.168.0.2:3260", + iqn: "iqn.2003-01.io.k8s:e2e.volume-1-lun-4", + count: 1, + }, + { + name: "nonexistent path", + baseDir: filepath.Join(baseDir, "this_path_should_not_exist"), + portal: "127.0.0.1:3260", + iqn: "iqn.2003-01.io.k8s:e2e.unknown", + count: 0, + }, + } + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - count, err := getVolCount(baseDir, tc.portal, tc.iqn) + count, err := getVolCount(tc.baseDir, tc.portal, tc.iqn) if err != nil { t.Errorf("expected no error, got %v", err) } @@ -409,7 +433,7 @@ func TestGetVolCount(t *testing.T) { } } -func createFakePluginDir() (string, error) { +func createFakePluginDirs() (string, error) { dir, err := ioutil.TempDir("", "refcounter") if err != nil { return "", err @@ -419,6 +443,8 @@ func createFakePluginDir() (string, error) { "iface-127.0.0.1:3260:pv1/127.0.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-3", "iface-127.0.0.1:3260:pv2/127.0.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-2", "iface-127.0.0.1:3260:pv2/192.168.0.1:3260-iqn.2003-01.io.k8s:e2e.volume-1-lun-1", + 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"), + 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"), } for _, d := range subdirs {