XL: allow meta bucket name appended with tmp meta prefix. (#2007)

object-api-putobject_test.go

@@ -18,7 +18,12 @@ package main
 
 import (
 	"bytes"
+	"crypto/md5"
+	"encoding/hex"
 	"fmt"
+	"io/ioutil"
+	"os"
+	"path"
 	"testing"
 )
 
@@ -229,3 +234,114 @@ func testObjectAPIPutObjectDiskNotFOund(obj ObjectLayer, instanceType string, di
 		}
 	}
 }
+
+// Wrapper for calling PutObject tests for both XL multiple disks and single node setup.
+func TestObjectAPIPutObjectStaleFiles(t *testing.T) {
+	ExecObjectLayerStaleFilesTest(t, testObjectAPIPutObjectStaleFiles)
+}
+
+// Tests validate correctness of PutObject.
+func testObjectAPIPutObjectStaleFiles(obj ObjectLayer, instanceType string, disks []string, t *testing.T) {
+	// Generating cases for which the PutObject fails.
+	bucket := "minio-bucket"
+	object := "minio-object"
+
+	// Create bucket.
+	err := obj.MakeBucket(bucket)
+	if err != nil {
+		// Failed to create newbucket, abort.
+		t.Fatalf("%s : %s", instanceType, err.Error())
+	}
+
+	data := []byte("hello, world")
+	// Create object.
+	_, err = obj.PutObject(bucket, object, int64(len(data)), bytes.NewReader(data), nil)
+	if err != nil {
+		// Failed to create object, abort.
+		t.Fatalf("%s : %s", instanceType, err.Error())
+	}
+
+	for _, disk := range disks {
+		tmpMetaDir := path.Join(disk, minioMetaBucket, tmpMetaPrefix)
+		if !isDirEmpty(tmpMetaDir) {
+			t.Fatalf("%s: expected: empty, got: non-empty", tmpMetaDir)
+		}
+	}
+}
+
+// Wrapper for calling Multipart PutObject tests for both XL multiple disks and single node setup.
+func TestObjectAPIMultipartPutObjectStaleFiles(t *testing.T) {
+	ExecObjectLayerStaleFilesTest(t, testObjectAPIMultipartPutObjectStaleFiles)
+}
+
+// Tests validate correctness of PutObject.
+func testObjectAPIMultipartPutObjectStaleFiles(obj ObjectLayer, instanceType string, disks []string, t *testing.T) {
+	// Generating cases for which the PutObject fails.
+	bucket := "minio-bucket"
+	object := "minio-object"
+
+	// Create bucket.
+	err := obj.MakeBucket(bucket)
+	if err != nil {
+		// Failed to create newbucket, abort.
+		t.Fatalf("%s : %s", instanceType, err.Error())
+	}
+
+	// Initiate Multipart Upload on the above created bucket.
+	uploadID, err := obj.NewMultipartUpload(bucket, object, nil)
+	if err != nil {
+		// Failed to create NewMultipartUpload, abort.
+		t.Fatalf("%s : %s", instanceType, err.Error())
+	}
+
+	// Upload part1.
+	fiveMBBytes := bytes.Repeat([]byte("a"), 5*1024*1024)
+	md5Writer := md5.New()
+	md5Writer.Write(fiveMBBytes)
+	etag1 := hex.EncodeToString(md5Writer.Sum(nil))
+	_, err = obj.PutObjectPart(bucket, object, uploadID, 1, int64(len(fiveMBBytes)), bytes.NewReader(fiveMBBytes), etag1)
+	if err != nil {
+		// Failed to upload object part, abort.
+		t.Fatalf("%s : %s", instanceType, err.Error())
+	}
+
+	// Upload part2.
+	data := []byte("hello, world")
+	md5Writer = md5.New()
+	md5Writer.Write(data)
+	etag2 := hex.EncodeToString(md5Writer.Sum(nil))
+	_, err = obj.PutObjectPart(bucket, object, uploadID, 2, int64(len(data)), bytes.NewReader(data), etag2)
+	if err != nil {
+		// Failed to upload object part, abort.
+		t.Fatalf("%s : %s", instanceType, err.Error())
+	}
+
+	// Complete multipart.
+	parts := []completePart{
+		{ETag: etag1, PartNumber: 1},
+		{ETag: etag2, PartNumber: 2},
+	}
+	_, err = obj.CompleteMultipartUpload(bucket, object, uploadID, parts)
+	if err != nil {
+		// Failed to complete multipart upload, abort.
+		t.Fatalf("%s : %s", instanceType, err.Error())
+	}
+
+	for _, disk := range disks {
+		tmpMetaDir := path.Join(disk, minioMetaBucket, tmpMetaPrefix)
+		files, err := ioutil.ReadDir(tmpMetaDir)
+		if err != nil {
+			// Its OK to have non-existen tmpMetaDir.
+			if os.IsNotExist(err) {
+				continue
+			}
+
+			// Print the error
+			t.Errorf("%s", err)
+		}
+
+		if len(files) != 0 {
+			t.Fatalf("%s: expected: empty, got: non-empty. content: %s", tmpMetaDir, files)
+		}
+	}
+}

posix-utils_nix.go

@@ -18,19 +18,12 @@
 
 package main
 
-import (
-	"os"
-	"strings"
-)
+import "os"
 
 // isValidVolname verifies a volname name in accordance with object
 // layer requirements.
 func isValidVolname(volname string) bool {
-	if len(volname) < 3 || len(volname) > 63 {
-		return false
-	}
-	// Volname shouldn't have '/' in it.
-	return !strings.ContainsAny(volname, "/")
+	return !(len(volname) < 3 || len(volname) > 63)
 }
 
 // mkdirAll creates a directory named path,

posix-utils_test.go

@@ -56,7 +56,7 @@ func TestIsValidVolname(t *testing.T) {
 		{"/", false},
 		{"a", false},
 		{"ab", false},
-		{"ab/", false},
+		{"ab/", true},
 		{"......", true},
 		{"lalalallalallalalalallalallalala-theString-size-is-greater-than-64", false},
 	}

posix-utils_windows.go

@@ -33,7 +33,7 @@ func isValidVolname(volname string) bool {
 		return false
 	}
 	// Volname shouldn't have reserved characters on windows in it.
-	return !strings.ContainsAny(volname, "/\\:*?\"<>|")
+	return !strings.ContainsAny(volname, `\:*?\"<>|`)
 }
 
 // mkdirAll creates a directory named path,

test-utils_test.go

@@ -592,3 +592,18 @@ func ExecObjectLayerDiskNotFoundTest(t *testing.T, objTest objTestDiskNotFoundTy
 	objTest(objLayer, xLTestStr, fsDirs, t)
 	defer removeRoots(fsDirs)
 }
+
+// Special object test type for stale files situations.
+type objTestStaleFilesType func(obj ObjectLayer, instanceType string, dirs []string, t *testing.T)
+
+// ExecObjectLayerStaleFilesTest - executes object layer tests those leaves stale
+// files/directories under .minio/tmp.  Creates XL ObjectLayer instance and runs test for XL layer.
+func ExecObjectLayerStaleFilesTest(t *testing.T, objTest objTestStaleFilesType) {
+	objLayer, fsDirs, err := getXLObjectLayer()
+	if err != nil {
+		t.Fatalf("Initialization of object layer failed for XL setup: %s", err.Error())
+	}
+	// Executing the object layer tests for XL.
+	objTest(objLayer, xLTestStr, fsDirs, t)
+	defer removeRoots(fsDirs)
+}

xl-v1-object.go

@@ -305,7 +305,8 @@ func (xl xlObjects) PutObject(bucket string, object string, size int64, data io.
 
 	uniqueID := getUUID()
 	tempErasureObj := path.Join(tmpMetaPrefix, uniqueID, "part.1")
-	tempObj := path.Join(tmpMetaPrefix, uniqueID)
+	minioMetaTmpBucket := path.Join(minioMetaBucket, tmpMetaPrefix)
+	tempObj := uniqueID
 
 	// Initialize xl meta.
 	xlMeta := newXLMetaV1(xl.dataBlocks, xl.parityBlocks)
@@ -372,7 +373,7 @@ func (xl xlObjects) PutObject(bucket string, object string, size int64, data io.
 	if md5Hex != "" {
 		if newMD5Hex != md5Hex {
 			// MD5 mismatch, delete the temporary object.
-			xl.deleteObject(minioMetaBucket, tempObj)
+			xl.deleteObject(minioMetaTmpBucket, tempObj)
 			// Returns md5 mismatch.
 			return "", BadDigest{md5Hex, newMD5Hex}
 		}
@@ -387,7 +388,7 @@ func (xl xlObjects) PutObject(bucket string, object string, size int64, data io.
 	// Rename if an object already exists to temporary location.
 	newUniqueID := getUUID()
 	if xl.isObject(bucket, object) {
-		err = xl.renameObject(bucket, object, minioMetaBucket, path.Join(tmpMetaPrefix, newUniqueID))
+		err = xl.renameObject(bucket, object, minioMetaTmpBucket, newUniqueID)
 		if err != nil {
 			return "", toObjectErr(err, bucket, object)
 		}
@@ -408,18 +409,18 @@ func (xl xlObjects) PutObject(bucket string, object string, size int64, data io.
 	}
 
 	// Write unique `xl.json` for each disk.
-	if err = xl.writeUniqueXLMetadata(minioMetaBucket, tempObj, partsMetadata); err != nil {
+	if err = xl.writeUniqueXLMetadata(minioMetaTmpBucket, tempObj, partsMetadata); err != nil {
 		return "", toObjectErr(err, bucket, object)
 	}
 
 	// Rename the successfully written temporary object to final location.
-	err = xl.renameObject(minioMetaBucket, tempObj, bucket, object)
+	err = xl.renameObject(minioMetaTmpBucket, tempObj, bucket, object)
 	if err != nil {
 		return "", toObjectErr(err, bucket, object)
 	}
 
 	// Delete the temporary object.
-	xl.deleteObject(minioMetaBucket, path.Join(tmpMetaPrefix, newUniqueID))
+	xl.deleteObject(minioMetaTmpBucket, newUniqueID)
 
 	// Return md5sum, successfully wrote object.
 	return newMD5Hex, nil