Remove ready directory which created in empty volumeMounter setUp func

Change-Id: I2384b07c7a044149e93e951a45f9f8a7bd9dba15

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 {

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 {