[RedMakeUp]

The RUNC_ALLOW_UNSAFE_TESTS variable set in the Cirrus CI env block does not reach the integration tests because they are executed via ssh -tt localhost "make ...", which starts a new login shell that does not inherit the caller's environment. As a result, unsafe tests are always skipped in Cirrus CI even though the intent is to run them.

Fix this by exporting the variable in /root/.bashrc (same way we already handle PATH), so the ssh session picks it up.

See #5212 (comment).

[RedMakeUp]

I've already checked the logs in CI:
ok 46 checkpoint and restore with container specific CRIU config without the # skip test requires unsafe comment,

which comfirm the RUNC_ALLOW_UNSAFE_TESTS works

[kolyshkin]

Since we always use integration tests via ssh, we can remove the var from the environment and add it to .bashrc directly. IOW

diff --git a/.cirrus.yml b/.cirrus.yml
index 0f845eb8..7ec3b1d6 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -15,8 +15,6 @@ task:
     BATS_VERSION: "v1.12.0"
     LIBPATHRS_VERSION: "0.2.4"
     RPMS: gcc git-core iptables jq glibc-static libseccomp-devel make criu fuse-sshfs container-selinux policycoreutils cargo lld wget
-    # Allow potentially unsafe tests.
-    RUNC_ALLOW_UNSAFE_TESTS: yes
     # yamllint disable rule:key-duplicates
     matrix:
       - DISTRO: almalinux-8
@@ -85,6 +83,8 @@ task:
     /home/runc/script/setup_rootless.sh
     # set PATH
     echo 'export PATH=/usr/local/go/bin:/usr/local/bin:$PATH' >> /root/.bashrc
+    # Allow potentially unsafe tests.
+    echo 'export RUNC_ALLOW_UNSAFE_TESTS=yes' >> /root/.bashrc
     # Setup ssh localhost for terminal emulation (script -e did not work)
     ssh-keygen -t ed25519 -f /root/.ssh/id_ed25519 -N ""
     cat /root/.ssh/id_ed25519.pub >> /root/.ssh/authorized_keys

[kolyshkin]

Can you please add

Fixes: 9932ad19 ("tests/int: introduce the concept of unsafe tests")

just before the Signed-off-by: line?

[kolyshkin]

NB: we do not have this problem with rootless tests (which are also executed via ssh -tt, see tests/rootless.sh) because all unsafe tests also require root.

[RedMakeUp]

Can you please add

Fixes: 9932ad19 ("tests/int: introduce the concept of unsafe tests")

just before the Signed-off-by: line?

Done

[RedMakeUp]

Since we always use integration tests via ssh, we can remove the var from the environment and add it to .bashrc directly. IOW

diff --git a/.cirrus.yml b/.cirrus.yml
index 0f845eb8..7ec3b1d6 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -15,8 +15,6 @@ task:
     BATS_VERSION: "v1.12.0"
     LIBPATHRS_VERSION: "0.2.4"
     RPMS: gcc git-core iptables jq glibc-static libseccomp-devel make criu fuse-sshfs container-selinux policycoreutils cargo lld wget
-    # Allow potentially unsafe tests.
-    RUNC_ALLOW_UNSAFE_TESTS: yes
     # yamllint disable rule:key-duplicates
     matrix:
       - DISTRO: almalinux-8
@@ -85,6 +83,8 @@ task:
     /home/runc/script/setup_rootless.sh
     # set PATH
     echo 'export PATH=/usr/local/go/bin:/usr/local/bin:$PATH' >> /root/.bashrc
+    # Allow potentially unsafe tests.
+    echo 'export RUNC_ALLOW_UNSAFE_TESTS=yes' >> /root/.bashrc
     # Setup ssh localhost for terminal emulation (script -e did not work)
     ssh-keygen -t ed25519 -f /root/.ssh/id_ed25519 -N ""
     cat /root/.ssh/id_ed25519.pub >> /root/.ssh/authorized_keys

Done

[kolyshkin]

LGTM except for a minor nit about the set PATH comment being modified.

[RedMakeUp]

LGTM except for a minor nit about the set PATH comment being modified.

oh, sorry about that. I've removed the unnecessary comment change.

[lifubang]

The fix works and follows the existing pattern (same as how PATH is propagated), so I'm not opposed to merging it. That said, I want to flag a subtle correctness concern for future reference.

ssh -tt localhost "make ..." -- even with -tt forcing PTY allocation -- runs a non-interactive shell because a command is provided. According to bash rules, a non-interactive shell does not source ~/.bashrc. The reason this works in practice is likely that this CI environment's /root/.bash_profile sources ~/.bashrc (a common distro default).

This means the fix is environment-specific and could silently break if the base image changes or a different distro is used.

A more portable alternative would be:

# Option 1: inline the variable in the ssh command itself
ssh -tt localhost "RUNC_ALLOW_UNSAFE_TESTS=yes make ..."

# Option 2: write to .bash_profile instead of .bashrc,
# since login shells source .bash_profile, not .bashrc directly
echo 'export RUNC_ALLOW_UNSAFE_TESTS=yes' >> /root/.bash_profile

If the team is confident the CI image always sources .bashrc from .bash_profile (which is reasonable given the PATH precedent), a short comment in .cirrus.yml explaining why .bashrc is used here would help future maintainers avoid confusion.

Minor concern overall -- the fix is pragmatic and demonstrably works. Just worth documenting the assumption.

[kolyshkin]

You're right in everything, including that most distros source ~/.bash_profile from ~/.bashrc, this is even described in bash manual:

   So, typically, your ‘~/.bash_profile’ contains the line
     if [ -f ~/.bashrc ]; then . ~/.bashrc; fi
after (or before) any login-specific initializations.

Therefore I guess we can rely on that.

In any case, I think if PATH is not set the tests won't work (go binary missing etc).

[lifubang]

LGTM, feel free to merge.