From 1132d26d479cb1cbcf88c108ee03467bdbfa7e46 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Thu, 16 Jul 2015 13:23:48 +0100 Subject: [PATCH 1/4] [proxy] do not weave attach containers in '--net=host|container:' --- proxy/create_container_interceptor.go | 2 +- proxy/create_exec_interceptor.go | 2 +- proxy/proxy.go | 7 ++++++- proxy/start_container_interceptor.go | 2 +- test/660_proxy_ipam_test.sh | 8 ++++++++ 5 files changed, 17 insertions(+), 4 deletions(-) diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index b7d8d0c2..db365c61 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -46,7 +46,7 @@ func (i *createContainerInterceptor) InterceptRequest(r *http.Request) error { return err } - if cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config); ok { + if cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig); ok { Info.Printf("Creating container with WEAVE_CIDR \"%s\"", strings.Join(cidrs, " ")) if container.HostConfig == nil { container.HostConfig = &docker.HostConfig{} diff --git a/proxy/create_exec_interceptor.go b/proxy/create_exec_interceptor.go index 33e655d9..d8a15815 100644 --- a/proxy/create_exec_interceptor.go +++ b/proxy/create_exec_interceptor.go @@ -30,7 +30,7 @@ func (i *createExecInterceptor) InterceptRequest(r *http.Request) error { } _, hasWeaveWait := container.Volumes["/w"] - cidrs, hasCIDRs := i.proxy.weaveCIDRsFromConfig(container.Config) + cidrs, hasCIDRs := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) if hasCIDRs && hasWeaveWait { Info.Printf("Exec in container %s with WEAVE_CIDR \"%s\"", container.ID, strings.Join(cidrs, " ")) cmd := append(weaveWaitEntrypoint, "-s") diff --git a/proxy/proxy.go b/proxy/proxy.go index c391431f..8964b074 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -115,7 +115,12 @@ func (proxy *Proxy) ListenAndServe() error { return (&http.Server{Handler: proxy}).Serve(listener) } -func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config) ([]string, bool) { +func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config, hostConfig *docker.HostConfig) ([]string, bool) { + if hostConfig != nil && + hostConfig.NetworkMode != "" && + hostConfig.NetworkMode != "bridge" { + return nil, false + } for _, e := range config.Env { if strings.HasPrefix(e, "WEAVE_CIDR=") { if e[11:] == "none" { diff --git a/proxy/start_container_interceptor.go b/proxy/start_container_interceptor.go index 4d54680d..ece4069f 100644 --- a/proxy/start_container_interceptor.go +++ b/proxy/start_container_interceptor.go @@ -21,7 +21,7 @@ func (i *startContainerInterceptor) InterceptResponse(r *http.Response) error { return err } - cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config) + cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) if !ok { Debug.Print("No Weave CIDR, ignoring") return nil diff --git a/test/660_proxy_ipam_test.sh b/test/660_proxy_ipam_test.sh index 11137995..ed1e589b 100755 --- a/test/660_proxy_ipam_test.sh +++ b/test/660_proxy_ipam_test.sh @@ -26,13 +26,21 @@ start $HOST1 --name=auto start $HOST1 --name=none -e WEAVE_CIDR=none start $HOST2 --name=zero -e WEAVE_CIDR= start $HOST2 --name=no-default +start $HOST1 --name=bridge --net=bridge +start $HOST1 --name=host --net=host +start $HOST1 --name=other --net=container:auto AUTO=$(container_ip $HOST1 auto) ZERO=$(container_ip $HOST2 zero) +BRIDGE=$(container_ip $HOST1 bridge) +OTHER=$(container_ip $HOST1 other) assert_raises "proxy exec_on $HOST1 auto $PING $ZERO" assert_raises "proxy exec_on $HOST2 zero $PING $AUTO" +assert_raises "proxy exec_on $HOST2 zero $PING $BRIDGE" +assert_raises "proxy exec_on $HOST2 zero $PING $OTHER" assert_no_ethwe $HOST1 none assert_no_ethwe $HOST2 no-default +assert_no_ethwe $HOST1 host end_suite From c13154eea9ce83256ee75c0842088706709c973a Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Thu, 16 Jul 2015 13:26:47 +0100 Subject: [PATCH 2/4] updating comment as per golint --- proxy/create_container_interceptor.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index db365c61..da038b21 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -23,9 +23,9 @@ type createContainerRequestBody struct { MacAddress string `json:"MacAddress,omitempty" yaml:"MacAddress,omitempty"` } -// Replacement for docker.NoSuchImage, which does not contain the -// image name, which in turn breaks docker clients post 1.7.0 since -// they expect the image name to be present in errors. +// ErrNoSuchImage replaces docker.NoSuchImage, which does not contain the image +// name, which in turn breaks docker clients post 1.7.0 since they expect the +// image name to be present in errors. type ErrNoSuchImage struct { Name string } From 9619a27f8c042d2f0c8987c9a57fa9c8a08a9a9b Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Thu, 16 Jul 2015 13:57:00 +0100 Subject: [PATCH 3/4] [proxy] adding log message for ignored containers --- proxy/proxy.go | 15 ++++++++++++--- proxy/start_container_interceptor.go | 4 ++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/proxy/proxy.go b/proxy/proxy.go index 8964b074..2e29b844 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -116,9 +116,7 @@ func (proxy *Proxy) ListenAndServe() error { } func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config, hostConfig *docker.HostConfig) ([]string, bool) { - if hostConfig != nil && - hostConfig.NetworkMode != "" && - hostConfig.NetworkMode != "bridge" { + if !validNetworkMode(hostConfig) { return nil, false } for _, e := range config.Env { @@ -131,3 +129,14 @@ func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config, hostConfig *dock } return nil, !proxy.NoDefaultIPAM } + +func validNetworkMode(hostConfig *docker.HostConfig) bool { + return networkMode(hostConfig) == "bridge" +} + +func networkMode(hostConfig *docker.HostConfig) string { + if hostConfig == nil || hostConfig.NetworkMode == "" { + return "bridge" + } + return hostConfig.NetworkMode +} diff --git a/proxy/start_container_interceptor.go b/proxy/start_container_interceptor.go index ece4069f..b020fd34 100644 --- a/proxy/start_container_interceptor.go +++ b/proxy/start_container_interceptor.go @@ -21,6 +21,10 @@ func (i *startContainerInterceptor) InterceptResponse(r *http.Response) error { return err } + if !validNetworkMode(container.HostConfig) { + Debug.Printf("Ignoring container %s with --net=%s", container.ID, networkMode(container.HostConfig)) + return nil + } cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) if !ok { Debug.Print("No Weave CIDR, ignoring") From c6fa9cb17edc6614d51ac55f8b0a2d1a047ae318 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Thu, 16 Jul 2015 14:42:20 +0100 Subject: [PATCH 4/4] refactor weaveCIDRsFromConfig to return an error message --- proxy/create_container_interceptor.go | 4 +++- proxy/create_exec_interceptor.go | 6 ++++-- proxy/proxy.go | 31 +++++++++++++-------------- proxy/start_container_interceptor.go | 10 +++------ 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index da038b21..574b5c01 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -46,7 +46,9 @@ func (i *createContainerInterceptor) InterceptRequest(r *http.Request) error { return err } - if cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig); ok { + if cidrs, err := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig); err != nil { + Info.Printf("Ignoring container due to %s", err) + } else { Info.Printf("Creating container with WEAVE_CIDR \"%s\"", strings.Join(cidrs, " ")) if container.HostConfig == nil { container.HostConfig = &docker.HostConfig{} diff --git a/proxy/create_exec_interceptor.go b/proxy/create_exec_interceptor.go index d8a15815..f15ed56f 100644 --- a/proxy/create_exec_interceptor.go +++ b/proxy/create_exec_interceptor.go @@ -30,8 +30,10 @@ func (i *createExecInterceptor) InterceptRequest(r *http.Request) error { } _, hasWeaveWait := container.Volumes["/w"] - cidrs, hasCIDRs := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) - if hasCIDRs && hasWeaveWait { + cidrs, err := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) + if err != nil { + Info.Printf("Ignoring container %s due to %s", container.ID, err) + } else if hasWeaveWait { Info.Printf("Exec in container %s with WEAVE_CIDR \"%s\"", container.ID, strings.Join(cidrs, " ")) cmd := append(weaveWaitEntrypoint, "-s") options.Cmd = append(cmd, options.Cmd...) diff --git a/proxy/proxy.go b/proxy/proxy.go index 2e29b844..d454a06c 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -2,6 +2,7 @@ package proxy import ( "crypto/tls" + "errors" "net" "net/http" "net/url" @@ -22,6 +23,10 @@ var ( containerCreateRegexp = regexp.MustCompile("^(/v[0-9\\.]*)?/containers/create$") containerStartRegexp = regexp.MustCompile("^(/v[0-9\\.]*)?/containers/[^/]*/(re)?start$") execCreateRegexp = regexp.MustCompile("^(/v[0-9\\.]*)?/containers/[^/]*/exec$") + + ErrInvalidNetworkMode = errors.New("--net option") + ErrWeaveCIDRNone = errors.New("WEAVE_CIDR=none") + ErrNoDefaultIPAM = errors.New("--no-default-ipam option") ) type Proxy struct { @@ -115,28 +120,22 @@ func (proxy *Proxy) ListenAndServe() error { return (&http.Server{Handler: proxy}).Serve(listener) } -func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config, hostConfig *docker.HostConfig) ([]string, bool) { - if !validNetworkMode(hostConfig) { - return nil, false +func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config, hostConfig *docker.HostConfig) ([]string, error) { + if hostConfig != nil && + hostConfig.NetworkMode != "" && + hostConfig.NetworkMode != "bridge" { + return nil, ErrInvalidNetworkMode } for _, e := range config.Env { if strings.HasPrefix(e, "WEAVE_CIDR=") { if e[11:] == "none" { - return nil, false + return nil, ErrWeaveCIDRNone } - return strings.Fields(e[11:]), true + return strings.Fields(e[11:]), nil } } - return nil, !proxy.NoDefaultIPAM -} - -func validNetworkMode(hostConfig *docker.HostConfig) bool { - return networkMode(hostConfig) == "bridge" -} - -func networkMode(hostConfig *docker.HostConfig) string { - if hostConfig == nil || hostConfig.NetworkMode == "" { - return "bridge" + if proxy.NoDefaultIPAM { + return nil, ErrNoDefaultIPAM } - return hostConfig.NetworkMode + return nil, nil } diff --git a/proxy/start_container_interceptor.go b/proxy/start_container_interceptor.go index b020fd34..803ca869 100644 --- a/proxy/start_container_interceptor.go +++ b/proxy/start_container_interceptor.go @@ -21,13 +21,9 @@ func (i *startContainerInterceptor) InterceptResponse(r *http.Response) error { return err } - if !validNetworkMode(container.HostConfig) { - Debug.Printf("Ignoring container %s with --net=%s", container.ID, networkMode(container.HostConfig)) - return nil - } - cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) - if !ok { - Debug.Print("No Weave CIDR, ignoring") + cidrs, err := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) + if err != nil { + Info.Printf("Ignoring container %s due to %s", container.ID, err) return nil } Info.Printf("Attaching container %s with WEAVE_CIDR \"%s\" to weave network", container.ID, strings.Join(cidrs, " "))