[virt-tools-list] [virt-install PATCH 3/3] virt-install: Add --unattended support
Cole Robinson
crobinso at redhat.com
Thu Feb 7 22:12:56 UTC 2019
On 2/6/19 8:21 AM, Fabiano Fidêncio wrote:
> This patch introduces the preliminary support for unattended
> installations using virt-install.
>
> The command-line added is something like:
> --unattended
> profile="jeos|desktop",user-password="...",admin-password="...",product-key="..."
>
> In case no profile is passed, "jeos" is used as the default one.
> In case no user or admin password is passed, those are going to be
> automatically generated and printed to the user.
> The "product-key" argument is needed for Windows(es) installations.
>
> The --unattended argument works with:
> - --cdrom, thus the user can pass a media to be used;
> - --location, thus the user can pass a tree/media to be used;
> - with no arguments, then the tree is taken from osinfo-db for that
> distro;
>
> Using --os-variant is required for when using --unattended.
>
> There are still a few things missing in this path that will require some
> more work to be done, as:
> - Windowses:
> - Handle more than one script (usually needed for installing drivers);
> - Support a second disk containing drivers;
> - General:
> - Have a proper check and help about which OSes support which kind of
> unattended installations;
> - Have a proper way to detect the keyboard layout being used by the
> user (currently we just guess it by the user's system language);
>
Thanks for working on this! Will definitely be nice to have this easily
accessible with virt-install.
Does it work with Fedora? I tried --os-variant fedora28 --unattended but
it isn't working... osinfo-db doesn't list 'initrd' as an injection
method. Are there missing osinfo-db patches?
Reviewing this gave me some ideas for cleanups and improvements that
will smooth out this process a bit. I pushed some commits to git and
rebased your patch on top, see the attachment. So in my points below
I'll call out some stuff I added/changed
There's a lot of different usages here and they should probably all
be their own patches. Here's how I think the series should go in broad
changes:
1) grabbing URL from libosinfo and doing a network install
This should be decoupled from the unattended stuff. The cli invocation
is the open question. In .git now, --os-variant is a VirtCLIParser, just
taking name= (the default) and full_id= values. This plumbing can be
used for something like:
virt-install --os-variant fedora28,install=tree|media
But with just the tree part implemented. Maybe we want to use
install=location|cdrom to match virt-install names too, I'm not quite
sure, pick one and we can change or extend it later if we want. The
other option is a --location suboption but that doesn't seem as nice.
(Later we can discuss whether we want plain 'virt-install --os-variant
fedora28' to do that by default, but that will be part of a bigger
discussion to use libosinfo defaults much more for default naming etc.)
When this is split out I will help test and write tests for it. At least
a case with --os-variant $windows that should fail of course because no
URL is available, and maybe some invocation in test_urls to test an
actual working case, I'll need to play with it.
2) Some of the plumbing for actually generating the script content. Most
if not all should live in osdict. We could add a couple basic unit tests
in tests/osdict.py to generate a couple different scripts and do just
basic string searches over them to verify the output is sane.
2) --unattended install cli plumbing: just a no-op for starters. Add an
_add_argcomplete_cmd test in clitest.py. This can be the virt-install
option collision checks and everything up until actually altering any
data passed to the installer. Kind of an artificial split but we could
commit this early and it will reduce the diff
4) --unattended linux network install. should handle implicit location
and explicit location. ignore cdrom handling for now. testing is an open
question, we will probably need to add some env variables or hacks in
the code to let us mock test this from clitest.py, but I will help with that
5) --unattended linux network install but with passed in --cdrom media.
I see this below, where we turn options.cdrom into options.location.
That might take some testing, maybe improve the error message in cases
this won't work, like if a Fedora livecd is passed or something. I'm
just calling this case out explicitly because I need to consider it more
6) windows --cdrom unattended support. Now if this comes last it will be
easier to see all the ways this alters the previous code, right now it's
mixed in and a bit difficult to parse where the differentiation is at.
Also since this requires invoking external tools we need to consider
things like RPM and testing deps, and will probably need some hacks to
get unittest coverage here, so it will take more consideration.
Okay hopefully that doesn't sound too daunting. Some specific comments
below.
> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> ---
> virt-install | 46 +++++++++++--
> virtinst/cli.py | 31 ++++++++-
> virtinst/installer.py | 116 +++++++++++++++++++++++++++++++++
> virtinst/installertreemedia.py | 17 +++++
> virtinst/osdict.py | 106 ++++++++++++++++++++++++++++++
> 5 files changed, 311 insertions(+), 5 deletions(-)
>
> diff --git a/virt-install b/virt-install
> index a7cf0c2d..af465df3 100755
> --- a/virt-install
> +++ b/virt-install
> @@ -355,6 +355,11 @@ def validate_required_options(options, guest, installer):
> _("An install method must be specified\n(%(methods)s)") %
> {"methods": install_methods})
>
> + if options.unattended:
> + if not options.distro_variant:
> + msg += "\n" + (
> + _("An OS variant must be specified\n"))
> +
In the rebased patch, I tweaked this a bit to specifically mention
--unattended
> if msg:
> fail(msg)
>
> @@ -398,6 +403,10 @@ def check_option_collisions(options, guest, installer):
> fail(_("--initrd-inject only works if specified with --location.") +
> cdrom_err)
>
> + if options.unattended:
> + if options.initrd_inject or options.extra_args:
> + fail(_("--unattended does not support --initrd-inject nor --extra-args."))
> +
>
> def _show_nographics_warnings(options, guest, installer):
> if guest.devices.graphics:
> @@ -472,8 +481,21 @@ def build_installer(options, guest):
> location_kernel = None
> location_initrd = None
> install_bootdev = None
> + network_install = False
>
> has_installer = True
> + if options.unattended:
> + if not guest.osinfo.is_windows():
> + if options.cdrom:
> + options.location = options.cdrom
> + options.cdrom = None
> + elif options.location:
> + if not options.location.endswith(".iso"):
> + network_install = True
> + else:
> + options.location = guest.osinfo.get_url(guest.os.arch)
> + network_install = True
> +
> if options.location:
> (location,
> location_kernel,
> @@ -502,10 +524,21 @@ def build_installer(options, guest):
> install_bootdev=install_bootdev)
> if cdrom and options.livecd:
> installer.livecd = True
> - if options.extra_args:
> - installer.extra_args = options.extra_args
> - if options.initrd_inject:
> - installer.set_initrd_injections(options.initrd_inject)
> +
> + if network_install:
> + installer.networkinstall = True
> +
> + if options.unattended:
> + unattended_data = cli.parse_unattended(options.unattended)
> + options.unattended = None
> +
> + installer.set_unattended(guest, options.distro_variant, options.name, unattended_data)
> + else:
> + if options.extra_args:
> + installer.extra_args = options.extra_args
> + if options.initrd_inject:
> + installer.set_initrd_injections(options.initrd_inject)
> +
> if options.autostart:
> installer.autostart = True
>
> @@ -531,6 +564,9 @@ def build_guest_instance(conn, options):
> guest.os.machine = options.machine
>
> guest.set_capabilities_defaults()
> + if options.unattended:
> + guest.set_os_name(options.distro_variant)
> +
My patches took care of the movement here. It did away with
set_distro_variant, since it was conflating two cases: when an explicit
--os-variant was specified, and when we are detecting os-variant from
explicit media specified. With more stuff being determined from the
os-variant, the first case needed to be handled later.
> installer = build_installer(options, guest)
> if installer:
> set_distro_variant(options, guest, installer)
> @@ -787,6 +823,8 @@ def parse_args():
> "booted from --location"))
> insg.add_argument("--initrd-inject", action="append",
> help=_("Add given file to root of initrd from --location"))
> + insg.add_argument("--unattended", const="profile=jeos", default="", nargs='?',
> + help=_("Unattended installation"))
>
I don't think the const= is needed, and the default= can probably be
dropped too.
> # Takes a URL and just prints to stdout the detected distro name
> insg.add_argument("--test-media-detection", help=argparse.SUPPRESS)
> diff --git a/virtinst/cli.py b/virtinst/cli.py
> index 52a876e1..ea36b015 100644
> --- a/virtinst/cli.py
> +++ b/virtinst/cli.py
> @@ -462,7 +462,7 @@ def get_meter():
> ###########################
>
> def _get_completer_parsers():
> - return VIRT_PARSERS + [ParseCLICheck, ParserLocation]
> + return VIRT_PARSERS + [ParseCLICheck, ParserLocation, ParseCLIUnattended]
>
>
> def _virtparser_completer(prefix, **kwargs):
> @@ -1417,6 +1417,35 @@ class VirtCLIParser(metaclass=InitClass):
> """Do nothing callback"""
>
>
> +########################
> +# --unattended parsing #
> +########################
> +
> +class ParseCLIUnattended(VirtCLIParser):
> + cli_arg_name = "unattended"
> +
> + @classmethod
> + def __init_class__(cls, **kwargs):
> + VirtCLIParser.__init_class__(**kwargs)
> + cls.add_arg("profile", "profile")
> + cls.add_arg("admin_password", "admin-password")
> + cls.add_arg("user_password", "user-password")
> + cls.add_arg("product_key", "product-key")
> +
> +def parse_unattended(unattended):
> + class UnattendedData:
> + def __init__(self):
> + self.profile = "jeos"
> + self.admin_password = None
> + self.user_password = None
> + self.product_key = None
> +
> + ret = UnattendedData()
> + parser = ParseCLIUnattended(None, unattended)
> + parser.parse(ret)
> + return ret
> +
> +
> ###################
> # --check parsing #
> ###################
> diff --git a/virtinst/installer.py b/virtinst/installer.py
> index e3ccfa42..31772527 100644
> --- a/virtinst/installer.py
> +++ b/virtinst/installer.py
> @@ -8,6 +8,7 @@
>
> import os
> import logging
> +import subprocess
>
I'd like all the floppy subprocess stuff to go into its own file, along
the lines of initrdinject.py
> import libvirt
>
> @@ -17,6 +18,11 @@ from .osdict import OSDB
> from .installertreemedia import InstallerTreeMedia
> from . import util
>
> +import gi
> +gi.require_version('Libosinfo', '1.0')
> +from gi.repository import Libosinfo as libosinfo
> +from gi.repository import Gio as gio
> +
>
It would be nice if all the libosinfo stuff could live in osdict.py. I
didn't look too deeply at what's happening in this file though so let me
know if you disagree.
> class Installer(object):
> """
> @@ -39,6 +45,7 @@ class Installer(object):
> location_kernel=None, location_initrd=None):
> self.conn = conn
>
> + self.networkinstall = False
> self.livecd = False
> self.extra_args = []
>
> @@ -49,6 +56,8 @@ class Installer(object):
> self._install_kernel = None
> self._install_initrd = None
> self._install_cdrom_device = None
> + self._unattended_floppy_device = None
> + self._unattended_files = []
> self._defaults_are_set = False
>
> if location_kernel or location_initrd:
> @@ -111,6 +120,28 @@ class Installer(object):
> self._install_cdrom_device.path = None
> self._install_cdrom_device.sync_path_props()
>
> + def _add_unattended_floppy_device(self, guest, location):
> + if self._unattended_floppy_device:
> + return
> + dev = DeviceDisk(self.conn)
> + dev.device = dev.DEVICE_FLOPPY
> + dev.path = location
> + dev.sync_path_props()
> + dev.validate()
> + self._unattended_floppy_device = dev
> + guest.add_device(self._unattended_floppy_device)
> +
> + def _remove_unattended_floppy_device(self, guest):
> + if not self._unattended_floppy_device:
> + return
> +
> + self._unattended_floppy_device.path = None
> + self._unattended_floppy_device.sync_path_props()
> +
> + def _cleanup_unattended_files(self):
> + for f in self._unattended_files:
> + os.unlink(f)
> +
> def _build_boot_order(self, guest, bootdev):
> bootorder = [bootdev]
>
> @@ -168,6 +199,8 @@ class Installer(object):
> def _cleanup(self, guest):
> if self._treemedia:
> self._treemedia.cleanup(guest)
> + else:
> + self._cleanup_unattended_files()
>
> def _get_postinstall_bootdev(self, guest):
> if self.cdrom and self.livecd:
> @@ -272,6 +305,88 @@ class Installer(object):
> logging.debug("installer.detect_distro returned=%s", ret)
> return ret
>
> + def set_unattended(self, guest, variant, name, unattended_data):
> + """
> + Set up the unattended installation based on the OS variant of the
> + guest. In case some error happens, a warning will be printed and
> + no unattended installation will happen.
> + """
> + # Those bits about installation source and injection method are
> + # mostly needed for Linuxes and they become a no-op on Windows(es)
> + # Guests.
> + installation_source = libosinfo.InstallScriptInstallationSource.MEDIA \
> + if not self.networkinstall \
> + else libosinfo.InstallScriptInstallationSource.NETWORK
> +
> + injection_method = libosinfo.InstallScriptInjectionMethod.FLOPPY \
> + if guest.osinfo.is_windows() \
> + else libosinfo.InstallScriptInjectionMethod.INITRD
> +
> + script = guest.osinfo.get_install_script(unattended_data.profile)
> +
> + supported_injection_methods = script.get_injection_methods()
> +
> + if (injection_method & supported_injection_methods == 0):
> + logging.warning(
> + _("%s does not support unattended installation for the %s profile "
> + "when using initrd as injection method."), name, profile)
> + return
> +
I turned this into an explicit failure. We can loosen it later if
desired, but I'd rather be strict now, will make testing easier.
> + script.set_preferred_injection_method(injection_method)
> + script.set_installation_source(installation_source)
> +
> + config = guest.osinfo.get_install_script_config(script, guest.os.arch, name)
> +
> + if unattended_data.admin_password and not unattended_data.user_password:
> + unattended_data.user_password = unattended_data.admin_password
> +
> + if unattended_data.user_password and not unattended_data.admin_password:
> + unattended_data.admin_password = unattended_data.user_password
> +
> + if unattended_data.user_password or unattended_data.admin_password:
> + config.set_admin_password(unattended_data.admin_password)
> + config.set_user_password(unattended_data.user_password)
> + else:
> + # "desktop" profiles will always have a user set up. The same
> + # happen for Windows(es) installations, even the "jeos" ones.
> + if unattended_data.profile == "desktop" or guest.osinfo.is_windows():
> + print(_("User password: %s" % config.get_user_password()))
> + print(_("Admin password: %s" % config.get_admin_password()))
> +
> + if self._treemedia:
> + cmdline = self._treemedia.generate_install_script(guest, script, config)
> + self.extra_args = [cmdline]
> + else:
> + # This branch targets Windows(es) unattended installations, where
> + # we have to:
> + # - set the target disk accordingly;
> + # - set the product key
> + # - create a floppy drive with a MS-DOS filesystem
> + # - copy the installation files to the drive
> + config.set_target_disk("C")
> + config.set_reg_product_key(unattended_data.product_key)
> +
> + scratch = os.path.join(util.get_cache_dir(), "unattended")
> + if not os.path.exists(scratch):
> + os.makedirs(scratch, 0o751)
> +
> + img = os.path.join(scratch, name + "-unattended.img")
> + self._unattended_files.append(img)
> +
> + guest.osinfo.generate_install_script_output(script, config, gio.File.new_for_path(scratch))
> + path = os.path.join(scratch, script.get_expected_filename())
> + self._unattended_files.append(path)
> +
> + cmd = ["mkfs.msdos", "-C", img, "1440"]
> + logging.debug("Running mkisofs: %s", cmd)
> + output = subprocess.check_output(cmd)
> +
> + cmd = ["mcopy", "-i", img, path, "::"]
> + logging.debug("Running mcopy: %s", cmd)
> + output = subprocess.check_output(cmd)
> +
> + self._add_unattended_floppy_device(guest, img)
> +
>
This actual floppy creation can't happen here, it needs to happen via
the _prepare step
Actually since I see the UnattendedData class from cli.py is passed all
the way down to here, it should be public. Maybe the floppy and
UnattendedData stuff could go into its own file like installunattended.
Something to consider
I didn't wrap my head around the details too much, I'll look more
closely on a v2
Thanks,
Cole
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-virt-install-Add-unattended-support.patch
Type: text/x-patch
Size: 18878 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20190207/59adc201/attachment.bin>
More information about the virt-tools-list
mailing list