Skip to content

Commit

Permalink
Refactor volume store's error usage
Browse files Browse the repository at this point in the history
Uses an errors API similar the `net` package.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
  • Loading branch information
cpuguy83 committed Nov 11, 2015
1 parent 62619a1 commit 43012fe
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 30 deletions.
4 changes: 2 additions & 2 deletions daemon/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (

"github.com/Sirupsen/logrus"
derr "github.com/docker/docker/errors"
"github.com/docker/docker/volume/store"
volumestore "github.com/docker/docker/volume/store"
)

// ContainerRmConfig is a holder for passing in runtime config.
Expand Down Expand Up @@ -153,7 +153,7 @@ func (daemon *Daemon) VolumeRm(name string) error {
return err
}
if err := daemon.volumes.Remove(v); err != nil {
if err == store.ErrVolumeInUse {
if volumestore.IsInUse(err) {
return derr.ErrorCodeRmVolumeInUse.WithArgs(err)
}
return derr.ErrorCodeRmVolume.WithArgs(name, err)
Expand Down
4 changes: 2 additions & 2 deletions daemon/mounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"strings"

derr "github.com/docker/docker/errors"
"github.com/docker/docker/volume/store"
volumestore "github.com/docker/docker/volume/store"
)

func (daemon *Daemon) prepareMountPoints(container *Container) error {
Expand Down Expand Up @@ -34,7 +34,7 @@ func (daemon *Daemon) removeMountPoints(container *Container, rm bool) error {
// not an error, but an implementation detail.
// This prevents docker from logging "ERROR: Volume in use"
// where there is another container using the volume.
if err != nil && err != store.ErrVolumeInUse {
if err != nil && !volumestore.IsInUse(err) {
rmErrors = append(rmErrors, err.Error())
}
}
Expand Down
58 changes: 58 additions & 0 deletions volume/store/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package store

import "errors"

var (
// errVolumeInUse is a typed error returned when trying to remove a volume that is currently in use by a container
errVolumeInUse = errors.New("volume is in use")
// errNoSuchVolume is a typed error returned if the requested volume doesn't exist in the volume store
errNoSuchVolume = errors.New("no such volume")
// errInvalidName is a typed error returned when creating a volume with a name that is not valid on the platform
errInvalidName = errors.New("volume name is not valid on this platform")
)

// OpErr is the error type returned by functions in the store package. It describes
// the operation, volume name, and error.
type OpErr struct {
// Err is the error that occurred during the operation.
Err error
// Op is the operation which caused the error, such as "create", or "list".
Op string
// Name is the name of the resource being requested for this op, typically the volume name or the driver name.
Name string
}

// Error satifies the built-in error interface type.
func (e *OpErr) Error() string {
if e == nil {
return "<nil>"
}
s := e.Op
if e.Name != "" {
s = s + " " + e.Name
}

s = s + ": " + e.Err.Error()
return s
}

// IsInUse returns a boolean indicating whether the error indicates that a
// volume is in use
func IsInUse(err error) bool {
return isErr(err, errVolumeInUse)
}

// IsNotExist returns a boolean indicating whether the error indicates that the volume does not exist
func IsNotExist(err error) bool {
return isErr(err, errNoSuchVolume)
}

func isErr(err error, expected error) bool {
switch pe := err.(type) {
case nil:
return false
case *OpErr:
err = pe.Err
}
return err == expected
}
26 changes: 8 additions & 18 deletions volume/store/store.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package store

import (
"errors"
"sync"

"github.com/Sirupsen/logrus"
Expand All @@ -10,15 +9,6 @@ import (
"github.com/docker/docker/volume/drivers"
)

var (
// ErrVolumeInUse is a typed error returned when trying to remove a volume that is currently in use by a container
ErrVolumeInUse = errors.New("volume is in use")
// ErrNoSuchVolume is a typed error returned if the requested volume doesn't exist in the volume store
ErrNoSuchVolume = errors.New("no such volume")
// ErrInvalidName is a typed error returned when creating a volume with a name that is not valid on the platform
ErrInvalidName = errors.New("volume name is not valid on this platform")
)

// New initializes a VolumeStore to keep
// reference counting of volumes in the system.
func New() *VolumeStore {
Expand Down Expand Up @@ -81,7 +71,7 @@ func (s *VolumeStore) Create(name, driverName string, opts map[string]string) (v

vd, err := volumedrivers.GetDriver(driverName)
if err != nil {
return nil, err
return nil, &OpErr{Err: err, Name: driverName, Op: "create"}
}

// Validate the name in a platform-specific manner
Expand All @@ -90,12 +80,12 @@ func (s *VolumeStore) Create(name, driverName string, opts map[string]string) (v
return nil, err
}
if !valid {
return nil, ErrInvalidName
return nil, &OpErr{Err: errInvalidName, Name: name, Op: "create"}
}

v, err := vd.Create(name, opts)
if err != nil {
return nil, err
return nil, &OpErr{Op: "create", Name: name, Err: err}
}

s.set(name, &volumeCounter{v, 0})
Expand All @@ -110,7 +100,7 @@ func (s *VolumeStore) Get(name string) (volume.Volume, error) {

vc, exists := s.get(name)
if !exists {
return nil, ErrNoSuchVolume
return nil, &OpErr{Err: errNoSuchVolume, Name: name, Op: "get"}
}
return vc.Volume, nil
}
Expand All @@ -124,19 +114,19 @@ func (s *VolumeStore) Remove(v volume.Volume) error {
logrus.Debugf("Removing volume reference: driver %s, name %s", v.DriverName(), name)
vc, exists := s.get(name)
if !exists {
return ErrNoSuchVolume
return &OpErr{Err: errNoSuchVolume, Name: name, Op: "remove"}
}

if vc.count > 0 {
return ErrVolumeInUse
return &OpErr{Err: errVolumeInUse, Name: name, Op: "remove"}
}

vd, err := volumedrivers.GetDriver(vc.DriverName())
if err != nil {
return err
return &OpErr{Err: err, Name: vc.DriverName(), Op: "remove"}
}
if err := vd.Remove(vc.Volume); err != nil {
return err
return &OpErr{Err: err, Name: name, Op: "remove"}
}

s.remove(name)
Expand Down
18 changes: 10 additions & 8 deletions volume/store/store_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package store

import (
"errors"
"testing"

"github.com/docker/docker/volume"
Expand Down Expand Up @@ -30,8 +31,8 @@ func TestGet(t *testing.T) {
t.Fatalf("Expected fake1 volume, got %v", v)
}

if _, err := s.Get("fake4"); err != ErrNoSuchVolume {
t.Fatalf("Expected ErrNoSuchVolume error, got %v", err)
if _, err := s.Get("fake4"); !IsNotExist(err) {
t.Fatalf("Expected IsNotExist error, got %v", err)
}
}

Expand All @@ -54,24 +55,25 @@ func TestCreate(t *testing.T) {
}

_, err = s.Create("fakeError", "fake", map[string]string{"error": "create error"})
if err == nil || err.Error() != "create error" {
t.Fatalf("Expected create error, got %v", err)
expected := &OpErr{Op: "create", Name: "fakeError", Err: errors.New("create error")}
if err != nil && err.Error() != expected.Error() {
t.Fatalf("Expected create fakeError: create error, got %v", err)
}
}

func TestRemove(t *testing.T) {
volumedrivers.Register(vt.FakeDriver{}, "fake")
s := New()
if err := s.Remove(vt.NoopVolume{}); err != ErrNoSuchVolume {
t.Fatalf("Expected ErrNoSuchVolume error, got %v", err)
if err := s.Remove(vt.NoopVolume{}); !IsNotExist(err) {
t.Fatalf("Expected IsNotExist error, got %v", err)
}
v, err := s.Create("fake1", "fake", nil)
if err != nil {
t.Fatal(err)
}
s.Increment(v)
if err := s.Remove(v); err != ErrVolumeInUse {
t.Fatalf("Expected ErrVolumeInUse error, got %v", err)
if err := s.Remove(v); !IsInUse(err) {
t.Fatalf("Expected IsInUse error, got %v", err)
}
s.Decrement(v)
if err := s.Remove(v); err != nil {
Expand Down

0 comments on commit 43012fe

Please sign in to comment.