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

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

Closed
wants to merge 3 commits into from

Conversation

cgwalters
Copy link
Member

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


Raw format is fine to use on systems that have reflinks for example.
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.
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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

jlebon commented Feb 6, 2024

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
Copy link
Member

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
Copy link
Member

jlebon commented Feb 6, 2024

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
Copy link
Member Author

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
Copy link
Member Author

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:

  • Both are in Go
  • Both directly spawn qemu
  • Both want to do stuff with Ignition (but not exclusively, as there's some discussion of podman machine not depending on FCOS)
  • podman machine (on Linux) really wants to use virtiofsd, which we have code for here

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
Copy link
Member

jlebon commented May 9, 2024

I think this is related to the discussions in containers/podman-bootc#28. See especially containers/podman-bootc#28 (comment).

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.

@jlebon jlebon closed this May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants