Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ubuntu-drivers debug wrapper #2026

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

Chris-Peterson444
Copy link
Contributor

Introduces:

  • A debug mode for wrapping all of the ubuntu-drivers calls with a umockdev wrapper script (Closes Fake ubuntu drivers #1959)
  • A sub-module, subiquity.system_scripts, to store scripts which subiquity can write to the system and call externally.

@Chris-Peterson444
Copy link
Contributor Author

After out of band discussion, the system_scripts approach isn't what we want to go with. There's a different approach with just using binaries vendored in /bin that I'll investigate alongside some other work. Marking this PR as draft until I refactor based on that.

@Chris-Peterson444 Chris-Peterson444 marked this pull request as draft July 11, 2024 21:48
.flake8 Outdated
@@ -3,3 +3,5 @@
builtins = _
max-line-length = 88
extend-ignore = E203
exclude =
subiquity/system_scripts
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's under subiquity I want it passing flake8. It looks like it's just 2 things to fix, noqa is fine for long strings that really do need to be long.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. I guess this is all moot since I'm going to tear this out anyways, but I was concerned something like

scripts = """\
some text
                                          really long line  # noqa
                                          another really long line  # noqa
"""

would be weird since it technically embeds the comment in the string, but you can just do

scripts = """\
some text
                                          really long line
                                          another really long line
"""  # noqa

it turns out.

@@ -0,0 +1,69 @@
script = """\
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as already discussed, I don't want to write these scripts out to use, I want them directly in the snap.

scripts/kvm-test.py Outdated Show resolved Hide resolved
scripts/kvm-test.py Outdated Show resolved Hide resolved
self.wrapper_script_source: str = umockdev_wrapper.script
self.script_path = "/tmp/umockdev_wrapper.py"

self.dev_config = configs.broadcom
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit hardcoded which means that we can't actually use the other config blobs, such as dell_certified_with_nvidia, without source modification. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking with moving away from the system_scripts module, we have a default config like:

        self.dev_config = {
            "devices": [
                {
                    "modalias": "pci:v000014E4d00004353sv00sd01bc02sc80i00",
                    "vendor": "0x14E4",
                    "device": "0x4353",
                }
            ]
        }

which gets written on boot to a known location (i.e. /tmp/umockdev_config.yaml) and then we just modify that with early-commands or similar before continuing with the install. This is also the inspiration for 1d56545. What do you think?

Adds the *_ORIG variables to the subiquity-server environment
so orig_environment will correctly replace mocked variables (e.g.,
$PATH).
Introduces a special set of binaries we will use to be run in the
system environment. This commit introduces subiquity-umockdev-wrapper,
which can be used to wrap ubuntu-drivers calls to fake hardware.
Utility to generate an environment for running programs outside of the
snap with the addition of the programs vendored in system_scripts.
@Chris-Peterson444 Chris-Peterson444 force-pushed the fake-ubuntu-drivers-new branch 3 times, most recently from f9e9cbc to 2e09905 Compare July 17, 2024 21:58
@Chris-Peterson444 Chris-Peterson444 marked this pull request as ready for review July 17, 2024 22:07
Introduces a new UbuntuDriversInterface class that will wrap
all "ubuntu-drivers" calls with the subiquity-umockdev-wrapper
script.

install_drivers no-op until further work to copy system_scripts to
target is done.
Add option to pass "subiquity-fake-pci-devices" on the kernel
command line.
For server installs, we specifically pass "--gpgpu" to ubuntu-drivers
calls, but this stops broadcom-sta-dkms drivers from being
a candidate to install. It's faster to test on server, so we can
enable this flag to test installs of braodcom-sta-dkms on server
(which normally shouldn't happen).
Add option to pass "subiquity-server-force-no-gpgpu" on the kernel
command line.
Wrap the ubuntu-drivers command on subiquity-umockdev-wrapper during
install too. Works by copying the wrapper script to /target/usr/bin
so it will be found within $PATH by default.
@Chris-Peterson444
Copy link
Contributor Author

Apologies for the noise. Somehow I didn't have pre-commit installed.

This is now ready including wrapping the install step, albeit not very elegantly for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants