From 2eba08c4b7ed08233d3c090950e2ede4b059ecfe Mon Sep 17 00:00:00 2001 From: Steeve Morin Date: Thu, 1 Aug 2013 02:42:22 +0200 Subject: [PATCH 1/3] Handle ip route showing mask-less IP addresses Sometimes `ip route` will show mask-less IPs, so net.ParseCIDR will fail. If it does we check if we can net.ParseIP, and fail only if we can't. Fixes #1214 Fixes #362 Upstream-commit: 2e72882216ce13169a578614202830a5b084bfb4 Component: engine --- components/engine/network.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/components/engine/network.go b/components/engine/network.go index 2e2dc7785c..daffbfcde4 100644 --- a/components/engine/network.go +++ b/components/engine/network.go @@ -104,7 +104,11 @@ func checkRouteOverlaps(dockerNetwork *net.IPNet) error { continue } if _, network, err := net.ParseCIDR(strings.Split(line, " ")[0]); err != nil { - return fmt.Errorf("Unexpected ip route output: %s (%s)", err, line) + // is this a mask-less IP address? + if ip := net.ParseIP(strings.Split(line, " ")[0]); ip == nil { + // fail only if it's neither a network nor a mask-less IP address + return fmt.Errorf("Unexpected ip route output: %s (%s)", err, line) + } } else if networkOverlaps(dockerNetwork, network) { return fmt.Errorf("Network %s is already routed: '%s'", dockerNetwork.String(), line) } From 55962b1db1a3c6b3b8beaa51424e8e27f414096d Mon Sep 17 00:00:00 2001 From: Daniel Mizyrycki Date: Mon, 29 Jul 2013 09:45:19 -0700 Subject: [PATCH 2/3] testing, issue #1331: Add registry functional test to docker-ci Upstream-commit: 2e7df5182cc94e3699ebc1031e89b1605f9d42f9 Component: engine --- components/engine/testing/README.rst | 4 ++++ components/engine/testing/Vagrantfile | 4 +++- .../engine/testing/buildbot/credentials.cfg | 5 +++++ components/engine/testing/buildbot/master.cfg | 20 +++++++++++++++++-- .../engine/testing/buildbot/requirements.txt | 1 + .../testing/buildbot/setup_credentials.sh | 17 ++++++++++++++++ .../testing/functionaltests/test_registry.sh | 11 ++++++++++ 7 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 components/engine/testing/buildbot/credentials.cfg create mode 100755 components/engine/testing/buildbot/setup_credentials.sh create mode 100755 components/engine/testing/functionaltests/test_registry.sh diff --git a/components/engine/testing/README.rst b/components/engine/testing/README.rst index 3b11092f9f..ce5aa837a4 100644 --- a/components/engine/testing/README.rst +++ b/components/engine/testing/README.rst @@ -40,6 +40,10 @@ Deployment export SMTP_USER=xxxxxxxxxxxx export SMTP_PWD=xxxxxxxxxxxx + # Define docker registry functional test credentials + export REGISTRY_USER=xxxxxxxxxxxx + export REGISTRY_PWD=xxxxxxxxxxxx + # Checkout docker git clone git://github.com/dotcloud/docker.git diff --git a/components/engine/testing/Vagrantfile b/components/engine/testing/Vagrantfile index 47257201dc..e76a951508 100644 --- a/components/engine/testing/Vagrantfile +++ b/components/engine/testing/Vagrantfile @@ -29,7 +29,9 @@ Vagrant::Config.run do |config| "chown #{USER}.#{USER} /data; cd /data; " \ "#{CFG_PATH}/setup.sh #{USER} #{CFG_PATH} #{ENV['BUILDBOT_PWD']} " \ "#{ENV['IRC_PWD']} #{ENV['IRC_CHANNEL']} #{ENV['SMTP_USER']} " \ - "#{ENV['SMTP_PWD']} #{ENV['EMAIL_RCP']}; " + "#{ENV['SMTP_PWD']} #{ENV['EMAIL_RCP']}; " \ + "#{CFG_PATH}/setup_credentials.sh #{USER} " \ + "#{ENV['REGISTRY_USER']} #{ENV['REGISTRY_PWD']}; " # Install docker dependencies pkg_cmd << "apt-get install -q -y python-software-properties; " \ "add-apt-repository -y ppa:dotcloud/docker-golang/ubuntu; apt-get update -qq; " \ diff --git a/components/engine/testing/buildbot/credentials.cfg b/components/engine/testing/buildbot/credentials.cfg new file mode 100644 index 0000000000..fbdd35d578 --- /dev/null +++ b/components/engine/testing/buildbot/credentials.cfg @@ -0,0 +1,5 @@ +# Credentials for tests. Buildbot source this file on tests +# when needed. + +# Docker registry credentials. Format: 'username:password' +export DOCKER_CREDS='' diff --git a/components/engine/testing/buildbot/master.cfg b/components/engine/testing/buildbot/master.cfg index 61912808ec..29926dbe5f 100644 --- a/components/engine/testing/buildbot/master.cfg +++ b/components/engine/testing/buildbot/master.cfg @@ -19,6 +19,7 @@ TEST_USER = 'buildbot' # Credential to authenticate build triggers TEST_PWD = 'docker' # Credential to authenticate build triggers BUILDER_NAME = 'docker' GITHUB_DOCKER = 'github.com/dotcloud/docker' +BUILDBOT_PATH = '/data/buildbot' DOCKER_PATH = '/data/docker' BUILDER_PATH = '/data/buildbot/slave/{0}/build'.format(BUILDER_NAME) DOCKER_BUILD_PATH = BUILDER_PATH + '/src/github.com/dotcloud/docker' @@ -41,16 +42,19 @@ c['db'] = {'db_url':"sqlite:///state.sqlite"} c['slaves'] = [BuildSlave('buildworker', BUILDBOT_PWD)] c['slavePortnum'] = PORT_MASTER + # Schedulers c['schedulers'] = [ForceScheduler(name='trigger', builderNames=[BUILDER_NAME, - 'coverage'])] + 'registry','coverage'])] c['schedulers'] += [SingleBranchScheduler(name="all", change_filter=filter.ChangeFilter(branch='master'), treeStableTimer=None, builderNames=[BUILDER_NAME])] -c['schedulers'] += [Nightly(name='daily', branch=None, builderNames=['coverage'], +c['schedulers'] += [Nightly(name='daily', branch=None, builderNames=['coverage','registry'], hour=0, minute=30)] + # Builders +# Docker commit test factory = BuildFactory() factory.addStep(ShellCommand(description='Docker',logEnviron=False,usePTY=True, command=["sh", "-c", Interpolate("cd ..; rm -rf build; export GOPATH={0}; " @@ -58,6 +62,7 @@ factory.addStep(ShellCommand(description='Docker',logEnviron=False,usePTY=True, "go test -v".format(BUILDER_PATH,GITHUB_DOCKER,DOCKER_BUILD_PATH))])) c['builders'] = [BuilderConfig(name=BUILDER_NAME,slavenames=['buildworker'], factory=factory)] + # Docker coverage test coverage_cmd = ('GOPATH=`pwd` go get -d github.com/dotcloud/docker\n' 'GOPATH=`pwd` go get github.com/axw/gocov/gocov\n' @@ -69,6 +74,17 @@ factory.addStep(ShellCommand(description='Coverage',logEnviron=False,usePTY=True c['builders'] += [BuilderConfig(name='coverage',slavenames=['buildworker'], factory=factory)] +# Registry Functionaltest builder +factory = BuildFactory() +factory.addStep(ShellCommand(description='registry', logEnviron=False, + command='. {0}/master/credentials.cfg; ' + '{1}/testing/functionaltests/test_registry.sh'.format(BUILDBOT_PATH, + DOCKER_PATH), usePTY=True)) + +c['builders'] += [BuilderConfig(name='registry',slavenames=['buildworker'], + factory=factory)] + + # Status authz_cfg = authz.Authz(auth=auth.BasicAuth([(TEST_USER, TEST_PWD)]), forceBuild='auth') diff --git a/components/engine/testing/buildbot/requirements.txt b/components/engine/testing/buildbot/requirements.txt index 0e451b017d..4e183ba062 100644 --- a/components/engine/testing/buildbot/requirements.txt +++ b/components/engine/testing/buildbot/requirements.txt @@ -4,3 +4,4 @@ buildbot==0.8.7p1 buildbot_slave==0.8.7p1 nose==1.2.1 requests==1.1.0 +flask==0.10.1 diff --git a/components/engine/testing/buildbot/setup_credentials.sh b/components/engine/testing/buildbot/setup_credentials.sh new file mode 100755 index 0000000000..f093815d60 --- /dev/null +++ b/components/engine/testing/buildbot/setup_credentials.sh @@ -0,0 +1,17 @@ +#!/bin/bash + +# Setup of test credentials. Called by Vagrantfile +export PATH="/bin:sbin:/usr/bin:/usr/sbin:/usr/local/bin" + +USER=$1 +REGISTRY_USER=$2 +REGISTRY_PWD=$3 + +BUILDBOT_PATH="/data/buildbot" +DOCKER_PATH="/data/docker" + +function run { su $USER -c "$1"; } + +run "cp $DOCKER_PATH/testing/buildbot/credentials.cfg $BUILDBOT_PATH/master" +cd $BUILDBOT_PATH/master +run "sed -i -E 's#(export DOCKER_CREDS=).+#\1\"$REGISTRY_USER:$REGISTRY_PWD\"#' credentials.cfg" diff --git a/components/engine/testing/functionaltests/test_registry.sh b/components/engine/testing/functionaltests/test_registry.sh new file mode 100755 index 0000000000..095a731631 --- /dev/null +++ b/components/engine/testing/functionaltests/test_registry.sh @@ -0,0 +1,11 @@ +#!/bin/sh + +# Cleanup +rm -rf docker-registry + +# Get latest docker registry +git clone https://github.com/dotcloud/docker-registry.git + +# Configure and run registry tests +cd docker-registry; cp config_sample.yml config.yml +cd test; python -m unittest workflow From ce7a658e04b8465e50d5d463b69b30a2deb2fe6e Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Thu, 1 Aug 2013 18:12:39 -0700 Subject: [PATCH 3/3] Make sure the routes IP are taken into consideration + add unit test for network overlap detection Upstream-commit: f5a8e90d101cd2dbb4ce19543ed15fff48579877 Component: engine --- components/engine/network.go | 31 ++++++++++++++++++++----------- components/engine/network_test.go | 19 +++++++++++++++++++ 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/components/engine/network.go b/components/engine/network.go index daffbfcde4..eefd36df3b 100644 --- a/components/engine/network.go +++ b/components/engine/network.go @@ -93,24 +93,29 @@ func iptables(args ...string) error { return nil } -func checkRouteOverlaps(dockerNetwork *net.IPNet) error { - output, err := ip("route") - if err != nil { - return err - } - utils.Debugf("Routes:\n\n%s", output) - for _, line := range strings.Split(output, "\n") { +func checkRouteOverlaps(routes string, dockerNetwork *net.IPNet) error { + utils.Debugf("Routes:\n\n%s", routes) + for _, line := range strings.Split(routes, "\n") { if strings.Trim(line, "\r\n\t ") == "" || strings.Contains(line, "default") { continue } - if _, network, err := net.ParseCIDR(strings.Split(line, " ")[0]); err != nil { + _, network, err := net.ParseCIDR(strings.Split(line, " ")[0]) + if err != nil { // is this a mask-less IP address? if ip := net.ParseIP(strings.Split(line, " ")[0]); ip == nil { // fail only if it's neither a network nor a mask-less IP address return fmt.Errorf("Unexpected ip route output: %s (%s)", err, line) + } else { + _, network, err = net.ParseCIDR(ip.String() + "/32") + if err != nil { + return err + } + } + } + if err == nil && network != nil { + if networkOverlaps(dockerNetwork, network) { + return fmt.Errorf("Network %s is already routed: '%s'", dockerNetwork, line) } - } else if networkOverlaps(dockerNetwork, network) { - return fmt.Errorf("Network %s is already routed: '%s'", dockerNetwork.String(), line) } } return nil @@ -146,7 +151,11 @@ func CreateBridgeIface(ifaceName string) error { if err != nil { return err } - if err := checkRouteOverlaps(dockerNetwork); err == nil { + routes, err := ip("route") + if err != nil { + return err + } + if err := checkRouteOverlaps(routes, dockerNetwork); err == nil { ifaceAddr = addr break } else { diff --git a/components/engine/network_test.go b/components/engine/network_test.go index 8e6eaad773..bd3a16a1be 100644 --- a/components/engine/network_test.go +++ b/components/engine/network_test.go @@ -383,3 +383,22 @@ func TestNetworkOverlaps(t *testing.T) { //netX starts and ends before netY AssertNoOverlap("172.16.1.1/25", "172.16.2.1/24", t) } + +func TestCheckRouteOverlaps(t *testing.T) { + routes := `default via 10.0.2.2 dev eth0 +10.0.2.0 dev eth0 proto kernel scope link src 10.0.2.15 +10.0.3.0/24 dev lxcbr0 proto kernel scope link src 10.0.3.1 +10.0.42.0/24 dev testdockbr0 proto kernel scope link src 10.0.42.1 +172.16.42.0/24 dev docker0 proto kernel scope link src 172.16.42.1 +192.168.142.0/24 dev eth1 proto kernel scope link src 192.168.142.142` + + _, netX, _ := net.ParseCIDR("172.16.0.1/24") + if err := checkRouteOverlaps(routes, netX); err != nil { + t.Fatal(err) + } + + _, netX, _ = net.ParseCIDR("10.0.2.0/24") + if err := checkRouteOverlaps(routes, netX); err == nil { + t.Fatalf("10.0.2.0/24 and 10.0.2.0 should overlap but it doesn't") + } +}