From 612fb1793eff62e9926f4aa656b83daccc311731 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Fri, 4 Oct 2019 13:59:44 +0200 Subject: [PATCH 1/3] Test global block directory in reconstruction tests --- test/e2e/storage/testsuites/disruptive.go | 14 ++++++------ test/e2e/storage/utils/utils.go | 26 +++++++++++++++++------ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/test/e2e/storage/testsuites/disruptive.go b/test/e2e/storage/testsuites/disruptive.go index 7201944d8bb..540628401ea 100644 --- a/test/e2e/storage/testsuites/disruptive.go +++ b/test/e2e/storage/testsuites/disruptive.go @@ -140,8 +140,9 @@ func (s *disruptiveTestSuite) defineTests(driver TestDriver, pattern testpattern } for _, test := range disruptiveTestTable { - if test.runTestFile != nil { - func(t disruptiveTest) { + func(t disruptiveTest) { + if (pattern.VolMode == v1.PersistentVolumeBlock && t.runTestBlock != nil) || + (pattern.VolMode == v1.PersistentVolumeFilesystem && t.runTestFile != nil) { ginkgo.It(t.testItStmt, func() { init() defer cleanup() @@ -158,13 +159,14 @@ func (s *disruptiveTestSuite) defineTests(driver TestDriver, pattern testpattern l.pod, err = e2epod.CreateSecPodWithNodeSelection(l.cs, l.ns.Name, pvcs, inlineSources, false, "", false, false, e2epv.SELinuxLabel, nil, e2epod.NodeSelection{Name: l.config.ClientNodeName}, framework.PodStartTimeout) framework.ExpectNoError(err, "While creating pods for kubelet restart test") - if pattern.VolMode == v1.PersistentVolumeBlock { + if pattern.VolMode == v1.PersistentVolumeBlock && t.runTestBlock != nil { t.runTestBlock(l.cs, l.config.Framework, l.pod) - } else { + } + if pattern.VolMode == v1.PersistentVolumeFilesystem && t.runTestFile != nil { t.runTestFile(l.cs, l.config.Framework, l.pod) } }) - }(test) - } + } + }(test) } } diff --git a/test/e2e/storage/utils/utils.go b/test/e2e/storage/utils/utils.go index 7239763b667..a02aafb45d5 100644 --- a/test/e2e/storage/utils/utils.go +++ b/test/e2e/storage/utils/utils.go @@ -318,18 +318,28 @@ func TestVolumeUnmapsFromDeletedPodWithForceOption(c clientset.Interface, f *fra nodeIP = nodeIP + ":22" // Creating command to check whether path exists - command := fmt.Sprintf("ls /var/lib/kubelet/pods/%s/volumeDevices/*/ | grep '.'", clientPod.UID) + podDirectoryCmd := fmt.Sprintf("ls /var/lib/kubelet/pods/%s/volumeDevices/*/ | grep '.'", clientPod.UID) if isSudoPresent(nodeIP, framework.TestContext.Provider) { - command = fmt.Sprintf("sudo sh -c \"%s\"", command) + podDirectoryCmd = fmt.Sprintf("sudo sh -c \"%s\"", podDirectoryCmd) + } + // Directories in the global directory have unpredictable names, however, device symlinks + // have the same name as pod.UID. So just find anything with pod.UID name. + globalBlockDirectoryCmd := fmt.Sprintf("find /var/lib/kubelet/plugins -name %s", clientPod.UID) + if isSudoPresent(nodeIP, framework.TestContext.Provider) { + globalBlockDirectoryCmd = fmt.Sprintf("sudo sh -c \"%s\"", globalBlockDirectoryCmd) } ginkgo.By("Expecting the symlinks from PodDeviceMapPath to be found.") - result, err := e2essh.SSH(command, nodeIP, framework.TestContext.Provider) + result, err := e2essh.SSH(podDirectoryCmd, nodeIP, framework.TestContext.Provider) e2essh.LogResult(result) framework.ExpectNoError(err, "Encountered SSH error.") framework.ExpectEqual(result.Code, 0, fmt.Sprintf("Expected grep exit code of 0, got %d", result.Code)) - // TODO: Needs to check GetGlobalMapPath and descriptor lock, as well. + ginkgo.By("Expecting the symlinks from global map path to be found.") + result, err = e2essh.SSH(globalBlockDirectoryCmd, nodeIP, framework.TestContext.Provider) + e2essh.LogResult(result) + framework.ExpectNoError(err, "Encountered SSH error.") + framework.ExpectEqual(result.Code, 0, fmt.Sprintf("Expected find exit code of 0, got %d", result.Code)) // This command is to make sure kubelet is started after test finishes no matter it fails or not. defer func() { @@ -358,12 +368,16 @@ func TestVolumeUnmapsFromDeletedPodWithForceOption(c clientset.Interface, f *fra } ginkgo.By("Expecting the symlink from PodDeviceMapPath not to be found.") - result, err = e2essh.SSH(command, nodeIP, framework.TestContext.Provider) + result, err = e2essh.SSH(podDirectoryCmd, nodeIP, framework.TestContext.Provider) e2essh.LogResult(result) framework.ExpectNoError(err, "Encountered SSH error.") gomega.Expect(result.Stdout).To(gomega.BeEmpty(), "Expected grep stdout to be empty.") - // TODO: Needs to check GetGlobalMapPath and descriptor lock, as well. + ginkgo.By("Expecting the symlinks from global map path not to be found.") + result, err = e2essh.SSH(globalBlockDirectoryCmd, nodeIP, framework.TestContext.Provider) + e2essh.LogResult(result) + framework.ExpectNoError(err, "Encountered SSH error.") + gomega.Expect(result.Stdout).To(gomega.BeEmpty(), "Expected find stdout to be empty.") framework.Logf("Volume unmaped on node %s", clientPod.Spec.NodeName) } From 44a006e0d0308d91421ac91797f54356f58b664d Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Fri, 4 Oct 2019 14:03:35 +0200 Subject: [PATCH 2/3] Fix volume map path during reconstruction --- pkg/kubelet/volumemanager/reconciler/reconciler.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler.go b/pkg/kubelet/volumemanager/reconciler/reconciler.go index 4199dd6b527..1e346ab79cb 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconciler.go +++ b/pkg/kubelet/volumemanager/reconciler/reconciler.go @@ -24,6 +24,7 @@ import ( "io/ioutil" "os" "path" + "path/filepath" "time" v1 "k8s.io/api/core/v1" @@ -518,7 +519,8 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume, pod.UID, newMapperErr) } - checkPath, _ = volumeMapper.GetPodDeviceMapPath() + mapDir, linkName := volumeMapper.GetPodDeviceMapPath() + checkPath = filepath.Join(mapDir, linkName) } else { var err error volumeMounter, err = plugin.NewMounter( From 1c8c86100975725ae169f81ff3cec413d3466102 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Fri, 4 Oct 2019 14:03:35 +0200 Subject: [PATCH 3/3] Reconstruct block PV name in all volume plugins The PV name is often necessary to build correct symlink paths /var/lib/kubelet/pods/{podUid}}/{DefaultKubeletVolumeDevicesDirName}/{escapeQualifiedPluginName}/{PV name} --- pkg/volume/awsebs/aws_ebs_block.go | 8 ++++++-- pkg/volume/awsebs/aws_ebs_block_test.go | 7 +++++-- pkg/volume/cinder/cinder_block.go | 10 +++++++--- pkg/volume/cinder/cinder_block_test.go | 9 ++++++--- pkg/volume/gcepd/gce_pd_block.go | 8 ++++++-- pkg/volume/gcepd/gce_pd_block_test.go | 7 +++++-- pkg/volume/vsphere_volume/vsphere_volume_block.go | 10 +++++++--- pkg/volume/vsphere_volume/vsphere_volume_block_test.go | 9 ++++++--- 8 files changed, 48 insertions(+), 20 deletions(-) diff --git a/pkg/volume/awsebs/aws_ebs_block.go b/pkg/volume/awsebs/aws_ebs_block.go index a991474a91f..9511393c5a5 100644 --- a/pkg/volume/awsebs/aws_ebs_block.go +++ b/pkg/volume/awsebs/aws_ebs_block.go @@ -25,6 +25,7 @@ import ( "strings" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/klog" "k8s.io/kubernetes/pkg/util/mount" @@ -51,10 +52,10 @@ func (plugin *awsElasticBlockStorePlugin) ConstructBlockVolumeSpec(podUID types. return nil, fmt.Errorf("failed to get volume plugin information from globalMapPathUUID: %v", globalMapPathUUID) } - return getVolumeSpecFromGlobalMapPath(globalMapPath) + return getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath) } -func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error) { +func getVolumeSpecFromGlobalMapPath(volumeName string, globalMapPath string) (*volume.Spec, error) { // Get volume spec information from globalMapPath // globalMapPath example: // plugins/kubernetes.io/{PluginName}/{DefaultKubeletVolumeDevicesDirName}/{volumeID} @@ -68,6 +69,9 @@ func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error) } block := v1.PersistentVolumeBlock awsVolume := &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: volumeName, + }, Spec: v1.PersistentVolumeSpec{ PersistentVolumeSource: v1.PersistentVolumeSource{ AWSElasticBlockStore: &v1.AWSElasticBlockStoreVolumeSource{ diff --git a/pkg/volume/awsebs/aws_ebs_block_test.go b/pkg/volume/awsebs/aws_ebs_block_test.go index 05ed2fa7ea6..f2a49456b78 100644 --- a/pkg/volume/awsebs/aws_ebs_block_test.go +++ b/pkg/volume/awsebs/aws_ebs_block_test.go @@ -52,16 +52,19 @@ func TestGetVolumeSpecFromGlobalMapPath(t *testing.T) { expectedGlobalPath := filepath.Join(tmpVDir, testGlobalPath) //Bad Path - badspec, err := getVolumeSpecFromGlobalMapPath("") + badspec, err := getVolumeSpecFromGlobalMapPath("", "") if badspec != nil || err == nil { t.Fatalf("Expected not to get spec from GlobalMapPath but did") } // Good Path - spec, err := getVolumeSpecFromGlobalMapPath(expectedGlobalPath) + spec, err := getVolumeSpecFromGlobalMapPath("myVolume", expectedGlobalPath) if spec == nil || err != nil { t.Fatalf("Failed to get spec from GlobalMapPath: %v", err) } + if spec.PersistentVolume.Name != "myVolume" { + t.Errorf("Invalid PV name from GlobalMapPath spec: %s", spec.PersistentVolume.Name) + } if spec.PersistentVolume.Spec.AWSElasticBlockStore.VolumeID != testVolName { t.Errorf("Invalid volumeID from GlobalMapPath spec: %s", spec.PersistentVolume.Spec.AWSElasticBlockStore.VolumeID) } diff --git a/pkg/volume/cinder/cinder_block.go b/pkg/volume/cinder/cinder_block.go index c03310aa7a4..483170ef284 100644 --- a/pkg/volume/cinder/cinder_block.go +++ b/pkg/volume/cinder/cinder_block.go @@ -22,7 +22,8 @@ import ( "fmt" "path/filepath" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/klog" "k8s.io/kubernetes/pkg/util/mount" @@ -53,10 +54,10 @@ func (plugin *cinderPlugin) ConstructBlockVolumeSpec(podUID types.UID, volumeNam return nil, fmt.Errorf("failed to get volume plugin information from globalMapPathUUID: %v", globalMapPathUUID) } - return getVolumeSpecFromGlobalMapPath(globalMapPath) + return getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath) } -func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error) { +func getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath string) (*volume.Spec, error) { // Get volume spec information from globalMapPath // globalMapPath example: // plugins/kubernetes.io/{PluginName}/{DefaultKubeletVolumeDevicesDirName}/{volumeID} @@ -67,6 +68,9 @@ func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error) } block := v1.PersistentVolumeBlock cinderVolume := &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: volumeName, + }, Spec: v1.PersistentVolumeSpec{ PersistentVolumeSource: v1.PersistentVolumeSource{ Cinder: &v1.CinderPersistentVolumeSource{ diff --git a/pkg/volume/cinder/cinder_block_test.go b/pkg/volume/cinder/cinder_block_test.go index 3f9dbe6b999..a28d64c0d12 100644 --- a/pkg/volume/cinder/cinder_block_test.go +++ b/pkg/volume/cinder/cinder_block_test.go @@ -23,7 +23,7 @@ import ( "path/filepath" "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" utiltesting "k8s.io/client-go/util/testing" @@ -52,16 +52,19 @@ func TestGetVolumeSpecFromGlobalMapPath(t *testing.T) { expectedGlobalPath := filepath.Join(tmpVDir, testGlobalPath) //Bad Path - badspec, err := getVolumeSpecFromGlobalMapPath("") + badspec, err := getVolumeSpecFromGlobalMapPath("", "") if badspec != nil || err == nil { t.Errorf("Expected not to get spec from GlobalMapPath but did") } // Good Path - spec, err := getVolumeSpecFromGlobalMapPath(expectedGlobalPath) + spec, err := getVolumeSpecFromGlobalMapPath("myVolume", expectedGlobalPath) if spec == nil || err != nil { t.Fatalf("Failed to get spec from GlobalMapPath: %v", err) } + if spec.PersistentVolume.Name != "myVolume" { + t.Errorf("Invalid PV name from GlobalMapPath spec: %s", spec.PersistentVolume.Name) + } if spec.PersistentVolume.Spec.Cinder.VolumeID != testVolName { t.Errorf("Invalid volumeID from GlobalMapPath spec: %s", spec.PersistentVolume.Spec.Cinder.VolumeID) } diff --git a/pkg/volume/gcepd/gce_pd_block.go b/pkg/volume/gcepd/gce_pd_block.go index 671f5d7178d..59f4e1c821b 100644 --- a/pkg/volume/gcepd/gce_pd_block.go +++ b/pkg/volume/gcepd/gce_pd_block.go @@ -24,6 +24,7 @@ import ( "strconv" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/klog" "k8s.io/kubernetes/pkg/util/mount" @@ -54,10 +55,10 @@ func (plugin *gcePersistentDiskPlugin) ConstructBlockVolumeSpec(podUID types.UID return nil, fmt.Errorf("failed to get volume plugin information from globalMapPathUUID: %v", globalMapPathUUID) } - return getVolumeSpecFromGlobalMapPath(globalMapPath) + return getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath) } -func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error) { +func getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath string) (*volume.Spec, error) { // Get volume spec information from globalMapPath // globalMapPath example: // plugins/kubernetes.io/{PluginName}/{DefaultKubeletVolumeDevicesDirName}/{volumeID} @@ -68,6 +69,9 @@ func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error) } block := v1.PersistentVolumeBlock gceVolume := &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: volumeName, + }, Spec: v1.PersistentVolumeSpec{ PersistentVolumeSource: v1.PersistentVolumeSource{ GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ diff --git a/pkg/volume/gcepd/gce_pd_block_test.go b/pkg/volume/gcepd/gce_pd_block_test.go index 6f992fbe075..6338a8a4156 100644 --- a/pkg/volume/gcepd/gce_pd_block_test.go +++ b/pkg/volume/gcepd/gce_pd_block_test.go @@ -52,16 +52,19 @@ func TestGetVolumeSpecFromGlobalMapPath(t *testing.T) { expectedGlobalPath := filepath.Join(tmpVDir, testGlobalPath) //Bad Path - badspec, err := getVolumeSpecFromGlobalMapPath("") + badspec, err := getVolumeSpecFromGlobalMapPath("", "") if badspec != nil || err == nil { t.Errorf("Expected not to get spec from GlobalMapPath but did") } // Good Path - spec, err := getVolumeSpecFromGlobalMapPath(expectedGlobalPath) + spec, err := getVolumeSpecFromGlobalMapPath("myVolume", expectedGlobalPath) if spec == nil || err != nil { t.Fatalf("Failed to get spec from GlobalMapPath: %v", err) } + if spec.PersistentVolume.Name != "myVolume" { + t.Errorf("Invalid PV name from GlobalMapPath spec: %s", spec.PersistentVolume.Name) + } if spec.PersistentVolume.Spec.GCEPersistentDisk.PDName != testPdName { t.Errorf("Invalid pdName from GlobalMapPath spec: %s", spec.PersistentVolume.Spec.GCEPersistentDisk.PDName) } diff --git a/pkg/volume/vsphere_volume/vsphere_volume_block.go b/pkg/volume/vsphere_volume/vsphere_volume_block.go index b660cd3cd4d..8f1ffd3b3cf 100644 --- a/pkg/volume/vsphere_volume/vsphere_volume_block.go +++ b/pkg/volume/vsphere_volume/vsphere_volume_block.go @@ -23,7 +23,8 @@ import ( "path/filepath" "strings" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/klog" "k8s.io/kubernetes/pkg/util/mount" @@ -49,10 +50,10 @@ func (plugin *vsphereVolumePlugin) ConstructBlockVolumeSpec(podUID types.UID, vo if len(globalMapPath) <= 1 { return nil, fmt.Errorf("failed to get volume plugin information from globalMapPathUUID: %v", globalMapPathUUID) } - return getVolumeSpecFromGlobalMapPath(globalMapPath) + return getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath) } -func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error) { +func getVolumeSpecFromGlobalMapPath(volumeName, globalMapPath string) (*volume.Spec, error) { // Construct volume spec from globalMapPath // globalMapPath example: // plugins/kubernetes.io/{PluginName}/{DefaultKubeletVolumeDevicesDirName}/{volumeID} @@ -64,6 +65,9 @@ func getVolumeSpecFromGlobalMapPath(globalMapPath string) (*volume.Spec, error) } block := v1.PersistentVolumeBlock vsphereVolume := &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: volumeName, + }, Spec: v1.PersistentVolumeSpec{ PersistentVolumeSource: v1.PersistentVolumeSource{ VsphereVolume: &v1.VsphereVirtualDiskVolumeSource{ diff --git a/pkg/volume/vsphere_volume/vsphere_volume_block_test.go b/pkg/volume/vsphere_volume/vsphere_volume_block_test.go index 941202e84a8..3377b4c8b43 100644 --- a/pkg/volume/vsphere_volume/vsphere_volume_block_test.go +++ b/pkg/volume/vsphere_volume/vsphere_volume_block_test.go @@ -23,7 +23,7 @@ import ( "path/filepath" "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" utiltesting "k8s.io/client-go/util/testing" @@ -51,16 +51,19 @@ func TestGetVolumeSpecFromGlobalMapPath(t *testing.T) { expectedGlobalPath := filepath.Join(tmpVDir, testGlobalPath) // Bad Path - badspec, err := getVolumeSpecFromGlobalMapPath("") + badspec, err := getVolumeSpecFromGlobalMapPath("", "") if badspec != nil || err == nil { t.Errorf("Expected not to get spec from GlobalMapPath but did") } // Good Path - spec, err := getVolumeSpecFromGlobalMapPath(expectedGlobalPath) + spec, err := getVolumeSpecFromGlobalMapPath("myVolume", expectedGlobalPath) if spec == nil || err != nil { t.Fatalf("Failed to get spec from GlobalMapPath: %s", err) } + if spec.PersistentVolume.Name != "myVolume" { + t.Errorf("Invalid PV name from GlobalMapPath spec: %s", spec.PersistentVolume.Name) + } if spec.PersistentVolume.Spec.VsphereVolume.VolumePath != testVolumePath { t.Fatalf("Invalid volumePath from GlobalMapPath spec: %s", spec.PersistentVolume.Spec.VsphereVolume.VolumePath) }