[pve-devel] [PATCH] Let PVE QEMU Cloud-Init adapt Cloudbase-init

[Mira Limbeck]

Hi,

Thank you for the patch. Looks like the patch still lacks a commit message.

It might also better to separate those 4 (almost) independent changes as 
@Thomas mentioned yesterday.

For example first the 'genisoimage' patch as none of the others work 
correctly without this patch? (At least based on my limited 
cloudbase-init tests regarding static network configuration this is 
necessary for cloudbase-init to recognize the iso correctly. Or do some 
of the features work without this?)


Some further comments inline.


On 4/12/19 12:36 AM, Austin Chan wrote:
> ---
>   PVE/API2/Qemu.pm            |  8 ++++++--
>   PVE/QemuServer/Cloudinit.pm | 13 +++++++++----
>   2 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 7195ec2..a591d7a 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1020,10 +1020,14 @@ my $update_vm_api  = sub {
>   
>       my $background_delay = extract_param($param, 'background_delay');
>   
> +    my $ostype = PVE::QemuConfig->load_config($vmid)->{ostype};
> +
>       if (defined(my $cipassword = $param->{cipassword})) {
>   	# Same logic as in cloud-init (but with the regex fixed...)
> -	$param->{cipassword} = PVE::Tools::encrypt_pw($cipassword)
> -	    if $cipassword !~ /^\$(?:[156]|2[ay])(\$.+){2}/;
> +	if (!PVE::QemuServer::windows_version($ostype)) {
> +	    $param->{cipassword} = PVE::Tools::encrypt_pw($cipassword)
> +		if $cipassword !~ /^\$(?:[156]|2[ay])(\$.+){2}/;
> +	}
>       }
I haven't gotten around to testing this yet, but I will as soon as I get 
to it.
>   
>       my @paramarr = (); # used for log message
> diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
> index 445c777..4f39a2b 100644
> --- a/PVE/QemuServer/Cloudinit.pm
> +++ b/PVE/QemuServer/Cloudinit.pm
> @@ -43,7 +43,7 @@ sub commit_cloudinit_disk {
>       my $size = PVE::Storage::file_size_info($iso_path);
>   
>       eval {
> -	run_command([['genisoimage', '-R', '-V', $label, $path],
> +	run_command([['genisoimage','-iso-level', '3', '-R', '-V', $label, $path],
>   		     ['qemu-img', 'dd', '-n', '-f', 'raw', '-O', $format,
>   		      'isize=0', "osize=$size", "of=$iso_path"]]);
This is a change that I made locally as well to test it last time, still 
have to test it more thoroughly, but looks good so far.
>       };
> @@ -159,11 +159,11 @@ sub configdrive2_network {
>       my ($searchdomains, $nameservers) = get_dns_conf($conf);
>       if ($nameservers && @$nameservers) {
>   	$nameservers = join(' ', @$nameservers);
> -	$content .= "        dns_nameservers $nameservers\n";
> +	$content .= "        dns-nameservers $nameservers\n";
>       }
>       if ($searchdomains && @$searchdomains) {
>   	$searchdomains = join(' ', @$searchdomains);
> -	$content .= "        dns_search $searchdomains\n";
> +	$content .= "        dns-search $searchdomains\n";
>       }
>   

Tested this yesterday, still couldn't set any dns-nameservers. How did 
you make it work? Which cloudbase-init config did you use and what did 
you add in the PVE GUI?

ipv4 nameservers worked when I added them to the ipv4 network interfaces 
directly instead of to 'lo', but ipv6 didn't work at all.

>       my @ifaces = grep(/^net(\d+)$/, keys %$conf);
> @@ -203,9 +203,14 @@ sub configdrive2_network {
>   
>   sub configdrive2_metadata {
>       my ($uuid) = @_;
> +	my ($conf, $vmid, $uuid) = @_;
> +	my ($hostname, $fqdn) = get_hostname_fqdn($conf, $vmid);
> +	my $password = $conf->{cipassword};
>       return <<"EOF";
>   {
>        "uuid": "$uuid",
> +	 "hostname": "$hostname",
> +	 "admin_pass": "$password",
>        "network_config": { "content_path": "/content/0000" }
>   }
>   EOF
> @@ -222,7 +227,7 @@ sub generate_configdrive2 {
>   	my $digest_data = $user_data . $network_data;
>   	my $uuid_str = Digest::SHA::sha1_hex($digest_data);
>   
> -	$meta_data = configdrive2_metadata($uuid_str);
> +	$meta_data = configdrive2_metadata($conf, $vmid, $uuid_str);
>       }
>       my $files = {
>   	'/openstack/latest/user_data' => $user_data,
Same as above, haven't gotten around to testing this yet, but of course 
will test it as soon as I get to it.