Adds StorageDriverFactory, unifying creation of StorageDrivers

Custom storage drivers can register a factory to create the driver by
name, similar to the database/sql package's Register and Open
factory.Create returns an in-process driver if registered or an IPC
driver if one can be found, erroring otherwise
This standardizes parameter passing for creation of storage drivers

Also adds documentation for storagedriver package and children

Author: BrianBland

main/storagedriver/filesystem/filesystem.go

@@ -8,19 +8,14 @@ import (
 	"github.com/docker/docker-registry/storagedriver/ipc"
 )
 
+// An out-of-process filesystem driver, intended to be run by ipc.NewDriverClient
 func main() {
 	parametersBytes := []byte(os.Args[1])
-	var parameters map[string]interface{}
+	var parameters map[string]string
 	err := json.Unmarshal(parametersBytes, &parameters)
 	if err != nil {
 		panic(err)
 	}
-	rootDirectory := "/tmp/registry"
-	if parameters != nil {
-		rootDirParam, ok := parameters["RootDirectory"].(string)
-		if ok && rootDirParam != "" {
-			rootDirectory = rootDirParam
-		}
-	}
-	ipc.Server(filesystem.NewDriver(rootDirectory))
+
+	ipc.StorageDriverServer(filesystem.FromParameters(parameters))
 }

main/storagedriver/inmemory/inmemory.go

@@ -5,6 +5,8 @@ import (
 	"github.com/docker/docker-registry/storagedriver/ipc"
 )
 
+// An out-of-process inmemory driver, intended to be run by ipc.NewDriverClient
+// This exists primarily for example and testing purposes
 func main() {
-	ipc.Server(inmemory.NewDriver())
+	ipc.StorageDriverServer(inmemory.New())
 }

main/storagedriver/s3/s3.go

@@ -3,55 +3,24 @@ package main
 import (
 	"encoding/json"
 	"os"
-	"strconv"
 
-	"github.com/crowdmob/goamz/aws"
 	"github.com/docker/docker-registry/storagedriver/ipc"
 	"github.com/docker/docker-registry/storagedriver/s3"
 )
 
+// An out-of-process S3 driver, intended to be run by ipc.NewDriverClient
 func main() {
 	parametersBytes := []byte(os.Args[1])
-	var parameters map[string]interface{}
+	var parameters map[string]string
 	err := json.Unmarshal(parametersBytes, &parameters)
 	if err != nil {
 		panic(err)
 	}
 
-	accessKey, ok := parameters["accessKey"].(string)
-	if !ok || accessKey == "" {
-		panic("No accessKey parameter")
-	}
-
-	secretKey, ok := parameters["secretKey"].(string)
-	if !ok || secretKey == "" {
-		panic("No secretKey parameter")
-	}
-
-	region, ok := parameters["region"].(string)
-	if !ok || region == "" {
-		panic("No region parameter")
-	}
-
-	bucket, ok := parameters["bucket"].(string)
-	if !ok || bucket == "" {
-		panic("No bucket parameter")
-	}
-
-	encrypt, ok := parameters["encrypt"].(string)
-	if !ok {
-		panic("No encrypt parameter")
-	}
-
-	encryptBool, err := strconv.ParseBool(encrypt)
-	if err != nil {
-		panic(err)
-	}
-
-	driver, err := s3.NewDriver(accessKey, secretKey, aws.GetRegion(region), encryptBool, bucket)
+	driver, err := s3.FromParameters(parameters)
 	if err != nil {
 		panic(err)
 	}
 
-	ipc.Server(driver)
+	ipc.StorageDriverServer(driver)
 }

storagedriver/factory/factory.go

@@ -0,0 +1,64 @@
+package factory
+
+import (
+	"fmt"
+
+	"github.com/docker/docker-registry/storagedriver"
+	"github.com/docker/docker-registry/storagedriver/ipc"
+)
+
+// Internal mapping between storage driver names and their respective factories
+var driverFactories = make(map[string]StorageDriverFactory)
+
+// Factory interface for the storagedriver.StorageDriver interface
+// Storage drivers should call Register() with a factory to make the driver available by name
+type StorageDriverFactory interface {
+	// Creates and returns a new storagedriver.StorageDriver with the given parameters
+	// Parameters will vary by driver and may be ignored
+	// Each parameter key must only consist of lowercase letters and numbers
+	Create(parameters map[string]string) (storagedriver.StorageDriver, error)
+}
+
+// Register makes a storage driver available by the provided name.
+// If Register is called twice with the same name or if driver factory is nil, it panics.
+func Register(name string, factory StorageDriverFactory) {
+	if factory == nil {
+		panic("Must not provide nil StorageDriverFactory")
+	}
+	_, registered := driverFactories[name]
+	if registered {
+		panic(fmt.Sprintf("StorageDriverFactory named %s already registered", name))
+	}
+
+	driverFactories[name] = factory
+}
+
+// Create a new storagedriver.StorageDriver with the given name and parameters
+// To run in-process, the StorageDriverFactory must first be registered with the given name
+// If no in-process drivers are found with the given name, this attempts to create an IPC driver
+// If no in-process or external drivers are found, an InvalidStorageDriverError is returned
+func Create(name string, parameters map[string]string) (storagedriver.StorageDriver, error) {
+	driverFactory, ok := driverFactories[name]
+	if !ok {
+		// No registered StorageDriverFactory found, try ipc
+		driverClient, err := ipc.NewDriverClient(name, parameters)
+		if err != nil {
+			return nil, InvalidStorageDriverError{name}
+		}
+		err = driverClient.Start()
+		if err != nil {
+			return nil, err
+		}
+		return driverClient, nil
+	}
+	return driverFactory.Create(parameters)
+}
+
+// Error returned when attempting to construct an unregistered storage driver
+type InvalidStorageDriverError struct {
+	Name string
+}
+
+func (err InvalidStorageDriverError) Error() string {
+	return fmt.Sprintf("StorageDriver not registered: %s", err.Name)
+}

storagedriver/filesystem/filesystem.go

@@ -8,20 +8,54 @@ import (
 	"strings"
 
 	"github.com/docker/docker-registry/storagedriver"
+	"github.com/docker/docker-registry/storagedriver/factory"
 )
 
+const DriverName = "filesystem"
+const DefaultRootDirectory = "/tmp/registry/storage"
+
+func init() {
+	factory.Register(DriverName, &filesystemDriverFactory{})
+}
+
+// Implements the factory.StorageDriverFactory interface
+type filesystemDriverFactory struct{}
+
+func (factory *filesystemDriverFactory) Create(parameters map[string]string) (storagedriver.StorageDriver, error) {
+	return FromParameters(parameters), nil
+}
+
+// Storage Driver backed by a local filesystem
+// All provided paths will be subpaths of the RootDirectory
 type FilesystemDriver struct {
 	rootDirectory string
 }
 
-func NewDriver(rootDirectory string) *FilesystemDriver {
+// Constructs a new FilesystemDriver with a given parameters map
+// Optional Parameters:
+// - rootdirectory
+func FromParameters(parameters map[string]string) *FilesystemDriver {
+	var rootDirectory = DefaultRootDirectory
+	if parameters != nil {
+		rootDir, ok := parameters["rootdirectory"]
+		if ok {
+			rootDirectory = rootDir
+		}
+	}
+	return New(rootDirectory)
+}
+
+// Constructs a new FilesystemDriver with a given rootDirectory
+func New(rootDirectory string) *FilesystemDriver {
 	return &FilesystemDriver{rootDirectory}
 }
 
 func (d *FilesystemDriver) subPath(subPath string) string {
 	return path.Join(d.rootDirectory, subPath)
 }
 
+// Implement the storagedriver.StorageDriver interface
+
 func (d *FilesystemDriver) GetContent(path string) ([]byte, error) {
 	contents, err := ioutil.ReadFile(d.subPath(path))
 	if err != nil {

storagedriver/filesystem/filesystem_test.go

@@ -17,8 +17,8 @@ func init() {
 	os.RemoveAll(rootDirectory)
 
 	filesystemDriverConstructor := func() (storagedriver.StorageDriver, error) {
-		return NewDriver(rootDirectory), nil
+		return New(rootDirectory), nil
 	}
 	testsuites.RegisterInProcessSuite(filesystemDriverConstructor, testsuites.NeverSkip)
-	testsuites.RegisterIPCSuite("filesystem", map[string]string{"RootDirectory": rootDirectory}, testsuites.NeverSkip)
+	testsuites.RegisterIPCSuite(DriverName, map[string]string{"rootdirectory": rootDirectory}, testsuites.NeverSkip)
 }

storagedriver/inmemory/inmemory.go

@@ -10,14 +10,31 @@ import (
 	"sync"
 
 	"github.com/docker/docker-registry/storagedriver"
+	"github.com/docker/docker-registry/storagedriver/factory"
 )
 
+const DriverName = "inmemory"
+
+func init() {
+	factory.Register(DriverName, &inMemoryDriverFactory{})
+}
+
+// Implements the factory.StorageDriverFactory interface
+type inMemoryDriverFactory struct{}
+
+func (factory *inMemoryDriverFactory) Create(parameters map[string]string) (storagedriver.StorageDriver, error) {
+	return New(), nil
+}
+
+// InMemory Storage Driver backed by a map
+// Intended solely for example and testing purposes
 type InMemoryDriver struct {
 	storage map[string][]byte
 	mutex   sync.RWMutex
 }
 
-func NewDriver() *InMemoryDriver {
+// Constructs a new InMemoryDriver
+func New() *InMemoryDriver {
 	return &InMemoryDriver{storage: make(map[string][]byte)}
 }
 

storagedriver/inmemory/inmemory_test.go

@@ -13,8 +13,8 @@ func Test(t *testing.T) { TestingT(t) }
 
 func init() {
 	inmemoryDriverConstructor := func() (storagedriver.StorageDriver, error) {
-		return NewDriver(), nil
+		return New(), nil
 	}
 	testsuites.RegisterInProcessSuite(inmemoryDriverConstructor, testsuites.NeverSkip)
-	testsuites.RegisterIPCSuite("inmemory", nil, testsuites.NeverSkip)
+	testsuites.RegisterIPCSuite(DriverName, nil, testsuites.NeverSkip)
 }

storagedriver/ipc/client.go

@@ -15,13 +15,21 @@ import (
 	"github.com/docker/libchan/spdy"
 )
 
+// Storage Driver implementation using a managed child process communicating over IPC
 type StorageDriverClient struct {
 	subprocess *exec.Cmd
 	socket     *os.File
 	transport  *spdy.Transport
 	sender     libchan.Sender
 }
 
+// Constructs a new out-of-process storage driver using the driver name and configuration parameters
+// Must call Start() on this driver client before remote method calls can be made
+//
+// Looks for drivers in the following locations in order:
+// - Storage drivers directory (to be determined, yet not implemented)
+// - $GOPATH/bin
+// - $PATH
 func NewDriverClient(name string, parameters map[string]string) (*StorageDriverClient, error) {
 	paramsBytes, err := json.Marshal(parameters)
 	if err != nil {
@@ -46,6 +54,7 @@ func NewDriverClient(name string, parameters map[string]string) (*StorageDriverC
 	}, nil
 }
 
+// Starts the designated child process storage driver and binds a socket to this process for IPC
 func (driver *StorageDriverClient) Start() error {
 	fileDescriptors, err := syscall.Socketpair(syscall.AF_LOCAL, syscall.SOCK_STREAM, 0)
 	if err != nil {
@@ -93,6 +102,8 @@ func (driver *StorageDriverClient) Start() error {
 	return nil
 }
 
+// Stops the child process storage driver
+// storagedriver.StorageDriver methods called after Stop() will fail
 func (driver *StorageDriverClient) Stop() error {
 	closeSenderErr := driver.sender.Close()
 	closeTransportErr := driver.transport.Close()
@@ -109,6 +120,8 @@ func (driver *StorageDriverClient) Stop() error {
 	return killErr
 }
 
+// Implement the storagedriver.StorageDriver interface over IPC
+
 func (driver *StorageDriverClient) GetContent(path string) ([]byte, error) {
 	receiver, remoteSender := libchan.Pipe()
 

storagedriver/ipc/ipc.go

@@ -10,12 +10,16 @@ import (
 	"github.com/docker/libchan"
 )
 
+// Defines a remote method call request
+// A return value struct is to be sent over the ResponseChannel
 type Request struct {
 	Type            string
 	Parameters      map[string]interface{}
 	ResponseChannel libchan.Sender
 }
 
+// A simple wrapper around an io.ReadCloser that implements the io.ReadWriteCloser interface
+// Writes are disallowed and will return an error if ever called
 type noWriteReadWriteCloser struct {
 	io.ReadCloser
 }
@@ -24,6 +28,8 @@ func (r noWriteReadWriteCloser) Write(p []byte) (n int, err error) {
 	return 0, errors.New("Write unsupported")
 }
 
+// Wraps an io.Reader as an io.ReadWriteCloser with a nop Close and unsupported Write method
+// Has no effect when an io.ReadWriteCloser is passed in
 func WrapReader(reader io.Reader) io.ReadWriteCloser {
 	if readWriteCloser, ok := reader.(io.ReadWriteCloser); ok {
 		return readWriteCloser
@@ -39,6 +45,7 @@ type responseError struct {
 	Message string
 }
 
+// Wraps an error in a serializable struct containing the error's type and message
 func ResponseError(err error) *responseError {
 	if err == nil {
 		return nil
@@ -53,29 +60,37 @@ func (err *responseError) Error() string {
 	return fmt.Sprintf("%s: %s", err.Type, err.Message)
 }
 
+// IPC method call response object definitions
+
+// Response for a ReadStream request
 type ReadStreamResponse struct {
 	Reader io.ReadWriteCloser
 	Error  *responseError
 }
 
+// Response for a WriteStream request
 type WriteStreamResponse struct {
 	Error *responseError
 }
 
+// Response for a ResumeWritePosition request
 type ResumeWritePositionResponse struct {
 	Position uint64
 	Error    *responseError
 }
 
+// Response for a List request
 type ListResponse struct {
 	Keys  []string
 	Error *responseError
 }
 
+// Response for a Move request
 type MoveResponse struct {
 	Error *responseError
 }
 
+// Response for a Delete request
 type DeleteResponse struct {
 	Error *responseError
 }