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

[Thomas Lamprecht]

Am 2/7/19 um 10:38 AM schrieb David Limbeck:> On 2/7/19 10:32 AM, Thomas Lamprecht wrote:
>> Am 2/7/19 um 10:20 AM schrieb David Limbeck:> On 2/7/19 8:23 AM, Thomas Lamprecht wrote:
>>>> 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.
> Wrong number, it's 2068 not 2038, will fix this as well in v2.
>>>>>
>>>>> 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',
>>>>> +    },
>>>>> +};
>>> Maybe rename it to networkdata, userdata and metadata (v2)? Or keep it network_data, user_data, meta_data(v2)?

or drop data postfix? we're already in the cicustom property here, so a:
> cicustom: network=volid,user=volid,meta=volid
could be enough in addition to the properties description...

>>>>> +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?
>>> This would require a yaml parser and we would have to make sure indentation is compatible.
>> We already use CPAN::Meta::YAML from perl-modules in pve-common, so non-issue.
>> A basic syntax check may also be nice for a user, may allow fixing such things
>> earlier.
>>
>>> It's probably a lot of work for little to no benefit.
>> with the parser available, the work amount depends on the capabillity of ci
>> (which I do not have fully in mind, atm, sorry), i.e., do you have much
>> interpendent options in different subtrees (YAML/JSON objects), or is it mostly
>> enough to replace things a the highest level, e.g., if internal represented as
>> hash just overwrite the values of everything below if the keys are the same,
>> because that would be easy.
>>
> There's not that little nesting and at the moment overwriting files allows the use of network config v2 for example. In addition people can set netplan specific configs like 'on-link: true' to get routes working (don't work in network config v1 on ubuntu) for gateways not in the subnet. Of course, with your suggestion of adding an addition parameter to enable/disable merging it might be possible. I will look into it.

OK, thanks for providing a bit background! So, great if you look into it,
but with the additional parameter we can move forward with the simpler overwrite
approach for now, and add the merge stuff later, if it does seems reasonable then.
Maybe it's even good to wait a bit and do it the simple way for now, so we see if
users even want this or if it's not really useful/needed anyway.

>>> Maybe an additional option ('qm generate_cloud_config' or so) would make more sense that generates the files based on our configuration and people can then use it as a base or in a pre-start hook.
>> besides the obvious blocker of additional underscores in (sub) commands ( ;-)
>> ), this could be nice in general as we basically can get this for free...  I'd
>> really use and alternative CLI syntax, though, maybe:
>>
>> qm cloudinit config VMID
>>