search

coreos / coreos-assembler

Tooling container to assemble CoreOS-like systems
https://coreos.github.io/coreos-assembler/
Apache License 2.0
335 stars 165 forks source link

devshell: Support `-S/--root-ssh-smbios` #3712

Closed cgwalters closed 4 months ago

cgwalters commented 7 months ago

qemu: Detect raw format too

Raw format is fine to use on systems that have reflinks for example.

qemu: Support injecting SSH keys via systemd credentials over SMBIOS

Since it's in systemd it's available as a baseline way to inject things like a SSH key. That said, not all systems support SMBIOS. But for those that do let's support it.

devshell: Support -S/--root-ssh-smbios

We've built up a lot of generically useful qemu code, and I am considering factoring it out into a Go package suitable for sharing in particular with "podman machine" and other use cases.

Related to this, I'm building systems now that don't use Ignition by default.

We only need a small bit of code to inject a root ssh key via systemd creds over SMBIOS; change things to support that.

Concretely with this I can now cosa run -S --qemu-image ~/build/centos-bootc.raw

cgwalters commented 7 months ago

OK, got this one past the random CI flakes. Any takers? Should be pretty safe.

That said, one thing I am thinking about doing is lifting the qemu library into a distinct separate git repository.

dustymabe commented 7 months ago

Interesting idea. Why should we do this in COSA and not in say virt-install?

If we do think COSA is the best place to do something like this: since we have the ability to inject kernel args, should we instead consider using the systemd.set_credential_binary= kernel arguments as the credential passing so it's not limited to just systems that support SMBIOS?

cgwalters commented 7 months ago

Why should we do this in COSA and not in say virt-install?

They're different but related things. One can already pass credentials via SMBIOS by directly using --qemu-commandline - though that wants cmdline sugar.

But virt-install doesn't do the "snapshot disk image by default, inject auto-generated ssh key and ssh in".

The other big thing is that virt-install is Python, whereas this code is in Go and that will make it easier (possible, really) to share with podman-machine and other Go projects in the future. Basically while this change isn't directly useful for FCOS, it starts to generalize our qemu/ssh code to be potentially more sharable with other use cases.

cgwalters commented 7 months ago

since we have the ability to inject kernel args, should we instead consider using the systemd.set_credential_binary= kernel arguments as the credential passing so it's not limited to just systems that support SMBIOS?

Yes, we could though ultimately I think that case is going to be solved with using virtiofs for metadata.

dustymabe commented 7 months ago

since we have the ability to inject kernel args, should we instead consider using the systemd.set_credential_binary= kernel arguments as the credential passing so it's not limited to just systems that support SMBIOS?

Yes, we could though ultimately I think that case is going to be solved with using virtiofs for metadata.

Oh nice. Is there a link where I can learn more about that?

dustymabe commented 7 months ago

Why should we do this in COSA and not in say virt-install?

They're different but related things. One can already pass credentials via SMBIOS by directly using --qemu-commandline - though that wants cmdline sugar.

But virt-install doesn't do the "snapshot disk image by default, inject auto-generated ssh key and ssh in".

Right. Those are quite nice additions to a dev/test workflow. I could totally see virt-install (or maybe another named tool for specific functionality that does the $0 switch) picking up this behavior.

My concern here is that we are adding features for non-CoreOS users essentially and I don't think our team(s) are in a position to field future feature request or fix bugs when those users have problems.

The other big thing is that virt-install is Python, whereas this code is in Go and that will make it easier (possible, really) to share with podman-machine and other Go projects in the future. Basically while this change isn't directly useful for FCOS, it starts to generalize our qemu/ssh code to be potentially more sharable with other use cases.

I'd be hesitant to start maintaining code here (or even in a split out library) that ended up as load bearing in other projects. Most of the code we wrote here was for a specific purpose and I'm not sure it would generalize well.

All that being said, I'm interested in what other contributors here think?

jlebon commented 7 months ago

Yeah, feels odd to be adding code here that's not really relevant to FCOS/RHCOS. Even if long-term we move away from Ignition, our tools would probably switch over to providing e.g. cloud-init data instead, right?

I'd be hesitant to start maintaining code here (or even in a split out library) that ended up as load bearing in other projects. Most of the code we wrote here was for a specific purpose and I'm not sure it would generalize well.

There's for sure a lot of CoreOS-specific things in there. Though we definitely also have bits that are of the more generic "convenience QEMU wrapper" aspect which I could imagine splitting out. Wrapping QEMU is common enough that it'd make sense to unite efforts a bit.

So my strawman is to create that library first (could literally copy/paste qemu.go to start), and then add this there. Immediately, the need for surfacing it to the kola qemuexec CLI drops out (I presume you'd add it to e.g. podman machine init instead). And then once it's cleaned up more and stabilized, we could rebase cosa on top of it.

dustymabe commented 7 months ago

Though we definitely also have bits that are of the more generic "convenience QEMU wrapper"

I'm just wondering why we couldn't contribute these pieces into a library more dedicated to that goal so it's useful for everyone? i.e. https://github.com/digitalocean/go-qemu or https://libvirt.org/go/libvirt

jlebon commented 7 months ago

Though we definitely also have bits that are of the more generic "convenience QEMU wrapper"

I'm just wondering why we couldn't contribute these pieces into a library more dedicated to that goal so it's useful for everyone? i.e. digitalocean/go-qemu or libvirt.org/go/libvirt

Wasn't aware of go-qemu. I may be missing it, but it seems geared towards managing existing QEMU VMs launched via libvirt rather than launching QEMU instances itself? Searching for some common qemu command-line arguments didn't yield anything.

Re. libvirt, I think not requiring a daemon and purely trying to make using the QEMU CLI for ephemeral VMs more convenient is worthwhile.

cgwalters commented 7 months ago

our tools would probably switch over to providing e.g. cloud-init data instead, right?

No, I don't think we should or can require cloud-init. systemd's credential stuff (which is added here, it's a trivial amount of code) is a good enough baseline bootstrap mechanism to get SSH from a generic disk image (except with apple Virtualization.framework, but that's a distinct issue).

cgwalters commented 7 months ago

I'm just wondering why we couldn't contribute these pieces into a library more dedicated to that goal so it's useful for everyone? i.e. https://github.com/digitalocean/go-qemu or https://libvirt.org/go/libvirt

Depending on libvirt I think is not out of the question, but a baseline requirement is running on MacOS, and libvirt is a really huge dependency there. I don't think there's much excitement about doing that. Also on Linux even; a tricky thing is libvirt doesn't have inherent namespacing, and that can be complex to deal with in general.

The other thing is libvirt just kind of breaks the "run a VM as a child process" workflow that we have right now, it gets hard to avoid leaking VMs in corner cases.

What I would again narrow in on here is that when you look at coreos-assembler and podman-machine:

It's fine, we don't have to merge this PR, I can just fork the qemu code here and merge it in another place. But, having this here to start would help re-aligning things later.

jlebon commented 4 months ago

I think this is related to the discussions in https://github.com/containers/podman-bootc/pull/28. See especially https://github.com/containers/podman-bootc/pull/28#issuecomment-2099600924.

I.e. in this case for example, I think eventually cosa should instead inherit from a shared space that knows how to do this stuff. And I think in that process, it'd make a lot of sense to move a lot of the qemu code in cosa to that shared tooling. Anyway, let's close this for now.