From 533e5905da45c6b244b9cfa4310bba22c8f7b504 Mon Sep 17 00:00:00 2001 From: mcasquer Date: Tue, 1 Oct 2024 13:14:52 +0200 Subject: [PATCH 1/7] Makes the pidfile accessible by everyone Creates the pidfile at an accessible location for every user, this way the manage pidfile warning is avoided. Signed-off-by: mcasquer --- tmt/steps/execute/internal.py | 2 +- tmt/steps/provision/__init__.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tmt/steps/execute/internal.py b/tmt/steps/execute/internal.py index 92a5cd4aed..b3d339f5db 100644 --- a/tmt/steps/execute/internal.py +++ b/tmt/steps/execute/internal.py @@ -34,7 +34,7 @@ TEST_PIDFILE_LOCK_FILENAME = f'{TEST_PIDFILE_FILENAME}.lock' #: The default directory for storing test pid file. -TEST_PIDFILE_ROOT = Path('/var/tmp') # noqa: S108 insecure usage of temporary dir +TEST_PIDFILE_ROOT = Path('/var/tmp/pid') # noqa: S108 insecure usage of temporary dir def effective_pidfile_root() -> Path: diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index 5c440a6000..ca80e63fa7 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -923,6 +923,9 @@ def setup(self) -> None: Setup the guest after it has been started. It is called after :py:meth:`Guest.start`. """ + from tmt.steps.execute.internal import effective_pidfile_root + pid_directory = effective_pidfile_root() + self.execute(ShellScript(f"mkdir -p {pid_directory} && chmod ugo+rwx {pid_directory}")) # A couple of requiremens for this field: # From 61a14dabfa211bc290dd7badbada2a0451e40af4 Mon Sep 17 00:00:00 2001 From: mcasquer Date: Mon, 4 Nov 2024 11:50:59 +0100 Subject: [PATCH 2/7] Adds a check to only create the pid directory when it doesn't exist. Signed-off-by: mcasquer --- tmt/steps/execute/internal.py | 2 +- tmt/steps/provision/__init__.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tmt/steps/execute/internal.py b/tmt/steps/execute/internal.py index b3d339f5db..b9a94259cb 100644 --- a/tmt/steps/execute/internal.py +++ b/tmt/steps/execute/internal.py @@ -34,7 +34,7 @@ TEST_PIDFILE_LOCK_FILENAME = f'{TEST_PIDFILE_FILENAME}.lock' #: The default directory for storing test pid file. -TEST_PIDFILE_ROOT = Path('/var/tmp/pid') # noqa: S108 insecure usage of temporary dir +TEST_PIDFILE_ROOT = Path('/var/tmp/tmt/pid') # noqa: S108 insecure usage of temporary dir def effective_pidfile_root() -> Path: diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index ca80e63fa7..89a90b38af 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -925,7 +925,9 @@ def setup(self) -> None: """ from tmt.steps.execute.internal import effective_pidfile_root pid_directory = effective_pidfile_root() - self.execute(ShellScript(f"mkdir -p {pid_directory} && chmod ugo+rwx {pid_directory}")) + command = f"if [ ! -d {pid_directory} ]; then mkdir -p {pid_directory} \ + && chmod ugo+rwx {pid_directory}; fi" + self.execute(ShellScript(command)) # A couple of requiremens for this field: # From 46d6991740505de0773eb0571155f3cecffbf7f3 Mon Sep 17 00:00:00 2001 From: mcasquer Date: Wed, 6 Nov 2024 13:36:10 +0100 Subject: [PATCH 3/7] Fixes the tmt reboot test, updating the pidfile location in the scripts. Also formats the pid directory creation command. Signed-off-by: mcasquer --- tests/execute/reboot/out-of-session.sh | 8 +++++++- tests/provision/become/test.sh | 12 ++++++------ tmt/steps/execute/internal.py | 4 ++++ tmt/steps/execute/scripts/tmt-reboot | 4 +++- tmt/steps/execute/scripts/tmt-reboot-core | 4 +++- tmt/steps/provision/__init__.py | 11 ++++++++--- 6 files changed, 31 insertions(+), 12 deletions(-) diff --git a/tests/execute/reboot/out-of-session.sh b/tests/execute/reboot/out-of-session.sh index 119b03028c..16d14cb9be 100755 --- a/tests/execute/reboot/out-of-session.sh +++ b/tests/execute/reboot/out-of-session.sh @@ -4,6 +4,12 @@ rlJournalStart rlPhaseStartSetup rlRun "PROVISION_HOW=${PROVISION_HOW:-container}" + + # To allow running the test outside of tmt. + rlRun "TMT_TEST_PIDFILE_ROOT=${TMT_TEST_PIDFILE_ROOT:-/var/tmp/tmt/pid}" + rlRun "TMT_TEST_PIDFILE=${TMT_TEST_PIDFILE:-$TMT_TEST_PIDFILE_ROOT/tmt-test.pid}" + rlRun "TMT_TEST_PIDFILE_LOCK=${TMT_TEST_PIDFILE_LOCK:-$TMT_TEST_PIDFILE.lock}" + rlRun "run=\$(mktemp -d -p /var/tmp)" 0 "Create run directory" rlRun "set -o pipefail" rlRun "pushd out-of-session" @@ -39,7 +45,7 @@ rlJournalStart # and use it to issue a `tmt-reboot` from outside the direct process tree of the test. set -x - tmt_reboot_command="export TMT_TEST_PIDFILE=/var/tmp/tmt-test.pid; export TMT_TEST_PIDFILE_LOCK=/var/tmp/tmt-test.pid.lock; export TMT_DEBUG=1; tmt-reboot" + tmt_reboot_command="export TMT_TEST_PIDFILE=$TMT_TEST_PIDFILE; export TMT_TEST_PIDFILE_LOCK=$TMT_TEST_PIDFILE_LOCK; export TMT_DEBUG=1; tmt-reboot" if [ "$PROVISION_HOW" = "container" ]; then podman_exec="$(sed -nr 's/\s*Run command: (podman exec .*) \/bin\/bash.*cd.*/\1/p' tmt.output | head -1)" diff --git a/tests/provision/become/test.sh b/tests/provision/become/test.sh index 9480ead94c..6d61940c7e 100755 --- a/tests/provision/become/test.sh +++ b/tests/provision/become/test.sh @@ -13,27 +13,27 @@ rlJournalStart rlPhaseEnd rlPhaseStartTest "$PROVISION_HOW, test with become=true" - rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /test/root" + rlRun "sudo tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /test/root" rlPhaseEnd rlPhaseStartTest "$PROVISION_HOW, test with become=false" - rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /test/user" + rlRun "sudo tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /test/user" rlPhaseEnd rlPhaseStartTest "$PROVISION_HOW, prepare/finish inline with become=true" - rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/root/inline" + rlRun "sudo tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/root/inline" rlPhaseEnd rlPhaseStartTest "$PROVISION_HOW, prepare/finish inline with become=false" - rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/user/inline" + rlRun "sudo tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/user/inline" rlPhaseEnd rlPhaseStartTest "$PROVISION_HOW, prepare/finish scripts with become=true" - rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/root/scripts" + rlRun "sudo tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/root/scripts" rlPhaseEnd rlPhaseStartTest "$PROVISION_HOW, prepare/finish scripts with become=false" - rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/user/scripts" + rlRun "sudo tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/user/scripts" rlPhaseEnd if [[ "$PROVISION_HOW" == "virtual" ]]; then diff --git a/tmt/steps/execute/internal.py b/tmt/steps/execute/internal.py index b9a94259cb..dcd554e9c0 100644 --- a/tmt/steps/execute/internal.py +++ b/tmt/steps/execute/internal.py @@ -30,6 +30,8 @@ format_timestamp, ) +# Note: if modified, pidfile root, filename, and lock filename must be +# also changed in `tmt-reboot` and `tmt-reboot-core` scripts. TEST_PIDFILE_FILENAME = 'tmt-test.pid' TEST_PIDFILE_LOCK_FILENAME = f'{TEST_PIDFILE_FILENAME}.lock' @@ -258,6 +260,8 @@ def _test_environment( assert isinstance(self.parent, tmt.steps.execute.Execute) assert self.parent.plan.my_run is not None + environment['TMT_TEST_PIDFILE_ROOT'] = EnvVarValue( + effective_pidfile_root()) environment['TMT_TEST_PIDFILE'] = EnvVarValue( effective_pidfile_root() / TEST_PIDFILE_FILENAME) environment['TMT_TEST_PIDFILE_LOCK'] = EnvVarValue( diff --git a/tmt/steps/execute/scripts/tmt-reboot b/tmt/steps/execute/scripts/tmt-reboot index 259dbec0b2..3851170ad0 100755 --- a/tmt/steps/execute/scripts/tmt-reboot +++ b/tmt/steps/execute/scripts/tmt-reboot @@ -6,7 +6,9 @@ [ -n "$TMT_DEBUG" ] && set -x -TMT_TEST_PIDFILE_LOCK="${TMT_TEST_PIDFILE_LOCK:-/var/tmp/tmt-test.pid}" +TMT_TEST_PIDFILE_ROOT="${TMT_TEST_PIDFILE_ROOT:-/var/tmp/tmt/pid}" +TMT_TEST_PIDFILE="${TMT_TEST_PIDFILE:-$TMT_TEST_PIDFILE_ROOT/tmt-test.pid}" +TMT_TEST_PIDFILE_LOCK="${TMT_TEST_PIDFILE_LOCK:-$TMT_TEST_PIDFILE.lock}" PATH=/sbin:/usr/sbin:$PATH diff --git a/tmt/steps/execute/scripts/tmt-reboot-core b/tmt/steps/execute/scripts/tmt-reboot-core index 96ed3b8aba..3057cfa2e0 100755 --- a/tmt/steps/execute/scripts/tmt-reboot-core +++ b/tmt/steps/execute/scripts/tmt-reboot-core @@ -2,7 +2,9 @@ [ -n "$TMT_DEBUG" ] && set -x -TMT_TEST_PIDFILE="${TMT_TEST_PIDFILE:-/var/tmp/tmt-test.pid}" +TMT_TEST_PIDFILE_ROOT="${TMT_TEST_PIDFILE_ROOT:-/var/tmp/tmt/pid}" +TMT_TEST_PIDFILE="${TMT_TEST_PIDFILE:-$TMT_TEST_PIDFILE_ROOT/tmt-test.pid}" +TMT_TEST_PIDFILE_LOCK="${TMT_TEST_PIDFILE_LOCK:-$TMT_TEST_PIDFILE.lock}" if [ ! -e "$TMT_TEST_PIDFILE" ]; then echo "There is no running test to signal the reboot to!" diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index 89a90b38af..38ed15721b 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -925,9 +925,14 @@ def setup(self) -> None: """ from tmt.steps.execute.internal import effective_pidfile_root pid_directory = effective_pidfile_root() - command = f"if [ ! -d {pid_directory} ]; then mkdir -p {pid_directory} \ - && chmod ugo+rwx {pid_directory}; fi" - self.execute(ShellScript(command)) + self.execute(ShellScript( + f""" + if [ ! -d {pid_directory} ]; then \ + mkdir -p {pid_directory} \ + && chmod ugo+rwx {pid_directory}; \ + fi + """ + )) # A couple of requiremens for this field: # From 490d93989eb7b3322e2c09fe55e195373215894a Mon Sep 17 00:00:00 2001 From: mcasquer Date: Wed, 6 Nov 2024 20:42:51 +0100 Subject: [PATCH 4/7] Updates the environment variables in out-of-session test. This way we avoid TestingFarm to set the old values. Signed-off-by: mcasquer --- tests/execute/reboot/out-of-session.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/execute/reboot/out-of-session.sh b/tests/execute/reboot/out-of-session.sh index 16d14cb9be..8b36cfe33f 100755 --- a/tests/execute/reboot/out-of-session.sh +++ b/tests/execute/reboot/out-of-session.sh @@ -6,9 +6,9 @@ rlJournalStart rlRun "PROVISION_HOW=${PROVISION_HOW:-container}" # To allow running the test outside of tmt. - rlRun "TMT_TEST_PIDFILE_ROOT=${TMT_TEST_PIDFILE_ROOT:-/var/tmp/tmt/pid}" - rlRun "TMT_TEST_PIDFILE=${TMT_TEST_PIDFILE:-$TMT_TEST_PIDFILE_ROOT/tmt-test.pid}" - rlRun "TMT_TEST_PIDFILE_LOCK=${TMT_TEST_PIDFILE_LOCK:-$TMT_TEST_PIDFILE.lock}" + rlRun "TMT_TEST_PIDFILE_ROOT=/var/tmp/tmt/pid" + rlRun "TMT_TEST_PIDFILE=$TMT_TEST_PIDFILE_ROOT/tmt-test.pid" + rlRun "TMT_TEST_PIDFILE_LOCK=$TMT_TEST_PIDFILE.lock" rlRun "run=\$(mktemp -d -p /var/tmp)" 0 "Create run directory" rlRun "set -o pipefail" From a450d3abbd2ebc10cfad36b2bf70ffe441dc5e82 Mon Sep 17 00:00:00 2001 From: mcasquer Date: Wed, 6 Nov 2024 21:54:45 +0100 Subject: [PATCH 5/7] Removes the sudo(s) from tmt commands and instead apply it directly in the pidfile creation. Also includes the super().setup() call to the setup GuestSsh function. Signed-off-by: mcasquer --- tests/provision/become/test.sh | 12 ++++++------ tmt/steps/provision/__init__.py | 6 ++++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/provision/become/test.sh b/tests/provision/become/test.sh index 6d61940c7e..9480ead94c 100755 --- a/tests/provision/become/test.sh +++ b/tests/provision/become/test.sh @@ -13,27 +13,27 @@ rlJournalStart rlPhaseEnd rlPhaseStartTest "$PROVISION_HOW, test with become=true" - rlRun "sudo tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /test/root" + rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /test/root" rlPhaseEnd rlPhaseStartTest "$PROVISION_HOW, test with become=false" - rlRun "sudo tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /test/user" + rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /test/user" rlPhaseEnd rlPhaseStartTest "$PROVISION_HOW, prepare/finish inline with become=true" - rlRun "sudo tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/root/inline" + rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/root/inline" rlPhaseEnd rlPhaseStartTest "$PROVISION_HOW, prepare/finish inline with become=false" - rlRun "sudo tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/user/inline" + rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/user/inline" rlPhaseEnd rlPhaseStartTest "$PROVISION_HOW, prepare/finish scripts with become=true" - rlRun "sudo tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/root/scripts" + rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/root/scripts" rlPhaseEnd rlPhaseStartTest "$PROVISION_HOW, prepare/finish scripts with become=false" - rlRun "sudo tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/user/scripts" + rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/user/scripts" rlPhaseEnd if [[ "$PROVISION_HOW" == "virtual" ]]; then diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index 38ed15721b..907d17dc9c 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -924,12 +924,13 @@ def setup(self) -> None: Setup the guest after it has been started. It is called after :py:meth:`Guest.start`. """ from tmt.steps.execute.internal import effective_pidfile_root + sudo = 'sudo' if not self.facts.is_superuser and self.become else '' pid_directory = effective_pidfile_root() self.execute(ShellScript( f""" if [ ! -d {pid_directory} ]; then \ - mkdir -p {pid_directory} \ - && chmod ugo+rwx {pid_directory}; \ + {sudo} mkdir -p {pid_directory} \ + && {sudo} chmod ugo+rwx {pid_directory}; \ fi """ )) @@ -1719,6 +1720,7 @@ def is_ready(self) -> bool: return self.primary_address is not None def setup(self) -> None: + super().setup() if self.is_dry_run: return if not self.facts.is_superuser and self.become: From f11013aa3e306f85ca9f3fc3417a19b2496ff7ab Mon Sep 17 00:00:00 2001 From: mcasquer Date: Thu, 7 Nov 2024 09:14:57 +0100 Subject: [PATCH 6/7] Removes the self.become this way the condition is applied every time the user has no privileges Signed-off-by: mcasquer --- tmt/steps/provision/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index 907d17dc9c..7dcda9d39c 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -924,7 +924,7 @@ def setup(self) -> None: Setup the guest after it has been started. It is called after :py:meth:`Guest.start`. """ from tmt.steps.execute.internal import effective_pidfile_root - sudo = 'sudo' if not self.facts.is_superuser and self.become else '' + sudo = 'sudo' if not self.facts.is_superuser else '' pid_directory = effective_pidfile_root() self.execute(ShellScript( f""" From 21ba16d33d8d87f842f4c18dfc7e29d9b3a83fd5 Mon Sep 17 00:00:00 2001 From: mcasquer Date: Tue, 12 Nov 2024 07:48:21 +0100 Subject: [PATCH 7/7] Changes bfu_* options to use unprivileged Fedora container image and fedora user Signed-off-by: mcasquer --- tests/provision/facts/test-guest.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/provision/facts/test-guest.sh b/tests/provision/facts/test-guest.sh index 9a043b6c5a..fc61293007 100755 --- a/tests/provision/facts/test-guest.sh +++ b/tests/provision/facts/test-guest.sh @@ -2,6 +2,8 @@ # vim: dict+=/usr/share/beakerlib/dictionary.vim cpt=.,w,b,u,t,i,k . /usr/share/beakerlib/beakerlib.sh || exit 1 +TEST_IMAGE_PREFIX="localhost/tmt/tests/container" + rlJournalStart rlPhaseStartSetup rlRun "PROVISION_HOW=${PROVISION_HOW:-local}" @@ -40,8 +42,8 @@ rlJournalStart fi elif [ "$PROVISION_HOW" = "container" ]; then - provision_options="--image fedora:39" - bfu_provision_options="$provision_options --user=nobody" + provision_options="--image $TEST_IMAGE_PREFIX/fedora/39:latest" + bfu_provision_options="--image $TEST_IMAGE_PREFIX/fedora/39/unprivileged:latest --user=fedora" arch="$(arch)" distro="Fedora Linux 39 (Container Image)"