[pve-devel] [PATCH qemu-server] cloudinit: add sshdeletehostkeys option

[pve-devel] [PATCH qemu-server] cloudinit: add sshdeletehostkeys option

[Alexandre Derumier]

This define behaviour of ssh server keys generation on cloudinit
config change.

different value:

- once : only once at vmstart  (default value)
- no : never generate ssh key
- yes: always generate ssh key

When value is defined to 'once', the value is rewriten to 'no'
in vmconfig after vm start

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 PVE/QemuServer.pm           |  9 ++++++++-
 PVE/QemuServer/Cloudinit.pm | 11 +++++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 54278e5..cd6c26c 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -760,6 +760,13 @@ my $confdesc_cloudinit = {
 	format => 'urlencoded',
 	description => "cloud-init: Setup public SSH keys (one key per line, OpenSSH format).",
     },
+    sshdeletehostkeys => {
+	optional => 1,
+	type => 'string',
+	enum => [qw(once yes no)],
+	default_key => 1,
+	description => "cloud-init: Regenerate host SSH keys on config change.",
+    },
 };
 
 # what about other qemu settings ?
@@ -4943,7 +4950,7 @@ sub vm_start_nolock {
 	$conf = PVE::QemuConfig->load_config($vmid); # update/reload
     }
 
-    PVE::QemuServer::Cloudinit::generate_cloudinitconfig($conf, $vmid);
+    PVE::QemuServer::Cloudinit::generate_cloudinitconfig($conf, $vmid, 1);
 
     my $defaults = load_defaults();
 
diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
index dd643c1..4dbc4d6 100644
--- a/PVE/QemuServer/Cloudinit.pm
+++ b/PVE/QemuServer/Cloudinit.pm
@@ -135,7 +135,7 @@ sub cloudinit_userdata {
 	    $content .= "  - $k\n";
 	}
     }
-    $content .= "ssh_deletekeys: false\n" if PVE::QemuServer::check_running($vmid);
+    $content .= "ssh_deletekeys: false\n" if defined($conf->{sshdeletehostkeys}) && $conf->{sshdeletehostkeys} eq 'no'; 
 
     $content .= "chpasswd:\n";
     $content .= "  expire: False\n";
@@ -464,9 +464,10 @@ my $cloudinit_methods = {
 };
 
 sub generate_cloudinitconfig {
-    my ($conf, $vmid) = @_;
+    my ($conf, $vmid, $vmstart) = @_;
 
     my $format = get_cloudinit_format($conf);
+    my $generated = undef;
 
     PVE::QemuConfig->foreach_volume($conf, sub {
         my ($ds, $drive) = @_;
@@ -479,7 +480,13 @@ sub generate_cloudinitconfig {
 	    or die "missing cloudinit methods for format '$format'\n";
 
 	$generator->($conf, $vmid, $drive, $volname, $storeid);
+	$generated = 1;
     });
+
+    if ($vmstart && $generated && (!defined($conf->{sshdeletehostkeys}) || $conf->{sshdeletehostkeys} eq 'once')) {
+        $conf->{sshdeletehostkeys} = 'no';
+        PVE::QemuConfig->write_config($vmid, $conf);
+    }
 }
 
 sub dump_cloudinit_config {
-- 
2.20.1

Re: [pve-devel] [PATCH qemu-server] cloudinit: add sshdeletehostkeys option

[Mira Limbeck]

Thank you for the patch.

It doesn't apply on the latest qemu-server master. Looks like your 
Cloudinit.pm file already contained changes which are not part of the patch.

Was it just the previous patch you sent?


Some additional comments inline.

On 1/14/21 6:11 PM, Alexandre Derumier wrote:
> This define behaviour of ssh server keys generation on cloudinit
> config change.
>
> different value:
>
> - once : only once at vmstart  (default value)
> - no : never generate ssh key
> - yes: always generate ssh key
>
> When value is defined to 'once', the value is rewriten to 'no'
> in vmconfig after vm start

This is exactly the use case of vendor data (run once at boot): 
https://cloudinit.readthedocs.io/en/latest/topics/vendordata.html

Maybe this could be done in addition to the instance-id change suggested 
below?

>
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>   PVE/QemuServer.pm           |  9 ++++++++-
>   PVE/QemuServer/Cloudinit.pm | 11 +++++++++--
>   2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 54278e5..cd6c26c 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -760,6 +760,13 @@ my $confdesc_cloudinit = {
>   	format => 'urlencoded',
>   	description => "cloud-init: Setup public SSH keys (one key per line, OpenSSH format).",
>       },
> +    sshdeletehostkeys => {
> +	optional => 1,
> +	type => 'string',
> +	enum => [qw(once yes no)],
> +	default_key => 1,
> +	description => "cloud-init: Regenerate host SSH keys on config change.",
> +    },
>   };

Consensus was that we do not want additional cloud-init options in the 
global options namespace. So instead it would be better to add it to 
cicustom instead and open that up for other custom options (as was 
initially intended).

Regarding the enum => [qw(once yes no)] line, we probably want to accept 
everything type 'Boolean' accepts, not just 'yes' and 'no'.

>   
>   # what about other qemu settings ?
> @@ -4943,7 +4950,7 @@ sub vm_start_nolock {
>   	$conf = PVE::QemuConfig->load_config($vmid); # update/reload
>       }
>   
> -    PVE::QemuServer::Cloudinit::generate_cloudinitconfig($conf, $vmid);
> +    PVE::QemuServer::Cloudinit::generate_cloudinitconfig($conf, $vmid, 1);
>   
>       my $defaults = load_defaults();
>   
> diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
> index dd643c1..4dbc4d6 100644
> --- a/PVE/QemuServer/Cloudinit.pm
> +++ b/PVE/QemuServer/Cloudinit.pm
> @@ -135,7 +135,7 @@ sub cloudinit_userdata {
>   	    $content .= "  - $k\n";
>   	}
>       }
> -    $content .= "ssh_deletekeys: false\n" if PVE::QemuServer::check_running($vmid);
> +    $content .= "ssh_deletekeys: false\n" if defined($conf->{sshdeletehostkeys}) && $conf->{sshdeletehostkeys} eq 'no';
>   
>       $content .= "chpasswd:\n";
>       $content .= "  expire: False\n";
> @@ -464,9 +464,10 @@ my $cloudinit_methods = {
>   };
>   
>   sub generate_cloudinitconfig {
> -    my ($conf, $vmid) = @_;
> +    my ($conf, $vmid, $vmstart) = @_;
>   
>       my $format = get_cloudinit_format($conf);
> +    my $generated = undef;
>   
>       PVE::QemuConfig->foreach_volume($conf, sub {
>           my ($ds, $drive) = @_;
> @@ -479,7 +480,13 @@ sub generate_cloudinitconfig {
>   	    or die "missing cloudinit methods for format '$format'\n";
>   
>   	$generator->($conf, $vmid, $drive, $volname, $storeid);
> +	$generated = 1;
>       });
> +
> +    if ($vmstart && $generated && (!defined($conf->{sshdeletehostkeys}) || $conf->{sshdeletehostkeys} eq 'once')) {
> +        $conf->{sshdeletehostkeys} = 'no';
> +        PVE::QemuConfig->write_config($vmid, $conf);
> +    }
>   }

Maybe it would make sense to create an instance id once and only change 
it if requested afterwards, instead of basing it on the user and network 
configs? This would also remove the need for this option.

Then we could simply regenerate the instance id on a clone, or if 
requested when restoring from a backup to a new VMID. What do you think?


I'll probably extend the documentation with info on preparing a 
cloudimg, as sometimes they do not work out of the box and require 
cleaning of the cloud-init artifacts [0] as well as changing the 
pre-configured cloud.cfg file.


[0] https://cloudinit.readthedocs.io/en/latest/topics/cli.html#clean

>   
>   sub dump_cloudinit_config {

Re: [pve-devel] [PATCH qemu-server] cloudinit: add sshdeletehostkeys option

[aderumier]

Le mercredi 27 janvier 2021 à 17:56 +0100, Mira Limbeck a écrit :
> Thank you for the patch.
> 
> It doesn't apply on the latest qemu-server master. Looks like your 
> Cloudinit.pm file already contained changes which are not part of the
> patch.
> 
> Was it just the previous patch you sent?
> 
Hi, sorry, I didn't see your response.
I'll rebase my patch.


> 
> Some additional comments inline.
> 
> On 1/14/21 6:11 PM, Alexandre Derumier wrote:
> > This define behaviour of ssh server keys generation on cloudinit
> > config change.
> > 
> > different value:
> > 
> > - once : only once at vmstart  (default value)
> > - no : never generate ssh key
> > - yes: always generate ssh key
> > 
> > When value is defined to 'once', the value is rewriten to 'no'
> > in vmconfig after vm start
> 
> This is exactly the use case of vendor data (run once at boot): 
> https://cloudinit.readthedocs.io/en/latest/topics/vendordata.html
> 
> Maybe this could be done in addition to the instance-id change
> suggested 
> below?
> 
> 
> Maybe it would make sense to create an instance id once and only
> change 
> it if requested afterwards, instead of basing it on the user and
> network 
> configs? This would also remove the need for this option.
> 
> Then we could simply regenerate the instance id on a clone, or if 
> requested when restoring from a backup to a new VMID. What do you
> think?
> 
> 
> I'll probably extend the documentation with info on preparing a 
> cloudimg, as sometimes they do not work out of the box and require 
> cleaning of the cloud-init artifacts [0] as well as changing the 
> pre-configured cloud.cfg file.
> 
> 
> [0] https://cloudinit.readthedocs.io/en/latest/topics/cli.html#clean


The main problem currently is indeed that we change instance-id at each
rebuild of the cloud-init disk.
But I'm not sure that's it's possible to change ip address when keeping
same instance-id, because ip configuration is done
at the cloudinit-init-local service, at it's already done once. 
Maybe this was the historic reason why we change the the instance-id
each time, I don't remember exactly.
I'll check that tomorrow to be sure, but indeed, keeping the instance-
id should be the clean way.



> > 
> > Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> > ---
> >   PVE/QemuServer.pm           |  9 ++++++++-
> >   PVE/QemuServer/Cloudinit.pm | 11 +++++++++--
> >   2 files changed, 17 insertions(+), 3 deletions(-)
> > 
> > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> > index 54278e5..cd6c26c 100644
> > --- a/PVE/QemuServer.pm
> > +++ b/PVE/QemuServer.pm
> > @@ -760,6 +760,13 @@ my $confdesc_cloudinit = {
> >         format => 'urlencoded',
> >         description => "cloud-init: Setup public SSH keys (one key
> > per line, OpenSSH format).",
> >       },
> > +    sshdeletehostkeys => {
> > +       optional => 1,
> > +       type => 'string',
> > +       enum => [qw(once yes no)],
> > +       default_key => 1,
> > +       description => "cloud-init: Regenerate host SSH keys on
> > config change.",
> > +    },
> >   };
> 
> Consensus was that we do not want additional cloud-init options in
> the 
> global options namespace. So instead it would be better to add it to 
> cicustom instead and open that up for other custom options (as was 
> initially intended).
> 
> Regarding the enum => [qw(once yes no)] line, we probably want to
> accept 
> everything type 'Boolean' accepts, not just 'yes' and 'no'.

Ok no problem, I'll change that


Thanks for the review !

Re: [pve-devel] [PATCH qemu-server] cloudinit: add sshdeletehostkeys option

[aderumier]

Le lundi 01 février 2021 à 17:12 +0100, aderumier@odiso.com a écrit :
> > 
> > [0] https://cloudinit.readthedocs.io/en/latest/topics/cli.html#clea
> > n
> 
> 
> The main problem currently is indeed that we change instance-id at
> each rebuild of the cloud-init disk.
> But I'm not sure that's it's possible to change ip address when
> keeping same instance-id, because ip configuration is done
> at the cloudinit-init-local service, at it's already done once. 
> Maybe this was the historic reason why we change the the instance-id
> each time, I don't remember exactly.
> I'll check that tomorrow to be sure, but indeed, keeping the
> instance-id should be the clean way.


Hi, I have redone tests with cloud-init.
So, the problem with networking config, is it's really only done
once by instance-id , and they are no way to change that.
(it's done in cloud-init local, and it's not possible to change that
like the others modules).

So, we really need to change instance-id each time.


Also, the ssh module manage both user ssh public keys,
and host private keys. So the only way is the trick of this patch to
disable the host key generation after first boot.


Currently I'm not using cloud-init, but I wonder if it's really
the good tool to manage configuration after the first boot.
(It's has been created by cloud-provider to really only run once at
first boot, and after that the configuration don't change).

I have looked on the net, and found than opennebula provide a nice 
tool to manage configuration at boot, but also changes in live.

https://github.com/OpenNebula/addon-context-linux

It's only simple a simple iso config drive with environment variables +
bash script + service for boot + udev rules to handle cdrom media
changes/nic plug/unplug live events.

Historically, opennebula was using cloud-init too (they are also an
opennebula config drive driver in cloud-init)
https://cloudinit.readthedocs.io/en/latest/topics/datasources/opennebula.html
 but it was not matching their workload, so they have created their own
script.
https://forum.opennebula.io/t/one-context-vs-cloud-init/1641


I have tested it, with a small patch in CloudInit.pm to generate the
cloudera context format, it's just working out of the box.
I can manage ips,hostname,sshkeys,resize partition  dynamically without
any trick.

I really like it, and with simple bash script + env var in config
drive, it's possible to easily adapt them for special need.
(I have some tricky network config in my production network)

a windows version is available too:
https://github.com/OpenNebula/addon-context-windows



What do you think to add this new datasource format support in current
Cloudinit.pm ? 

Here a sample (not cleaned yet):



+sub generate_opennebula {
+    my ($conf, $vmid, $drive, $volname, $storeid) = @_;
+
+    my ($hostname, $fqdn) = get_hostname_fqdn($conf, $vmid);
+
+    my $content = "";
+
+    my $username = $conf->{ciuser};
+    my $password = encode_base64($conf->{cipassword});

+        $keys = [map { my $key = $_; chomp $key; $key } split(/\n/,
$keys)];
+        $keys = [grep { /\S/ } @$keys];
+        $content .= "SSH_PUBLIC_KEY=\"";
+
+        foreach my $k (@$keys) {
+            $content .= "$k\n";
+        }
+        $content .= "\"\n";
+
+    }
+
+    my ($searchdomains, $nameservers) = get_dns_conf($conf);
+    if ($nameservers && @$nameservers) {
+        $nameservers = join(' ', @$nameservers);
+        $content .= "DNS=\"$nameservers\"\n";
+    }
+
+    $content .= "NETWORK=YES\n";
+    $content .= "SET_HOSTNAME=prout2\n";
+
+    if ($searchdomains && @$searchdomains) {
+        $searchdomains = join(' ', @$searchdomains);
+        $content .= "SEARCH_DOMAIN=\"$searchdomains\"\n";
+    }
+
+    my @ifaces = grep { /^net(\d+)$/ } keys %$conf;
+    foreach my $iface (sort @ifaces) {
+        (my $id = $iface) =~ s/^net//;
+        next if !$conf->{"ipconfig$id"};
+        my $net = PVE::QemuServer::parse_ipconfig($conf-
>{"ipconfig$id"});
+        my $ethid = "ETH$id";
+
+       my $mac = lc $net->{hwaddr};
+        if ($net->{ip}) {
+            if ($net->{ip} eq 'dhcp') {
+                $content .= "\n";  #opennebule don't handle DHCP
config.....
+            } else {
+                my ($addr, $mask) = split_ip4($net->{ip});
+               $content .= $ethid."_IP=$addr\n";
+               $content .= $ethid."_MASK=$mask\n";
+               $content .= $ethid."_MAC=$mac\n";
+               $content .= $ethid."_GATEWAY=$net->{gw}\n" if $net-
>{gw};
+            }
+        }
+
+    }
+
+    my $files = {
+       '/context.sh' => $content,
+    };
+    commit_cloudinit_disk($conf, $vmid, $drive, $volname, $storeid,
$files, 'CONTEXT');
+}
+
@@ -461,13 +531,14 @@ sub read_cloudinit_snippets_file {
 my $cloudinit_methods = {
     configdrive2 => \&generate_configdrive2,
     nocloud => \&generate_nocloud,
+    opennebula => \&generate_opennebula,
 };