/test

/test

RUN_AS_ROOT is only used for make run_bpf_tests. Will this cause the tests under bpf/tests to run with a cache but invalidate the cache for other workflows?

RUN_AS_ROOT is only used for make run_bpf_tests. Will this cause the tests under bpf/tests to run with a cache but invalidate the cache for other workflows?

@joestringer I'm not sure I follow, the tests workflows were not changed. This workflow is only used to generate the golang caches which there's a step further down in the workflow to revert the owner from root.

Now I'm confused 😅

The GitHub workflow before this PR:

        env:
          BUILDER_GOCACHE_DIR: "/tmp/.cache/go/.cache/go-build"
          BUILDER_GOMODCACHE_DIR: "/tmp/.cache/go/pkg"

which then invokes contrib/scripts/builder.sh that does:

if [ -z "${RUN_AS_ROOT:-}" ]; then                                                                                                                               
    USER_OPTION="--user $(id -u):$(id -g)"                                                                                                                       
    USER_PATH="$HOME"                                                                                                                                            
fi

...

if [ -n "${BUILDER_GOCACHE_DIR:-}" ]; then                                                                                                                       
    MOUNT_GOCACHE_DIR="-v ${BUILDER_GOCACHE_DIR}:${USER_PATH}/.cache/go-build"                                                                                   
fi

...

if [ -n "${BUILDER_GOMODCACHE_DIR:-}" ]; then                                                                                                                    
    MOUNT_GOMODCACHE_DIR="-v ${BUILDER_GOMODCACHE_DIR}:/go/pkg/mod"                                                                                              
fi 

...

docker run --rm \
   $USER_OPTION \
   $MOUNT_GOCACHE_DIR \
   $MOUNT_GOMODCACHE_DIR \
   $MOUNT_CCACHE_DIR \
   ...

So even if the script runs as non-root, I would expect it to respect the specified environment variables and place the relevant files under /tmp/.cache/go. We might not even need the chown because the docker container would be run as the current user by default instead.

One slightly fishy thing above is that the BUILDER_GOCACHE_DIR is being mounted into the container based on a path based on the user with a subdirectory under .cache, while the equivalent gomod environment variable BUILDER_GOMODCACHE_DIR mounts under /go/pkg/mod. I'm not sure where the go build would cache by default inside the container when run as non-root 🤔

EDIT: Well, there's an answer:

$ docker run --user $(id -u):$(id -g) \
             $(cat images/cilium/Dockerfile | grep '^ARG CILIUM_BUILDER_IMAGE=' | cut -d '=' -f 2) \
    go env \
  | grep CACHE
GOCACHE='/home/ubuntu/.cache/go-build'
GOCACHEPROG=''
GOMODCACHE='/go/pkg/mod'

(Note that my username on my host is not ubuntu; that is the username for uid=1000 in the builder image)

I guess the question I have is, why do we need to run this build as root? I don't see a reason, other than maybe it aligns some directory paths in a better way. But then which directory path is supposed to be different? Maybe we can just fix the directory path or something?

Here's my counter-proposal:

diff --git a/contrib/scripts/builder.sh b/contrib/scripts/builder.sh
index b094053dd46a..f26c60118cf8 100755
--- a/contrib/scripts/builder.sh
+++ b/contrib/scripts/builder.sh
@@ -20,7 +20,7 @@ fi
 
 if [ -z "${RUN_AS_ROOT:-}" ]; then
     USER_OPTION="--user $(id -u):$(id -g)"
-    USER_PATH="$HOME"
+    USER_PATH="/home/ubuntu"
 fi
 
 MOUNT_GOCACHE_DIR=""
@@ -44,7 +44,7 @@ fi
 MOUNT_CCACHE_DIR=""
 
 if [ -z "${BUILDER_CCACHE_DIR:-}" ]; then
-    MOUNT_CCACHE_DIR="-v ${USER_PATH}/.ccache:${USER_PATH}/.ccache"
+    MOUNT_CCACHE_DIR="-v ${HOME}/.ccache:${USER_PATH}/.ccache"
 else
     MOUNT_CCACHE_DIR="-v ${BUILDER_CCACHE_DIR}:${USER_PATH}/.ccache"
 fi

Here's my counter-proposal:

diff --git a/contrib/scripts/builder.sh b/contrib/scripts/builder.sh
index b094053dd46a..f26c60118cf8 100755
--- a/contrib/scripts/builder.sh
+++ b/contrib/scripts/builder.sh
@@ -20,7 +20,7 @@ fi
 
 if [ -z "${RUN_AS_ROOT:-}" ]; then
     USER_OPTION="--user $(id -u):$(id -g)"
-    USER_PATH="$HOME"
+    USER_PATH="/home/ubuntu"
 fi
 
 MOUNT_GOCACHE_DIR=""
@@ -44,7 +44,7 @@ fi
 MOUNT_CCACHE_DIR=""
 
 if [ -z "${BUILDER_CCACHE_DIR:-}" ]; then
-    MOUNT_CCACHE_DIR="-v ${USER_PATH}/.ccache:${USER_PATH}/.ccache"
+    MOUNT_CCACHE_DIR="-v ${HOME}/.ccache:${USER_PATH}/.ccache"
 else
     MOUNT_CCACHE_DIR="-v ${BUILDER_CCACHE_DIR}:${USER_PATH}/.ccache"
 fi

@joestringer I also tried that before but it doesn't work 😅 . It doesn't work because "--user $(id -u):$(id -g)" is translated to "--user 1001:1001" which is caused by the GH runners change from 1000 to 1001. This then prevents the builder image to create / modify the directory /home/ubuntu/"go-cache". That's why we will run it as root so that it can write into that directory regardless of the uid of the GH runners.