From f6d64738d05d62596d94a5567916e3b3b5209b2c Mon Sep 17 00:00:00 2001 From: Charles Hooper Date: Tue, 12 Mar 2013 02:58:39 +0000 Subject: [PATCH 1/9] Pull missing images on run. Addresses #31. chooper@chimay:~/projects/docker/bin$ ./docker images NAME ID CREATED PARENT chooper@chimay:~/projects/docker/bin$ ./docker run -a base echo "hello world" Downloading from http://s3.amazonaws.com/docker.io/images/base Unpacking to base ######################################################################## 100.0% base:e9cb4ad9173245ac hello world chooper@chimay:~/projects/docker/bin$ ./docker run -a base echo "hello world" hello world chooper@chimay:~/projects/docker/bin$ ./docker run -a nosuchimage echo "hello world" Downloading from http://s3.amazonaws.com/docker.io/images/nosuchimage Unpacking to nosuchimage ######################################################################## 100.0% Error: Error downloading image: nosuchimage chooper@chimay:~/projects/docker/bin$ --- server/server.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/server/server.go b/server/server.go index bc642cdbb0..4b73fec31c 100644 --- a/server/server.go +++ b/server/server.go @@ -800,6 +800,8 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string) fl_tty := cmd.Bool("t", false, "Allocate a pseudo-tty") fl_comment := cmd.String("c", "", "Comment") var fl_ports ports + var img *image.Image + cmd.Var(&fl_ports, "p", "Map a network port to the container") if err := cmd.Parse(args); err != nil { return nil @@ -820,11 +822,20 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string) *fl_attach = true cmdline = []string{"/bin/bash", "-i"} } + // Find the image - img := srv.images.Find(name) + img = srv.images.Find(name) if img == nil { - return errors.New("No such image: " + name) + devnull, err := os.Open("/dev/null") + if err != nil { + return errors.New("Error opening /dev/null") + } + if srv.CmdPull(devnull, stdout, name) != nil { + return errors.New("Error downloading image: " + name) + } + img = srv.images.Find(name) } + // Create new container container, err := srv.CreateContainer(img, fl_ports, *fl_user, *fl_tty, *fl_stdin, *fl_comment, cmdline[0], cmdline[1:]...) if err != nil { From 63edf8a4a1830a939432e369dd075ebade0e03a9 Mon Sep 17 00:00:00 2001 From: Charles Hooper Date: Tue, 12 Mar 2013 03:18:34 +0000 Subject: [PATCH 2/9] Use ioutil.NopCloser instead of opening /dev/null for ReadCloser in CmdRun. Related to #31 --- server/server.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/server/server.go b/server/server.go index 4b73fec31c..17dc7705c4 100644 --- a/server/server.go +++ b/server/server.go @@ -11,6 +11,7 @@ import ( "github.com/dotcloud/docker/image" "github.com/dotcloud/docker/rcli" "io" + "io/ioutil" "net/http" "net/url" "os" @@ -826,11 +827,8 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string) // Find the image img = srv.images.Find(name) if img == nil { - devnull, err := os.Open("/dev/null") - if err != nil { - return errors.New("Error opening /dev/null") - } - if srv.CmdPull(devnull, stdout, name) != nil { + stdin_noclose := ioutil.NopCloser(stdin) + if srv.CmdPull(stdin_noclose, stdout, name) != nil { return errors.New("Error downloading image: " + name) } img = srv.images.Find(name) From b8219b5275f5bbb049e97a1459b87d194878aebc Mon Sep 17 00:00:00 2001 From: Charles Hooper Date: Tue, 12 Mar 2013 04:31:49 +0000 Subject: [PATCH 3/9] Don't allow images with colons in name and reject pegging versions on pull/import. Addresses #49 and #52 --- image/image.go | 6 ++++++ server/server.go | 10 ++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/image/image.go b/image/image.go index 4702919ab4..d9326fbd17 100644 --- a/image/image.go +++ b/image/image.go @@ -46,6 +46,9 @@ func New(root string) (*Store, error) { // Import creates a new image from the contents of `archive` and registers it in the store as `name`. // If `parent` is not nil, it will registered as the parent of the new image. func (store *Store) Import(name string, archive io.Reader, parent *Image) (*Image, error) { + if strings.Contains(name, ":") { + return nil, errors.New("Invalid image name: " + name) + } layer, err := store.Layers.AddLayer(archive) if err != nil { return nil, err @@ -62,6 +65,9 @@ func (store *Store) Import(name string, archive io.Reader, parent *Image) (*Imag } func (store *Store) Create(name string, source string, layers ...string) (*Image, error) { + if strings.Contains(name, ":") { + return nil, errors.New("Invalid image name: " + name) + } image, err := NewImage(name, layers, source) if err != nil { return nil, err diff --git a/server/server.go b/server/server.go index 17dc7705c4..d52666d8f6 100644 --- a/server/server.go +++ b/server/server.go @@ -411,6 +411,9 @@ func (srv *Server) CmdPull(stdin io.ReadCloser, stdout io.Writer, args ...string if name == "" { return errors.New("Not enough arguments") } + if strings.Contains(name, ":") { + return errors.New("Invalid image name: " + name) + } u, err := url.Parse(name) if err != nil { return err @@ -828,10 +831,13 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string) img = srv.images.Find(name) if img == nil { stdin_noclose := ioutil.NopCloser(stdin) - if srv.CmdPull(stdin_noclose, stdout, name) != nil { - return errors.New("Error downloading image: " + name) + if err := srv.CmdPull(stdin_noclose, stdout, name); err != nil { + return err } img = srv.images.Find(name) + if img == nil { + return errors.New("Could not find image after downloading: " + name) + } } // Create new container From 8e0986caec91f61e0496e1990a4bec745a9596fc Mon Sep 17 00:00:00 2001 From: Charles Hooper Date: Tue, 12 Mar 2013 05:49:57 +0000 Subject: [PATCH 4/9] Error gracefully when an image is not found on pull. Addresses #50 and comments from #49 --- future/future.go | 5 ++++- server/server.go | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/future/future.go b/future/future.go index 33f1f8925c..208484df1a 100644 --- a/future/future.go +++ b/future/future.go @@ -89,7 +89,7 @@ func Pv(src io.Reader, info io.Writer) io.Reader { // the body of the response. If `stderr` is not nil, a progress bar will be // written to it. func Curl(url string, stderr io.Writer) (io.Reader, error) { - curl := exec.Command("curl", "-#", "-L", url) + curl := exec.Command("curl", "-#", "-L", "--fail", url) output, err := curl.StdoutPipe() if err != nil { return nil, err @@ -98,5 +98,8 @@ func Curl(url string, stderr io.Writer) (io.Reader, error) { if err := curl.Start(); err != nil { return nil, err } + if err := curl.Wait(); err != nil { + return nil, err + } return output, nil } diff --git a/server/server.go b/server/server.go index d52666d8f6..d70b413161 100644 --- a/server/server.go +++ b/server/server.go @@ -428,12 +428,16 @@ func (srv *Server) CmdPull(stdin io.ReadCloser, stdout io.Writer, args ...string } fmt.Fprintf(stdout, "Downloading from %s\n", u.String()) // Download with curl (pretty progress bar) - // If curl is not available, fallback to http.Get() + // If curl is not available or receives a HTTP error, fallback + // to http.Get() archive, err := future.Curl(u.String(), stdout) if err != nil { if resp, err := http.Get(u.String()); err != nil { return err } else { + if resp.StatusCode >= 400 { + return errors.New("Got HTTP status code >= 400: " + resp.Status) + } archive = resp.Body } } From 49554f47f60568c3df942f274ab45d92f451fbcc Mon Sep 17 00:00:00 2001 From: Charles Hooper Date: Tue, 12 Mar 2013 20:50:11 +0000 Subject: [PATCH 5/9] Add Download method for native Go downloading (to replace curl). Catch 404s. Ref: #49, #50 --- future/future.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/future/future.go b/future/future.go index 208484df1a..a3dd9f7efa 100644 --- a/future/future.go +++ b/future/future.go @@ -3,9 +3,11 @@ package future import ( "bytes" "crypto/sha256" + "errors" "fmt" "io" "math/rand" + "net/http" "os/exec" "time" ) @@ -103,3 +105,21 @@ func Curl(url string, stderr io.Writer) (io.Reader, error) { } return output, nil } + +// Request a given URL and return an io.Reader +func Download(url string, stderr io.Writer) (io.Reader, error) { + var resp *http.Response + var archive io.ReadCloser = nil + var err error = nil + + fmt.Fprintf(stderr, "Download start\n") // FIXME: Replace with progress bar + if resp, err = http.Get(url); err != nil { + return nil, err + } + if resp.StatusCode >= 400 { + return nil, errors.New("Got HTTP status code >= 400: " + resp.Status) + } + archive = resp.Body + fmt.Fprintf(stderr, "Download end\n") // FIXME: Replace with progress bar + return archive, nil +} From 1bfd827701d80a40e65509d45abaa61cd5c748ec Mon Sep 17 00:00:00 2001 From: Charles Hooper Date: Tue, 12 Mar 2013 22:08:10 +0000 Subject: [PATCH 6/9] Add native Go downloading functionality with progress bar. Ref #49, #50 --- future/future.go | 30 ++++++++++++++++++++++++++---- server/server.go | 29 +++++++++++++++-------------- 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/future/future.go b/future/future.go index a3dd9f7efa..6a1b4e2311 100644 --- a/future/future.go +++ b/future/future.go @@ -107,9 +107,8 @@ func Curl(url string, stderr io.Writer) (io.Reader, error) { } // Request a given URL and return an io.Reader -func Download(url string, stderr io.Writer) (io.Reader, error) { +func Download(url string, stderr io.Writer) (*http.Response, error) { var resp *http.Response - var archive io.ReadCloser = nil var err error = nil fmt.Fprintf(stderr, "Download start\n") // FIXME: Replace with progress bar @@ -119,7 +118,30 @@ func Download(url string, stderr io.Writer) (io.Reader, error) { if resp.StatusCode >= 400 { return nil, errors.New("Got HTTP status code >= 400: " + resp.Status) } - archive = resp.Body fmt.Fprintf(stderr, "Download end\n") // FIXME: Replace with progress bar - return archive, nil + return resp, nil +} + +// Reader with progress bar +type progressReader struct { + reader io.ReadCloser // Stream to read from + output io.Writer // Where to send progress bar to + read_total int // Expected stream length (bytes) + read_progress int // How much has been read so far (bytes) +} +func (r *progressReader) Read(p []byte) (n int, err error) { + read, err := io.ReadCloser(r.reader).Read(p) + // FIXME: Don't print progress bar on every read + r.read_progress += read + fmt.Fprintf(r.output, "%d/%d (%.2f%%)\n", + r.read_progress, + r.read_total, + float64(r.read_progress) / float64(r.read_total) * 100) + return read, err +} +func (r *progressReader) Close() error { + return io.ReadCloser(r.reader).Close() +} +func ProgressReader(r io.ReadCloser, size int, output io.Writer) *progressReader { + return &progressReader{r, output, size, 0} } diff --git a/server/server.go b/server/server.go index d70b413161..9c61ffaee8 100644 --- a/server/server.go +++ b/server/server.go @@ -12,7 +12,6 @@ import ( "github.com/dotcloud/docker/rcli" "io" "io/ioutil" - "net/http" "net/url" "os" "path" @@ -427,19 +426,11 @@ func (srv *Server) CmdPull(stdin io.ReadCloser, stdout io.Writer, args ...string u.Path = path.Join("/docker.io/images", u.Path) } fmt.Fprintf(stdout, "Downloading from %s\n", u.String()) - // Download with curl (pretty progress bar) - // If curl is not available or receives a HTTP error, fallback - // to http.Get() - archive, err := future.Curl(u.String(), stdout) + resp, err := future.Download(u.String(), stdout) + // FIXME: Validate ContentLength + archive := future.ProgressReader(resp.Body, int(resp.ContentLength), stdout) if err != nil { - if resp, err := http.Get(u.String()); err != nil { - return err - } else { - if resp.StatusCode >= 400 { - return errors.New("Got HTTP status code >= 400: " + resp.Status) - } - archive = resp.Body - } + return err } fmt.Fprintf(stdout, "Unpacking to %s\n", name) img, err := srv.images.Import(name, archive, nil) @@ -815,7 +806,10 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string) return nil } name := cmd.Arg(0) + var img_name string + //var img_version string // Only here for reference var cmdline []string + if len(cmd.Args()) >= 2 { cmdline = cmd.Args()[1:] } @@ -823,6 +817,13 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string) if name == "" { name = "base" } + + // Separate the name:version tag + if strings.Contains(name, ":") { + parts := strings.SplitN(name, ":", 2) + img_name = parts[0] + //img_version = parts[1] // Only here for reference + } // Choose a default command if needed if len(cmdline) == 0 { *fl_stdin = true @@ -835,7 +836,7 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string) img = srv.images.Find(name) if img == nil { stdin_noclose := ioutil.NopCloser(stdin) - if err := srv.CmdPull(stdin_noclose, stdout, name); err != nil { + if err := srv.CmdPull(stdin_noclose, stdout, img_name); err != nil { return err } img = srv.images.Find(name) From 2feefb43758e2477b70044e73ed0d800453fe8de Mon Sep 17 00:00:00 2001 From: Charles Hooper Date: Tue, 12 Mar 2013 22:13:21 +0000 Subject: [PATCH 7/9] Fix "no such image" bug in docker run, introduced last commit. Ref #49,#50 --- server/server.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/server.go b/server/server.go index 9c61ffaee8..46060d471a 100644 --- a/server/server.go +++ b/server/server.go @@ -823,7 +823,10 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string) parts := strings.SplitN(name, ":", 2) img_name = parts[0] //img_version = parts[1] // Only here for reference - } + } else { + img_name = name + } + // Choose a default command if needed if len(cmdline) == 0 { *fl_stdin = true From b5c1cd7991b3b7ea81665b06ad1dd4cb1c8d8b0f Mon Sep 17 00:00:00 2001 From: Charles Hooper Date: Tue, 12 Mar 2013 23:02:50 +0000 Subject: [PATCH 8/9] Only update download/untar progress every 1%. Related to #49 --- future/future.go | 30 +++++++++++++++++------------- server/server.go | 2 +- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/future/future.go b/future/future.go index 6a1b4e2311..477fe7c320 100644 --- a/future/future.go +++ b/future/future.go @@ -110,38 +110,42 @@ func Curl(url string, stderr io.Writer) (io.Reader, error) { func Download(url string, stderr io.Writer) (*http.Response, error) { var resp *http.Response var err error = nil - - fmt.Fprintf(stderr, "Download start\n") // FIXME: Replace with progress bar if resp, err = http.Get(url); err != nil { return nil, err } if resp.StatusCode >= 400 { return nil, errors.New("Got HTTP status code >= 400: " + resp.Status) } - fmt.Fprintf(stderr, "Download end\n") // FIXME: Replace with progress bar return resp, nil } // Reader with progress bar type progressReader struct { - reader io.ReadCloser // Stream to read from - output io.Writer // Where to send progress bar to - read_total int // Expected stream length (bytes) - read_progress int // How much has been read so far (bytes) + reader io.ReadCloser // Stream to read from + output io.Writer // Where to send progress bar to + read_total int // Expected stream length (bytes) + read_progress int // How much has been read so far (bytes) + last_update int // How many bytes read at least update } func (r *progressReader) Read(p []byte) (n int, err error) { read, err := io.ReadCloser(r.reader).Read(p) - // FIXME: Don't print progress bar on every read r.read_progress += read - fmt.Fprintf(r.output, "%d/%d (%.2f%%)\n", - r.read_progress, - r.read_total, - float64(r.read_progress) / float64(r.read_total) * 100) + + // Only update progress for every 1% read + update_every := int(0.01 * float64(r.read_total)) + if r.read_progress - r.last_update > update_every { + fmt.Fprintf(r.output, "%d/%d (%.0f%%)\n", + r.read_progress, + r.read_total, + float64(r.read_progress) / float64(r.read_total) * 100) + r.last_update = r.read_progress + } + return read, err } func (r *progressReader) Close() error { return io.ReadCloser(r.reader).Close() } func ProgressReader(r io.ReadCloser, size int, output io.Writer) *progressReader { - return &progressReader{r, output, size, 0} + return &progressReader{r, output, size, 0, 0} } diff --git a/server/server.go b/server/server.go index 46060d471a..9c07d42213 100644 --- a/server/server.go +++ b/server/server.go @@ -432,7 +432,7 @@ func (srv *Server) CmdPull(stdin io.ReadCloser, stdout io.Writer, args ...string if err != nil { return err } - fmt.Fprintf(stdout, "Unpacking to %s\n", name) + fmt.Fprintf(stdout, "Downloading and unpacking to %s\n", name) img, err := srv.images.Import(name, archive, nil) if err != nil { return err From b0265e0a38a71fdbebf3b6ef53120249d399e293 Mon Sep 17 00:00:00 2001 From: Charles Hooper Date: Tue, 12 Mar 2013 23:06:07 +0000 Subject: [PATCH 9/9] Remove Curl function from future --- future/future.go | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/future/future.go b/future/future.go index 477fe7c320..0063794e0c 100644 --- a/future/future.go +++ b/future/future.go @@ -8,7 +8,6 @@ import ( "io" "math/rand" "net/http" - "os/exec" "time" ) @@ -87,25 +86,6 @@ func Pv(src io.Reader, info io.Writer) io.Reader { return r } -// Curl makes an http request by executing the unix command 'curl', and returns -// the body of the response. If `stderr` is not nil, a progress bar will be -// written to it. -func Curl(url string, stderr io.Writer) (io.Reader, error) { - curl := exec.Command("curl", "-#", "-L", "--fail", url) - output, err := curl.StdoutPipe() - if err != nil { - return nil, err - } - curl.Stderr = stderr - if err := curl.Start(); err != nil { - return nil, err - } - if err := curl.Wait(); err != nil { - return nil, err - } - return output, nil -} - // Request a given URL and return an io.Reader func Download(url string, stderr io.Writer) (*http.Response, error) { var resp *http.Response