[libvirt PATCH v2 4/4] ci: Makefile: Expose the CI_USER_LOGIN variable for users to use

Erik Skultety posted 4 patches 4 years, 12 months ago
[libvirt PATCH v2 4/4] ci: Makefile: Expose the CI_USER_LOGIN variable for users to use
Posted by Erik Skultety 4 years, 12 months ago
More often than not I find myself debugging in the containers which
means that I need to have root inside, but without manually tweaking
the Makefile each time the execution would simply fail thanks to the
uid/gid mapping we do. What if we expose the CI_USER_LOGIN variable, so
that when needed, the root can be simply passed with this variable and
voila - you have a root shell inside the container with CWD=~root.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 ci/Makefile | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/ci/Makefile b/ci/Makefile
index 9308738d2d..05a50c021c 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -45,15 +45,15 @@ CI_CLEAN = 1
 # preserved env
 CI_REUSE = 0
 
-# We need the container process to run with current host IDs
-# so that it can access the passed in build directory
-CI_UID = $(shell id -u)
-CI_GID = $(shell id -g)
-
-# We also need the user's login and home directory to prepare the
+# We need the user's login and home directory to prepare the
 # environment the way some programs expect it
-CI_USER_LOGIN = $(shell echo "$$USER")
-CI_USER_HOME = $(shell echo "$$HOME")
+CI_USER_LOGIN = $(shell whoami)
+CI_USER_HOME = $(shell eval echo "~$(CI_USER_LOGIN)")
+
+# We also need the container process to run with current host IDs
+# so that it can access the passed in build directory
+CI_UID = $(shell id -u "$(CI_USER_LOGIN)")
+CI_GID = $(shell id -g "$(CI_USER_LOGIN)")
 
 CI_ENGINE = auto
 # Container engine we are going to use, can be overridden per make
@@ -124,14 +124,16 @@ ifeq ($(CI_ENGINE),podman)
 	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)
+	ifneq ($(CI_UID), 0)
+		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)
+	endif
 endif
 
 # Args to use when cloning a git repo.
@@ -239,6 +241,7 @@ ci-help:
 	@echo "    CI_CLEAN=0          - do not delete '$(CI_SCRATCHDIR)' after completion"
 	@echo "    CI_REUSE=1          - re-use existing '$(CI_SCRATCHDIR)' content"
 	@echo "    CI_ENGINE=auto      - container engine to use (podman, docker)"
+	@echo "    CI_USER_LOGIN=$$USER      - which user should run in the container"
 	@echo "    CI_MESON_ARGS=      - extra arguments passed to meson"
 	@echo "    CI_NINJA_ARGS=      - extra arguments passed to ninja"
 	@echo
-- 
2.29.2

Re: [libvirt PATCH v2 4/4] ci: Makefile: Expose the CI_USER_LOGIN variable for users to use
Posted by Andrea Bolognani 4 years, 12 months ago
On Fri, 2021-02-12 at 15:19 +0100, Erik Skultety wrote:
> @@ -239,6 +241,7 @@ ci-help:
>  	@echo "    CI_CLEAN=0          - do not delete '$(CI_SCRATCHDIR)' after completion"
>  	@echo "    CI_REUSE=1          - re-use existing '$(CI_SCRATCHDIR)' content"
>  	@echo "    CI_ENGINE=auto      - container engine to use (podman, docker)"
> +	@echo "    CI_USER_LOGIN=$$USER      - which user should run in the container"

Alignment is completely broken here. But I just realized that $USER
will get expanded here, so we have basically no way of keeping that
consistent...

Basically, your original version of the help message was better,
please go back to that one :)

Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH v2 4/4] ci: Makefile: Expose the CI_USER_LOGIN variable for users to use
Posted by Erik Skultety 4 years, 12 months ago
On Fri, Feb 12, 2021 at 04:04:04PM +0100, Andrea Bolognani wrote:
> On Fri, 2021-02-12 at 15:19 +0100, Erik Skultety wrote:
> > @@ -239,6 +241,7 @@ ci-help:
> >  	@echo "    CI_CLEAN=0          - do not delete '$(CI_SCRATCHDIR)' after completion"
> >  	@echo "    CI_REUSE=1          - re-use existing '$(CI_SCRATCHDIR)' content"
> >  	@echo "    CI_ENGINE=auto      - container engine to use (podman, docker)"
> > +	@echo "    CI_USER_LOGIN=$$USER      - which user should run in the container"
> 
> Alignment is completely broken here. But I just realized that $USER
> will get expanded here, so we have basically no way of keeping that
> consistent...

I didn't want to point it out earlier, since to me it's not a deal breaker :).

> 
> Basically, your original version of the help message was better,
> please go back to that one :)

Sure, will do, thanks for the review.

Erik

> 
> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
> 
> -- 
> Andrea Bolognani / Red Hat / Virtualization
>