Skip to content
This repository has been archived by the owner on Sep 12, 2018. It is now read-only.

Adds prototype NG driver apis #630

Closed
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions contrib/golang_impl_ng/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
FROM golang:1.3.3

WORKDIR /go/src/

RUN ["mkdir", "-p", "github.com/docker/"]

VOLUME ["/Users/bbland/code/docker/docker-registry:github.com/docker/docker-registry"]

# WORKDIR github.com/docker/docker-registry/contrib/golang_impl_ng

RUN ["go", "get", "-d", "-v", "./..."]
RUN ["go", "fmt", "./..."]
RUN ["go", "install", "./..."]
ENTRYPOINT ["/go/bin/driverclient", "-driver"]
CMD ["inmemory"]
53 changes: 53 additions & 0 deletions contrib/golang_impl_ng/driver/driver.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package driver
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small mini-rant, but I think there's potential in breaking this package out of the registry completely as a filesystem API package to contribute back to the Go community, similar to other projects in the pkg/ directory in docker/docker: https://github.com/docker/docker/tree/master/pkg


import (
"fmt"
"io"
)

type Driver interface {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Driver seems too generic. How about FileStore?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Driver is probably needlessly generic. I'm open to naming suggestions.

GetContent(path string) ([]byte, error)
PutContent(path string, content []byte) error
ReadStream(path string) (io.ReadCloser, error)
WriteStream(path string, offset uint64, readCloser io.ReadCloser) error
ResumeWritePosition(path string) (uint64, error)
Move(sourcePath string, destPath string) error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep consistency, should this be changed to MoveContent, or should GetContent/PutContent be changed to Get/Put?

Delete(path string) error
}

func ImageManifestPath(imageId string) string {
return fmt.Sprintf("images/%s/manifest.json", imageId)
}

func ImagePrivatePath(imageId string) string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should disappear entirely. Authorization is not up to the registry to decide, nor the driver.

return fmt.Sprintf("images/%s/_private", imageId)
}

func ImageDeletionPath(imageId string) string {
return fmt.Sprintf("images/%s/_deleted", imageId)
}

func ImageLayerPath(layerTarsum string) string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Layer, manifest paths etc: should be entirely opaque to the driver.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing, I'll remove all of this boilerplate.

return fmt.Sprintf("layers/%s/layer", layerTarsum)
}

func LayerChecksumPath(layerTarsum string) string {
return fmt.Sprintf("layers/%s/checksum", layerTarsum)
}

type PathNotFoundError struct {
Path string
}

func (err PathNotFoundError) Error() string {
return fmt.Sprintf("Path not found: %s", err.Path)
}

type InvalidOffsetError struct {
Path string
Offset uint64
}

func (err InvalidOffsetError) Error() string {
return fmt.Sprintf("Invalid offset: %d for path: %s", err.Offset, err.Path)
}
121 changes: 121 additions & 0 deletions contrib/golang_impl_ng/driver/filesystem/filesystem.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
package filesystem

import (
"github.com/docker/docker-registry/contrib/golang_impl_ng/driver"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imports outside of Go's stdlib should be listed below the stdlib imports

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be the default behavior of go fmt if you don't split the imports into two logical blocks. Cleaning this up now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've hit that too, though this seems to be the "proper" format. Upstream's bug, not ours! ;)

"io"
"io/ioutil"
"os"
"path"
)

type FilesystemDriver struct {
rootDirectory string
}

func NewDriver(rootDirectory string) *FilesystemDriver {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of previous filesystem driver configuration, there was a lot more than just the root directory. For example, in the S3 driver, you had to set up your access key/secret key to connect to the S3 bucket. For the swift driver, you needed to specify your tenant name, region name, your keystone API key, etc. How will this fit in with this implementation?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current intention is that each driver will have a main() method that handles (json) serialized parameters when run out-of-process over ipc. If you reference contrib/golang_impl_ng/main/driver/filesystem/filesystem.go, you can see that we're pulling the RootDirectory parameter out of the first command line argument, which is a json blob of all required configuration params.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Mon, Oct 13, 2014 at 04:43:12PM -0700, Matthew Fisher wrote:

In terms of previous filesystem driver configuration, there was a
lot more than just the root directory. For example, in the S3
driver, you had to set up your access key/secret key to connect to
the S3 bucket. For the swift driver, you needed to specify your
tenant name, region name, your keystone API key, etc. How will this
fit in with this implementation?

Detached libchan storage drivers can just read their own config files
or CLI arguments or whatever when they spin up. For same-process
drivers, I'm fine with either the current “just pass through the whole
config hash structure” or a similar approach using environment
variables.

return &FilesystemDriver{rootDirectory}
}

func (d *FilesystemDriver) subPath(subPath string) string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to add additional cruft, as it's just a one-liner function. There's no true difference between calling d.subPath() and path.Join(), IMO.

return path.Join(d.rootDirectory, subPath)
}

func (d *FilesystemDriver) GetContent(path string) ([]byte, error) {
contents, err := ioutil.ReadFile(d.subPath(path))
if err != nil {
return nil, driver.PathNotFoundError{path}
}
return contents, nil
}

func (d *FilesystemDriver) PutContent(subPath string, contents []byte) error {
fullPath := d.subPath(subPath)
parentDir := path.Dir(fullPath)
err := os.MkdirAll(parentDir, 0755)
if err != nil {
return err
}

err = ioutil.WriteFile(fullPath, contents, 0644)
return err
}

func (d *FilesystemDriver) ReadStream(path string) (io.ReadCloser, error) {
file, err := os.OpenFile(d.subPath(path), os.O_RDONLY, 0644)
if err != nil {
return nil, err
}
return file, nil
}

func (d *FilesystemDriver) WriteStream(subPath string, offset uint64, reader io.ReadCloser) error {
defer reader.Close()

resumableOffset, err := d.ResumeWritePosition(subPath)
if err != nil {
return err
}
if offset > resumableOffset {
return driver.InvalidOffsetError{subPath, offset}
}

fullPath := d.subPath(subPath)
parentDir := path.Dir(fullPath)
err = os.MkdirAll(parentDir, 0755)
if err != nil {
return err
}

file, err := os.Create(fullPath)
if err != nil {
return err
}
defer file.Close()

buf := make([]byte, 32*1024)
for {
bytesRead, er := reader.Read(buf)
if bytesRead > 0 {
bytesWritten, ew := file.WriteAt(buf[0:bytesRead], int64(offset))
if bytesWritten > 0 {
offset += uint64(bytesWritten)
}
if ew != nil {
err = ew
break
}
if bytesRead != bytesWritten {
err = io.ErrShortWrite
break
}
}
if er == io.EOF {
break
}
if er != nil {
err = er
break
}
}
return err
}

func (d *FilesystemDriver) ResumeWritePosition(subPath string) (uint64, error) {
fullPath := d.subPath(subPath)

fileInfo, err := os.Stat(fullPath)
if err != nil {
return 0, err
}
return uint64(fileInfo.Size()), nil
}

func (d *FilesystemDriver) Move(sourcePath string, destPath string) error {
err := os.Rename(d.subPath(sourcePath), d.subPath(destPath))
return err
}

func (d *FilesystemDriver) Delete(path string) error {
err := os.RemoveAll(d.subPath(path))
return err
}
30 changes: 30 additions & 0 deletions contrib/golang_impl_ng/driver/filesystem/filesystem_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package filesystem

import (
"github.com/docker/docker-registry/contrib/golang_impl_ng/driver"
"github.com/docker/docker-registry/contrib/golang_impl_ng/driver/ipc"
. "gopkg.in/check.v1"
"os"
"testing"
)

// Hook up gocheck into the "go test" runner.
func Test(t *testing.T) { TestingT(t) }

var rootDirectory = "/tmp/driver"
var _ = os.RemoveAll(rootDirectory)

var filesystemDriverConstructor = func() (driver.Driver, error) {
return NewDriver(rootDirectory), nil
}

var InProcessSuite = Suite(&driver.InProcessDriverSuite{
DriverConstructor: filesystemDriverConstructor,
})

var IPCSuite = Suite(&ipc.IPCDriverSuite{
TestDriverConfig: &ipc.TestDriverConfig{
"filesystem",
map[string]string{"RootDirectory": rootDirectory},
},
})
100 changes: 100 additions & 0 deletions contrib/golang_impl_ng/driver/inmemory/inmemory.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package inmemory

import (
"bytes"
"github.com/docker/docker-registry/contrib/golang_impl_ng/driver"
"io"
"io/ioutil"
"strings"
)

type InMemoryDriver struct {
storage map[string][]byte
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to ipc.Server driver is passed to many goroutines at the same time. So there's a concurrent access to storage. It should be thread-safe to avoid data races in tests.

}

func NewDriver() *InMemoryDriver {
return &InMemoryDriver{make(map[string][]byte)}
}

func (d *InMemoryDriver) GetContent(path string) ([]byte, error) {
contents, ok := d.storage[path]
if !ok {
return nil, driver.PathNotFoundError{path}
}
return contents, nil
}

func (d *InMemoryDriver) PutContent(path string, contents []byte) error {
d.storage[path] = contents
return nil
}

func (d *InMemoryDriver) ReadStream(path string) (io.ReadCloser, error) {
contents, err := d.GetContent(path)
if err != nil {
return nil, err
}
return ioutil.NopCloser(bytes.NewReader(contents)), nil
}

func (d *InMemoryDriver) WriteStream(path string, offset uint64, reader io.ReadCloser) error {
defer reader.Close()

resumableOffset, err := d.ResumeWritePosition(path)
if err != nil {
return err
}

if offset > resumableOffset {
return driver.InvalidOffsetError{path, offset}
}

contents, err := ioutil.ReadAll(reader)
if err != nil {
return err
}
d.storage[path] = contents
return nil
}

func (d *InMemoryDriver) ResumeWritePosition(path string) (uint64, error) {
contents, ok := d.storage[path]
if !ok {
return 0, nil
}
return uint64(len(contents)), nil
}

func (d *InMemoryDriver) Move(sourcePath string, destPath string) error {
contents, ok := d.storage[sourcePath]
if !ok {
return driver.PathNotFoundError{sourcePath}
}
d.storage[destPath] = contents
delete(d.storage, sourcePath)
return nil
}

func (d *InMemoryDriver) Delete(path string) error {
_, ok := d.storage[path]
if ok {
delete(d.storage, path)
return nil
}

subPaths := make([]string, 0)
for k := range d.storage {
if k != path && strings.HasPrefix(k, path) {
subPaths = append(subPaths, k)
}
}

if len(subPaths) == 0 {
return driver.PathNotFoundError{path}
}

for _, subPath := range subPaths {
delete(d.storage, subPath)
}
return nil
}
25 changes: 25 additions & 0 deletions contrib/golang_impl_ng/driver/inmemory/inmemory_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package inmemory

import (
"github.com/docker/docker-registry/contrib/golang_impl_ng/driver"
"github.com/docker/docker-registry/contrib/golang_impl_ng/driver/ipc"
. "gopkg.in/check.v1"
"testing"
)

// Hook up gocheck into the "go test" runner.
func Test(t *testing.T) { TestingT(t) }

var filesystemDriverConstructor = func() (driver.Driver, error) {
return NewDriver(), nil
}

var InProcessSuite = Suite(&driver.InProcessDriverSuite{
DriverConstructor: filesystemDriverConstructor,
})

var IPCSuite = Suite(&ipc.IPCDriverSuite{
TestDriverConfig: &ipc.TestDriverConfig{
Name: "inmemory",
},
})
Loading