From a4aa494cc1c99a0a9e58d135ff43eb9a965d4f96 Mon Sep 17 00:00:00 2001 From: Jing Xu Date: Wed, 21 Oct 2020 12:13:46 -0700 Subject: [PATCH] Remove ready directory which created in empty volumeMounter setUp func Change-Id: I2384b07c7a044149e93e951a45f9f8a7bd9dba15 --- pkg/volume/emptydir/empty_dir.go | 20 ++++- pkg/volume/emptydir/empty_dir_test.go | 112 +++++++++++++++++++------- 2 files changed, 99 insertions(+), 33 deletions(-) diff --git a/pkg/volume/emptydir/empty_dir.go b/pkg/volume/emptydir/empty_dir.go index d619765c875..af68575ee9a 100644 --- a/pkg/volume/emptydir/empty_dir.go +++ b/pkg/volume/emptydir/empty_dir.go @@ -208,11 +208,21 @@ func (ed *emptyDir) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { // storage medium is the default, then the volume is ready. If the // medium is memory, and a mountpoint is present, then the volume is // ready. - if volumeutil.IsReady(ed.getMetaDir()) { + readyDir := ed.getMetaDir() + if volumeutil.IsReady(readyDir) { if ed.medium == v1.StorageMediumMemory && !notMnt { return nil } else if ed.medium == v1.StorageMediumDefault { - return nil + // Further check dir exists + if _, err := os.Stat(dir); err == nil { + return nil + } + // This situation should not happen unless user manually delete volume dir. + // In this case, delete ready file and print a warning for it. + klog.Warningf("volume ready file dir %s exist, but volume dir %s does not. Remove ready dir", readyDir, dir) + if err := os.RemoveAll(readyDir); err != nil && !os.IsNotExist(err) { + klog.Warningf("failed to remove ready dir [%s]: %v", readyDir, err) + } } } @@ -421,6 +431,12 @@ func (ed *emptyDir) TearDown() error { // TearDownAt simply discards everything in the directory. func (ed *emptyDir) TearDownAt(dir string) error { + // First remove ready dir which created in SetUp func + readyDir := ed.getMetaDir() + if removeErr := os.RemoveAll(readyDir); removeErr != nil && !os.IsNotExist(removeErr) { + return fmt.Errorf("failed to remove ready dir [%s]: %v", readyDir, removeErr) + } + if pathExists, pathErr := mount.PathExists(dir); pathErr != nil { return fmt.Errorf("Error checking if path exists: %v", pathErr) } else if !pathExists { diff --git a/pkg/volume/emptydir/empty_dir_test.go b/pkg/volume/emptydir/empty_dir_test.go index a3dfd858931..978d11646d3 100644 --- a/pkg/volume/emptydir/empty_dir_test.go +++ b/pkg/volume/emptydir/empty_dir_test.go @@ -25,7 +25,7 @@ import ( "path/filepath" "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -36,6 +36,7 @@ import ( "k8s.io/kubernetes/pkg/volume" volumetest "k8s.io/kubernetes/pkg/volume/testing" "k8s.io/kubernetes/pkg/volume/util" + volumeutil "k8s.io/kubernetes/pkg/volume/util" "k8s.io/mount-utils" ) @@ -81,6 +82,26 @@ func (fake *fakeMountDetector) GetMountMedium(path string, requestedMedium v1.St func TestPluginEmptyRootContext(t *testing.T) { doTestPlugin(t, pluginTestConfig{ + volumeDirExists: true, + readyDirExists: true, + medium: v1.StorageMediumDefault, + expectedSetupMounts: 0, + expectedTeardownMounts: 0}) + doTestPlugin(t, pluginTestConfig{ + volumeDirExists: false, + readyDirExists: false, + medium: v1.StorageMediumDefault, + expectedSetupMounts: 0, + expectedTeardownMounts: 0}) + doTestPlugin(t, pluginTestConfig{ + volumeDirExists: true, + readyDirExists: false, + medium: v1.StorageMediumDefault, + expectedSetupMounts: 0, + expectedTeardownMounts: 0}) + doTestPlugin(t, pluginTestConfig{ + volumeDirExists: false, + readyDirExists: true, medium: v1.StorageMediumDefault, expectedSetupMounts: 0, expectedTeardownMounts: 0}) @@ -117,8 +138,11 @@ func TestPluginHugetlbfs(t *testing.T) { } type pluginTestConfig struct { - medium v1.StorageMedium - idempotent bool + medium v1.StorageMedium + //volumeDirExists indicates whether volumeDir already/still exists before volume setup/teardown + volumeDirExists bool + //readyDirExists indicates whether readyDir already/still exists before volume setup/teardown + readyDirExists bool expectedSetupMounts int shouldBeMountedBeforeTeardown bool expectedTeardownMounts int @@ -163,7 +187,7 @@ func doTestPlugin(t *testing.T, config pluginTestConfig) { } ) - if config.idempotent { + if config.readyDirExists { physicalMounter.MountPoints = []mount.MountPoint{ { Path: volumePath, @@ -188,29 +212,14 @@ func doTestPlugin(t *testing.T, config pluginTestConfig) { if volPath != volumePath { t.Errorf("Got unexpected path: %s", volPath) } - - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { - t.Errorf("Expected success, got: %v", err) + if config.volumeDirExists { + if err := os.MkdirAll(volPath, perm); err != nil { + t.Errorf("fail to create path: %s", volPath) + } } // Stat the directory and check the permission bits - fileinfo, err := os.Stat(volPath) - if !config.idempotent { - if err != nil { - if os.IsNotExist(err) { - t.Errorf("SetUp() failed, volume path not created: %s", volPath) - } else { - t.Errorf("SetUp() failed: %v", err) - } - } - if e, a := perm, fileinfo.Mode().Perm(); e != a { - t.Errorf("Unexpected file mode for %v: expected: %v, got: %v", volPath, e, a) - } - } else if err == nil { - // If this test is for idempotency and we were able - // to stat the volume path, it's an error. - t.Errorf("Volume directory was created unexpectedly") - } + testSetUp(mounter, metadataDir, volPath) log := physicalMounter.GetLog() // Check the number of mounts performed during setup @@ -236,14 +245,19 @@ func doTestPlugin(t *testing.T, config pluginTestConfig) { t.Errorf("Got a nil Unmounter") } - // Tear down the volume - if err := unmounter.TearDown(); err != nil { - t.Errorf("Expected success, got: %v", err) + if !config.readyDirExists { + if err := os.RemoveAll(metadataDir); err != nil && !os.IsNotExist(err) { + t.Errorf("failed to remove ready dir [%s]: %v", metadataDir, err) + } } - if _, err := os.Stat(volPath); err == nil { - t.Errorf("TearDown() failed, volume path still exists: %s", volPath) - } else if !os.IsNotExist(err) { - t.Errorf("TearDown() failed: %v", err) + if !config.volumeDirExists { + if err := os.RemoveAll(volPath); err != nil && !os.IsNotExist(err) { + t.Errorf("failed to remove ready dir [%s]: %v", metadataDir, err) + } + } + // Tear down the volume + if err := testTearDown(unmounter, metadataDir, volPath); err != nil { + t.Errorf("Test failed with error %v", err) } log = physicalMounter.GetLog() @@ -256,6 +270,42 @@ func doTestPlugin(t *testing.T, config pluginTestConfig) { physicalMounter.ResetLog() } +func testSetUp(mounter volume.Mounter, metadataDir, volPath string) error { + if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + return fmt.Errorf("Expected success, got: %v", err) + } + // Stat the directory and check the permission bits + if !volumeutil.IsReady(metadataDir) { + return fmt.Errorf("SetUp() failed, ready file is not created") + } + fileinfo, err := os.Stat(volPath) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("SetUp() failed, volume path not created: %s", volPath) + } + return fmt.Errorf("SetUp() failed: %v", err) + } + if e, a := perm, fileinfo.Mode().Perm(); e != a { + return fmt.Errorf("Unexpected file mode for %v: expected: %v, got: %v", volPath, e, a) + } + return nil +} + +func testTearDown(unmounter volume.Unmounter, metadataDir, volPath string) error { + if err := unmounter.TearDown(); err != nil { + return err + } + if volumeutil.IsReady(metadataDir) { + return fmt.Errorf("Teardown() failed, ready file still exists") + } + if _, err := os.Stat(volPath); err == nil { + return fmt.Errorf("TearDown() failed, volume path still exists: %s", volPath) + } else if !os.IsNotExist(err) { + return fmt.Errorf("TearDown() failed: %v", err) + } + return nil +} + func TestPluginBackCompat(t *testing.T) { basePath, err := utiltesting.MkTmpdir("emptydirTest") if err != nil {