[virt-tools-list] virt-install and cloud-init, feedback wanted

[Cole Robinson]

On 11/21/19 5:28 AM, Daniel P. Berrangé wrote:
> On Wed, Nov 20, 2019 at 05:49:45PM -0500, Cole Robinson wrote:
>> Hi all. The purpose of this mail is to get some feedback on pending
>> cloud-init support in virt-install. If you're on the CC list here, I
>> either pulled your email from a cloud-init discussion on the the
>> virt-tools-list mailing list, or from the CC list of this virt-install bug:
>>
>> RFE: Provide cloud-init integration for VMs
>> https://bugzilla.redhat.com/show_bug.cgi?id=981693
>>
>> For GSOC 2019 Athina Plaskasoviti completed some cloud-init integration
>> work in virt-install. You can read her wrap up here:
>> https://athinapl.home.blog/2019/08/25/gsoc-2019-cloud-init-configuration-for-virt-manager-virt-install/
>>
>> Right now the code is sitting in a virt-manager.git branch, not yet
>> pushed to master:
>> https://github.com/virt-manager/virt-manager/tree/cloudinit
>>
>> I'll summarize the current behavior, and then ask some questions.
>>
>>
>> The branch adds a new 'virt-install --cloud-init' argument with several
>> sub options. When specified, virt-install generates an empty meta-data
>> file, a user-data file with the requested changes, stuffs them both into
>> a cidata iso, which is used for the first VM boot and then deleted.
>>
>> This behavior is only triggered when --cloud-init is specified in some
>> form, there's no automagic invocation of this support.
>>
>> The command sub options are:
>>
>> $ ./virt-install --cloud-init=help
>> --cloud-init options:
>>   disable
>>   root-password-file
>>   root-password-generate
>>   ssh-key
>>   user-data
>>
>> Their behavior:
>>
>> * disable=yes: boolean option to disable cloud-init in the VM for
>> subsequent boots. Adds this block to user-data:
>>   runcmd:
>>   - [ sudo, touch, /etc/cloud/cloud-init.disabled ]
>>
>> * root-password-file=/MY/PATH: set the desired root password from the
>> content of /MY/PATH on the host
>>
>> * root-password-generate=yes: boolean option, generate a random root
>> password, set it in user-data, print it to the host text console and
>> pause for a bit for the user to see and copy it. sorta inspired by
>> virt-builder
>>
>> * ssh-key=/MY/KEY.pub: inject the ssh key from /MY/KEY.pub on the host
>> into the cloud-init user-data
>>
>> * user-data=/PATH/TO/user-data: ignore all other options and copy this
>> file to the .iso as user-data
>>
>> When bare '--cloud-init' is specified, we default to generating a random
>> root password and disabling cloud-init for subsequent boots:
>> --cloud-init root-password-generate=yes,disable=yes
>>
>> We've explicitly rejected something like root-password=MY-PASSWORD
>> because of the security implications of encouraging a password to end up
>> in command line history. We've already had a CVE for something similar
>> in virt-install.
>>
>> Also, I don't want virt-install to be in the business of specifying
>> every cloud-init option under the sun, there's gotta be a better tool
>> for that already. So I'd like to keep --cloud-init suboptions
>> specifically targeted to expected virt use cases, and anything else can
>> be served with custom user-data=
>>
>> One more point: my main interaction with cloud-init has historically
>> been by grabbing a Fedora/RHEL cloud image, passing it to
>> virt-install/virt-manager, and watching the boot hang, because there's
>> no data provider and cloud-init times out talking to the network, and
>> then I can't log in. I expect many people have hit this issue before.
>> I've always worked around this by using 'virt-customize' to disable
>> cloud-init and reset the root password. That's about the extent of my
>> usage here, which is broadly why the bare `--cloud-init` is the way it was.
>>
>> I'm also thinking to the future, if one day virt-install can detect that
>> it was passed a distro cloud-init image, perhaps we can invoke some
>> default behavior that gives the user a better chance of this config
>> being usable out of the box. I figure that will match whatever we choose
>> for the bare '--cloud-init' behavior
> 
> One option is to take the lazy approach and always enable cloud init
> if we are given a pre-built disk image.
> 
>  * If the disk image doesn't support cloud init, then its fairly
>    harmless to provide cloud-init data.
> 
>  * If the disk image does support cloud init but has already been
>    booted, and the user has marked cloud init as disable, then it
>    is again harmless to provide cloud-init data
> 
>  * If the disk image does support cloud init but has already been
>    booted, and the user left cloud init enabled so that it runs on
>    every boot, then passing cloud init data is probably good thing.
> 
>  * If the disk image does support cloud init and has never been
>    booted, then obviously providing cloud init data is good
>    

If we had a SMBIOS or -fw_cfg option, then maybe this is an option. But
I don't think I could stomach doing this by default with the current
'cdrom' approach, surely there would be complications, and at minimum
complaints about a needless attached CDROM device.

>> * What are the usecases you see for virt-install cloud-init support?
> 
> To me the critical use case is simply providing a way for the user to
> login to the guest. This is covered by password and/or SSH key inject.
> Once they have that working, anything else is just nice to have.
> 
>> * Does the above meet your expectations?
> 
> Yep
> 
>> * Are we missing anything vital?
> 
> Probably the main question will be around the transport for providing
> the metadata.
> 
> The impl does config-drive:
> 
>   https://cloudinit.readthedocs.io/en/latest/topics/datasources/configdrive.html
> 
> As you see from the above list, there are many other approaches
> possible, though the majority of them end up being based on a
> magic IP address.
> 
> The main downside of using a config-drive is that it is visible
> to the guest as as new disk. In simple cases this is no big deal,
> but if the user is setting up a VM with multiple disks, it can be
> annoying to have an extra disk added for cloud-init, as it can
> cause the drive-letters to be reshuffled.  OpenStack tried to
> mitigate this by making the cloud-init config drive the very
> last one to be added.
> 
> This isn't perfect though. eg consider you boot the VM with
> config drive, then hotplug a disk, now reboot the VM. The config
> drive disk is now gone, and the hotplugged disk may well have
> changed its drive letter.
> 
> This is one reason for the use of the IP based metadata service.
> 
> The challenge in any of the IP based solutions is that something on
> the host needs to provide a server to listen for these requests.
> 
> I think that providing such a server is out of scope for virt-install.
> 
> I could see value in someone creating a new project that provides a
> simple self-contained metadata server, as all the currentl metadata
> server impls are part of some larger project codebase and can't be
> used independantly (AFAIK - someone correct me if a simple standalone
> impl already exists somewhere).
> 
> In any case if someone wants to use a network metadata server, then
> virt-install doesn't need todo anything at all. Any config is entirely
> respponsibility of whomever is running that service.
> 
> 
> If neither config drive, or IP based solutions are viable, then you
> are left with platform specific options. Ignition uses fw_cfg however
> this is really frowned upon by QEMU maintainers, as fw_cfg is not
> intended for general purpose application usage.  The suggested replacement
> for fw_cfg for app usage is the SMBIOS OEM strings table.
> 
> Neither of these are platform portable, just x86 + aarch64 right now.
> 

Thanks for the breakdown. There's obviously a lot to consider here and a
large test matrix to consider if we go with the non-cdrom case.

I think the 'cdrom' case is good enough for a first crack at this and
anything else can build on that initial support. I think the command
line is flexible enough that in the future we could have a --cloud-init
method=SMBIOS/... for users to explicitly choose what injection method
they want, but otherwise virt-install will choose a default, which may
be different depending on the arch.

> 
>> * Do you have an opinion of what behavior bare '--cloud-init' should give?
> 
> My feeling is that a bare --cloud-init should inject the current user's
> SSH identity and/or authorized keys list.
> 
> ie either we grab $HOME/.ssh/id_rsa.pub, or we grab all the keys from
> the SSH agent connection (ie ssh-add -l)
> 
> Given that virt-intsall is run as root, there probably isn't a
> /root/.ssh/id_rsa.pub file for most people. Grabbing the ssh-agent
> authorized keys is probably best default approach.

How to handle ssh keys kinda makes my head spin. I've shrunk the CC list
on this mail to just the folks that mentioned ssh in some way in their
replies

If I'm thinking of the ideal interactive virt-install UI, it would be:

$ virt-install --name MYVM --disk Fedora-Cloud-...qcow2 --cloud-init
INFO: SSH keys added to MYVM
INFO: Connecting to MYVM with: ssh fedora at 192.168.122.XXX
...
[fedora at localhost:~] $


If all the pieces come together, that's pretty nice. But there's a lot
of missing pieces needed for that, and some unclear bits:

* What is the cloud-init user name? Passing in an ssh key gives access
to the default cloud-init configured username, which is different
depending on the distro (might be fedora, ubuntu, etc). To properly
handle this, we either need libosinfo distro detection, or use
libguestfs to fish the default user out of /etc/cloud/cloud.cfg in the
VM. And root account is typically locked, and ssh access disabled, and
even if we could hack things to give root ssh access it seems like going
against the grain WRT cloud images.

So best I can think of for an immediate solution for SSH is to print
something explanatory:

INFO: you can ssh to the VM using the distro's default username. This is
usually one of: fedora, ubuntu, ...

Or something along those lines. Maybe we can tie that into --os-variant
as well so the user can inform us what distro it is. Still it will take
some work.

* Where to get ssh keys from? You mention ssh-add -l which is nice, I
didn't know about that. There's the issue with running virt-install
under sudo, but not sure there's anything we can do about that to make
it 'just work'. Sounds like `ssh-add -l` is the way though for some sort
of ssh-key=auto mode

* What to do if we can't find any ssh-keys to add? I guess explicitly
fail, and print a descriptive error: We didn't find any SSH keys to add.
Maybe you are running under sudo. You can manually specify a key with
XXX or generate root password with YYY. (something better than that of
course)

* virt-install support for launching an ssh session as the UI. This is
probably worth doing regardless. I'd like to add an explicit
--autoconsole none|graphical|text anyways, adding ssh to that seems
reasonable. Requires waiting for VM IP address to pop up, but that
shouldn't be too bad to implement. The main problem for the --cloud-init
case is that I don't know if it makes sense to use this if we are just
guessing about the what the default username is?


So, to get that ideal UI, or at least point the user in the right
direction, we need to:

* determine the default user name for the disk image, which we don't
have a solution for presently
* successfully inject a relevant ssh key, which may not be available
like under sudo

So there's missing non-trivial work, and there's ways it can fail.

The current UI, with root-password-generate=on (plus some work to
default to '--console text' when --cloud-init is passed), is more
reliable as it should work in all situations, but even when it works
it's not as friendly as the working ssh case (need to copy the printed
password and paste it into a terminal). And it doesn't sound like it's
what people expect the default to be. And giving root access seems to be
against the spirit of the cloud images, but maybe that's just for ssh access

Maybe we shoot for the `--cloud-init ssh-key=auto` behavior as default,
but just fail as descriptively as possible if all the pieces aren't in
place. We can use the error to point people to manual cli workarounds,
like specifying an ssh-key path, some cli way to specify the ssh
username (--autoconsole ssh-user=FOO ?), or the root-generate-password=on

But I'm still kinda confused :) Ideas welcome.

Thanks,
Cole