* [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1 @ 2017-07-13 14:43 Daniel P. Berrange 2017-07-14 2:29 ` Fam Zheng 0 siblings, 1 reply; 6+ messages in thread From: Daniel P. Berrange @ 2017-07-13 14:43 UTC (permalink / raw) To: qemu-devel Cc: Alex Bennée, Fam Zheng, Philippe Mathieu-Daudé, Daniel P. Berrange 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 variable to allow enablement of networking separately from DEBUG=1. This can be used in two ways. To enable the default docker network backend make docker-test-build@fedora NETWORK=1 while to enable a specific network backend, eg join the network associated with the container 'wibble': make docker-test-build@fedora NETWORK=container:wibble Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- Changed in v2: - Remove reference to eth0 - Allow arbitrary docker network backend to be given tests/docker/Makefile.include | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 037cb9e9e7..ec8e8d935d 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -106,6 +106,8 @@ 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 virtual network interface with default backend.' + @echo ' NETWORK=$BACKEND Enable virtual network interface with $BACKEND.' @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 +134,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),$(if $(subst $(NETWORK),,1)$(subst 1,,$(NETWORK)),--net=$(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.13.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1 2017-07-13 14:43 [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1 Daniel P. Berrange @ 2017-07-14 2:29 ` Fam Zheng 2017-07-14 11:41 ` Daniel P. Berrange 0 siblings, 1 reply; 6+ messages in thread From: Fam Zheng @ 2017-07-14 2:29 UTC (permalink / raw) To: Daniel P. Berrange Cc: qemu-devel, Alex Bennée, Philippe Mathieu-Daudé On Thu, 07/13 15:43, 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 variable to allow enablement of > networking separately from DEBUG=1. This can be used in two > ways. To enable the default docker network backend > > make docker-test-build@fedora NETWORK=1 > > while to enable a specific network backend, eg join the network > associated with the container 'wibble': > > make docker-test-build@fedora NETWORK=container:wibble > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > > Changed in v2: > > - Remove reference to eth0 > - Allow arbitrary docker network backend to be given > > tests/docker/Makefile.include | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include > index 037cb9e9e7..ec8e8d935d 100644 > --- a/tests/docker/Makefile.include > +++ b/tests/docker/Makefile.include > @@ -106,6 +106,8 @@ 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 virtual network interface with default backend.' > + @echo ' NETWORK=$BACKEND Enable virtual network interface with $BACKEND.' > @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 +134,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),$(if $(subst $(NETWORK),,1)$(subst 1,,$(NETWORK)),--net=$(NETWORK),),--net=none) \ Isn't the first subst enough? We already know $(NETWORK) is non-empty. If it is "1", $(subst 1,,1) is empty; otherwise, $(subst foo,,1) is non-empty: $(if $(NETWORK),$(if $(subst $(NETWORK),,1),,--net=$(NETWORK)),--net=none) \ The second subst is only a necessary condition. Another way to write it is: $(if $(NETWORK),$(if $(findstring $(NETWORK),1),,--net=$(NETWORK)),--net=none) \ Fam > -e TARGET_LIST=$(TARGET_LIST) \ > -e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \ > -e V=$V -e J=$J -e DEBUG=$(DEBUG) \ > -- > 2.13.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1 2017-07-14 2:29 ` Fam Zheng @ 2017-07-14 11:41 ` Daniel P. Berrange 2017-07-14 11:50 ` Fam Zheng 0 siblings, 1 reply; 6+ messages in thread From: Daniel P. Berrange @ 2017-07-14 11:41 UTC (permalink / raw) To: Fam Zheng; +Cc: qemu-devel, Alex Bennée, Philippe Mathieu-Daudé On Fri, Jul 14, 2017 at 10:29:30AM +0800, Fam Zheng wrote: > On Thu, 07/13 15:43, 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 variable to allow enablement of > > networking separately from DEBUG=1. This can be used in two > > ways. To enable the default docker network backend > > > > make docker-test-build@fedora NETWORK=1 > > > > while to enable a specific network backend, eg join the network > > associated with the container 'wibble': > > > > make docker-test-build@fedora NETWORK=container:wibble > > > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > > --- > > > > Changed in v2: > > > > - Remove reference to eth0 > > - Allow arbitrary docker network backend to be given > > > > tests/docker/Makefile.include | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include > > index 037cb9e9e7..ec8e8d935d 100644 > > --- a/tests/docker/Makefile.include > > +++ b/tests/docker/Makefile.include > > @@ -106,6 +106,8 @@ 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 virtual network interface with default backend.' > > + @echo ' NETWORK=$BACKEND Enable virtual network interface with $BACKEND.' > > @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 +134,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),$(if $(subst $(NETWORK),,1)$(subst 1,,$(NETWORK)),--net=$(NETWORK),),--net=none) \ > > Isn't the first subst enough? We already know $(NETWORK) is non-empty. If it is > "1", $(subst 1,,1) is empty; otherwise, $(subst foo,,1) is non-empty: > > $(if $(NETWORK),$(if $(subst $(NETWORK),,1),,--net=$(NETWORK)),--net=none) \ This looks inverted to me - NETWORK=1, will generate --net=1 and NETWORK=foo generates empty string. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1 2017-07-14 11:41 ` Daniel P. Berrange @ 2017-07-14 11:50 ` Fam Zheng 2017-07-14 11:52 ` Daniel P. Berrange 0 siblings, 1 reply; 6+ messages in thread From: Fam Zheng @ 2017-07-14 11:50 UTC (permalink / raw) To: Daniel P. Berrange Cc: qemu-devel, Alex Bennée, Philippe Mathieu-Daudé On Fri, 07/14 12:41, Daniel P. Berrange wrote: > On Fri, Jul 14, 2017 at 10:29:30AM +0800, Fam Zheng wrote: > > On Thu, 07/13 15:43, 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 variable to allow enablement of > > > networking separately from DEBUG=1. This can be used in two > > > ways. To enable the default docker network backend > > > > > > make docker-test-build@fedora NETWORK=1 > > > > > > while to enable a specific network backend, eg join the network > > > associated with the container 'wibble': > > > > > > make docker-test-build@fedora NETWORK=container:wibble > > > > > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > > > --- > > > > > > Changed in v2: > > > > > > - Remove reference to eth0 > > > - Allow arbitrary docker network backend to be given > > > > > > tests/docker/Makefile.include | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include > > > index 037cb9e9e7..ec8e8d935d 100644 > > > --- a/tests/docker/Makefile.include > > > +++ b/tests/docker/Makefile.include > > > @@ -106,6 +106,8 @@ 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 virtual network interface with default backend.' > > > + @echo ' NETWORK=$BACKEND Enable virtual network interface with $BACKEND.' > > > @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 +134,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),$(if $(subst $(NETWORK),,1)$(subst 1,,$(NETWORK)),--net=$(NETWORK),),--net=none) \ > > > > Isn't the first subst enough? We already know $(NETWORK) is non-empty. If it is > > "1", $(subst 1,,1) is empty; otherwise, $(subst foo,,1) is non-empty: > > > > $(if $(NETWORK),$(if $(subst $(NETWORK),,1),,--net=$(NETWORK)),--net=none) \ > > This looks inverted to me - NETWORK=1, will generate --net=1 and > NETWORK=foo generates empty string. You are right, I was confused (subst succeed is FALSE branch of if). I think s/,,/,/ and it will work? Fam ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1 2017-07-14 11:50 ` Fam Zheng @ 2017-07-14 11:52 ` Daniel P. Berrange 2017-07-14 12:12 ` Fam Zheng 0 siblings, 1 reply; 6+ messages in thread From: Daniel P. Berrange @ 2017-07-14 11:52 UTC (permalink / raw) To: Fam Zheng; +Cc: qemu-devel, Alex Bennée, Philippe Mathieu-Daudé On Fri, Jul 14, 2017 at 07:50:03PM +0800, Fam Zheng wrote: > On Fri, 07/14 12:41, Daniel P. Berrange wrote: > > On Fri, Jul 14, 2017 at 10:29:30AM +0800, Fam Zheng wrote: > > > On Thu, 07/13 15:43, 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 variable to allow enablement of > > > > networking separately from DEBUG=1. This can be used in two > > > > ways. To enable the default docker network backend > > > > > > > > make docker-test-build@fedora NETWORK=1 > > > > > > > > while to enable a specific network backend, eg join the network > > > > associated with the container 'wibble': > > > > > > > > make docker-test-build@fedora NETWORK=container:wibble > > > > > > > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > > > > --- > > > > > > > > Changed in v2: > > > > > > > > - Remove reference to eth0 > > > > - Allow arbitrary docker network backend to be given > > > > > > > > tests/docker/Makefile.include | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include > > > > index 037cb9e9e7..ec8e8d935d 100644 > > > > --- a/tests/docker/Makefile.include > > > > +++ b/tests/docker/Makefile.include > > > > @@ -106,6 +106,8 @@ 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 virtual network interface with default backend.' > > > > + @echo ' NETWORK=$BACKEND Enable virtual network interface with $BACKEND.' > > > > @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 +134,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),$(if $(subst $(NETWORK),,1)$(subst 1,,$(NETWORK)),--net=$(NETWORK),),--net=none) \ > > > > > > Isn't the first subst enough? We already know $(NETWORK) is non-empty. If it is > > > "1", $(subst 1,,1) is empty; otherwise, $(subst foo,,1) is non-empty: > > > > > > $(if $(NETWORK),$(if $(subst $(NETWORK),,1),,--net=$(NETWORK)),--net=none) \ > > > > This looks inverted to me - NETWORK=1, will generate --net=1 and > > NETWORK=foo generates empty string. > > You are right, I was confused (subst succeed is FALSE branch of if). I think > s/,,/,/ and it will work? Yep Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1 2017-07-14 11:52 ` Daniel P. Berrange @ 2017-07-14 12:12 ` Fam Zheng 0 siblings, 0 replies; 6+ messages in thread From: Fam Zheng @ 2017-07-14 12:12 UTC (permalink / raw) To: Daniel P. Berrange Cc: Alex Bennée, qemu-devel, Philippe Mathieu-Daudé On Fri, 07/14 12:52, Daniel P. Berrange wrote: > > > > > @@ -132,7 +134,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),$(if $(subst $(NETWORK),,1)$(subst 1,,$(NETWORK)),--net=$(NETWORK),),--net=none) \ > > > > > > > > Isn't the first subst enough? We already know $(NETWORK) is non-empty. If it is > > > > "1", $(subst 1,,1) is empty; otherwise, $(subst foo,,1) is non-empty: > > > > > > > > $(if $(NETWORK),$(if $(subst $(NETWORK),,1),,--net=$(NETWORK)),--net=none) \ > > > > > > This looks inverted to me - NETWORK=1, will generate --net=1 and > > > NETWORK=foo generates empty string. > > > > You are right, I was confused (subst succeed is FALSE branch of if). I think > > s/,,/,/ and it will work? > > Yep OK, so dropping the second "$(subst ...)" and the comma we have: $(if $(NETWORK),$(if $(subst $(NETWORK),,1),--net=$(NETWORK)),--net=none) \ With that, queued for 2.10: https://github.com/famz/qemu/tree/staging Fam ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-07-14 12:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-07-13 14:43 [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1 Daniel P. Berrange 2017-07-14 2:29 ` Fam Zheng 2017-07-14 11:41 ` Daniel P. Berrange 2017-07-14 11:50 ` Fam Zheng 2017-07-14 11:52 ` Daniel P. Berrange 2017-07-14 12:12 ` Fam Zheng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).