[virt-tools-list] [virt-manager PATCH v3] Introduction of cloud-init configuration in virt-install

[Cole Robinson]

On 6/28/19 12:05 PM, Athina Plaskasoviti wrote:
> Usage:
> --cloud-init
> 
> Signed-off-by: Athina Plaskasoviti <athina.plaskasoviti at gmail.com>
> ---
>  virt-install                        |  5 +++
>  virtinst/cli.py                     | 25 +++++++++++++
>  virtinst/install/cloudinit.py       | 57 +++++++++++++++++++++++++++++
>  virtinst/install/installer.py       | 17 +++++++++
>  virtinst/install/installerinject.py | 20 +++++-----
>  5 files changed, 115 insertions(+), 9 deletions(-)
>  create mode 100644 virtinst/install/cloudinit.py
> 

Thanks. Rather than keeping going back and forth with a lot of the same
code repeated in future patch versions, I've pushed your patch and one
of my own to the 'cloudinit' branch on the upstream repo.

https://github.com/virt-manager/virt-manager/tree/cloudinit

My patch on top fixes the password generation to actually work for the
root password.

Now we can fix individual issues in the patch with smaller incremental
patches. And maybe squash them together before moving them to git.

The first issue to fix in that branch is that the test suite is still
broken.

> diff --git a/virtinst/install/cloudinit.py b/virtinst/install/cloudinit.py
> new file mode 100644
> index 00000000..41667f4b
> --- /dev/null
> +++ b/virtinst/install/cloudinit.py
> @@ -0,0 +1,57 @@
> +import tempfile
> +import random
> +import string
> +import time
> +from ..logger import log
> +
> +
> +class CloudInitData():
> +    root_password = None
> +
> +
> +def create_metadata(scratchdir, hostname=None):
> +    if hostname:
> +        instance = hostname
> +    else:
> +        hostname = instance = "localhost"
> +    content = 'instance-id: %s\n' % instance
> +    content += 'hostname: %s\n' % hostname
> +    log.debug("Generated cloud-init metadata:\n%s", content)
> +

Like I mentioned in one of the previous mails, none of this data is
required. So we can just set content="" here and write that. hostname=
will be unused after that and can be removed

> +    fileobj = tempfile.NamedTemporaryFile(
> +            prefix="virtinst-", suffix="-metadata",
> +            dir=scratchdir, delete=False)
> +    filename = fileobj.name
> +
> +    with open(filename, "w") as f:
> +        f.write(content)
> +    return filename
> +
> +
> +def create_userdata(scratchdir, cloudinit_data, username=None, password=None):

Because we are going to use the cloudinit_data class here, username and
password values should be dropped.

> +    if not password:
> +        password = ""
> +        for dummy in range(16):
> +            password += random.choice(string.ascii_letters + string.digits)
> +    content = "#cloud-config\n"
> +    if username:
> +        content += "name: %s\n" % username
> +    if cloudinit_data.root_password == "generate":
> +        pass
> +    else:
> +        content += "password: %s\n" % password
> +        log.debug("Generated password for first boot: \n%s", password)
> +        time.sleep(20)
> +    content += "runcmd:\n"
> +    content += "- [ sudo, touch, /etc/cloud/cloud-init.disabled ]\n"
> +    log.debug("Generated cloud-init userdata:\n%s", content)
> +

I think we should also add a --cloud-init disable=yes|no option as well
that controls this behavior. Plain '--cloud-init' will then default to
--cloud-init root-password=generate,disable=yes. I think if users are
specifying manual options we can require them to pass explicit
disable=yes if they want it.

These changes can all be their own individual patches that are sent as
you implement them.

Thanks,
Cole