From 8002af43fb54254ef786e4478ada7824e726baf7 Mon Sep 17 00:00:00 2001 From: shin- Date: Mon, 18 Feb 2013 21:10:19 -0800 Subject: [PATCH] Better error context when failing to create a new image. Added basic tag support + unit tests --- fs/layers.go | 27 +++++++++++++++---------- fs/store.go | 52 +++++++++++++++++++++++++++++++++++++++++++----- fs/store_test.go | 31 +++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 15 deletions(-) diff --git a/fs/layers.go b/fs/layers.go index fa03ca5865..f841b08091 100644 --- a/fs/layers.go +++ b/fs/layers.go @@ -8,6 +8,7 @@ import ( "io/ioutil" "os" "os/exec" + "fmt" "github.com/dotcloud/docker/future" ) @@ -15,6 +16,14 @@ type LayerStore struct { Root string } +type Compression uint32 + +const ( + Uncompressed Compression = iota + Bzip2 + Gzip +) + func NewLayerStore(root string) (*LayerStore, error) { abspath, err := filepath.Abs(root) if err != nil { @@ -93,7 +102,7 @@ func (store *LayerStore) AddLayer(id string, archive Archive, stderr io.Writer, tmp, err := store.Mktemp() defer os.RemoveAll(tmp) if err != nil { - return "", err + return "", errors.New(fmt.Sprintf("Mktemp failed: %s", err)) } extractFlags := "-x" if compression == Bzip2 { @@ -104,16 +113,16 @@ func (store *LayerStore) AddLayer(id string, archive Archive, stderr io.Writer, untarCmd := exec.Command("tar", "-C", tmp, extractFlags) untarW, err := untarCmd.StdinPipe() if err != nil { - return "", err + return "", errors.New(fmt.Sprintf("Could not obtain stdin pipe: %s", err)) } untarStderr, err := untarCmd.StderrPipe() if err != nil { - return "", err + return "", errors.New(fmt.Sprintf("Could not obtain stderr pipe: %s", err)) } go io.Copy(stderr, untarStderr) untarStdout, err := untarCmd.StdoutPipe() if err != nil { - return "", err + return "", errors.New(fmt.Sprintf("Could not obtain stdout pipe: %s", err)) } go io.Copy(stderr, untarStdout) untarCmd.Start() @@ -122,19 +131,17 @@ func (store *LayerStore) AddLayer(id string, archive Archive, stderr io.Writer, untarW.Close() return err }) - if err != nil { - return "", err - } + if err := untarCmd.Wait(); err != nil { - return "", err + return "", errors.New(fmt.Sprintf("Error while waiting for untar command to complete: %s", err)) } if err := <-job_copy; err != nil { - return "", err + return "", errors.New(fmt.Sprintf("Error while copying: %s", err)) } layer := store.layerPath(id) if !store.Exists(id) { if err := os.Rename(tmp, layer); err != nil { - return "", err + return "", errors.New(fmt.Sprintf("Could not rename temp dir to layer %s: %s", layer, err)) } } return layer, nil diff --git a/fs/store.go b/fs/store.go index 9bbc52e18e..d66fa46324 100644 --- a/fs/store.go +++ b/fs/store.go @@ -8,6 +8,8 @@ import ( "io" "path" "github.com/dotcloud/docker/future" + "fmt" + "errors" ) type Store struct { @@ -31,6 +33,7 @@ func New(root string) (*Store, error) { orm.AddTableWithName(Image{}, "images").SetKeys(false, "Id") orm.AddTableWithName(Path{}, "paths").SetKeys(false, "Path", "Image") orm.AddTableWithName(Mountpoint{}, "mountpoints").SetKeys(false, "Root") + orm.AddTableWithName(Tag{}, "tags").SetKeys(false, "TagName") if err := orm.CreateTables(); err != nil { return nil, err } @@ -105,7 +108,7 @@ func (store *Store) Create(layerData Archive, parent *Image, pth, comment string // FIXME: Archive should contain compression info. For now we only support uncompressed. _, err := store.layers.AddLayer(img.Id, layerData, os.Stderr, Uncompressed) if err != nil { - return nil, err + return nil, errors.New(fmt.Sprintf("Could not add layer: %s", err)) } path := &Path{ Path: path.Clean(pth), @@ -113,16 +116,16 @@ func (store *Store) Create(layerData Archive, parent *Image, pth, comment string } trans, err := store.orm.Begin() if err != nil { - return nil, err + return nil, errors.New(fmt.Sprintf("Could not begin transaction:", err)) } if err := trans.Insert(img); err != nil { - return nil, err + return nil, errors.New(fmt.Sprintf("Could not insert image info: %s", err)) } if err := trans.Insert(path); err != nil { - return nil, err + return nil, errors.New(fmt.Sprintf("Could not insert path info: %s", err)) } if err := trans.Commit(); err != nil { - return nil, err + return nil, errors.New(fmt.Sprintf("Could not commit transaction: %s", err)) } return img, nil } @@ -183,8 +186,47 @@ func (image *Image) Mountpoints() ([]*Mountpoint, error) { return mountpoints, nil } +func (store *Store) AddTag(imageId, tagName string) error { + if image, err := store.Get(imageId); err != nil { + return err + } else if image == nil { + return errors.New("No image with ID " + imageId) + } + + err2 := store.orm.Insert(&Tag{ + TagName: tagName, + Image: imageId, + }) + + return err2 +} + +func (store *Store) GetByTag(tagName string) (*Image, error) { + res, err := store.orm.Get(Tag{}, tagName) + if err != nil { + return nil, err + } else if res == nil { + return nil, errors.New("No image associated to tag \"" + tagName + "\"") + } + + tag := res.(*Tag) + + img, err2 := store.Get(tag.Image) + if err2 != nil { + return nil, err2 + } else if img == nil { + return nil, errors.New("Tag was found but image seems to be inexistent.") + } + + return img, nil +} type Path struct { Path string Image string } + +type Tag struct { + TagName string + Image string +} \ No newline at end of file diff --git a/fs/store_test.go b/fs/store_test.go index 89198f94d5..2898049404 100644 --- a/fs/store_test.go +++ b/fs/store_test.go @@ -52,6 +52,37 @@ func TestCreate(t *testing.T) { } } +func TestTag(t *testing.T) { + store, err := TempStore("testtag") + if err != nil { + t.Fatal(err) + } + defer nuke(store) + archive, err := fake.FakeTar() + if err != nil { + t.Fatal(err) + } + image, err := store.Create(archive, nil, "foo", "Testing") + if err != nil { + t.Fatal(err) + } + if images, err := store.Images(); err != nil { + t.Fatal(err) + } else if l := len(images); l != 1 { + t.Fatalf("Wrong number of images. Should be %d, not %d", 1, l) + } + + if err := store.AddTag(image.Id, "baz"); err != nil { + t.Fatalf("Error while adding a tag to created image: %s", err) + } + + if taggedImage, err := store.GetByTag("baz"); err != nil { + t.Fatalf("Error while trying to retrieve image for tag 'baz': %s", err) + } else if taggedImage.Id != image.Id { + t.Fatalf("Expected to retrieve image %s but found %s instead", image.Id, taggedImage.Id) + } +} + // Copy an image to a new path func TestCopyNewPath(t *testing.T) { store, err := TempStore("testcopynewpath")