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

[David Limbeck]

On 2/7/19 10:55 AM, Thomas Lamprecht wrote:
> 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.
I'll go ahead and work on a v2 then with your suggestions but without 
the merging for now.
>>>> 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
>>>