Remove more bloated code - simplify

pkg/storage/donut/bucket.go

@@ -41,12 +41,12 @@ type bucket struct {
 	acl       string
 	time      time.Time
 	donutName string
-	nodes     map[string]Node
+	nodes     map[string]node
 	lock      *sync.RWMutex
 }
 
 // newBucket - instantiate a new bucket
-func newBucket(bucketName, aclType, donutName string, nodes map[string]Node) (bucket, map[string]string, error) {
+func newBucket(bucketName, aclType, donutName string, nodes map[string]node) (bucket, map[string]string, error) {
 	errParams := map[string]string{
 		"bucketName": bucketName,
 		"donutName":  donutName,

pkg/storage/donut/donut.go

@@ -28,14 +28,13 @@ import (
 	"sync"
 
 	"github.com/minio/minio/pkg/iodine"
-	"github.com/minio/minio/pkg/storage/donut/disk"
 )
 
 // donut struct internal data
 type donut struct {
 	name    string
 	buckets map[string]bucket
-	nodes   map[string]Node
+	nodes   map[string]node
 	lock    *sync.RWMutex
 }
 
@@ -54,28 +53,10 @@ const (
 	donutObjectMetadataVersion = "1.0"
 )
 
-// attachDonutNode - wrapper function to instantiate a new node for associated donut
+// attachDonutNode - wrapper function to instantiate a new node for associatedt donut
 // based on the provided configuration
-func (d donut) attachDonutNode(hostname string, disks []string) error {
+func (dt donut) attachDonutNode(hostname string, disks []string) error {
-	node, err := NewNode(hostname)
+	if err := dt.AttachNode(hostname, disks); err != nil {
-	if err != nil {
-		return iodine.New(err, nil)
-	}
-	donutName := d.name
-	for i, d := range disks {
-		// Order is necessary for maps, keep order number separately
-		newDisk, err := disk.New(d)
-		if err != nil {
-			return iodine.New(err, nil)
-		}
-		if err := newDisk.MakeDir(donutName); err != nil {
-			return iodine.New(err, nil)
-		}
-		if err := node.AttachDisk(newDisk, i); err != nil {
-			return iodine.New(err, nil)
-		}
-	}
-	if err := d.AttachNode(node); err != nil {
 		return iodine.New(err, nil)
 	}
 	return nil
@@ -86,7 +67,7 @@ func NewDonut(donutName string, nodeDiskMap map[string][]string) (Donut, error)
 	if donutName == "" || len(nodeDiskMap) == 0 {
 		return nil, iodine.New(InvalidArgument{}, nil)
 	}
-	nodes := make(map[string]Node)
+	nodes := make(map[string]node)
 	buckets := make(map[string]bucket)
 	d := donut{
 		name:    donutName,
@@ -107,27 +88,26 @@ func NewDonut(donutName string, nodeDiskMap map[string][]string) (Donut, error)
 }
 
 // MakeBucket - make a new bucket
-func (d donut) MakeBucket(bucket, acl string) error {
+func (dt donut) MakeBucket(bucket, acl string) error {
-	d.lock.Lock()
+	dt.lock.Lock()
-	defer d.lock.Unlock()
+	defer dt.lock.Unlock()
 	if bucket == "" || strings.TrimSpace(bucket) == "" {
 		return iodine.New(InvalidArgument{}, nil)
 	}
-	return d.makeDonutBucket(bucket, acl)
+	return dt.makeDonutBucket(bucket, acl)
 }
 
 // GetBucketMetadata - get bucket metadata
-func (d donut) GetBucketMetadata(bucket string) (map[string]string, error) {
+func (dt donut) GetBucketMetadata(bucket string) (map[string]string, error) {
-	d.lock.RLock()
+	dt.lock.RLock()
-	defer d.lock.RUnlock()
+	defer dt.lock.RUnlock()
-	err := d.getDonutBuckets()
+	if err := dt.getDonutBuckets(); err != nil {
-	if err != nil {
 		return nil, iodine.New(err, nil)
 	}
-	if _, ok := d.buckets[bucket]; !ok {
+	if _, ok := dt.buckets[bucket]; !ok {
 		return nil, iodine.New(BucketNotFound{Bucket: bucket}, nil)
 	}
-	metadata, err := d.getDonutBucketMetadata()
+	metadata, err := dt.getDonutBucketMetadata()
 	if err != nil {
 		return nil, iodine.New(err, nil)
 	}
@@ -135,14 +115,13 @@ func (d donut) GetBucketMetadata(bucket string) (map[string]string, error) {
 }
 
 // SetBucketMetadata - set bucket metadata
-func (d donut) SetBucketMetadata(bucket string, bucketMetadata map[string]string) error {
+func (dt donut) SetBucketMetadata(bucket string, bucketMetadata map[string]string) error {
-	d.lock.Lock()
+	dt.lock.Lock()
-	defer d.lock.Unlock()
+	defer dt.lock.Unlock()
-	err := d.getDonutBuckets()
+	if err := dt.getDonutBuckets(); err != nil {
-	if err != nil {
 		return iodine.New(err, nil)
 	}
-	metadata, err := d.getDonutBucketMetadata()
+	metadata, err := dt.getDonutBucketMetadata()
 	if err != nil {
 		return iodine.New(err, nil)
 	}
@@ -150,32 +129,30 @@ func (d donut) SetBucketMetadata(bucket string, bucketMetadata map[string]string
 	// TODO ignore rest of the keys for now, only mutable data is "acl"
 	oldBucketMetadata["acl"] = bucketMetadata["acl"]
 	metadata[bucket] = oldBucketMetadata
-	return d.setDonutBucketMetadata(metadata)
+	return dt.setDonutBucketMetadata(metadata)
 }
 
 // ListBuckets - return list of buckets
-func (d donut) ListBuckets() (metadata map[string]map[string]string, err error) {
+func (dt donut) ListBuckets() (metadata map[string]map[string]string, err error) {
-	d.lock.RLock()
+	dt.lock.RLock()
-	defer d.lock.RUnlock()
+	defer dt.lock.RUnlock()
-	err = d.getDonutBuckets()
+	if err := dt.getDonutBuckets(); err != nil {
-	if err != nil {
 		return nil, iodine.New(err, nil)
 	}
-	dummyMetadata := make(map[string]map[string]string)
+	metadata, err = dt.getDonutBucketMetadata()
-	metadata, err = d.getDonutBucketMetadata()
 	if err != nil {
 		// intentionally left out the error when Donut is empty
 		// but we need to revisit this area in future - since we need
 		// to figure out between acceptable and unacceptable errors
-		return dummyMetadata, nil
+		return make(map[string]map[string]string), nil
 	}
 	return metadata, nil
 }
 
 // ListObjects - return list of objects
-func (d donut) ListObjects(bucket, prefix, marker, delimiter string, maxkeys int) ([]string, []string, bool, error) {
+func (dt donut) ListObjects(bucket, prefix, marker, delimiter string, maxkeys int) ([]string, []string, bool, error) {
-	d.lock.RLock()
+	dt.lock.RLock()
-	defer d.lock.RUnlock()
+	defer dt.lock.RUnlock()
 	errParams := map[string]string{
 		"bucket":    bucket,
 		"prefix":    prefix,
@@ -183,14 +160,13 @@ func (d donut) ListObjects(bucket, prefix, marker, delimiter string, maxkeys int
 		"delimiter": delimiter,
 		"maxkeys":   strconv.Itoa(maxkeys),
 	}
-	err := d.getDonutBuckets()
+	if err := dt.getDonutBuckets(); err != nil {
-	if err != nil {
 		return nil, nil, false, iodine.New(err, errParams)
 	}
-	if _, ok := d.buckets[bucket]; !ok {
+	if _, ok := dt.buckets[bucket]; !ok {
 		return nil, nil, false, iodine.New(BucketNotFound{Bucket: bucket}, errParams)
 	}
-	objectList, err := d.buckets[bucket].ListObjects()
+	objectList, err := dt.buckets[bucket].ListObjects()
 	if err != nil {
 		return nil, nil, false, iodine.New(err, errParams)
 	}
@@ -249,9 +225,9 @@ func (d donut) ListObjects(bucket, prefix, marker, delimiter string, maxkeys int
 }
 
 // PutObject - put object
-func (d donut) PutObject(bucket, object, expectedMD5Sum string, reader io.ReadCloser, metadata map[string]string) (string, error) {
+func (dt donut) PutObject(bucket, object, expectedMD5Sum string, reader io.ReadCloser, metadata map[string]string) (string, error) {
-	d.lock.Lock()
+	dt.lock.Lock()
-	defer d.lock.Unlock()
+	defer dt.lock.Unlock()
 	errParams := map[string]string{
 		"bucket": bucket,
 		"object": object,
@@ -262,14 +238,13 @@ func (d donut) PutObject(bucket, object, expectedMD5Sum string, reader io.ReadCl
 	if object == "" || strings.TrimSpace(object) == "" {
 		return "", iodine.New(InvalidArgument{}, errParams)
 	}
-	err := d.getDonutBuckets()
+	if err := dt.getDonutBuckets(); err != nil {
-	if err != nil {
 		return "", iodine.New(err, errParams)
 	}
-	if _, ok := d.buckets[bucket]; !ok {
+	if _, ok := dt.buckets[bucket]; !ok {
 		return "", iodine.New(BucketNotFound{Bucket: bucket}, nil)
 	}
-	objectList, err := d.buckets[bucket].ListObjects()
+	objectList, err := dt.buckets[bucket].ListObjects()
 	if err != nil {
 		return "", iodine.New(err, nil)
 	}
@@ -278,7 +253,7 @@ func (d donut) PutObject(bucket, object, expectedMD5Sum string, reader io.ReadCl
 			return "", iodine.New(ObjectExists{Object: object}, nil)
 		}
 	}
-	md5sum, err := d.buckets[bucket].WriteObject(object, reader, expectedMD5Sum, metadata)
+	md5sum, err := dt.buckets[bucket].WriteObject(object, reader, expectedMD5Sum, metadata)
 	if err != nil {
 		return "", iodine.New(err, errParams)
 	}
@@ -286,9 +261,9 @@ func (d donut) PutObject(bucket, object, expectedMD5Sum string, reader io.ReadCl
 }
 
 // GetObject - get object
-func (d donut) GetObject(bucket, object string) (reader io.ReadCloser, size int64, err error) {
+func (dt donut) GetObject(bucket, object string) (reader io.ReadCloser, size int64, err error) {
-	d.lock.RLock()
+	dt.lock.RLock()
-	defer d.lock.RUnlock()
+	defer dt.lock.RUnlock()
 	errParams := map[string]string{
 		"bucket": bucket,
 		"object": object,
@@ -299,32 +274,30 @@ func (d donut) GetObject(bucket, object string) (reader io.ReadCloser, size int6
 	if object == "" || strings.TrimSpace(object) == "" {
 		return nil, 0, iodine.New(InvalidArgument{}, errParams)
 	}
-	err = d.getDonutBuckets()
+	if err := dt.getDonutBuckets(); err != nil {
-	if err != nil {
 		return nil, 0, iodine.New(err, nil)
 	}
-	if _, ok := d.buckets[bucket]; !ok {
+	if _, ok := dt.buckets[bucket]; !ok {
 		return nil, 0, iodine.New(BucketNotFound{Bucket: bucket}, errParams)
 	}
-	return d.buckets[bucket].ReadObject(object)
+	return dt.buckets[bucket].ReadObject(object)
 }
 
 // GetObjectMetadata - get object metadata
-func (d donut) GetObjectMetadata(bucket, object string) (map[string]string, error) {
+func (dt donut) GetObjectMetadata(bucket, object string) (map[string]string, error) {
-	d.lock.RLock()
+	dt.lock.RLock()
-	defer d.lock.RUnlock()
+	defer dt.lock.RUnlock()
 	errParams := map[string]string{
 		"bucket": bucket,
 		"object": object,
 	}
-	err := d.getDonutBuckets()
+	if err := dt.getDonutBuckets(); err != nil {
-	if err != nil {
 		return nil, iodine.New(err, errParams)
 	}
-	if _, ok := d.buckets[bucket]; !ok {
+	if _, ok := dt.buckets[bucket]; !ok {
 		return nil, iodine.New(BucketNotFound{Bucket: bucket}, errParams)
 	}
-	objectList, err := d.buckets[bucket].ListObjects()
+	objectList, err := dt.buckets[bucket].ListObjects()
 	if err != nil {
 		return nil, iodine.New(err, errParams)
 	}
@@ -336,16 +309,16 @@ func (d donut) GetObjectMetadata(bucket, object string) (map[string]string, erro
 }
 
 // getDiskWriters -
-func (d donut) getBucketMetadataWriters() ([]io.WriteCloser, error) {
+func (dt donut) getBucketMetadataWriters() ([]io.WriteCloser, error) {
 	var writers []io.WriteCloser
-	for _, node := range d.nodes {
+	for _, node := range dt.nodes {
 		disks, err := node.ListDisks()
 		if err != nil {
 			return nil, iodine.New(err, nil)
 		}
 		writers = make([]io.WriteCloser, len(disks))
-		for order, disk := range disks {
+		for order, d := range disks {
-			bucketMetaDataWriter, err := disk.CreateFile(filepath.Join(d.name, bucketMetadataConfig))
+			bucketMetaDataWriter, err := d.CreateFile(filepath.Join(dt.name, bucketMetadataConfig))
 			if err != nil {
 				return nil, iodine.New(err, nil)
 			}
@@ -355,16 +328,16 @@ func (d donut) getBucketMetadataWriters() ([]io.WriteCloser, error) {
 	return writers, nil
 }
 
-func (d donut) getBucketMetadataReaders() ([]io.ReadCloser, error) {
+func (dt donut) getBucketMetadataReaders() ([]io.ReadCloser, error) {
 	var readers []io.ReadCloser
-	for _, node := range d.nodes {
+	for _, node := range dt.nodes {
 		disks, err := node.ListDisks()
 		if err != nil {
 			return nil, iodine.New(err, nil)
 		}
 		readers = make([]io.ReadCloser, len(disks))
-		for order, disk := range disks {
+		for order, d := range disks {
-			bucketMetaDataReader, err := disk.OpenFile(filepath.Join(d.name, bucketMetadataConfig))
+			bucketMetaDataReader, err := d.OpenFile(filepath.Join(dt.name, bucketMetadataConfig))
 			if err != nil {
 				return nil, iodine.New(err, nil)
 			}
@@ -375,8 +348,8 @@ func (d donut) getBucketMetadataReaders() ([]io.ReadCloser, error) {
 }
 
 //
-func (d donut) setDonutBucketMetadata(metadata map[string]map[string]string) error {
+func (dt donut) setDonutBucketMetadata(metadata map[string]map[string]string) error {
-	writers, err := d.getBucketMetadataWriters()
+	writers, err := dt.getBucketMetadataWriters()
 	if err != nil {
 		return iodine.New(err, nil)
 	}
@@ -392,9 +365,9 @@ func (d donut) setDonutBucketMetadata(metadata map[string]map[string]string) err
 	return nil
 }
 
-func (d donut) getDonutBucketMetadata() (map[string]map[string]string, error) {
+func (dt donut) getDonutBucketMetadata() (map[string]map[string]string, error) {
 	metadata := make(map[string]map[string]string)
-	readers, err := d.getBucketMetadataReaders()
+	readers, err := dt.getBucketMetadataReaders()
 	if err != nil {
 		return nil, iodine.New(err, nil)
 	}
@@ -410,41 +383,40 @@ func (d donut) getDonutBucketMetadata() (map[string]map[string]string, error) {
 	return metadata, nil
 }
 
-func (d donut) makeDonutBucket(bucketName, acl string) error {
+func (dt donut) makeDonutBucket(bucketName, acl string) error {
-	err := d.getDonutBuckets()
+	if err := dt.getDonutBuckets(); err != nil {
-	if err != nil {
 		return iodine.New(err, nil)
 	}
-	if _, ok := d.buckets[bucketName]; ok {
+	if _, ok := dt.buckets[bucketName]; ok {
 		return iodine.New(BucketExists{Bucket: bucketName}, nil)
 	}
-	bucket, bucketMetadata, err := newBucket(bucketName, acl, d.name, d.nodes)
+	bucket, bucketMetadata, err := newBucket(bucketName, acl, dt.name, dt.nodes)
 	if err != nil {
 		return iodine.New(err, nil)
 	}
 	nodeNumber := 0
-	d.buckets[bucketName] = bucket
+	dt.buckets[bucketName] = bucket
-	for _, node := range d.nodes {
+	for _, node := range dt.nodes {
 		disks, err := node.ListDisks()
 		if err != nil {
 			return iodine.New(err, nil)
 		}
 		for order, disk := range disks {
 			bucketSlice := fmt.Sprintf("%s$%d$%d", bucketName, nodeNumber, order)
-			err := disk.MakeDir(filepath.Join(d.name, bucketSlice))
+			err := disk.MakeDir(filepath.Join(dt.name, bucketSlice))
 			if err != nil {
 				return iodine.New(err, nil)
 			}
 		}
 		nodeNumber = nodeNumber + 1
 	}
-	metadata, err := d.getDonutBucketMetadata()
+	metadata, err := dt.getDonutBucketMetadata()
 	if err != nil {
 		err = iodine.ToError(err)
 		if os.IsNotExist(err) {
 			metadata := make(map[string]map[string]string)
 			metadata[bucketName] = bucketMetadata
-			err = d.setDonutBucketMetadata(metadata)
+			err = dt.setDonutBucketMetadata(metadata)
 			if err != nil {
 				return iodine.New(err, nil)
 			}
@@ -453,21 +425,21 @@ func (d donut) makeDonutBucket(bucketName, acl string) error {
 		return iodine.New(err, nil)
 	}
 	metadata[bucketName] = bucketMetadata
-	err = d.setDonutBucketMetadata(metadata)
+	err = dt.setDonutBucketMetadata(metadata)
 	if err != nil {
 		return iodine.New(err, nil)
 	}
 	return nil
 }
 
-func (d donut) getDonutBuckets() error {
+func (dt donut) getDonutBuckets() error {
-	for _, node := range d.nodes {
+	for _, node := range dt.nodes {
 		disks, err := node.ListDisks()
 		if err != nil {
 			return iodine.New(err, nil)
 		}
 		for _, disk := range disks {
-			dirs, err := disk.ListDir(d.name)
+			dirs, err := disk.ListDir(dt.name)
 			if err != nil {
 				return iodine.New(err, nil)
 			}
@@ -477,12 +449,12 @@ func (d donut) getDonutBuckets() error {
 					return iodine.New(CorruptedBackend{Backend: dir.Name()}, nil)
 				}
 				bucketName := splitDir[0]
-				// we dont need this NewBucket once we cache from makeDonutBucket()
+				// we dont need this once we cache from makeDonutBucket()
-				bucket, _, err := newBucket(bucketName, "private", d.name, d.nodes)
+				bucket, _, err := newBucket(bucketName, "private", dt.name, dt.nodes)
 				if err != nil {
 					return iodine.New(err, nil)
 				}
-				d.buckets[bucketName] = bucket
+				dt.buckets[bucketName] = bucket
 			}
 		}
 	}

pkg/storage/donut/interfaces.go

@@ -16,11 +16,7 @@
 
 package donut
 
-import (
+import "io"
-	"io"
-
-	"github.com/minio/minio/pkg/storage/donut/disk"
-)
 
 // Collection of Donut specification interfaces
 
@@ -53,20 +49,9 @@ type Management interface {
 	Rebalance() error
 	Info() (map[string][]string, error)
 
-	AttachNode(node Node) error
+	AttachNode(hostname string, disks []string) error
-	DetachNode(node Node) error
+	DetachNode(hostname string) error
 
 	SaveConfig() error
 	LoadConfig() error
 }
-
-// Node interface for node management
-type Node interface {
-	ListDisks() (map[int]disk.Disk, error)
-	AttachDisk(disk disk.Disk, diskOrder int) error
-	DetachDisk(diskOrder int) error
-
-	GetNodeName() string
-	SaveConfig() error
-	LoadConfig() error
-}

pkg/storage/donut/management.go

@@ -1,3 +1,19 @@
+/*
+ * Minimalist Object Storage, (C) 2015 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
 package donut
 
 import (
@@ -5,17 +21,18 @@ import (
 	"path/filepath"
 
 	"github.com/minio/minio/pkg/iodine"
+	"github.com/minio/minio/pkg/storage/donut/disk"
 )
 
 // Heal - heal a donut and fix bad data blocks
-func (d donut) Heal() error {
+func (dt donut) Heal() error {
 	return iodine.New(NotImplemented{Function: "Heal"}, nil)
 }
 
 // Info - return info about donut configuration
-func (d donut) Info() (nodeDiskMap map[string][]string, err error) {
+func (dt donut) Info() (nodeDiskMap map[string][]string, err error) {
 	nodeDiskMap = make(map[string][]string)
-	for nodeName, node := range d.nodes {
+	for nodeName, node := range dt.nodes {
 		disks, err := node.ListDisks()
 		if err != nil {
 			return nil, iodine.New(err, nil)
@@ -30,30 +47,46 @@ func (d donut) Info() (nodeDiskMap map[string][]string, err error) {
 }
 
 // AttachNode - attach node
-func (d donut) AttachNode(node Node) error {
+func (dt donut) AttachNode(hostname string, disks []string) error {
-	if node == nil {
+	if hostname == "" || len(disks) == 0 {
 		return iodine.New(InvalidArgument{}, nil)
 	}
-	d.nodes[node.GetNodeName()] = node
+	node, err := newNode(hostname)
+	if err != nil {
+		return iodine.New(err, nil)
+	}
+	dt.nodes[hostname] = node
+	for i, d := range disks {
+		newDisk, err := disk.New(d)
+		if err != nil {
+			return iodine.New(err, nil)
+		}
+		if err := newDisk.MakeDir(dt.name); err != nil {
+			return iodine.New(err, nil)
+		}
+		if err := node.AttachDisk(newDisk, i); err != nil {
+			return iodine.New(err, nil)
+		}
+	}
 	return nil
 }
 
 // DetachNode - detach node
-func (d donut) DetachNode(node Node) error {
+func (dt donut) DetachNode(hostname string) error {
-	delete(d.nodes, node.GetNodeName())
+	delete(dt.nodes, hostname)
 	return nil
 }
 
 // SaveConfig - save donut configuration
-func (d donut) SaveConfig() error {
+func (dt donut) SaveConfig() error {
 	nodeDiskMap := make(map[string][]string)
-	for hostname, node := range d.nodes {
+	for hostname, node := range dt.nodes {
 		disks, err := node.ListDisks()
 		if err != nil {
 			return iodine.New(err, nil)
 		}
 		for order, disk := range disks {
-			donutConfigPath := filepath.Join(d.name, donutConfig)
+			donutConfigPath := filepath.Join(dt.name, donutConfig)
 			donutConfigWriter, err := disk.CreateFile(donutConfigPath)
 			defer donutConfigWriter.Close()
 			if err != nil {
@@ -70,6 +103,6 @@ func (d donut) SaveConfig() error {
 }
 
 // LoadConfig - load configuration
-func (d donut) LoadConfig() error {
+func (dt donut) LoadConfig() error {
 	return iodine.New(NotImplemented{Function: "LoadConfig"}, nil)
 }

pkg/storage/donut/node.go

@@ -27,10 +27,10 @@ type node struct {
 	disks    map[int]disk.Disk
 }
 
-// NewNode - instantiates a new node
+// newNode - instantiates a new node
-func NewNode(hostname string) (Node, error) {
+func newNode(hostname string) (node, error) {
 	if hostname == "" {
-		return nil, iodine.New(InvalidArgument{}, nil)
+		return node{}, iodine.New(InvalidArgument{}, nil)
 	}
 	disks := make(map[int]disk.Disk)
 	n := node{
@@ -40,8 +40,8 @@ func NewNode(hostname string) (Node, error) {
 	return n, nil
 }
 
-// GetNodeName - return hostname
+// GetHostname - return hostname
-func (n node) GetNodeName() string {
+func (n node) GetHostname() string {
 	return n.hostname
 }