On Fri, Jun 21, 2019 at 01:50:51PM -0400, Cole Robinson wrote:
> On 6/21/19 1:34 PM, Daniel P. Berrangé wrote:
> > On Fri, Jun 21, 2019 at 08:18:45PM +0300, Athina Plaskasoviti wrote:
> >> Triggered by:
> >> --install is_cloud=yes ... --import
> >>
> >> Signed-off-by: Athina Plaskasoviti <athina.plaskasoviti@xxxxxxxxx>
> >> ---
> >> virt-install | 9 ++++---
> >> virtinst/cli.py | 2 ++
> >> virtinst/install/cloudinit.py | 41 +++++++++++++++++++++++++++++
> >> virtinst/install/installer.py | 16 ++++++++++-
> >> virtinst/install/installerinject.py | 20 +++++++-------
> >> 5 files changed, 75 insertions(+), 13 deletions(-)
> >> create mode 100644 virtinst/install/cloudinit.py
> >>
> >> diff --git a/virt-install b/virt-install
> >> index ee2b9006..b3608662 100755
> >> --- a/virt-install
> >> +++ b/virt-install
> >> @@ -399,6 +399,7 @@ def build_installer(options, guest, installdata):
> >> install_kernel_args = installdata.kernel_args
> >> install_os = installdata.os
> >> no_install = installdata.no_install
> >> + is_cloud = installdata.is_cloud
> >> if installdata.kernel_args:
> >> if installdata.kernel_args_overwrite:
> >> install_kernel_args = installdata.kernel_args
> >> @@ -417,10 +418,11 @@ def build_installer(options, guest, installdata):
> >> no_install = True
> >> elif options.pxe:
> >> install_bootdev = "network"
> >> + elif options.import_install:
> >> + no_install = True
> >> elif installdata.is_set:
> >> pass
> >> - elif (options.import_install or
> >> - options.xmlonly or
> >> + elif (options.xmlonly or
> >> options.boot):
> >> no_install = True
> >>
> >> @@ -433,7 +435,8 @@ def build_installer(options, guest, installdata):
> >> install_kernel=install_kernel,
> >> install_initrd=install_initrd,
> >> install_kernel_args=install_kernel_args,
> >> - no_install=no_install)
> >> + no_install=no_install,
> >> + is_cloud=is_cloud)
> >>
> >> if options.unattended:
> >> unattended_data = cli.parse_unattended(options.unattended)
> >> diff --git a/virtinst/cli.py b/virtinst/cli.py
> >> index 9a1fe2f6..a2a501a5 100644
> >> --- a/virtinst/cli.py
> >> +++ b/virtinst/cli.py
> >> @@ -1580,6 +1580,7 @@ class ParserInstall(VirtCLIParser):
> >> is_onoff=True)
> >> cls.add_arg("os", "os")
> >> cls.add_arg("no_install", "no_install", is_onoff=True)
> >> + cls.add_arg("is_cloud", "is_cloud", is_onoff=True)
> >>
> >>
> >> class InstallData:
> >> @@ -1592,6 +1593,7 @@ class InstallData:
> >> self.os = None
> >> self.is_set = False
> >> self.no_install = None
> >> + self.is_cloud = None
> >>
> >>
> >> def parse_install(optstr):
> >> diff --git a/virtinst/install/cloudinit.py b/virtinst/install/cloudinit.py
> >> new file mode 100644
> >> index 00000000..25b2a79b
> >> --- /dev/null
> >> +++ b/virtinst/install/cloudinit.py
> >> @@ -0,0 +1,41 @@
> >> +import tempfile
> >> +from ..logger import log
> >> +
> >> +
> >> +def create_metadata(scratchdir, hostname=None):
> >> + if hostname:
> >> + instance = hostname
> >> + else:
> >> + hostname = instance = "localhost"
> >> +
> >> + fileobj = tempfile.NamedTemporaryFile(
> >> + prefix="virtinst-", suffix="-metadata",
> >> + dir=scratchdir, delete=False)
> >> + filename = fileobj.name
> >> +
> >> + with open(filename, "w") as f:
> >> + log.debug("Writing instance-id and hostname to file meta-data")
> >> + f.writelines(['instance-id: %s\n' % instance,
> >> + 'hostname: %s\n' % hostname])
> >> + return filename
> >
>
> Just a general note, for now we are just trying to straighten out the
> plumbing and get something testable. I'm not planning on pushing
> anything without a broader discussion which I will start soon
>
> > We should probably use the UUID for the newly to-be-created VM as
> > the instance-id value, and probably not set hostname at all unless
> > we know what it will end up being.
> >
>
> What practical effect does instance-id have? It wasn't obvious to me
> from reading the docs
No idea, probably have to read the source.
> >> +def create_userdata(scratchdir, username=None, password=None):
> >> + if not password:
> >> + password = "password"
> >
> > We must not do this - it is a secret flaw to have a hardcoded
> > password.
> >
> > If the user doesn't spply a password, we should simply not
> > set one.
> >
>
> Yes, final commit will not have a hardcoded password. We couldn't find a
> way with native cloud-init commands to unset the password though,
> unclear if there's a way besides calling host commands. We can generate
> a random password, print it to stdout, to it and set it to expire on
> first login. virt-builder does the random password thing IIRC
>
> FWIW though fedora cloud images already have a notion of a default
> password, it's just 'fedora' if it finds cloud-init config that doesn't
> overwrite the password. Something like that... I haven't dug enough into
> the details to know how all the pieces fit together.
Hmm, I thought it would not have any password set at all, forcing login
via ssh pub keys.
Generating a random 20 char password and print to stdout seems
the best bet as dfault behaviour.
> > As default behaviour it is probably more useful to
> > take $HOME/.ssh/authorized_keys and inject it into
> > the guest. We should also allow an alterantive file
> > in case they want different keys in some cases.
> >
>
> Hmm interesting. We can't depend on that option as necessary for login
> because not everyone will have ssh keys configured.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
