-
Notifications
You must be signed in to change notification settings - Fork 77
test: add support to check for server-qcow2 (no cloud-init) #1995
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
base: main
Are you sure you want to change the base?
Conversation
|
This PR is stale because it had no activity for the past 30 days. Remove the "Stale" label or add a comment, otherwise this PR will be closed in 7 days. |
|
Although I was skeptical to this, I think this is the best option. I spent two days experimenting with two approaches of "injecting" host check script into a VM:
Both lead to dead ends with triple amount of code compared to this solution, it was simply ugly and fragile. This looks so much better and it builds on top of the very awesome |
4639017 to
c2921a1
Compare
Similar to the unattended-iso.json configs lets make sure we have the `osbuild` user in all-customizations. This ensures that we can inject credentials when needed. This commit also changes the kernel commandline in all customizations from debug to systemd.journal.forward_to_console to make debugging boots easier.
a82b5f9 to
67d05da
Compare
This commit adds support for non-cloud init image types like the `server-qcow2`. To test those we need to mutate the image so that we can inject our test user and credetials (note that in the future with test.thing and systemd credentials passing this will be no longer necessary). We also need a customization that contains the "osbuild" user. It can be run via: ``` ./test/scripts/build-image fedora-43 server-qcow2 ./test/configs/all-customizations ./test/scripts/boot-image ./build/fedora_42-x86_64-server_qcow2 ```
67d05da to
13df30e
Compare
| # server-qcow2 has no cloud-init so we need at least a osbuild user | ||
| if info.get("image-type") == "server-qcow2": | ||
| if not any(u["name"] == "osbuild" for u in customizations.get("user", [])): | ||
| raise CannotRunQemuTest("server-qcow2 needs a customization with the osbuild user") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we assume the manifest is always available, which in our testing it is, we could make this check more correct by checking if cloud-init is installed in the image and have it work the same way for any image type we'd want to test this way.
| # add osbuild ssh key for systems that lack systemd credentials, this assumes | ||
| # the customization has an osbuild user that can do sudo | ||
| uid = 1000 | ||
| ssh_dir = "/home/osbuild/.ssh" | ||
| g.mkdir_p(ssh_dir) | ||
| g.chmod(0o700, ssh_dir) | ||
| g.chown(uid, uid, ssh_dir) | ||
| auth_keys_file = f"{ssh_dir}/authorized_keys" | ||
| g.write_append(auth_keys_file, pubkey.encode('utf-8')) | ||
| g.chmod(0o600, auth_keys_file) | ||
| g.chown(uid, uid, auth_keys_file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused now. The ensure_can_run_qemu_test() called above will raise an exception if server-qcow2 is built without an osbuild user. Are we writing the ssh key regardless of the customization just to be sure? Or is it a different key that's being written here that's specific to the test?
If we're modifying the image anyway, would it be simpler to add the key to the root user and ssh in as root instead of osbuild, saving us from needing to add the sudoers override as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the text to capture that while the customizations add an osbuild user we may not have access to the ssh key. We could of course force the use of a ssh key in the customizations that is available via the CI_PRIV_SSH_KEY env. The downside would be that running it locally requires more setup, this is why I have a slight preference for this but of course the upside is that this way we don't need to modify the image which is a very nice bonus.
If we want to go this way (which is fine with me) I will close this PR and we can add the server-qcow2 in a simpler way (i.e. by just ensuring that we have an osbuild user that can can do passwordless sudo. We will also have to ensure that passwordless sudo is possible, I don't think we support that outside of kickstarts right now but maybe I just missed the option :) If its missing it seems like a very nice feature to add.
As for why not just "root" - that was my initial idea, we can still just do it this way, the downsides:
a) less uniform, right now we always run "check-base-host" via the osbuild user so we would have to add conditions on vm.run() what user to pick and the behavior of the host check would be (slightly) different. Not a big deal of course
b) some systems do not allow root-login at all and having to debug when this is the case can be a slight headache (i.e. remembering that the failure might be because of that). by going via osbuild this is not a concern.
But of course happy to adjust as needed.
| # make sudo available | ||
| sudoers_d_path = "/etc/sudoers" | ||
| g.write_append(sudoers_d_path, "osbuild ALL=(ALL) NOPASSWD: ALL\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sudoers_d_path implies /etc/sudoers.d. Which makes me think writing a drop-in file would be a more correct thing to do here instead of appending to /etc/sudoers.
thozza
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides Achilleas's comments, this looks good. 👍
test: add support to check for server-qcow2 (no cloud-init)
This commit adds support for non-cloud init image types like
the
server-qcow2. To test those we need to mutate theimage so that we can inject our test user and credetials
(note that in the future with test.thing and systemd
credentials passing this will be no longer necessary).
We also need a customization that contains the "osbuild"
user.
It can be run via:
test: tweak all-customizations config to include a osbuild user
Similar to the unattended-iso.json configs lets make sure we have
the
osbuilduser in all-customizations. This ensures that wecan inject credentials when needed.
This commit also changes the kernel commandline in all customizations
from debug to systemd.journal.forward_to_console to make debugging
boots easier.