This way more users can run our CI builds locally.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 Makefile.ci | 125 ++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 93 insertions(+), 32 deletions(-)

diff --git a/Makefile.ci b/Makefile.ci
index 12a62167cc67..e2989ada313c 100644
--- a/Makefile.ci
+++ b/Makefile.ci
@@ -17,7 +17,7 @@ CI_GIT_ROOT = $(shell git rev-parse --show-toplevel)
 CI_HOST_SRCDIR = $(CI_SCRATCHDIR)/src
 
 # The directory holding the source inside the
-# container. ie where we told Docker to expose
+# container. ie where we want to expose
 # the $(CI_HOST_SRCDIR) directory from the host
 CI_CONT_SRCDIR = /src
 
@@ -46,14 +46,13 @@ CI_CONFIGURE_ARGS =
 # cloning them
 CI_SUBMODULES = $(shell git submodule | awk '{ print $$2 }')
 
-# Location of the Docker images we're going to pull
+# Location of the container images we're going to pull
 # Can be useful to overridde to use a locally built
 # image instead
 CI_IMAGE_PREFIX = quay.io/libvirt/buildenv-
 
-# Docker defaults to pulling the ':latest' tag but
-# if the Docker repo above uses different conventions
-# this can override it
+# The default tag is ':latest' but if the container
+# repo above uses different conventions this can override it
 CI_IMAGE_TAG = :master
 
 # We delete the virtual root after completion, set
@@ -71,24 +70,82 @@ CI_REUSE = 0
 CI_UID = $(shell id -u)
 CI_GID = $(shell id -g)
 
-# Docker doesn't require the IDs you run as to exist in
+# Container engine runtime we are going to use, can be overridden per make
+# invocation, if it is not, we try podman and then default to docker.
+ifeq ($(CI_CENGINE),)
+	CI_CENGINE = $(shell podman version >/dev/null && echo podman || echo docker)
+endif
+
+# IDs you run as do not need to exist in
 # the container's /etc/passwd & /etc/group files, but
-# if they do not, then libvirt's  'make check' will fail
+# if they do not, then libvirt's 'make check' will fail
 # many tests.
-#
-# We do not directly mount /etc/{passwd,group} as Docker
-# is liable to mess with SELinux labelling which will
-# then prevent the host accessing them. Copying them
-# first is safer.
-CI_PWDB_MOUNTS = \
-	--volume $(CI_SCRATCHDIR)/group:/etc/group:ro,z \
-	--volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \
-	$(NULL)
+ifeq ($(CI_CENGINE),podman)
+	CI_PWDB_MOUNTS = \
+		--volume /etc/group:/etc/group:ro,z \
+		--volume /etc/passwd:/etc/passwd:ro,z \
+		$(NULL)
+else
+	# We do not directly mount /etc/{passwd,group} as Docker
+	# is liable to mess with SELinux labelling which will
+	# then prevent the host accessing them. Copying them
+	# first is safer.
+	CI_PWDB_MOUNTS = \
+		--volume $(CI_SCRATCHDIR)/group:/etc/group:ro,z \
+		--volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \
+		$(NULL)
+endif
+
+ifeq ($(CI_CENGINE),docker)
+	# Docker containers can have very large ulimits
+	# for nofiles - as much as 1048576. This makes
+	# libvirt very slow at exec'ing programs.
+	CI_ULIMIT_FILES = 1024
+endif
 
-# Docker containers can have very large ulimits
-# for nofiles - as much as 1048576. This makes
-# libvirt very slow at exec'ing programs.
-CI_ULIMIT_FILES = 1024
+ifeq ($(CI_CENGINE),podman)
+	# Podman cannot reuse host namespace when running non-root containers.  Until
+	# support for --keep-uid is added we can just create another mapping that will
+	# do that for us.  Beware, that in {uid,git}map=container_id:host_id:range,
+	# the host_id does actually refer to the uid in the first mapping where 0
+	# (root) is mapped to the current user and rest is offset.
+
+	# In order to set up this mapping, we need to keep all the user IDs to prevent
+	# possible errors as some images might expect UIDs up to 90000 (looking at you
+	# fedora), so we don't want the overflowuid to be used for them.  For mapping
+	# all the other users properly ther eneeds to be some math done.  Don't worry,
+	# it's just addition and subtraction.
+
+	# 65536 ought to be enough (tm), but for really rare cases the maximums might
+	# need to be higher, but that only happens when your /etc/sub{u,g}id allow
+	# users to have more IDs.  Unless --keep-uid is supported, let's do this in a
+	# way that should work for everyone.
+	CI_MAX_UID = $(shell sed -n "s/^$USER:[^:]\+://p" /etc/subuid)
+	CI_MAX_GID = $(shell sed -n "s/^$USER:[^:]\+://p" /etc/subgid)
+	ifeq ($(CI_MAX_UID),)
+		CI_MAX_UID = 65536
+	endif
+	ifeq ($(CI_MAX_GID),)
+		CI_MAX_GID = 65536
+	endif
+	CI_UID_OTHER = $(shell echo $$(($(CI_UID)+1)))
+	CI_GID_OTHER = $(shell echo $$(($(CI_GID)+1)))
+	CI_UID_OTHER_RANGE = $(shell echo $$(($(CI_MAX_UID)-$(CI_UID))))
+	CI_GID_OTHER_RANGE = $(shell echo $$(($(CI_MAX_GID)-$(CI_GID))))
+
+	CI_PODMAN_ARGS = \
+		--uidmap 0:1:$(CI_UID) \
+		--uidmap $(CI_UID):0:1 \
+		--uidmap $(CI_UID_OTHER):$(CI_UID_OTHER):$(CI_UID_OTHER_RANGE) \
+		--gidmap 0:1:$(CI_GID) \
+		--gidmap $(CI_GID):0:1 \
+		--gidmap $(CI_GID_OTHER):$(CI_GID_OTHER):$(CI_GID_OTHER_RANGE) \
+		$(NULL)
+else
+	CI_DOCKER_ARGS = \
+		--ulimit nofile=$(CI_ULIMIT_FILES):$(CI_ULIMIT_FILES) \
+		$(NULL)
+endif
 
 # Args to use when cloning a git repo.
 #  -c  stop it complaining about checking out a random hash
@@ -100,7 +157,7 @@ CI_GIT_ARGS = \
 	--local  \
 	$(NULL)
 
-# Args to use when running the Docker env
+# Args to use when running the container
 #   --rm      stop inactive containers getting left behind
 #   --user    we execute as the same user & group account
 #             as dev so that file ownership matches host
@@ -110,27 +167,30 @@ CI_GIT_ARGS = \
 #   --ulimit  lower files limit for performance reasons
 #   --interactive
 #   --tty     Ensure we have ability to Ctrl-C the build
-CI_DOCKER_ARGS = \
+CI_CENGINE_ARGS = \
 	--rm \
 	--user $(CI_UID):$(CI_GID) \
 	--interactive \
 	--tty \
+	$(CI_PODMAN_ARGS) \
+	$(CI_DOCKER_ARGS) \
 	$(CI_PWDB_MOUNTS) \
 	--volume $(CI_HOST_SRCDIR):$(CI_CONT_SRCDIR):z \
 	--workdir $(CI_CONT_SRCDIR) \
-	--ulimit nofile=$(CI_ULIMIT_FILES):$(CI_ULIMIT_FILES) \
 	$(NULL)
 
-ci-check-docker:
-	@echo -n "Checking if Docker is available and running..." && \
-	docker version 1>/dev/null && echo "yes"
+ci-check-cengine:
+	@echo -n "Checking if $(CI_CENGINE) is available..." && \
+	$(CI_CENGINE) version 1>/dev/null && echo "yes"
 
-ci-prepare-tree: ci-check-docker
+ci-prepare-tree: ci-check-cengine
 	@test "$(CI_REUSE)" != "1" && rm -rf $(CI_SCRATCHDIR) || :
 	@if ! test -d $(CI_SCRATCHDIR) ; then \
 		mkdir -p $(CI_SCRATCHDIR); \
-		cp /etc/passwd $(CI_SCRATCHDIR); \
-		cp /etc/group $(CI_SCRATCHDIR); \
+		if test "$(CI_CENGINE)" != "podman"; then \
+			cp /etc/passwd $(CI_SCRATCHDIR); \
+			cp /etc/group $(CI_SCRATCHDIR); \
+		fi; \
 		echo "Cloning $(CI_GIT_ROOT) to $(CI_HOST_SRCDIR)"; \
 		git clone $(CI_GIT_ARGS) $(CI_GIT_ROOT) $(CI_HOST_SRCDIR) || exit 1; \
 		for mod in $(CI_SUBMODULES) ; \
@@ -150,7 +210,7 @@ ci-prepare-tree: ci-check-docker
 # gl_public_submodule_commit= to disable gnulib's submodule check
 # which breaks due to way we clone the submodules
 ci-build@%: ci-prepare-tree
-	docker run $(CI_DOCKER_ARGS) $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) \
+	$(CI_CENGINE) run $(CI_CENGINE_ARGS) $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) \
 		/bin/bash -c '\
 		mkdir -p $(CI_CONT_BUILDDIR) || exit 1 ; \
 		cd $(CI_CONT_BUILDDIR) ; \
@@ -179,11 +239,11 @@ ci-check@%:
 	$(MAKE) -f $(CI_MAKEFILE) ci-build@$* CI_MAKE_ARGS="check"
 
 ci-shell@%: ci-prepare-tree
-	docker run $(CI_DOCKER_ARGS) $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) /bin/bash
+	$(CI_CENGINE) run $(CI_CENGINE_ARGS) $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) /bin/bash
 	@test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || :
 
 ci-help:
-	@echo "Build libvirt inside Docker containers used for CI"
+	@echo "Build libvirt inside containers used for CI"
 	@echo
 	@echo "Available targets:"
 	@echo
@@ -215,6 +275,7 @@ ci-help:
 	@echo
 	@echo "Available make variables:"
 	@echo
+	@echo "    CI_CENGINE=engine - container engine to use (podman (default) or docker)"
 	@echo "    CI_CLEAN=0 - do not delete '$(CI_SCRATCHDIR)' after completion"
 	@echo "    CI_REUSE=1 - re-use existing '$(CI_SCRATCHDIR)' content"
 	@echo
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, 2019-05-07 at 17:45 +0200, Martin Kletzander wrote:
>  	@echo
>  	@echo "Available make variables:"
>  	@echo
> +	@echo "    CI_CENGINE=engine - container engine to use (podman (default) or docker)"
>  	@echo "    CI_CLEAN=0 - do not delete '$(CI_SCRATCHDIR)' after completion"
>  	@echo "    CI_REUSE=1 - re-use existing '$(CI_SCRATCHDIR)' content"
>  	@echo

I have not looked at the code in detail, but I wanted to point this
out before you respin: I really don't like the name CI_CENGINE :)

Since we're dealing with containers exclusively I'm pretty sure we
can just go with CI_ENGINE and not cause any confusion for users.

The description given above is also not accurate, as far as I can
tell: we use Podman instead of Docker only if we detect that the
former is available, so what we really do is *prefer* Podman, not
default to it. I would document it along the lines of

  CI_ENGINE=auto - container engine to use (podman, docker)

and implement it like

  CI_ENGINE=auto

  # ...

  ifeq ($(CI_ENGINE),auto)
    CI_ENGINE = $(shell ...)
  endif

  # Validate that CI_ENGINE is either podman or docker at this
  # point, and error out otherwise

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, May 10, 2019 at 10:08:05AM +0200, Andrea Bolognani wrote:
>On Tue, 2019-05-07 at 17:45 +0200, Martin Kletzander wrote:
>>  	@echo
>>  	@echo "Available make variables:"
>>  	@echo
>> +	@echo "    CI_CENGINE=engine - container engine to use (podman (default) or docker)"
>>  	@echo "    CI_CLEAN=0 - do not delete '$(CI_SCRATCHDIR)' after completion"
>>  	@echo "    CI_REUSE=1 - re-use existing '$(CI_SCRATCHDIR)' content"
>>  	@echo
>
>I have not looked at the code in detail, but I wanted to point this
>out before you respin: I really don't like the name CI_CENGINE :)
>
>Since we're dealing with containers exclusively I'm pretty sure we
>can just go with CI_ENGINE and not cause any confusion for users.
>
>The description given above is also not accurate, as far as I can
>tell: we use Podman instead of Docker only if we detect that the
>former is available, so what we really do is *prefer* Podman, not
>default to it. I would document it along the lines of
>
>  CI_ENGINE=auto - container engine to use (podman, docker)
>
>and implement it like
>
>  CI_ENGINE=auto
>
>  # ...
>
>  ifeq ($(CI_ENGINE),auto)
>    CI_ENGINE = $(shell ...)

This needs an override directive [1], because make, see "Overriding" [2].  But
it still looks better, so I'll change v2 with this.

[1] https://www.gnu.org/software/make/manual/html_node/Override-Directive.html#Override-Directive
[2] https://www.gnu.org/software/make/manual/html_node/Overriding.html

>  endif
>
>  # Validate that CI_ENGINE is either podman or docker at this
>  # point, and error out otherwise
>

I can't really do that, that would have to be in a rule to fail properly.  But
it's (kind of) done in ci-check-engine.  To check that it is one of the strings
I would have to parse it and check the first word, otherwise I couldn't do what
I can do now, e.g.:

  make -f Makefile.ci ci-check@debian-9 CI_ENGINE="podman --log-level=debug"

>--
>Andrea Bolognani / Red Hat / Virtualization
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, May 07, 2019 at 05:45:31PM +0200, Martin Kletzander wrote:
> This way more users can run our CI builds locally.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  Makefile.ci | 125 ++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 93 insertions(+), 32 deletions(-)
> 
> diff --git a/Makefile.ci b/Makefile.ci
> index 12a62167cc67..e2989ada313c 100644
> --- a/Makefile.ci
> +++ b/Makefile.ci
> @@ -17,7 +17,7 @@ CI_GIT_ROOT = $(shell git rev-parse --show-toplevel)
>  CI_HOST_SRCDIR = $(CI_SCRATCHDIR)/src
>  
>  # The directory holding the source inside the
> -# container. ie where we told Docker to expose
> +# container. ie where we want to expose
>  # the $(CI_HOST_SRCDIR) directory from the host
>  CI_CONT_SRCDIR = /src
>  
> @@ -46,14 +46,13 @@ CI_CONFIGURE_ARGS =
>  # cloning them
>  CI_SUBMODULES = $(shell git submodule | awk '{ print $$2 }')
>  
> -# Location of the Docker images we're going to pull
> +# Location of the container images we're going to pull
>  # Can be useful to overridde to use a locally built
>  # image instead
>  CI_IMAGE_PREFIX = quay.io/libvirt/buildenv-
>  
> -# Docker defaults to pulling the ':latest' tag but
> -# if the Docker repo above uses different conventions
> -# this can override it
> +# The default tag is ':latest' but if the container
> +# repo above uses different conventions this can override it
>  CI_IMAGE_TAG = :master
>  
>  # We delete the virtual root after completion, set
> @@ -71,24 +70,82 @@ CI_REUSE = 0
>  CI_UID = $(shell id -u)
>  CI_GID = $(shell id -g)
>  
> -# Docker doesn't require the IDs you run as to exist in
> +# Container engine runtime we are going to use, can be overridden per make
> +# invocation, if it is not, we try podman and then default to docker.
> +ifeq ($(CI_CENGINE),)
> +	CI_CENGINE = $(shell podman version >/dev/null && echo podman || echo docker)
> +endif
> +
> +# IDs you run as do not need to exist in
>  # the container's /etc/passwd & /etc/group files, but
> -# if they do not, then libvirt's  'make check' will fail
> +# if they do not, then libvirt's 'make check' will fail
>  # many tests.
> -#
> -# We do not directly mount /etc/{passwd,group} as Docker
> -# is liable to mess with SELinux labelling which will
> -# then prevent the host accessing them. Copying them
> -# first is safer.
> -CI_PWDB_MOUNTS = \
> -	--volume $(CI_SCRATCHDIR)/group:/etc/group:ro,z \
> -	--volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \
> -	$(NULL)
> +ifeq ($(CI_CENGINE),podman)
> +	CI_PWDB_MOUNTS = \
> +		--volume /etc/group:/etc/group:ro,z \
> +		--volume /etc/passwd:/etc/passwd:ro,z \
> +		$(NULL)
> +else
> +	# We do not directly mount /etc/{passwd,group} as Docker
> +	# is liable to mess with SELinux labelling which will
> +	# then prevent the host accessing them. Copying them
> +	# first is safer.
> +	CI_PWDB_MOUNTS = \
> +		--volume $(CI_SCRATCHDIR)/group:/etc/group:ro,z \
> +		--volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \
> +		$(NULL)
> +endif

Does this need to be conditionalized ?  Wouldn't podman just
work with the existing code.  How does podman end up giving
access to these files if it isn't changing the SELinux label
on them ?


> +
> +ifeq ($(CI_CENGINE),docker)
> +	# Docker containers can have very large ulimits
> +	# for nofiles - as much as 1048576. This makes
> +	# libvirt very slow at exec'ing programs.
> +	CI_ULIMIT_FILES = 1024
> +endif

Again, does this really need to be conditionalized ?

>  
> -# Docker containers can have very large ulimits
> -# for nofiles - as much as 1048576. This makes
> -# libvirt very slow at exec'ing programs.
> -CI_ULIMIT_FILES = 1024
> +ifeq ($(CI_CENGINE),podman)
> +	# Podman cannot reuse host namespace when running non-root containers.  Until
> +	# support for --keep-uid is added we can just create another mapping that will
> +	# do that for us.  Beware, that in {uid,git}map=container_id:host_id:range,
> +	# the host_id does actually refer to the uid in the first mapping where 0
> +	# (root) is mapped to the current user and rest is offset.
> +
> +	# In order to set up this mapping, we need to keep all the user IDs to prevent
> +	# possible errors as some images might expect UIDs up to 90000 (looking at you
> +	# fedora), so we don't want the overflowuid to be used for them.  For mapping
> +	# all the other users properly ther eneeds to be some math done.  Don't worry,
> +	# it's just addition and subtraction.
> +
> +	# 65536 ought to be enough (tm), but for really rare cases the maximums might
> +	# need to be higher, but that only happens when your /etc/sub{u,g}id allow
> +	# users to have more IDs.  Unless --keep-uid is supported, let's do this in a
> +	# way that should work for everyone.
> +	CI_MAX_UID = $(shell sed -n "s/^$USER:[^:]\+://p" /etc/subuid)
> +	CI_MAX_GID = $(shell sed -n "s/^$USER:[^:]\+://p" /etc/subgid)
> +	ifeq ($(CI_MAX_UID),)
> +		CI_MAX_UID = 65536
> +	endif
> +	ifeq ($(CI_MAX_GID),)
> +		CI_MAX_GID = 65536
> +	endif
> +	CI_UID_OTHER = $(shell echo $$(($(CI_UID)+1)))
> +	CI_GID_OTHER = $(shell echo $$(($(CI_GID)+1)))
> +	CI_UID_OTHER_RANGE = $(shell echo $$(($(CI_MAX_UID)-$(CI_UID))))
> +	CI_GID_OTHER_RANGE = $(shell echo $$(($(CI_MAX_GID)-$(CI_GID))))
> +
> +	CI_PODMAN_ARGS = \
> +		--uidmap 0:1:$(CI_UID) \
> +		--uidmap $(CI_UID):0:1 \
> +		--uidmap $(CI_UID_OTHER):$(CI_UID_OTHER):$(CI_UID_OTHER_RANGE) \
> +		--gidmap 0:1:$(CI_GID) \
> +		--gidmap $(CI_GID):0:1 \
> +		--gidmap $(CI_GID_OTHER):$(CI_GID_OTHER):$(CI_GID_OTHER_RANGE) \
> +		$(NULL)
> +else
> +	CI_DOCKER_ARGS = \
> +		--ulimit nofile=$(CI_ULIMIT_FILES):$(CI_ULIMIT_FILES) \
> +		$(NULL)
> +endif
>  
>  # Args to use when cloning a git repo.
>  #  -c  stop it complaining about checking out a random hash
> @@ -100,7 +157,7 @@ CI_GIT_ARGS = \
>  	--local  \
>  	$(NULL)
>  
> -# Args to use when running the Docker env
> +# Args to use when running the container
>  #   --rm      stop inactive containers getting left behind
>  #   --user    we execute as the same user & group account
>  #             as dev so that file ownership matches host
> @@ -110,27 +167,30 @@ CI_GIT_ARGS = \
>  #   --ulimit  lower files limit for performance reasons
>  #   --interactive
>  #   --tty     Ensure we have ability to Ctrl-C the build
> -CI_DOCKER_ARGS = \
> +CI_CENGINE_ARGS = \
>  	--rm \
>  	--user $(CI_UID):$(CI_GID) \
>  	--interactive \
>  	--tty \
> +	$(CI_PODMAN_ARGS) \
> +	$(CI_DOCKER_ARGS) \
>  	$(CI_PWDB_MOUNTS) \
>  	--volume $(CI_HOST_SRCDIR):$(CI_CONT_SRCDIR):z \
>  	--workdir $(CI_CONT_SRCDIR) \
> -	--ulimit nofile=$(CI_ULIMIT_FILES):$(CI_ULIMIT_FILES) \
>  	$(NULL)

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, May 09, 2019 at 02:06:18PM +0100, Daniel P. Berrangé wrote:
>On Tue, May 07, 2019 at 05:45:31PM +0200, Martin Kletzander wrote:
>> This way more users can run our CI builds locally.
>>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  Makefile.ci | 125 ++++++++++++++++++++++++++++++++++++++--------------
>>  1 file changed, 93 insertions(+), 32 deletions(-)
>>
>> diff --git a/Makefile.ci b/Makefile.ci
>> index 12a62167cc67..e2989ada313c 100644
>> --- a/Makefile.ci
>> +++ b/Makefile.ci
>> @@ -17,7 +17,7 @@ CI_GIT_ROOT = $(shell git rev-parse --show-toplevel)
>>  CI_HOST_SRCDIR = $(CI_SCRATCHDIR)/src
>>
>>  # The directory holding the source inside the
>> -# container. ie where we told Docker to expose
>> +# container. ie where we want to expose
>>  # the $(CI_HOST_SRCDIR) directory from the host
>>  CI_CONT_SRCDIR = /src
>>
>> @@ -46,14 +46,13 @@ CI_CONFIGURE_ARGS =
>>  # cloning them
>>  CI_SUBMODULES = $(shell git submodule | awk '{ print $$2 }')
>>
>> -# Location of the Docker images we're going to pull
>> +# Location of the container images we're going to pull
>>  # Can be useful to overridde to use a locally built
>>  # image instead
>>  CI_IMAGE_PREFIX = quay.io/libvirt/buildenv-
>>
>> -# Docker defaults to pulling the ':latest' tag but
>> -# if the Docker repo above uses different conventions
>> -# this can override it
>> +# The default tag is ':latest' but if the container
>> +# repo above uses different conventions this can override it
>>  CI_IMAGE_TAG = :master
>>
>>  # We delete the virtual root after completion, set
>> @@ -71,24 +70,82 @@ CI_REUSE = 0
>>  CI_UID = $(shell id -u)
>>  CI_GID = $(shell id -g)
>>
>> -# Docker doesn't require the IDs you run as to exist in
>> +# Container engine runtime we are going to use, can be overridden per make
>> +# invocation, if it is not, we try podman and then default to docker.
>> +ifeq ($(CI_CENGINE),)
>> +	CI_CENGINE = $(shell podman version >/dev/null && echo podman || echo docker)
>> +endif
>> +
>> +# IDs you run as do not need to exist in
>>  # the container's /etc/passwd & /etc/group files, but
>> -# if they do not, then libvirt's  'make check' will fail
>> +# if they do not, then libvirt's 'make check' will fail
>>  # many tests.
>> -#
>> -# We do not directly mount /etc/{passwd,group} as Docker
>> -# is liable to mess with SELinux labelling which will
>> -# then prevent the host accessing them. Copying them
>> -# first is safer.
>> -CI_PWDB_MOUNTS = \
>> -	--volume $(CI_SCRATCHDIR)/group:/etc/group:ro,z \
>> -	--volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \
>> -	$(NULL)
>> +ifeq ($(CI_CENGINE),podman)
>> +	CI_PWDB_MOUNTS = \
>> +		--volume /etc/group:/etc/group:ro,z \
>> +		--volume /etc/passwd:/etc/passwd:ro,z \
>> +		$(NULL)
>> +else
>> +	# We do not directly mount /etc/{passwd,group} as Docker
>> +	# is liable to mess with SELinux labelling which will
>> +	# then prevent the host accessing them. Copying them
>> +	# first is safer.
>> +	CI_PWDB_MOUNTS = \
>> +		--volume $(CI_SCRATCHDIR)/group:/etc/group:ro,z \
>> +		--volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \
>> +		$(NULL)
>> +endif
>
>Does this need to be conditionalized ?  Wouldn't podman just
>work with the existing code.  How does podman end up giving

It would, but it seemed pointless to copy the files when it is not needed.

>access to these files if it isn't changing the SELinux label
>on them ?
>

Oh right, I haven't tried with SELinux.  I kind of hope that fuse-overlayfs
would solve that, but they are just starting with these things, so yeah, you're
right, this needs to be there due to SELinux for both podman and docker.

>
>> +
>> +ifeq ($(CI_CENGINE),docker)
>> +	# Docker containers can have very large ulimits
>> +	# for nofiles - as much as 1048576. This makes
>> +	# libvirt very slow at exec'ing programs.
>> +	CI_ULIMIT_FILES = 1024
>> +endif
>
>Again, does this really need to be conditionalized ?
>

Not really, it can be left unconditional.

I'll send a clean v2.

>>
>> -# Docker containers can have very large ulimits
>> -# for nofiles - as much as 1048576. This makes
>> -# libvirt very slow at exec'ing programs.
>> -CI_ULIMIT_FILES = 1024
>> +ifeq ($(CI_CENGINE),podman)
>> +	# Podman cannot reuse host namespace when running non-root containers.  Until
>> +	# support for --keep-uid is added we can just create another mapping that will
>> +	# do that for us.  Beware, that in {uid,git}map=container_id:host_id:range,
>> +	# the host_id does actually refer to the uid in the first mapping where 0
>> +	# (root) is mapped to the current user and rest is offset.
>> +
>> +	# In order to set up this mapping, we need to keep all the user IDs to prevent
>> +	# possible errors as some images might expect UIDs up to 90000 (looking at you
>> +	# fedora), so we don't want the overflowuid to be used for them.  For mapping
>> +	# all the other users properly ther eneeds to be some math done.  Don't worry,
>> +	# it's just addition and subtraction.
>> +
>> +	# 65536 ought to be enough (tm), but for really rare cases the maximums might
>> +	# need to be higher, but that only happens when your /etc/sub{u,g}id allow
>> +	# users to have more IDs.  Unless --keep-uid is supported, let's do this in a
>> +	# way that should work for everyone.
>> +	CI_MAX_UID = $(shell sed -n "s/^$USER:[^:]\+://p" /etc/subuid)
>> +	CI_MAX_GID = $(shell sed -n "s/^$USER:[^:]\+://p" /etc/subgid)
>> +	ifeq ($(CI_MAX_UID),)
>> +		CI_MAX_UID = 65536
>> +	endif
>> +	ifeq ($(CI_MAX_GID),)
>> +		CI_MAX_GID = 65536
>> +	endif
>> +	CI_UID_OTHER = $(shell echo $$(($(CI_UID)+1)))
>> +	CI_GID_OTHER = $(shell echo $$(($(CI_GID)+1)))
>> +	CI_UID_OTHER_RANGE = $(shell echo $$(($(CI_MAX_UID)-$(CI_UID))))
>> +	CI_GID_OTHER_RANGE = $(shell echo $$(($(CI_MAX_GID)-$(CI_GID))))
>> +
>> +	CI_PODMAN_ARGS = \
>> +		--uidmap 0:1:$(CI_UID) \
>> +		--uidmap $(CI_UID):0:1 \
>> +		--uidmap $(CI_UID_OTHER):$(CI_UID_OTHER):$(CI_UID_OTHER_RANGE) \
>> +		--gidmap 0:1:$(CI_GID) \
>> +		--gidmap $(CI_GID):0:1 \
>> +		--gidmap $(CI_GID_OTHER):$(CI_GID_OTHER):$(CI_GID_OTHER_RANGE) \
>> +		$(NULL)
>> +else
>> +	CI_DOCKER_ARGS = \
>> +		--ulimit nofile=$(CI_ULIMIT_FILES):$(CI_ULIMIT_FILES) \
>> +		$(NULL)
>> +endif
>>
>>  # Args to use when cloning a git repo.
>>  #  -c  stop it complaining about checking out a random hash
>> @@ -100,7 +157,7 @@ CI_GIT_ARGS = \
>>  	--local  \
>>  	$(NULL)
>>
>> -# Args to use when running the Docker env
>> +# Args to use when running the container
>>  #   --rm      stop inactive containers getting left behind
>>  #   --user    we execute as the same user & group account
>>  #             as dev so that file ownership matches host
>> @@ -110,27 +167,30 @@ CI_GIT_ARGS = \
>>  #   --ulimit  lower files limit for performance reasons
>>  #   --interactive
>>  #   --tty     Ensure we have ability to Ctrl-C the build
>> -CI_DOCKER_ARGS = \
>> +CI_CENGINE_ARGS = \
>>  	--rm \
>>  	--user $(CI_UID):$(CI_GID) \
>>  	--interactive \
>>  	--tty \
>> +	$(CI_PODMAN_ARGS) \
>> +	$(CI_DOCKER_ARGS) \
>>  	$(CI_PWDB_MOUNTS) \
>>  	--volume $(CI_HOST_SRCDIR):$(CI_CONT_SRCDIR):z \
>>  	--workdir $(CI_CONT_SRCDIR) \
>> -	--ulimit nofile=$(CI_ULIMIT_FILES):$(CI_ULIMIT_FILES) \
>>  	$(NULL)
>
>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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list