Merged branch 79-rmi_regexp-feature

.gitignore

@@ -4,3 +4,5 @@ docker/docker
 a.out
 *.orig
 build_src
+command-line-arguments.test
+.flymake*

commands.go

@@ -54,7 +54,7 @@ func (srv *Server) Help() string {
 		{"reset", "Reset changes to a container's filesystem"},
 		{"restart", "Restart a running container"},
 		{"rm", "Remove a container"},
-		{"rmimage", "Remove an image"},
+		{"rmi", "Remove an image"},
 		{"run", "Run a command in a new container"},
 		{"start", "Start a stopped container"},
 		{"stop", "Stop a running container"},
@@ -356,30 +356,28 @@ func (srv *Server) CmdPort(stdin io.ReadCloser, stdout io.Writer, args ...string
 }
 
 // 'docker rmi NAME' removes all images with the name NAME
-func (srv *Server) CmdRmi(stdin io.ReadCloser, stdout io.Writer, args ...string) error {
+func (srv *Server) CmdRmi(stdin io.ReadCloser, stdout io.Writer, args ...string) (err error) {
 	cmd := rcli.Subcmd(stdout, "rmimage", "[OPTIONS] IMAGE", "Remove an image")
 	fl_all := cmd.Bool("a", false, "Use IMAGE as a path and remove ALL images in this path")
-	if err := cmd.Parse(args); err != nil {
+	fl_regexp := cmd.Bool("r", false, "Use IMAGE as a regular expression instead of an exact name")
-		cmd.Usage()
+	if cmd.Parse(args) != nil || cmd.NArg() < 1 {
-		return nil
-	}
-	if cmd.NArg() < 1 {
 		cmd.Usage()
 		return nil
 	}
 	for _, name := range cmd.Args() {
-		var err error
+		if *fl_regexp {
-		if *fl_all {
+			err = srv.images.RemoveRegexp(name)
+		} else if *fl_all {
 			err = srv.images.RemoveInPath(name)
 		} else {
-			image, err := srv.images.Get(name)
+			if image, err1 := srv.images.Find(name); err1 != nil {
-			if err != nil {
+				err = err1
-				return err
+			} else if err1 == nil && image == nil {
-			} else if image == nil {
+				err = fmt.Errorf("No such image: %s", name)
-				return errors.New("No such image: " + name)
+			} else {
-			}
 				err = srv.images.Remove(image)
 			}
+		}
 		if err != nil {
 			return err
 		}

docker_test.go

@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"os/user"
 	"testing"
 )
 
@@ -42,6 +43,12 @@ func init() {
 		return
 	}
 
+	if usr, err := user.Current(); err != nil {
+		panic(err)
+	} else if usr.Uid != "0" {
+		panic("docker tests needs to be run as root")
+	}
+
 	// Create a temp directory
 	root, err := ioutil.TempDir("", "docker-test")
 	if err != nil {

fs/remove_test.go

@@ -0,0 +1,223 @@
+package fs
+
+import (
+	"fmt"
+	"github.com/dotcloud/docker/fake"
+	"testing"
+)
+
+func countImages(store *Store) int {
+	paths, err := store.Images()
+	if err != nil {
+		panic(err)
+	}
+	return len(paths)
+}
+
+func TestRemoveInPath(t *testing.T) {
+	store, err := TempStore("test-remove-in-path")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer nuke(store)
+	archive, err := fake.FakeTar()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 0 {
+		t.Fatalf("Expected 0 images, %d found", c)
+	}
+
+	// Test 10 create / Delete all
+	for i := 0; i < 10; i++ {
+		if _, err := store.Create(archive, nil, "foo", "Testing"); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if c := countImages(store); c != 10 {
+		t.Fatalf("Expected 10 images, %d found", c)
+	}
+	if err := store.RemoveInPath("foo"); err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 0 {
+		t.Fatalf("Expected 0 images, %d found", c)
+	}
+
+	// Test 10 create / Delete 1
+	for i := 0; i < 10; i++ {
+		if _, err := store.Create(archive, nil, fmt.Sprintf("foo-%d", i), "Testing"); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if c := countImages(store); c != 10 {
+		t.Fatalf("Expected 10 images, %d found", c)
+	}
+	if err := store.RemoveInPath("foo-0"); err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 9 {
+		t.Fatalf("Expected 9 images, %d found", c)
+	}
+
+	// Delete failure
+	if err := store.RemoveInPath("Not_Foo"); err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 9 {
+		t.Fatalf("Expected 9 images, %d found", c)
+	}
+}
+
+func TestRemove(t *testing.T) {
+	store, err := TempStore("test-remove")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer nuke(store)
+	archive, err := fake.FakeTar()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 0 {
+		t.Fatalf("Expected 0 images, %d found", c)
+	}
+
+	// Test 1 create / 1 delete
+	img, err := store.Create(archive, nil, "foo", "Testing")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 1 {
+		t.Fatalf("Expected 1 images, %d found", c)
+	}
+	if err := store.Remove(img); err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 0 {
+		t.Fatalf("Expected 0 images, %d found", c)
+	}
+
+	// Test 2 create (same name) / 1 delete
+	img1, err := store.Create(archive, nil, "foo", "Testing")
+	if err != nil {
+		t.Fatal(err)
+	}
+	img2, err := store.Create(archive, nil, "foo", "Testing")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 2 {
+		t.Fatalf("Expected 2 images, %d found", c)
+	}
+	if err := store.Remove(img1); err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 1 {
+		t.Fatalf("Expected 1 images, %d found", c)
+	}
+
+	// Test delete wrong name
+	// Note: If we change orm and Delete of non existing return error, we will need to change this test
+	if err := store.Remove(&Image{Id: "Not_foo", store: img2.store}); err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 1 {
+		t.Fatalf("Expected 1 images, %d found", c)
+	}
+
+	// Test delete last one
+	if err := store.Remove(img2); err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 0 {
+		t.Fatalf("Expected 0 images, %d found", c)
+	}
+}
+
+func TestRemoveRegexp(t *testing.T) {
+	store, err := TempStore("test-remove-regexp")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer nuke(store)
+	archive, err := fake.FakeTar()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 0 {
+		t.Fatalf("Expected 0 images, %d found", c)
+	}
+
+	// Test 10 create with different names / Delete all good regexp
+	for i := 0; i < 10; i++ {
+		if _, err := store.Create(archive, nil, fmt.Sprintf("foo-%d", i), "Testing"); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if c := countImages(store); c != 10 {
+		t.Fatalf("Expected 10 images, %d found", c)
+	}
+	if err := store.RemoveRegexp("foo"); err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 0 {
+		t.Fatalf("Expected 0 images, %d found", c)
+	}
+
+	// Test 10 create with different names / Delete all good regexp globing
+	for i := 0; i < 10; i++ {
+		if _, err := store.Create(archive, nil, fmt.Sprintf("foo-%d", i), "Testing"); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if c := countImages(store); c != 10 {
+		t.Fatalf("Expected 10 images, %d found", c)
+	}
+	if err := store.RemoveRegexp("foo-*"); err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 0 {
+		t.Fatalf("Expected 0 images, %d found", c)
+	}
+
+	// Test 10 create with different names / Delete all bad regexp
+	for i := 0; i < 10; i++ {
+		if _, err := store.Create(archive, nil, fmt.Sprintf("foo-%d", i), "Testing"); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if c := countImages(store); c != 10 {
+		t.Fatalf("Expected 10 images, %d found", c)
+	}
+	if err := store.RemoveRegexp("oo-*"); err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 0 {
+		t.Fatalf("Expected 0 images, %d found", c)
+	}
+
+	// Test 10 create with different names / Delete none strict regexp
+	for i := 0; i < 10; i++ {
+		if _, err := store.Create(archive, nil, fmt.Sprintf("foo-%d", i), "Testing"); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if c := countImages(store); c != 10 {
+		t.Fatalf("Expected 10 images, %d found", c)
+	}
+	if err := store.RemoveRegexp("^oo-"); err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 10 {
+		t.Fatalf("Expected 10 images, %d found", c)
+	}
+
+	// Test delete 2
+	if err := store.RemoveRegexp("^foo-[1,2]$"); err != nil {
+		t.Fatal(err)
+	}
+	if c := countImages(store); c != 8 {
+		t.Fatalf("Expected 8 images, %d found", c)
+	}
+}

fs/store.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"syscall"
 	"time"
@@ -105,6 +106,27 @@ func (store *Store) RemoveInPath(pth string) error {
 	return nil
 }
 
+// DeleteMatch deletes all images whose name matches `pattern`
+func (store *Store) RemoveRegexp(pattern string) error {
+	// Retrieve all the paths
+	paths, err := store.Paths()
+	if err != nil {
+		return err
+	}
+	// Check the pattern on each elements
+	for _, pth := range paths {
+		if match, err := regexp.MatchString(pattern, pth); err != nil {
+			return err
+		} else if match {
+			// If there is a match, remove it
+			if err := store.RemoveInPath(pth); err != nil {
+				return nil
+			}
+		}
+	}
+	return nil
+}
+
 func (store *Store) Remove(img *Image) error {
 	_, err := store.orm.Delete(img)
 	return err

fs/store_test.go

@@ -1,7 +1,6 @@
 package fs
 
 import (
-	"errors"
 	"fmt"
 	"github.com/dotcloud/docker/fake"
 	"github.com/dotcloud/docker/future"
@@ -11,6 +10,8 @@ import (
 	"time"
 )
 
+// FIXME: Remove the Fake package
+
 func TestInit(t *testing.T) {
 	store, err := TempStore("testinit")
 	if err != nil {
@@ -26,6 +27,8 @@ func TestInit(t *testing.T) {
 	}
 }
 
+// FIXME: Do more extensive tests (ex: create multiple, delete, recreate;
+//       create multiple, check the amount of images and paths, etc..)
 func TestCreate(t *testing.T) {
 	store, err := TempStore("testcreate")
 	if err != nil {
@@ -229,63 +232,6 @@ func TestMountpointDuplicateRoot(t *testing.T) {
 	}
 }
 
-func TestMount(t *testing.T) {
-	store, err := TempStore("test-mount")
-	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)
-	}
-	// Create mount targets
-	root, err := ioutil.TempDir("", "docker-fs-test")
-	if err != nil {
-		t.Fatal(err)
-	}
-	rw, err := ioutil.TempDir("", "docker-fs-test")
-	if err != nil {
-		t.Fatal(err)
-	}
-	mountpoint, err := image.Mount(root, rw)
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer mountpoint.Umount()
-	// Mountpoint should be marked as mounted
-	if !mountpoint.Mounted() {
-		t.Fatal("Mountpoint not mounted")
-	}
-	// There should be one mountpoint registered
-	if mps, err := image.Mountpoints(); err != nil {
-		t.Fatal(err)
-	} else if len(mps) != 1 {
-		t.Fatal("Wrong number of mountpoints registered (should be %d, not %d)", 1, len(mps))
-	}
-	// Unmounting should work
-	if err := mountpoint.Umount(); err != nil {
-		t.Fatal(err)
-	}
-	// De-registering should work
-	if err := mountpoint.Deregister(); err != nil {
-		t.Fatal(err)
-	}
-	if mps, err := image.Mountpoints(); err != nil {
-		t.Fatal(err)
-	} else if len(mps) != 0 {
-		t.Fatal("Wrong number of mountpoints registered (should be %d, not %d)", 0, len(mps))
-	}
-	// General health check
-	if err := healthCheck(store); err != nil {
-		t.Fatal(err)
-	}
-}
-
 func TempStore(prefix string) (*Store, error) {
 	dir, err := ioutil.TempDir("", "docker-fs-test-"+prefix)
 	if err != nil {
@@ -314,7 +260,7 @@ func healthCheck(store *Store) error {
 		for _, img := range images {
 			// Check for duplicate IDs per path
 			if _, exists := IDs[img.Id]; exists {
-				return errors.New(fmt.Sprintf("Duplicate ID: %s", img.Id))
+				return fmt.Errorf("Duplicate ID: %s", img.Id)
 			} else {
 				IDs[img.Id] = true
 			}
@@ -327,7 +273,7 @@ func healthCheck(store *Store) error {
 	// Check non-existing parents
 	for parent := range parents {
 		if _, exists := parents[parent]; !exists {
-			return errors.New("Reference to non-registered parent: " + parent)
+			return fmt.Errorf("Reference to non-registered parent: %s", parent)
 		}
 	}
 	return nil

mount_test.go

@@ -0,0 +1,115 @@
+package docker
+
+import (
+	"fmt"
+	"github.com/dotcloud/docker/fake"
+	"github.com/dotcloud/docker/fs"
+	"io/ioutil"
+	"os"
+	"testing"
+)
+
+// Look for inconsistencies in a store.
+func healthCheck(store *fs.Store) error {
+	parents := make(map[string]bool)
+	paths, err := store.Paths()
+	if err != nil {
+		return err
+	}
+	for _, path := range paths {
+		images, err := store.List(path)
+		if err != nil {
+			return err
+		}
+		IDs := make(map[string]bool) // All IDs for this path
+		for _, img := range images {
+			// Check for duplicate IDs per path
+			if _, exists := IDs[img.Id]; exists {
+				return fmt.Errorf("Duplicate ID: %s", img.Id)
+			} else {
+				IDs[img.Id] = true
+			}
+			// Store parent for 2nd pass
+			if parent := img.Parent; parent != "" {
+				parents[parent] = true
+			}
+		}
+	}
+	// Check non-existing parents
+	for parent := range parents {
+		if _, exists := parents[parent]; !exists {
+			return fmt.Errorf("Reference to non-registered parent: %s", parent)
+		}
+	}
+	return nil
+}
+
+// Note: This test is in the docker package because he needs to be run as root
+func TestMount(t *testing.T) {
+	dir, err := ioutil.TempDir("", "docker-fs-test-mount")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	store, err := fs.New(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	archive, err := fake.FakeTar()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	image, err := store.Create(archive, nil, "foo", "Testing")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Create mount targets
+	root, err := ioutil.TempDir("", "docker-fs-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(root)
+
+	rw, err := ioutil.TempDir("", "docker-fs-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(rw)
+
+	mountpoint, err := image.Mount(root, rw)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer mountpoint.Umount()
+	// Mountpoint should be marked as mounted
+	if !mountpoint.Mounted() {
+		t.Fatal("Mountpoint not mounted")
+	}
+	// There should be one mountpoint registered
+	if mps, err := image.Mountpoints(); err != nil {
+		t.Fatal(err)
+	} else if len(mps) != 1 {
+		t.Fatal("Wrong number of mountpoints registered (should be %d, not %d)", 1, len(mps))
+	}
+	// Unmounting should work
+	if err := mountpoint.Umount(); err != nil {
+		t.Fatal(err)
+	}
+	// De-registering should work
+	if err := mountpoint.Deregister(); err != nil {
+		t.Fatal(err)
+	}
+	if mps, err := image.Mountpoints(); err != nil {
+		t.Fatal(err)
+	} else if len(mps) != 0 {
+		t.Fatal("Wrong number of mountpoints registered (should be %d, not %d)", 0, len(mps))
+	}
+	// General health check
+	if err := healthCheck(store); err != nil {
+		t.Fatal(err)
+	}
+}