From 1ba11384bf82f824b0efbab31aaca439cfba1b4f Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Tue, 26 Nov 2013 17:46:06 +0000 Subject: [PATCH 1/4] Refactor Opts --- buildfile.go | 2 +- commands.go | 117 ++++++++++------------------------------- docker/docker.go | 24 +++++---- opts.go | 132 +++++++++++++++++++++++++++++++++++++++++++++++ utils.go | 6 +-- utils/utils.go | 12 ----- 6 files changed, 178 insertions(+), 115 deletions(-) create mode 100644 opts.go diff --git a/buildfile.go b/buildfile.go index 4ac8e75bd4..bb84f54027 100644 --- a/buildfile.go +++ b/buildfile.go @@ -241,7 +241,7 @@ func (b *buildFile) CmdVolume(args string) error { volume = []string{args} } if b.config.Volumes == nil { - b.config.Volumes = PathOpts{} + b.config.Volumes = map[string]struct{}{} } for _, v := range volume { b.config.Volumes[v] = struct{}{} diff --git a/commands.go b/commands.go index d992db2e6c..ba91b14923 100644 --- a/commands.go +++ b/commands.go @@ -23,7 +23,6 @@ import ( "os" "os/signal" "path" - "path/filepath" "reflect" "regexp" "runtime" @@ -1635,54 +1634,6 @@ func (cli *DockerCli) CmdSearch(args ...string) error { // Ports type - Used to parse multiple -p flags type ports []int -// AttachOpts stores arguments to 'docker run -a', eg. which streams to attach to -type AttachOpts map[string]bool - -func (opts AttachOpts) String() string { return fmt.Sprintf("%v", map[string]bool(opts)) } -func (opts AttachOpts) Set(val string) error { - if val != "stdin" && val != "stdout" && val != "stderr" { - return fmt.Errorf("Unsupported stream name: %s", val) - } - opts[val] = true - return nil -} - -// LinkOpts stores arguments to `docker run -link` -type LinkOpts []string - -func (link *LinkOpts) String() string { return fmt.Sprintf("%v", []string(*link)) } -func (link *LinkOpts) Set(val string) error { - if _, err := parseLink(val); err != nil { - return err - } - *link = append(*link, val) - return nil -} - -// PathOpts stores a unique set of absolute paths -type PathOpts map[string]struct{} - -func (opts PathOpts) String() string { return fmt.Sprintf("%v", map[string]struct{}(opts)) } -func (opts PathOpts) Set(val string) error { - var containerPath string - - splited := strings.SplitN(val, ":", 2) - if len(splited) == 1 { - containerPath = splited[0] - val = filepath.Clean(splited[0]) - } else { - containerPath = splited[1] - val = fmt.Sprintf("%s:%s", splited[0], filepath.Clean(splited[1])) - } - - if !filepath.IsAbs(containerPath) { - utils.Debugf("%s is not an absolute path", containerPath) - return fmt.Errorf("%s is not an absolute path", containerPath) - } - opts[val] = struct{}{} - return nil -} - func (cli *DockerCli) CmdTag(args ...string) error { cmd := cli.Subcmd("tag", "[OPTIONS] IMAGE REPOSITORY[:TAG]", "Tag an image into a repository") force := cmd.Bool("f", false, "Force") @@ -1728,16 +1679,16 @@ func ParseRun(args []string, capabilities *Capabilities) (*Config, *HostConfig, func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Config, *HostConfig, *flag.FlagSet, error) { var ( // FIXME: use utils.ListOpts for attach and volumes? - flAttach = AttachOpts{} - flVolumes = PathOpts{} - flLinks = LinkOpts{} + flAttach = NewListOpts(ValidateAttach) + flVolumes = NewListOpts(ValidatePath) + flLinks = NewListOpts(ValidateLink) + flEnv = NewListOpts(ValidateEnv) - flPublish utils.ListOpts - flExpose utils.ListOpts - flEnv utils.ListOpts - flDns utils.ListOpts - flVolumesFrom utils.ListOpts - flLxcOpts utils.ListOpts + flPublish ListOpts + flExpose ListOpts + flDns ListOpts + flVolumesFrom ListOpts + flLxcOpts ListOpts flAutoRemove = cmd.Bool("rm", false, "Automatically remove the container when it exits (incompatible with -d)") flDetach = cmd.Bool("d", false, "Detached mode: Run container in the background, print new container id") @@ -1759,13 +1710,13 @@ func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Co _ = cmd.String("name", "", "Assign a name to the container") ) - cmd.Var(flAttach, "a", "Attach to stdin, stdout or stderr.") - cmd.Var(flVolumes, "v", "Bind mount a volume (e.g. from the host: -v /host:/container, from docker: -v /container)") + cmd.Var(&flAttach, "a", "Attach to stdin, stdout or stderr.") + cmd.Var(&flVolumes, "v", "Bind mount a volume (e.g. from the host: -v /host:/container, from docker: -v /container)") cmd.Var(&flLinks, "link", "Add link to another container (name:alias)") + cmd.Var(&flEnv, "e", "Set environment variables") cmd.Var(&flPublish, "p", fmt.Sprintf("Publish a container's port to the host (format: %s) (use 'docker port' to see the actual mapping)", PortSpecTemplateFormat)) cmd.Var(&flExpose, "expose", "Expose a port from the container without publishing it to your host") - cmd.Var(&flEnv, "e", "Set environment variables") cmd.Var(&flDns, "dns", "Set custom dns servers") cmd.Var(&flVolumesFrom, "volumes-from", "Mount volumes from the specified container(s)") cmd.Var(&flLxcOpts, "lxc-conf", "Add custom lxc options -lxc-conf=\"lxc.cgroup.cpuset.cpus = 0,1\"") @@ -1780,7 +1731,7 @@ func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Co } // Validate input params - if *flDetach && len(flAttach) > 0 { + if *flDetach && flAttach.Len() > 0 { return nil, nil, cmd, ErrConflictAttachDetach } if *flWorkingDir != "" && !path.IsAbs(*flWorkingDir) { @@ -1791,7 +1742,7 @@ func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Co } // If neither -d or -a are set, attach to everything by default - if len(flAttach) == 0 && !*flDetach { + if flAttach.Len() == 0 && !*flDetach { if !*flDetach { flAttach.Set("stdout") flAttach.Set("stderr") @@ -1801,17 +1752,6 @@ func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Co } } - var envs []string - for _, env := range flEnv { - arr := strings.Split(env, "=") - if len(arr) > 1 { - envs = append(envs, env) - } else { - v := os.Getenv(env) - envs = append(envs, env+"="+v) - } - } - var flMemory int64 if *flMemoryString != "" { parsedMemory, err := utils.RAMInBytes(*flMemoryString) @@ -1823,16 +1763,15 @@ func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Co var binds []string // add any bind targets to the list of container volumes - for bind := range flVolumes { - arr := strings.Split(bind, ":") - if len(arr) > 1 { + for bind := range flVolumes.GetMap() { + if arr := strings.Split(bind, ":"); len(arr) > 1 { if arr[0] == "/" { return nil, nil, cmd, fmt.Errorf("Invalid bind mount: source can't be '/'") } dstDir := arr[1] - flVolumes[dstDir] = struct{}{} + flVolumes.Set(dstDir) binds = append(binds, bind) - delete(flVolumes, bind) + flVolumes.Delete(bind) } } @@ -1867,13 +1806,13 @@ func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Co domainname = parts[1] } - ports, portBindings, err := parsePortSpecs(flPublish) + ports, portBindings, err := parsePortSpecs(flPublish.GetAll()) if err != nil { return nil, nil, cmd, err } // Merge in exposed ports to the map of published ports - for _, e := range flExpose { + for _, e := range flExpose.GetAll() { if strings.Contains(e, ":") { return nil, nil, cmd, fmt.Errorf("Invalid port format for -expose: %s", e) } @@ -1894,15 +1833,15 @@ func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Co OpenStdin: *flStdin, Memory: flMemory, CpuShares: *flCpuShares, - AttachStdin: flAttach["stdin"], - AttachStdout: flAttach["stdout"], - AttachStderr: flAttach["stderr"], - Env: envs, + AttachStdin: flAttach.Get("stdin"), + AttachStdout: flAttach.Get("stdout"), + AttachStderr: flAttach.Get("stderr"), + Env: flEnv.GetAll(), Cmd: runCmd, - Dns: flDns, + Dns: flDns.GetAll(), Image: image, - Volumes: flVolumes, - VolumesFrom: strings.Join(flVolumesFrom, ","), + Volumes: flVolumes.GetMap(), + VolumesFrom: strings.Join(flVolumesFrom.GetAll(), ","), Entrypoint: entrypoint, WorkingDir: *flWorkingDir, } @@ -1913,7 +1852,7 @@ func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Co LxcConf: lxcConf, Privileged: *flPrivileged, PortBindings: portBindings, - Links: flLinks, + Links: flLinks.GetAll(), PublishAllPorts: *flPublishAll, } diff --git a/docker/docker.go b/docker/docker.go index 83df5c2171..3f9be0a92a 100644 --- a/docker/docker.go +++ b/docker/docker.go @@ -33,26 +33,30 @@ func main() { flRoot := flag.String("g", "/var/lib/docker", "Path to use as the root of the docker runtime") flEnableCors := flag.Bool("api-enable-cors", false, "Enable CORS headers in the remote API") flDns := flag.String("dns", "", "Force docker to use specific DNS servers") - flHosts := utils.ListOpts{fmt.Sprintf("unix://%s", docker.DEFAULTUNIXSOCKET)} - flag.Var(&flHosts, "H", "Multiple tcp://host:port or unix://path/to/socket to bind in daemon mode, single connection otherwise") + flEnableIptables := flag.Bool("iptables", true, "Disable docker's addition of iptables rules") flDefaultIp := flag.String("ip", "0.0.0.0", "Default IP address to use when binding container ports") flInterContainerComm := flag.Bool("icc", true, "Enable inter-container communication") flGraphDriver := flag.String("s", "", "Force the docker runtime to use a specific storage driver") + flHosts := docker.ListOpts{} + flHosts.Set(fmt.Sprintf("unix://%s", docker.DEFAULTUNIXSOCKET)) + flag.Var(&flHosts, "H", "Multiple tcp://host:port or unix://path/to/socket to bind in daemon mode, single connection otherwise") + flag.Parse() if *flVersion { showVersion() return } - if len(flHosts) > 1 { - flHosts = flHosts[1:] //trick to display a nice default value in the usage - } - for i, flHost := range flHosts { + // if flHosts.Len() > 1 { + // flHosts = flHosts[1:] //trick to display a nice default value in the usage + // } + for _, flHost := range flHosts.GetAll() { host, err := utils.ParseHost(docker.DEFAULTHTTPHOST, docker.DEFAULTHTTPPORT, flHost) if err == nil { - flHosts[i] = host + flHosts.Set(host) + flHosts.Delete(flHost) } else { log.Fatal(err) } @@ -88,16 +92,16 @@ func main() { log.Fatal(err) } // Serve api - job = eng.Job("serveapi", flHosts...) + job = eng.Job("serveapi", flHosts.GetAll()...) job.SetenvBool("Logging", true) if err := job.Run(); err != nil { log.Fatal(err) } } else { - if len(flHosts) > 1 { + if flHosts.Len() > 1 { log.Fatal("Please specify only one -H") } - protoAddrParts := strings.SplitN(flHosts[0], "://", 2) + protoAddrParts := strings.SplitN(flHosts.GetAll()[0], "://", 2) if err := docker.ParseCommands(protoAddrParts[0], protoAddrParts[1], flag.Args()...); err != nil { if sterr, ok := err.(*utils.StatusError); ok { os.Exit(sterr.Status) diff --git a/opts.go b/opts.go new file mode 100644 index 0000000000..826091bea6 --- /dev/null +++ b/opts.go @@ -0,0 +1,132 @@ +package docker + +import ( + "fmt" + "github.com/dotcloud/docker/utils" + "os" + "path/filepath" + "strings" +) + +// ListOpts type +type ListOpts struct { + values []string + validator ValidatorFctType +} + +func NewListOpts(validator ValidatorFctType) ListOpts { + return ListOpts{ + validator: validator, + } +} + +func (opts *ListOpts) String() string { + return fmt.Sprintf("%v", []string(opts.values)) +} + +// Set validates if needed the input value and add it to the +// internal slice. +func (opts *ListOpts) Set(value string) error { + if opts.validator != nil { + v, err := opts.validator(value) + if err != nil { + return err + } + value = v + } + opts.values = append(opts.values, value) + return nil +} + +// Delete remove the given element from the slice. +func (opts *ListOpts) Delete(key string) { + for i, k := range opts.values { + if k == key { + opts.values = append(opts.values[:i], opts.values[i+1:]...) + return + } + } +} + +// GetMap returns the content of values in a map in order to avoid +// duplicates. +// FIXME: can we remove this? +func (opts *ListOpts) GetMap() map[string]struct{} { + ret := make(map[string]struct{}) + for _, k := range opts.values { + ret[k] = struct{}{} + } + return ret +} + +// GetAll returns the values' slice. +// FIXME: Can we remove this? +func (opts *ListOpts) GetAll() []string { + return opts.values +} + +// Get checks the existence of the given key. +func (opts *ListOpts) Get(key string) bool { + for _, k := range opts.values { + if k == key { + return true + } + } + return false +} + +// Len returns the amount of element in the slice. +func (opts *ListOpts) Len() int { + return len(opts.values) +} + +// Validators +type ValidatorFctType func(val string) (string, error) + +func ValidateAttach(val string) (string, error) { + if val != "stdin" && val != "stdout" && val != "stderr" { + return val, fmt.Errorf("Unsupported stream name: %s", val) + } + return val, nil +} + +func ValidateLink(val string) (string, error) { + if _, err := parseLink(val); err != nil { + return val, err + } + return val, nil +} + +func ValidatePath(val string) (string, error) { + var containerPath string + + splited := strings.SplitN(val, ":", 2) + if len(splited) == 1 { + containerPath = splited[0] + val = filepath.Clean(splited[0]) + } else { + containerPath = splited[1] + val = fmt.Sprintf("%s:%s", splited[0], filepath.Clean(splited[1])) + } + + if !filepath.IsAbs(containerPath) { + return val, fmt.Errorf("%s is not an absolute path", containerPath) + } + return val, nil +} + +func ValidateEnv(val string) (string, error) { + arr := strings.Split(val, "=") + if len(arr) > 1 { + return val, nil + } + return fmt.Sprintf("%s=%s", val, os.Getenv(val)), nil +} + +func ValidateHost(val string) (string, error) { + host, err := utils.ParseHost(DEFAULTHTTPHOST, DEFAULTHTTPPORT, val) + if err != nil { + return val, err + } + return host, nil +} diff --git a/utils.go b/utils.go index f62e46104c..d723affbfe 100644 --- a/utils.go +++ b/utils.go @@ -215,9 +215,9 @@ func MergeConfig(userConf, imageConf *Config) error { return nil } -func parseLxcConfOpts(opts utils.ListOpts) ([]KeyValuePair, error) { - out := make([]KeyValuePair, len(opts)) - for i, o := range opts { +func parseLxcConfOpts(opts ListOpts) ([]KeyValuePair, error) { + out := make([]KeyValuePair, opts.Len()) + for i, o := range opts.GetAll() { k, v, err := parseLxcOpt(o) if err != nil { return nil, err diff --git a/utils/utils.go b/utils/utils.go index cfdc73bb2e..5eeaec6659 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -34,18 +34,6 @@ type Fataler interface { Fatal(args ...interface{}) } -// ListOpts type -type ListOpts []string - -func (opts *ListOpts) String() string { - return fmt.Sprint(*opts) -} - -func (opts *ListOpts) Set(value string) error { - *opts = append(*opts, value) - return nil -} - // Go is a basic promise implementation: it wraps calls a function in a goroutine, // and returns a channel which will later return the function's return value. func Go(f func() error) chan error { From 1beb5005d11b8fadd04e51c4a1341779d26a475d Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Tue, 26 Nov 2013 17:47:58 +0000 Subject: [PATCH 2/4] Format main() --- docker/docker.go | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/docker/docker.go b/docker/docker.go index 3f9be0a92a..43b93dc2f4 100644 --- a/docker/docker.go +++ b/docker/docker.go @@ -23,23 +23,24 @@ func main() { sysinit.SysInit() return } - // FIXME: Switch d and D ? (to be more sshd like) - flVersion := flag.Bool("v", false, "Print version information and quit") - flDaemon := flag.Bool("d", false, "Enable daemon mode") - flDebug := flag.Bool("D", false, "Enable debug mode") - flAutoRestart := flag.Bool("r", true, "Restart previously running containers") - bridgeName := flag.String("b", "", "Attach containers to a pre-existing network bridge; use 'none' to disable container networking") - pidfile := flag.String("p", "/var/run/docker.pid", "Path to use for daemon PID file") - flRoot := flag.String("g", "/var/lib/docker", "Path to use as the root of the docker runtime") - flEnableCors := flag.Bool("api-enable-cors", false, "Enable CORS headers in the remote API") - flDns := flag.String("dns", "", "Force docker to use specific DNS servers") - flEnableIptables := flag.Bool("iptables", true, "Disable docker's addition of iptables rules") - flDefaultIp := flag.String("ip", "0.0.0.0", "Default IP address to use when binding container ports") - flInterContainerComm := flag.Bool("icc", true, "Enable inter-container communication") - flGraphDriver := flag.String("s", "", "Force the docker runtime to use a specific storage driver") + var ( + flVersion = flag.Bool("v", false, "Print version information and quit") + flDaemon = flag.Bool("d", false, "Enable daemon mode") + flDebug = flag.Bool("D", false, "Enable debug mode") + flAutoRestart = flag.Bool("r", true, "Restart previously running containers") + bridgeName = flag.String("b", "", "Attach containers to a pre-existing network bridge; use 'none' to disable container networking") + pidfile = flag.String("p", "/var/run/docker.pid", "Path to use for daemon PID file") + flRoot = flag.String("g", "/var/lib/docker", "Path to use as the root of the docker runtime") + flEnableCors = flag.Bool("api-enable-cors", false, "Enable CORS headers in the remote API") + flDns = flag.String("dns", "", "Force docker to use specific DNS servers") + flEnableIptables = flag.Bool("iptables", true, "Disable docker's addition of iptables rules") + flDefaultIp = flag.String("ip", "0.0.0.0", "Default IP address to use when binding container ports") + flInterContainerComm = flag.Bool("icc", true, "Enable inter-container communication") + flGraphDriver = flag.String("s", "", "Force the docker runtime to use a specific storage driver") - flHosts := docker.ListOpts{} + flHosts = docker.ListOpts{} + ) flHosts.Set(fmt.Sprintf("unix://%s", docker.DEFAULTUNIXSOCKET)) flag.Var(&flHosts, "H", "Multiple tcp://host:port or unix://path/to/socket to bind in daemon mode, single connection otherwise") From 5e3f6e7023fbd0cfd1233a99c332801755340cfb Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Tue, 26 Nov 2013 17:50:43 +0000 Subject: [PATCH 3/4] Change the default Host affectation to not rely on slice --- docker/docker.go | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/docker/docker.go b/docker/docker.go index 43b93dc2f4..2e1f2beed8 100644 --- a/docker/docker.go +++ b/docker/docker.go @@ -38,10 +38,8 @@ func main() { flDefaultIp = flag.String("ip", "0.0.0.0", "Default IP address to use when binding container ports") flInterContainerComm = flag.Bool("icc", true, "Enable inter-container communication") flGraphDriver = flag.String("s", "", "Force the docker runtime to use a specific storage driver") - - flHosts = docker.ListOpts{} + flHosts = docker.NewListOpts(docker.ValidateHost) ) - flHosts.Set(fmt.Sprintf("unix://%s", docker.DEFAULTUNIXSOCKET)) flag.Var(&flHosts, "H", "Multiple tcp://host:port or unix://path/to/socket to bind in daemon mode, single connection otherwise") flag.Parse() @@ -50,17 +48,9 @@ func main() { showVersion() return } - // if flHosts.Len() > 1 { - // flHosts = flHosts[1:] //trick to display a nice default value in the usage - // } - for _, flHost := range flHosts.GetAll() { - host, err := utils.ParseHost(docker.DEFAULTHTTPHOST, docker.DEFAULTHTTPPORT, flHost) - if err == nil { - flHosts.Set(host) - flHosts.Delete(flHost) - } else { - log.Fatal(err) - } + if flHosts.Len() == 0 { + // If we do not have a host, default to unix socket + flHosts.Set(fmt.Sprintf("unix://%s", docker.DEFAULTUNIXSOCKET)) } if *flDebug { From 2bbc90e92ff140c29891e92c9ce320ca7d19cd57 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Thu, 28 Nov 2013 12:24:04 -0800 Subject: [PATCH 4/4] Make volumes opts more strict --- opts.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/opts.go b/opts.go index 826091bea6..80785a5161 100644 --- a/opts.go +++ b/opts.go @@ -100,6 +100,10 @@ func ValidateLink(val string) (string, error) { func ValidatePath(val string) (string, error) { var containerPath string + if strings.Count(val, ":") > 2 { + return val, fmt.Errorf("bad format for volumes: %s", val) + } + splited := strings.SplitN(val, ":", 2) if len(splited) == 1 { containerPath = splited[0]