Support Fedora Image Mode #3229

thrix · 2024-09-19T23:39:00Z

To support Fedora Image mode the following changes were required:

Fix rsync installation. The tool is not included in the distribution and
the installation method was completely broken, i.e. rpm-ostree has
no --install-root option. We did not hit it as there is no good
testing environment available that would cover this.
Introduce TMT_SCRIPTS_DEST_DIR environment variable set by default
to /var/tmp/tmt/bin which hosts the tmt scripts. The original path
is not writable on these systems. The PATH is set system wide via a
new /etc/profile.d/tmt.sh script.

Signed-off-by: Miroslav Vadkerti [email protected]

Pull Request Checklist

With tmt from this MR: teemtee/tmt#3229 Signed-off-by: Miroslav Vadkerti <[email protected]>

tmt/steps/execute/scripts/tmt.sh

tmt/steps/execute/__init__.py

tmt/steps/execute/scripts/tmt.sh.j2

To support Fedora Image mode the following changes were required: * Fix `rsync` installation. The tool is not included in the distribution and the installation method was completely broken, i.e. `rpm-ostree` has no `--install-root` option. We did not hit it as there is no good testing environment available that would cover this. * Introduce `TMT_SCRIPTS_DEST_DIR` environment variable set by default to `/var/tmp/tmt/bin` which hosts the `tmt` scripts. The original path is not writable on these systems. The `PATH` is set system wide via a new `/etc/profile.d/tmt.sh` script. Signed-off-by: Miroslav Vadkerti <[email protected]>

tmt/steps/execute/__init__.py

martinhoyer

Do I understand it correctly, that the intention is to have default directory which is writable on all platforms, as opposed to having a default directory, which would be changed if 'default' is not writable?

.github/workflows/shellcheck.yml

tmt/steps/execute/__init__.py

To support Fedora Image mode the following changes were required: * Fix `rsync` installation. The tool is not included in the distribution and the installation method was completely broken, i.e. `rpm-ostree` has no `--install-root` option. We did not hit it as there is no good testing environment available that would cover this. * Introduce `TMT_SCRIPTS_DEST_DIR` environment variable set by default to `/var/tmp/tmt/bin` which hosts the `tmt` scripts. The original path is not writable on these systems. The `PATH` is set system wide via a new `/etc/profile.d/tmt.sh` script. Signed-off-by: Miroslav Vadkerti <[email protected]>

tmt/steps/execute/__init__.py

tmt/steps/execute/scripts/tmt.sh.j2

Signed-off-by: Miroslav Vadkerti <[email protected]>

thrix · 2024-10-02T12:24:58Z

@happz @psss added docs, all comments should be resolved

thrix · 2024-10-02T15:46:09Z

Btw we agreed on getting this into Testing Farm sooner than 1.38, once we verify it does not break anything.

thrix · 2024-10-02T19:07:23Z

Please vote for the new location in this discussion poll:

#3260

kkaarreell · 2024-10-02T20:07:43Z

What about SELinux? Given this dir hosts executables, it should be assigned with the respective selinux context.

kkaarreell · 2024-10-02T20:27:49Z

Do I understand it correctly, that the intention is to have default directory which is writable on all platforms, as opposed to having a default directory, which would be changed if 'default' is not writable?

I am also interested to know. It looks like that alternative location would require some system "bending" and I am not sure if doing this by default Is a good approach. I expect that most tested systems won't be bootc.

thrix · 2024-10-03T06:56:16Z

What about SELinux? Given this dir hosts executables, it should be assigned with the respective selinux context.

Thanks, depending on the discussion we can adjust the SELinux context. I did not yet see a problem with this with the current path /var/tmp/tmt/bin but might be worth checking :)

thrix · 2024-10-03T06:56:59Z

Do I understand it correctly, that the intention is to have default directory which is writable on all platforms, as opposed to having a default directory, which would be changed if 'default' is not writable?

I am also interested to know. It looks like that alternative location would require some system "bending" and I am not sure if doing this by default Is a good approach. I expect that most tested systems won't be bootc.

Bending is already in place and we talked about proper documentation. Feel free to read the MR docs being added.

martinhoyer · 2024-10-03T07:59:44Z

tmt/steps/execute/__init__.py

-                    options=["-p", "--chmod=755"],
-                    superuser=guest.facts.is_superuser is not True)
+            with script as source:
+                for dest in [script.path, *script.aliases]:


@thrix Not related to the problem this PR is addressing, but I thought it might be a good idea to bring it up since we're doing changes in this code:
I've noticed the copying of the scripts can take quite some time when copying to remote host and I suspect it could be vastly sped-up if we copy them in batch instead of one at a time.

If you say it's more suitable to handle in separate PR, I'll open an issue to not forget about it :)

@martinhoyer yes please, not definitely something for this patch :)

martinhoyer · 2024-10-03T08:05:54Z

tmt/steps/execute/__init__.py

@@ -597,16 +680,18 @@ def prepare_tests(self, guest: Guest, logger: tmt.log.Logger) -> list[TestInvoca

    def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None:
        """ Prepare additional scripts for testing """
+        # Create scripts directory
+        guest.execute(Command("mkdir", "-p", str(effective_scripts_dest_dir())))


How about using rsync with --mkpath ?
Actually, there is also --executability or --perms/--chmod to get rid of the other ssh command.
(again, can be a separate PR)

@martinhoyer rsync --mkpath was added in rsync 3.2.3 (6 Aug 2020). I wonder if it will work on all supported OSes

I think I did check that before writing the comment.

That option has to exist for rsync on the guest as well, right? If so we definitely cannot use that as rhel-7 has just rsync-3.1.2

thrix · 2024-10-07T21:34:11Z

/packit test

thrix added this to the 1.37 milestone Sep 19, 2024

thrix self-assigned this Sep 19, 2024

thrix added a commit to testing-farm/gluetool-modules that referenced this pull request Sep 20, 2024

DNM: Image mode test worker 2024-08.13

4eec0b4

With tmt from this MR: teemtee/tmt#3229 Signed-off-by: Miroslav Vadkerti <[email protected]>

thrix force-pushed the image-mode-fedora branch from 8c1c5db to b447a6a Compare September 30, 2024 20:18

github-advanced-security bot found potential problems Sep 30, 2024

View reviewed changes

tmt/steps/execute/scripts/tmt.sh Fixed Show fixed Hide fixed

happz reviewed Sep 30, 2024

View reviewed changes

tmt/steps/execute/__init__.py Outdated Show resolved Hide resolved

happz reviewed Sep 30, 2024

View reviewed changes

tmt/steps/execute/__init__.py Outdated Show resolved Hide resolved

happz reviewed Sep 30, 2024

View reviewed changes

tmt/steps/execute/__init__.py Outdated Show resolved Hide resolved

happz reviewed Sep 30, 2024

View reviewed changes

tmt/steps/execute/__init__.py Outdated Show resolved Hide resolved

thrix force-pushed the image-mode-fedora branch 4 times, most recently from 5617f1e to 71c72d1 Compare October 1, 2024 08:19

thrix requested a review from happz October 1, 2024 08:20

github-advanced-security bot found potential problems Oct 1, 2024

View reviewed changes

thrix force-pushed the image-mode-fedora branch from 71c72d1 to 4d0abe1 Compare October 1, 2024 08:29

thrix marked this pull request as ready for review October 1, 2024 08:30

thrix requested review from psss, lukaszachy and janhavlin as code owners October 1, 2024 08:30

happz reviewed Oct 1, 2024

View reviewed changes

tmt/steps/execute/__init__.py Outdated Show resolved Hide resolved

happz reviewed Oct 1, 2024

View reviewed changes

tmt/steps/execute/__init__.py Outdated Show resolved Hide resolved

martinhoyer reviewed Oct 1, 2024

View reviewed changes

.github/workflows/shellcheck.yml Outdated Show resolved Hide resolved

tmt/steps/execute/__init__.py Outdated Show resolved Hide resolved

thrix modified the milestones: 1.37, 1.38 Oct 1, 2024

thrix added the priority | must high priority, must be included in the next release label Oct 1, 2024

happz added the step | execute Stuff related to the execute step label Oct 1, 2024

thrix force-pushed the image-mode-fedora branch from 517fb79 to 0b923ee Compare October 1, 2024 18:42

thrix force-pushed the image-mode-fedora branch from 0b923ee to 97d2b90 Compare October 1, 2024 18:44

thrix commented Oct 1, 2024

View reviewed changes

tmt/steps/execute/__init__.py Outdated Show resolved Hide resolved

thrix commented Oct 1, 2024

View reviewed changes

tmt/steps/execute/__init__.py Outdated Show resolved Hide resolved

thrix commented Oct 1, 2024

View reviewed changes

tmt/steps/execute/__init__.py Outdated Show resolved Hide resolved

Fix grammar

f34f045

happz reviewed Oct 1, 2024

View reviewed changes

tmt/steps/execute/__init__.py Show resolved Hide resolved

happz reviewed Oct 1, 2024

View reviewed changes

tmt/steps/execute/scripts/tmt.sh.j2 Outdated Show resolved Hide resolved

thrix added 2 commits October 2, 2024 12:51

Update tmt/steps/execute/scripts/tmt.sh.j2

3fc808d

Added docs

9dcaa75

Signed-off-by: Miroslav Vadkerti <[email protected]>

martinhoyer reviewed Oct 3, 2024

View reviewed changes

thrix added the ci | full test Pull request is ready for the full test execution label Oct 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Fedora Image Mode #3229

Support Fedora Image Mode #3229

thrix commented Sep 19, 2024 •

edited

Loading

martinhoyer left a comment

thrix commented Oct 2, 2024

thrix commented Oct 2, 2024

thrix commented Oct 2, 2024

kkaarreell commented Oct 2, 2024

kkaarreell commented Oct 2, 2024

thrix commented Oct 3, 2024

thrix commented Oct 3, 2024

martinhoyer Oct 3, 2024

thrix Oct 7, 2024

martinhoyer Oct 3, 2024

thrix Oct 7, 2024

martinhoyer Oct 8, 2024

lukaszachy Oct 9, 2024

thrix commented Oct 7, 2024

Support Fedora Image Mode #3229

Are you sure you want to change the base?

Support Fedora Image Mode #3229

Conversation

thrix commented Sep 19, 2024 • edited Loading

martinhoyer left a comment

Choose a reason for hiding this comment

thrix commented Oct 2, 2024

thrix commented Oct 2, 2024

thrix commented Oct 2, 2024

kkaarreell commented Oct 2, 2024

kkaarreell commented Oct 2, 2024

thrix commented Oct 3, 2024

thrix commented Oct 3, 2024

martinhoyer Oct 3, 2024

Choose a reason for hiding this comment

thrix Oct 7, 2024

Choose a reason for hiding this comment

martinhoyer Oct 3, 2024

Choose a reason for hiding this comment

thrix Oct 7, 2024

Choose a reason for hiding this comment

martinhoyer Oct 8, 2024

Choose a reason for hiding this comment

lukaszachy Oct 9, 2024

Choose a reason for hiding this comment

thrix commented Oct 7, 2024

thrix commented Sep 19, 2024 •

edited

Loading