[RFC PATCH] tests/docker: Allow passing --network option when building images

Philippe Mathieu-Daudé posted 1 patch 3 years, 3 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210119054502.531451-1-f4bug@amsat.org
tests/docker/Makefile.include | 1 +
1 file changed, 1 insertion(+)
[RFC PATCH] tests/docker: Allow passing --network option when building images
Posted by Philippe Mathieu-Daudé 3 years, 3 months ago
When using the Docker engine, build fails because the container is
unable to resolve hostnames:

  $ make docker-image-debian-s390x-cross NETWORK=host ENGINE=docker
    BUILD   debian10
  #6 9.679 Err:1 http://deb.debian.org/debian buster InRelease
  #6 9.679   Temporary failure resolving 'deb.debian.org'
  #6 16.69 Err:2 http://security.debian.org/debian-security buster/updates InRelease
  #6 16.69   Temporary failure resolving 'security.debian.org'
  #6 22.69 Err:3 http://deb.debian.org/debian buster-updates InRelease
  #6 22.69   Temporary failure resolving 'deb.debian.org'
  #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster/InRelease  Temporary failure resolving 'deb.debian.org'
  #6 22.74 W: Failed to fetch http://security.debian.org/debian-security/dists/buster/updates/InRelease  Temporary failure resolving 'security.debian.org'
  #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster-updates/InRelease  Temporary failure resolving 'deb.debian.org'
  #6 22.74 W: Some index files failed to download. They have been ignored, or old ones used instead.
  Traceback (most recent call last):
    File "./tests/docker/docker.py", line 709, in <module>
      sys.exit(main())
    File "./tests/docker/docker.py", line 705, in main
      return args.cmdobj.run(args, argv)
    File "./tests/docker/docker.py", line 498, in run
      dkr.build_image(tag, docker_dir, dockerfile,
    File "./tests/docker/docker.py", line 353, in build_image
      self._do_check(build_args,
    File "./tests/docker/docker.py", line 244, in _do_check
      return subprocess.check_call(self._command + cmd, **kwargs)
    File "/usr/lib64/python3.8/subprocess.py", line 364, in check_call
      raise CalledProcessError(retcode, cmd)
  make: *** [tests/docker/Makefile.include:61: docker-image-debian10] Error 1

Fix by passing the NETWORK variable with --network= argument.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/docker/Makefile.include | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index bdc53ddfcf9..b65fd684011 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -63,6 +63,7 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker
 		$(if $V,,--quiet) \
 		$(if $(NOCACHE),--no-cache, \
 			$(if $(DOCKER_REGISTRY),--registry $(DOCKER_REGISTRY))) \
+		$(if $(NETWORK),$(if $(subst $(NETWORK),,1),--network=$(NETWORK))) \
 		$(if $(NOUSER),,--add-current-user) \
 		$(if $(EXTRA_FILES),--extra-files $(EXTRA_FILES))\
 		$(if $(EXECUTABLE),--include-executable=$(EXECUTABLE)),\
-- 
2.26.2

Re: [RFC PATCH] tests/docker: Allow passing --network option when building images
Posted by Alex Bennée 3 years, 3 months ago
Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> When using the Docker engine, build fails because the container is
> unable to resolve hostnames:
>
>   $ make docker-image-debian-s390x-cross NETWORK=host ENGINE=docker
>     BUILD   debian10
>   #6 9.679 Err:1 http://deb.debian.org/debian buster InRelease
>   #6 9.679   Temporary failure resolving 'deb.debian.org'
>   #6 16.69 Err:2 http://security.debian.org/debian-security buster/updates InRelease
>   #6 16.69   Temporary failure resolving 'security.debian.org'
>   #6 22.69 Err:3 http://deb.debian.org/debian buster-updates InRelease
>   #6 22.69   Temporary failure resolving 'deb.debian.org'
>   #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster/InRelease  Temporary failure resolving 'deb.debian.org'
>   #6 22.74 W: Failed to fetch http://security.debian.org/debian-security/dists/buster/updates/InRelease  Temporary failure resolving 'security.debian.org'
>   #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster-updates/InRelease  Temporary failure resolving 'deb.debian.org'
>   #6 22.74 W: Some index files failed to download. They have been
>   ignored, or old ones used instead.

I'm confused by this one as it currently works for me. That said I
thought the actual behaviour was meant to be networking is enabled by
default and explicitly disabled by the run step (which shouldn't be
pulling extra stuff down).

This was last tweaked by Daniel in 8a2390a4f47

Have the defaults for docker engine changed?

>   Traceback (most recent call last):
>     File "./tests/docker/docker.py", line 709, in <module>
>       sys.exit(main())
>     File "./tests/docker/docker.py", line 705, in main
>       return args.cmdobj.run(args, argv)
>     File "./tests/docker/docker.py", line 498, in run
>       dkr.build_image(tag, docker_dir, dockerfile,
>     File "./tests/docker/docker.py", line 353, in build_image
>       self._do_check(build_args,
>     File "./tests/docker/docker.py", line 244, in _do_check
>       return subprocess.check_call(self._command + cmd, **kwargs)
>     File "/usr/lib64/python3.8/subprocess.py", line 364, in check_call
>       raise CalledProcessError(retcode, cmd)
>   make: *** [tests/docker/Makefile.include:61: docker-image-debian10] Error 1
>
> Fix by passing the NETWORK variable with --network= argument.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/docker/Makefile.include | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index bdc53ddfcf9..b65fd684011 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -63,6 +63,7 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker
>  		$(if $V,,--quiet) \
>  		$(if $(NOCACHE),--no-cache, \
>  			$(if $(DOCKER_REGISTRY),--registry $(DOCKER_REGISTRY))) \
> +		$(if $(NETWORK),$(if $(subst
> $(NETWORK),,1),--network=$(NETWORK))) \

which if it has we'll need to tweak both build and run steps?

>  		$(if $(NOUSER),,--add-current-user) \
>  		$(if $(EXTRA_FILES),--extra-files $(EXTRA_FILES))\
>  		$(if $(EXECUTABLE),--include-executable=$(EXECUTABLE)),\


-- 
Alex Bennée

Re: [RFC PATCH] tests/docker: Allow passing --network option when building images
Posted by Philippe Mathieu-Daudé 3 years, 3 months ago
On 1/19/21 12:27 PM, Alex Bennée wrote:
> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> 
>> When using the Docker engine, build fails because the container is
>> unable to resolve hostnames:
>>
>>   $ make docker-image-debian-s390x-cross NETWORK=host ENGINE=docker
>>     BUILD   debian10
>>   #6 9.679 Err:1 http://deb.debian.org/debian buster InRelease
>>   #6 9.679   Temporary failure resolving 'deb.debian.org'
>>   #6 16.69 Err:2 http://security.debian.org/debian-security buster/updates InRelease
>>   #6 16.69   Temporary failure resolving 'security.debian.org'
>>   #6 22.69 Err:3 http://deb.debian.org/debian buster-updates InRelease
>>   #6 22.69   Temporary failure resolving 'deb.debian.org'
>>   #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster/InRelease  Temporary failure resolving 'deb.debian.org'
>>   #6 22.74 W: Failed to fetch http://security.debian.org/debian-security/dists/buster/updates/InRelease  Temporary failure resolving 'security.debian.org'
>>   #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster-updates/InRelease  Temporary failure resolving 'deb.debian.org'
>>   #6 22.74 W: Some index files failed to download. They have been
>>   ignored, or old ones used instead.
> 
> I'm confused by this one as it currently works for me. That said I
> thought the actual behaviour was meant to be networking is enabled by
> default and explicitly disabled by the run step (which shouldn't be
> pulling extra stuff down).
> 
> This was last tweaked by Daniel in 8a2390a4f47
> 
> Have the defaults for docker engine changed?

No idea as I'm not following their development, but TBH it
becomes harder and harder to use without tricks (I had to
add systemd.unified_cgroup_hierarchy=0 to kernel cmdline
to keep using it).

Maybe I should switch to podman which is the recommended
engine on Fedora.

Cc'ing Marc-André who added podman support (Daniel is in Cc).

> 
>>   Traceback (most recent call last):
>>     File "./tests/docker/docker.py", line 709, in <module>
>>       sys.exit(main())
>>     File "./tests/docker/docker.py", line 705, in main
>>       return args.cmdobj.run(args, argv)
>>     File "./tests/docker/docker.py", line 498, in run
>>       dkr.build_image(tag, docker_dir, dockerfile,
>>     File "./tests/docker/docker.py", line 353, in build_image
>>       self._do_check(build_args,
>>     File "./tests/docker/docker.py", line 244, in _do_check
>>       return subprocess.check_call(self._command + cmd, **kwargs)
>>     File "/usr/lib64/python3.8/subprocess.py", line 364, in check_call
>>       raise CalledProcessError(retcode, cmd)
>>   make: *** [tests/docker/Makefile.include:61: docker-image-debian10] Error 1
>>
>> Fix by passing the NETWORK variable with --network= argument.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  tests/docker/Makefile.include | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>> index bdc53ddfcf9..b65fd684011 100644
>> --- a/tests/docker/Makefile.include
>> +++ b/tests/docker/Makefile.include
>> @@ -63,6 +63,7 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker
>>  		$(if $V,,--quiet) \
>>  		$(if $(NOCACHE),--no-cache, \
>>  			$(if $(DOCKER_REGISTRY),--registry $(DOCKER_REGISTRY))) \
>> +		$(if $(NETWORK),$(if $(subst
>> $(NETWORK),,1),--network=$(NETWORK))) \
> 
> which if it has we'll need to tweak both build and run steps?

I suppose you need need networking for git (submodule) clone?

Personally I don't require networking when running because I
share my QEMU source directory as a volume, so it never bothered
me.

> 
>>  		$(if $(NOUSER),,--add-current-user) \
>>  		$(if $(EXTRA_FILES),--extra-files $(EXTRA_FILES))\
>>  		$(if $(EXECUTABLE),--include-executable=$(EXECUTABLE)),\
> 
> 

Re: [RFC PATCH] tests/docker: Allow passing --network option when building images
Posted by Daniel P. Berrangé 3 years, 3 months ago
On Tue, Jan 19, 2021 at 02:40:13PM +0100, Philippe Mathieu-Daudé wrote:
> On 1/19/21 12:27 PM, Alex Bennée wrote:
> > Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> > 
> >> When using the Docker engine, build fails because the container is
> >> unable to resolve hostnames:
> >>
> >>   $ make docker-image-debian-s390x-cross NETWORK=host ENGINE=docker
> >>     BUILD   debian10
> >>   #6 9.679 Err:1 http://deb.debian.org/debian buster InRelease
> >>   #6 9.679   Temporary failure resolving 'deb.debian.org'
> >>   #6 16.69 Err:2 http://security.debian.org/debian-security buster/updates InRelease
> >>   #6 16.69   Temporary failure resolving 'security.debian.org'
> >>   #6 22.69 Err:3 http://deb.debian.org/debian buster-updates InRelease
> >>   #6 22.69   Temporary failure resolving 'deb.debian.org'
> >>   #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster/InRelease  Temporary failure resolving 'deb.debian.org'
> >>   #6 22.74 W: Failed to fetch http://security.debian.org/debian-security/dists/buster/updates/InRelease  Temporary failure resolving 'security.debian.org'
> >>   #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster-updates/InRelease  Temporary failure resolving 'deb.debian.org'
> >>   #6 22.74 W: Some index files failed to download. They have been
> >>   ignored, or old ones used instead.
> > 
> > I'm confused by this one as it currently works for me. That said I
> > thought the actual behaviour was meant to be networking is enabled by
> > default and explicitly disabled by the run step (which shouldn't be
> > pulling extra stuff down).
> > 
> > This was last tweaked by Daniel in 8a2390a4f47
> > 
> > Have the defaults for docker engine changed?
> 
> No idea as I'm not following their development, but TBH it
> becomes harder and harder to use without tricks (I had to
> add systemd.unified_cgroup_hierarchy=0 to kernel cmdline
> to keep using it).
> 
> Maybe I should switch to podman which is the recommended
> engine on Fedora.
> 
> Cc'ing Marc-André who added podman support (Daniel is in Cc).

I'm using podman exclusively since Docker doesn't work well with
modern distros that use Cgroups v2.


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 :|


Re: [RFC PATCH] tests/docker: Allow passing --network option when building images
Posted by Philippe Mathieu-Daudé 3 years, 3 months ago
On 1/19/21 3:20 PM, Daniel P. Berrangé wrote:
> On Tue, Jan 19, 2021 at 02:40:13PM +0100, Philippe Mathieu-Daudé wrote:
>> On 1/19/21 12:27 PM, Alex Bennée wrote:
>>> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
>>>
>>>> When using the Docker engine, build fails because the container is
>>>> unable to resolve hostnames:
>>>>
>>>>   $ make docker-image-debian-s390x-cross NETWORK=host ENGINE=docker
>>>>     BUILD   debian10
>>>>   #6 9.679 Err:1 http://deb.debian.org/debian buster InRelease
>>>>   #6 9.679   Temporary failure resolving 'deb.debian.org'
>>>>   #6 16.69 Err:2 http://security.debian.org/debian-security buster/updates InRelease
>>>>   #6 16.69   Temporary failure resolving 'security.debian.org'
>>>>   #6 22.69 Err:3 http://deb.debian.org/debian buster-updates InRelease
>>>>   #6 22.69   Temporary failure resolving 'deb.debian.org'
>>>>   #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster/InRelease  Temporary failure resolving 'deb.debian.org'
>>>>   #6 22.74 W: Failed to fetch http://security.debian.org/debian-security/dists/buster/updates/InRelease  Temporary failure resolving 'security.debian.org'
>>>>   #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster-updates/InRelease  Temporary failure resolving 'deb.debian.org'
>>>>   #6 22.74 W: Some index files failed to download. They have been
>>>>   ignored, or old ones used instead.
>>>
>>> I'm confused by this one as it currently works for me. That said I
>>> thought the actual behaviour was meant to be networking is enabled by
>>> default and explicitly disabled by the run step (which shouldn't be
>>> pulling extra stuff down).
>>>
>>> This was last tweaked by Daniel in 8a2390a4f47
>>>
>>> Have the defaults for docker engine changed?
>>
>> No idea as I'm not following their development, but TBH it
>> becomes harder and harder to use without tricks (I had to
>> add systemd.unified_cgroup_hierarchy=0 to kernel cmdline
>> to keep using it).
>>
>> Maybe I should switch to podman which is the recommended
>> engine on Fedora.
>>
>> Cc'ing Marc-André who added podman support (Daniel is in Cc).
> 
> I'm using podman exclusively since Docker doesn't work well with
> modern distros that use Cgroups v2.

OK this probably explains it.

Ideally we could add a check for this ("modern distro" -> docker
engine not recommended) but I guess I'm the only one using this
feature on Fedora (as nobody complained) so not a problem. I'll
see how to use podman.

Thanks,

Phil.

Re: [RFC PATCH] tests/docker: Allow passing --network option when building images
Posted by Daniel P. Berrangé 3 years, 3 months ago
On Tue, Jan 19, 2021 at 03:40:50PM +0100, Philippe Mathieu-Daudé wrote:
> On 1/19/21 3:20 PM, Daniel P. Berrangé wrote:
> > On Tue, Jan 19, 2021 at 02:40:13PM +0100, Philippe Mathieu-Daudé wrote:
> >> On 1/19/21 12:27 PM, Alex Bennée wrote:
> >>> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> >>>
> >>>> When using the Docker engine, build fails because the container is
> >>>> unable to resolve hostnames:
> >>>>
> >>>>   $ make docker-image-debian-s390x-cross NETWORK=host ENGINE=docker
> >>>>     BUILD   debian10
> >>>>   #6 9.679 Err:1 http://deb.debian.org/debian buster InRelease
> >>>>   #6 9.679   Temporary failure resolving 'deb.debian.org'
> >>>>   #6 16.69 Err:2 http://security.debian.org/debian-security buster/updates InRelease
> >>>>   #6 16.69   Temporary failure resolving 'security.debian.org'
> >>>>   #6 22.69 Err:3 http://deb.debian.org/debian buster-updates InRelease
> >>>>   #6 22.69   Temporary failure resolving 'deb.debian.org'
> >>>>   #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster/InRelease  Temporary failure resolving 'deb.debian.org'
> >>>>   #6 22.74 W: Failed to fetch http://security.debian.org/debian-security/dists/buster/updates/InRelease  Temporary failure resolving 'security.debian.org'
> >>>>   #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster-updates/InRelease  Temporary failure resolving 'deb.debian.org'
> >>>>   #6 22.74 W: Some index files failed to download. They have been
> >>>>   ignored, or old ones used instead.
> >>>
> >>> I'm confused by this one as it currently works for me. That said I
> >>> thought the actual behaviour was meant to be networking is enabled by
> >>> default and explicitly disabled by the run step (which shouldn't be
> >>> pulling extra stuff down).
> >>>
> >>> This was last tweaked by Daniel in 8a2390a4f47
> >>>
> >>> Have the defaults for docker engine changed?
> >>
> >> No idea as I'm not following their development, but TBH it
> >> becomes harder and harder to use without tricks (I had to
> >> add systemd.unified_cgroup_hierarchy=0 to kernel cmdline
> >> to keep using it).
> >>
> >> Maybe I should switch to podman which is the recommended
> >> engine on Fedora.
> >>
> >> Cc'ing Marc-André who added podman support (Daniel is in Cc).
> > 
> > I'm using podman exclusively since Docker doesn't work well with
> > modern distros that use Cgroups v2.
> 
> OK this probably explains it.
> 
> Ideally we could add a check for this ("modern distro" -> docker
> engine not recommended) but I guess I'm the only one using this
> feature on Fedora (as nobody complained) so not a problem. I'll
> see how to use podman.

I'm not sure we need to block it. If someone has docker installed
then its reasonable to assume they have ti working. We prefer
podman if both are installed.


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 :|


Re: [RFC PATCH] tests/docker: Allow passing --network option when building images
Posted by Alex Bennée 3 years, 3 months ago
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Jan 19, 2021 at 03:40:50PM +0100, Philippe Mathieu-Daudé wrote:
>> On 1/19/21 3:20 PM, Daniel P. Berrangé wrote:
>> > On Tue, Jan 19, 2021 at 02:40:13PM +0100, Philippe Mathieu-Daudé wrote:
>> >> On 1/19/21 12:27 PM, Alex Bennée wrote:
>> >>> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
>> >>>
>> >>>> When using the Docker engine, build fails because the container is
>> >>>> unable to resolve hostnames:
>> >>>>
>> >>>>   $ make docker-image-debian-s390x-cross NETWORK=host ENGINE=docker
>> >>>>     BUILD   debian10
>> >>>>   #6 9.679 Err:1 http://deb.debian.org/debian buster InRelease
>> >>>>   #6 9.679   Temporary failure resolving 'deb.debian.org'
>> >>>>   #6 16.69 Err:2 http://security.debian.org/debian-security buster/updates InRelease
>> >>>>   #6 16.69   Temporary failure resolving 'security.debian.org'
>> >>>>   #6 22.69 Err:3 http://deb.debian.org/debian buster-updates InRelease
>> >>>>   #6 22.69   Temporary failure resolving 'deb.debian.org'
>> >>>>   #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster/InRelease  Temporary failure resolving 'deb.debian.org'
>> >>>>   #6 22.74 W: Failed to fetch http://security.debian.org/debian-security/dists/buster/updates/InRelease  Temporary failure resolving 'security.debian.org'
>> >>>>   #6 22.74 W: Failed to fetch http://deb.debian.org/debian/dists/buster-updates/InRelease  Temporary failure resolving 'deb.debian.org'
>> >>>>   #6 22.74 W: Some index files failed to download. They have been
>> >>>>   ignored, or old ones used instead.
>> >>>
>> >>> I'm confused by this one as it currently works for me. That said I
>> >>> thought the actual behaviour was meant to be networking is enabled by
>> >>> default and explicitly disabled by the run step (which shouldn't be
>> >>> pulling extra stuff down).
>> >>>
>> >>> This was last tweaked by Daniel in 8a2390a4f47
>> >>>
>> >>> Have the defaults for docker engine changed?
>> >>
>> >> No idea as I'm not following their development, but TBH it
>> >> becomes harder and harder to use without tricks (I had to
>> >> add systemd.unified_cgroup_hierarchy=0 to kernel cmdline
>> >> to keep using it).
>> >>
>> >> Maybe I should switch to podman which is the recommended
>> >> engine on Fedora.
>> >>
>> >> Cc'ing Marc-André who added podman support (Daniel is in Cc).
>> > 
>> > I'm using podman exclusively since Docker doesn't work well with
>> > modern distros that use Cgroups v2.
>> 
>> OK this probably explains it.
>> 
>> Ideally we could add a check for this ("modern distro" -> docker
>> engine not recommended) but I guess I'm the only one using this
>> feature on Fedora (as nobody complained) so not a problem. I'll
>> see how to use podman.
>
> I'm not sure we need to block it. If someone has docker installed
> then its reasonable to assume they have ti working. We prefer
> podman if both are installed.

From my point of view podman is the odd man out (I run upstream
docker.com packages on Debian Buster). I had to jump through some hoops
to get podman installed on my Gentoo box but I think it's currently
broken because it's Gentoo.

IOW I assume the people that really care about podman will shout if it
breaks. It would be nice if we could catch cases in the CI though.

>
>
> Regards,
> Daniel


-- 
Alex Bennée