diff mbox

Don't enable networking as a side-effect of DEBUG=1

Message ID 20170712162550.8061-1-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé July 12, 2017, 4:25 p.m. UTC
When trying to debug problems with tests it is natural to set
DEBUG=1 when starting the docker environment. Unfortunately
this has a side-effect of enabling an eth0 network interface
in the container, which changes the operating environment of
the test suite. IOW tests with fail may suddenly start
working again if DEBUG=1 is set, due to changed network setup.

Add a separate NETWORK=1 option to allow enablement of
networking separately from DEBUG=1, since common debugging
tasks probably don't require networking anyway.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 tests/docker/Makefile.include | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé July 12, 2017, 9:46 p.m. UTC | #1
Hi Alex, Fam,

I wanted to try this patch but got:

$ make docker-test-quick@centos6 NETWORK=1
   BUILD   centos6
The command '/bin/sh -c yum install -y epel-release' returned a non-zero 
code: 139
Traceback (most recent call last):
   File "./tests/docker/docker.py", line 382, in <module>
     sys.exit(main())
   File "./tests/docker/docker.py", line 379, in main
     return args.cmdobj.run(args, argv)
   File "./tests/docker/docker.py", line 301, in run
     extra_files_cksum=cksum)
   File "./tests/docker/docker.py", line 185, in build_image
     quiet=quiet)
   File "./tests/docker/docker.py", line 123, in _do_check
     return subprocess.check_call(self._command + cmd, **kwargs)
   File "/usr/lib/python2.7/subprocess.py", line 186, in check_call
     raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['docker', 'build', '-t', 
'qemu:centos6', '-f', '/tmp/docker_buildIrIR2w/tmpxMPjZu.docker', 
'--build-arg=http_proxy=http://172.17.0.1:3142/', 
'/tmp/docker_buildIrIR2w']' returned non-zero exit status 139
tests/docker/Makefile.include:47: recipe for target 
'docker-image-centos6' failed
make: *** [docker-image-centos6] Error 1

looking further:

$ docker run --rm centos:6 cat /etc/redhat-release; echo $?
CentOS release 6.9 (Final)
0

$ docker run --rm centos:6 sh -c "cat /etc/redhat-release"; echo $?
139

uh?

$ docker run --rm centos:7 sh -c "cat /etc/redhat-release"; echo $?
CentOS Linux release 7.3.1611 (Core)
0

now trying old debian release:

$ docker run --rm -it debian:wheezy sh -c "cat /etc/debian_version"; echo $?
7.11
0

$ docker run --rm -it debian:wheezy bash -c "cat /etc/debian_version"; 
echo $?
139

hmmm

$ docker run --rm -it debian:jessie bash -c "cat /etc/debian_version"; 
echo $?
8.7
0

$ docker info
Server Version: 17.05.0-ce
Storage Driver: overlay2
  Backing Filesystem: extfs
Cgroup Driver: cgroupfs
Default Runtime: runc
Init Binary: docker-init
containerd version: 9048e5e50717ea4497b757314bad98ea3763c145
runc version: 9c2d8d184e5da67c95d601382adf14862e4f2228
init version: 949e6fa
Kernel Version: 4.11.0-1-amd64
Operating System: Debian GNU/Linux buster/sid
Architecture: x86_64

$ sudo journalctl -kb -l -o json-pretty
{
         "PRIORITY" : "6",
         "_TRANSPORT" : "kernel",
         "SYSLOG_FACILITY" : "0",
         "SYSLOG_IDENTIFIER" : "kernel",
         "MESSAGE" : "sh[23389] vsyscall attempted with vsyscall=none 
ip:ffffffffff600400 cs:33 sp:7ffcfd21a6c8 ax:ffffffffff600400 
si:7ffcfd21af6f di:0"
}
{
         "_TRANSPORT" : "kernel",
         "SYSLOG_FACILITY" : "0",
         "SYSLOG_IDENTIFIER" : "kernel",
         "MESSAGE" : "sh[23389]: segfault at ffffffffff600400 ip 
ffffffffff600400 sp 00007ffcfd21a6c8 error 15"
}

is it time to upgrade the docker image to centos:7 ?
Philippe Mathieu-Daudé July 12, 2017, 9:55 p.m. UTC | #2
Hi Daniel,

On 07/12/2017 01:25 PM, Daniel P. Berrange wrote:
> When trying to debug problems with tests it is natural to set
> DEBUG=1 when starting the docker environment. Unfortunately
> this has a side-effect of enabling an eth0 network interface
> in the container, which changes the operating environment of
> the test suite. IOW tests with fail may suddenly start
> working again if DEBUG=1 is set, due to changed network setup.
> 
> Add a separate NETWORK=1 option to allow enablement of
> networking separately from DEBUG=1, since common debugging
> tasks probably don't require networking anyway.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>   tests/docker/Makefile.include | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 037cb9e..a8c4b82 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -106,6 +106,7 @@ docker:
>   	@echo '                         (default is 1)'
>   	@echo '    DEBUG=1              Stop and drop to shell in the created container'
>   	@echo '                         before running the command.'
> +	@echo '    NETWORK=1            Enable eth0 virtual network interface.'

"eth0" is not always true...

This patch could be more generic, maybe documented as:

   NETWORK=host     Use full host network stack (default no network).'

>   	@echo '    NOUSER               Define to disable adding current user to containers passwd.'
>   	@echo '    NOCACHE=1            Ignore cache when build images.'
>   	@echo '    EXECUTABLE=<path>    Include executable in image.'
> @@ -132,7 +133,8 @@ docker-run: docker-qemu-src
>   		$(SRC_PATH)/tests/docker/docker.py run 			\
>   			$(if $(NOUSER),,-u $(shell id -u)) -t 		\
>   			$(if $V,,--rm) 					\
> -			$(if $(DEBUG),-i,--net=none) 			\
> +			$(if $(DEBUG),-i,)				\
> +			$(if $(NETWORK),,--net=none)			\

and here use directly:  --net=${NETWORK:-none}

so an experimented docker user could even run tests as:

   make docker-test-quick@centos6 NETWORK=container:qemu

(or NETWORK=bridge)

>   			-e TARGET_LIST=$(TARGET_LIST) 			\
>   			-e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
>   			-e V=$V -e J=$J -e DEBUG=$(DEBUG)		\
> 

Regards,

Phil.
Philippe Mathieu-Daudé July 12, 2017, 10:09 p.m. UTC | #3
On 07/12/2017 06:46 PM, Philippe Mathieu-Daudé wrote:
> now trying old debian release:
> 
> $ docker run --rm -it debian:wheezy sh -c "cat /etc/debian_version"; 
> echo $?
> 7.11
> 0
> 
> $ docker run --rm -it debian:wheezy bash -c "cat /etc/debian_version"; 
> echo $?
> 139

Indeed using debian:wheezy based dockerfile:

$ make docker-test-quick@debian7
[...]
Step 4/14 : RUN apt-get update
  ---> Running in 305758a09ca4
E: Method http has died unexpectedly!
E: Sub-process http received a segmentation fault.
The command '/bin/sh -c apt-get update' returned a non-zero code: 100

$ dmesg
sh[25336] vsyscall attempted with vsyscall=none ip:ffffffffff600400 
cs:33 sp:7fffa210e208 ax:ffffffffff600400 si:7fffa210ef60 di:0
sh[25336]: segfault at ffffffffff600400 ip ffffffffff600400 sp 
00007fffa210e208 error 15

note, this does test Fam's "docker.py: Improve subprocess exit code 
handling" :P
Fam Zheng July 13, 2017, 6:14 a.m. UTC | #4
On Wed, 07/12 17:25, Daniel P. Berrange wrote:
> When trying to debug problems with tests it is natural to set
> DEBUG=1 when starting the docker environment. Unfortunately
> this has a side-effect of enabling an eth0 network interface
> in the container, which changes the operating environment of
> the test suite. IOW tests with fail may suddenly start
> working again if DEBUG=1 is set, due to changed network setup.

Makes sense.

> 
> Add a separate NETWORK=1 option to allow enablement of
> networking separately from DEBUG=1, since common debugging
> tasks probably don't require networking anyway.

Not uncommon because fiddling with the package manager is often needed when
working on the test coverage. But that doesn't mean it's bad to control network
explicitly.

> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  tests/docker/Makefile.include | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 037cb9e..a8c4b82 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -106,6 +106,7 @@ docker:
>  	@echo '                         (default is 1)'
>  	@echo '    DEBUG=1              Stop and drop to shell in the created container'
>  	@echo '                         before running the command.'
> +	@echo '    NETWORK=1            Enable eth0 virtual network interface.'

As pointed out by Philippe, I'd just document it as "enable network".

>  	@echo '    NOUSER               Define to disable adding current user to containers passwd.'
>  	@echo '    NOCACHE=1            Ignore cache when build images.'
>  	@echo '    EXECUTABLE=<path>    Include executable in image.'
> @@ -132,7 +133,8 @@ docker-run: docker-qemu-src
>  		$(SRC_PATH)/tests/docker/docker.py run 			\
>  			$(if $(NOUSER),,-u $(shell id -u)) -t 		\
>  			$(if $V,,--rm) 					\
> -			$(if $(DEBUG),-i,--net=none) 			\
> +			$(if $(DEBUG),-i,)				\
> +			$(if $(NETWORK),,--net=none)			\
>  			-e TARGET_LIST=$(TARGET_LIST) 			\
>  			-e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
>  			-e V=$V -e J=$J -e DEBUG=$(DEBUG)		\
> -- 
> 2.9.4
> 

If others don't come up with objections, I'll apply this.

Thanks,
Fam
Alex Bennée July 13, 2017, 8:46 a.m. UTC | #5
Fam Zheng <famz@redhat.com> writes:

> On Wed, 07/12 17:25, Daniel P. Berrange wrote:
>> When trying to debug problems with tests it is natural to set
>> DEBUG=1 when starting the docker environment. Unfortunately
>> this has a side-effect of enabling an eth0 network interface
>> in the container, which changes the operating environment of
>> the test suite. IOW tests with fail may suddenly start
>> working again if DEBUG=1 is set, due to changed network setup.
>
> Makes sense.
>
>>
>> Add a separate NETWORK=1 option to allow enablement of
>> networking separately from DEBUG=1, since common debugging
>> tasks probably don't require networking anyway.
>
> Not uncommon because fiddling with the package manager is often needed when
> working on the test coverage. But that doesn't mean it's bad to control network
> explicitly.
>
>>
>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>> ---
>>  tests/docker/Makefile.include | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>> index 037cb9e..a8c4b82 100644
>> --- a/tests/docker/Makefile.include
>> +++ b/tests/docker/Makefile.include
>> @@ -106,6 +106,7 @@ docker:
>>  	@echo '                         (default is 1)'
>>  	@echo '    DEBUG=1              Stop and drop to shell in the created container'
>>  	@echo '                         before running the command.'
>> +	@echo '    NETWORK=1            Enable eth0 virtual network interface.'
>
> As pointed out by Philippe, I'd just document it as "enable network".
>
>>  	@echo '    NOUSER               Define to disable adding current user to containers passwd.'
>>  	@echo '    NOCACHE=1            Ignore cache when build images.'
>>  	@echo '    EXECUTABLE=<path>    Include executable in image.'
>> @@ -132,7 +133,8 @@ docker-run: docker-qemu-src
>>  		$(SRC_PATH)/tests/docker/docker.py run 			\
>>  			$(if $(NOUSER),,-u $(shell id -u)) -t 		\
>>  			$(if $V,,--rm) 					\
>> -			$(if $(DEBUG),-i,--net=none) 			\
>> +			$(if $(DEBUG),-i,)				\
>> +			$(if $(NETWORK),,--net=none)			\
>>  			-e TARGET_LIST=$(TARGET_LIST) 			\
>>  			-e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
>>  			-e V=$V -e J=$J -e DEBUG=$(DEBUG)		\
>> --
>> 2.9.4
>>
>
> If others don't come up with objections, I'll apply this.

Acked-by: Alex Bennée <alex.bennee@linaro.org>


--
Alex Bennée
Daniel P. Berrangé July 13, 2017, 9:24 a.m. UTC | #6
On Wed, Jul 12, 2017 at 06:55:45PM -0300, Philippe Mathieu-Daudé wrote:
> Hi Daniel,
> 
> On 07/12/2017 01:25 PM, Daniel P. Berrange wrote:
> > When trying to debug problems with tests it is natural to set
> > DEBUG=1 when starting the docker environment. Unfortunately
> > this has a side-effect of enabling an eth0 network interface
> > in the container, which changes the operating environment of
> > the test suite. IOW tests with fail may suddenly start
> > working again if DEBUG=1 is set, due to changed network setup.
> > 
> > Add a separate NETWORK=1 option to allow enablement of
> > networking separately from DEBUG=1, since common debugging
> > tasks probably don't require networking anyway.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >   tests/docker/Makefile.include | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> > index 037cb9e..a8c4b82 100644
> > --- a/tests/docker/Makefile.include
> > +++ b/tests/docker/Makefile.include
> > @@ -106,6 +106,7 @@ docker:
> >   	@echo '                         (default is 1)'
> >   	@echo '    DEBUG=1              Stop and drop to shell in the created container'
> >   	@echo '                         before running the command.'
> > +	@echo '    NETWORK=1            Enable eth0 virtual network interface.'
> 
> "eth0" is not always true...
> 
> This patch could be more generic, maybe documented as:
> 
>   NETWORK=host     Use full host network stack (default no network).'
> 
> >   	@echo '    NOUSER               Define to disable adding current user to containers passwd.'
> >   	@echo '    NOCACHE=1            Ignore cache when build images.'
> >   	@echo '    EXECUTABLE=<path>    Include executable in image.'
> > @@ -132,7 +133,8 @@ docker-run: docker-qemu-src
> >   		$(SRC_PATH)/tests/docker/docker.py run 			\
> >   			$(if $(NOUSER),,-u $(shell id -u)) -t 		\
> >   			$(if $V,,--rm) 					\
> > -			$(if $(DEBUG),-i,--net=none) 			\
> > +			$(if $(DEBUG),-i,)				\
> > +			$(if $(NETWORK),,--net=none)			\
> 
> and here use directly:  --net=${NETWORK:-none}
> 
> so an experimented docker user could even run tests as:
> 
>   make docker-test-quick@centos6 NETWORK=container:qemu
> 
> (or NETWORK=bridge)

This is a nice idea, though slightly more complicated. It would be good
to support NETWORK=1 as a short-cut for enabling the default docker
network backend, as well as being able to give an explicit backend for
those who really care about the flexibility.


Regards,
Daniel
diff mbox

Patch

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 037cb9e..a8c4b82 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -106,6 +106,7 @@  docker:
 	@echo '                         (default is 1)'
 	@echo '    DEBUG=1              Stop and drop to shell in the created container'
 	@echo '                         before running the command.'
+	@echo '    NETWORK=1            Enable eth0 virtual network interface.'
 	@echo '    NOUSER               Define to disable adding current user to containers passwd.'
 	@echo '    NOCACHE=1            Ignore cache when build images.'
 	@echo '    EXECUTABLE=<path>    Include executable in image.'
@@ -132,7 +133,8 @@  docker-run: docker-qemu-src
 		$(SRC_PATH)/tests/docker/docker.py run 			\
 			$(if $(NOUSER),,-u $(shell id -u)) -t 		\
 			$(if $V,,--rm) 					\
-			$(if $(DEBUG),-i,--net=none) 			\
+			$(if $(DEBUG),-i,)				\
+			$(if $(NETWORK),,--net=none)			\
 			-e TARGET_LIST=$(TARGET_LIST) 			\
 			-e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
 			-e V=$V -e J=$J -e DEBUG=$(DEBUG)		\