[pve-devel] [PATCH qemu-server] cloud-init: allow custom network/user data files via snippets

[Thomas Lamprecht]

Am 2/6/19 um 1:35 PM schrieb David Limbeck:
> Adds the 'cicustom' option to specify either or both network_data and
> user_data options as property strings. Their parameters are files
> in a snippets storage (e.g. local:snippets/network.yaml). If one or both
> are specified they are used instead of their respective generated
> configuration.
> This allows the use of completely custom configurations and is also a
> possible solution for bug #2038 by specifying a custom user_data file
> that contains package_upgrade: false.
> 
> Tested with Ubuntu 18.10
> 
> Signed-off-by: David Limbeck <d.limbeck at proxmox.com>
> ---
>  PVE/API2/Qemu.pm            |  1 +
>  PVE/QemuServer.pm           | 24 ++++++++++++++++++++++++
>  PVE/QemuServer/Cloudinit.pm | 37 +++++++++++++++++++++++++++++++++----
>  3 files changed, 58 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 22f9f6a..49aaa48 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -292,6 +292,7 @@ my $diskoptions = {
>  };
>  
>  my $cloudinitoptions = {
> +    cicustom => 1,
>      cipassword => 1,
>      citype => 1,
>      ciuser => 1,
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 4a903a6..7c39b97 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -623,6 +623,24 @@ EODESCR
>      },
>  };
>  
> +my $cicustom_fmt = {
> +    network_data => {
> +	type => 'string',
> +	optional => 1,
> +	description => 'Specify a custom file containing all network data passed to the VM via cloud-init.',
> +	format => 'pve-volume-id',
> +	format_description => 'volume',
> +    },
> +    user_data => {
> +	type => 'string',
> +	optional => 1,
> +	description => 'Specify a custom file containing all user data passed to the VM via cloud-init.',
> +	format => 'pve-volume-id',
> +	format_description => 'volume',
> +    },
> +};
> +PVE::JSONSchema::register_format('pve-qm-cicustom', $cicustom_fmt);
> +
>  my $confdesc_cloudinit = {
>      citype => {
>  	optional => 1,
> @@ -640,6 +658,12 @@ my $confdesc_cloudinit = {
>  	type => 'string',
>  	description => 'cloud-init: Password to assign the user. Using this is generally not recommended. Use ssh keys instead. Also note that older cloud-init versions do not support hashed passwords.',
>      },
> +    cicustom => {
> +	optional => 1,
> +	type => 'string',
> +	description => 'cloud-init: Specify custom files to replace the automatically generated ones at start.',

"to replace and enhance" ? Could it make sense to merge both?

> +	format => 'pve-qm-cicustom',
> +    },
>      searchdomain => {
>  	optional => 1,
>  	type => 'string',
> diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
> index 5be820c..9f36744 100644
> --- a/PVE/QemuServer/Cloudinit.pm
> +++ b/PVE/QemuServer/Cloudinit.pm
> @@ -208,8 +208,9 @@ EOF
>  sub generate_configdrive2 {
>      my ($conf, $vmid, $drive, $volname, $storeid) = @_;
>  
> -    my $user_data = cloudinit_userdata($conf, $vmid);
> -    my $network_data = configdrive2_network($conf);
> +    my ($user_data, $network_data) = get_custom_cloudinit_files($conf);
> +    $user_data = cloudinit_userdata($conf, $vmid) if !defined($user_data);
> +    $network_data = configdrive2_network($conf) if !defined($network_data);
>  
>      my $digest_data = $user_data . $network_data;
>      my $uuid_str = Digest::SHA::sha1_hex($digest_data);
> @@ -378,8 +379,9 @@ sub nocloud_metadata {
>  sub generate_nocloud {
>      my ($conf, $vmid, $drive, $volname, $storeid) = @_;
>  
> -    my $user_data = cloudinit_userdata($conf, $vmid);
> -    my $network_data = nocloud_network($conf);
> +    my ($user_data, $network_data) = get_custom_cloudinit_files($conf);
> +    $user_data = cloudinit_userdata($conf, $vmid) if !defined($user_data);
> +    $network_data = nocloud_network($conf) if !defined($network_data);

why not extend cloudinit_userdata and nocloud_network so that they merge
an external script into the generated data? if you pull out the repeating
parts (see below) this could be relatively straight forward and a nicer
methond interface, the cloudinit_userdata and nocloud_network get all info
needed ($conf) as parameter, so this could be a bit more transparent,
a hunk above it looks the same.

Merging may not be completely easy, but dooable. Maybe allow to switch it on/off
by a cicustom format boolean?

>  
>      my $digest_data = $user_data . $network_data;
>      my $uuid_str = Digest::SHA::sha1_hex($digest_data);
> @@ -394,6 +396,33 @@ sub generate_nocloud {
>      commit_cloudinit_disk($conf, $vmid, $drive, $volname, $storeid, $files, 'cidata');
>  }
>  
> +sub get_custom_cloudinit_files {
> +    my ($conf) = @_;
> +
> +    my $cicustom = $conf->{cicustom};
> +    my $files = $cicustom ? PVE::JSONSchema::parse_property_string('pve-qm-cicustom', $cicustom) : {};
> +
> +    my $network_data_file = $files->{network_data};
> +    my $user_data_file = $files->{user_data};
> +
> +    my $storage_conf = PVE::Storage::config();
> +
> +    my $network_data;
> +    if ($network_data_file) {
> +	my ($full_path, undef, $type) = PVE::Storage::path($storage_conf, $network_data_file);
> +	die "$network_data_file is not in the snippets directory\n" if $type ne 'snippets';
> +	$network_data = PVE::Tools::file_get_contents($full_path);
> +    }

hmm, this looks like it would fit good into its own submethod?

> +    my $user_data;
> +    if ($user_data_file) {
> +	my ($full_path, undef, $type) = PVE::Storage::path($storage_conf, $user_data_file);
> +	die "$user_data_file is not in the snippets directory\n" if $type ne 'snippets';
> +	$user_data = PVE::Tools::file_get_contents($full_path);
> +    }
> +
> +    return ($user_data, $network_data);
> +}
> +
>  my $cloudinit_methods = {
>      configdrive2 => \&generate_configdrive2,
>      nocloud => \&generate_nocloud,
>