[pve-devel] [PATCH] Implement support for Cloud-Init vendor data

Mira Limbeck m.limbeck at proxmox.com
Wed Sep 18 13:43:41 CEST 2019


Hi,

Thanks for the clarification. I haven't yet had time to take a closer look.

As this is your first patch please send the signed CLA[1] to 
office at proxmox.com

Some things inline.


[1] 
https://pve.proxmox.com/wiki/Developer_Documentation#Software_License_and_Copyright

On 9/4/19 4:29 PM, Marlin Cremers wrote:
> Op wo 4 sep. 2019 om 15:59 schreef Mira Limbeck <m.limbeck at proxmox.com>:
>> Hi,
>>
>> Thank you for the patch, will take a closer look as soon as possible.
>>
>> Three things:
>>
>> 1) What's your use case that 'vendor-data' is required or preferred
>> instead of 'user-data'?
>
> Vendor data is configuration information that is necessary or provided
> by a 'cloud'
> provider, this information can include particular mirrors for
> repositories, kernel
> options, packages that get installed by default, data to seed the randomness of
> the server etc.
>
> This data is separate from the user data so the 'cloud provider' doesn't have to
> merge their configuration with the user provided user-data. Cloud-Init
> also ensures
> that the user has the ability to override all the options set in the
> vendor data.
>
> As a 'cloud' provider this allows us to provide our own configuration
> while still
> allowing the user to set their own configuration.
>
> I have also moved the options that get set by Proxmox (the hostname, FQDN,
> username, password and SSH keys) which are set in the UI and config file to
> vendor data. This necessary because otherwise when user data is provided for
> the VM it won't get for example, the username and password set in the UI
> which is very confusing. The reason the username and password wouldn't be
> set was because Proxmox currently includes that information in user data instead
> of vendor data.
Thanks for the clarification.
>>
>> 2) Did you test it with configdrive2 (on Linux and Windows)? You change
>> it not only for nocloud, but also for configdrive2.
> That is a very good one, I have only tested this for nocloud, I can
> definitely try
> configdrive2 on Linux to see if that works. Testing Windows however I'm unsure
> about, as I don't have access to any Windows licenses.

You can test it with a Windows Server without a license as well for a 
limited time.

Please also try configdrive2 on Linux.

>
>> 3) To support 'vendor-data' you will have to also add it as a possible
>> option to 'cicustom' in the API (see the variable $cicustom_fmt in
>> PVE/QemuServer.pm).
> Thank you very much I'll look into that, currently I'm not overriding
> the vendor data
> myself so I haven't run into that yet. I'll try and change that.
>
>> And one more thing inline
> About the changes you requested, should I make a entirely new mailing list post
> with a new patch or should reply with a new patch?

Please make a new post, don't reply with the new patch to this one.

The subject prefix should also contain the repository you are targeting, 
in this case: PATCH qemu-server

As this will be the 2nd version of the patch, please also add v2 as version.

See 
https://pve.proxmox.com/wiki/Developer_Documentation#Preparing_Patches 
for more detailed instructions.

>> On 9/1/19 8:22 PM, Marlin Cremers wrote:
>>> The current implementation of Cloud-Init uses user data to set the
>>> hostname, username, password and SSH keys of the virtual machine.
>>> This has affect that overriding the user data also stops the logins
>>> from working using the information provided in the UI. This makes
>> Do you mean that users that changed the password in the VM can't login
>> after a reboot because the original password, set in the cloudinit
>> options (and in return in user-data) override the one set by the user?
>>
> What I meant was what I just explained in your first comment. Since Proxmox
> puts the username etc in user data this won't be in the user data when this is
> changes by the user. Which makes sense but is very confusing since those
> options are still configurable and don't get me wrong, it makes to have those
> be configurable beside the user data. Most 'cloud' providers allow you
> to provide
> SSH keys while still setting user data in their UI. This change
> essentially does that
> but for Proxmox.
>
>>> implementing a proper user data implementation very difficult as
>>> it would require merging the actual user data with the credentials
>>> or avoiding the Proxmox Cloud-Init implementation at all.
>>>
>>> This patch includes support for vendor data, this allows Proxmox
>>> to set the credentials and other settings it needs but still allows
>>> users to set their own user data. The vendor data can also be
>>> overridden when necessary.
>>>
>>> Signed-off-by: Marlin Cremers <mcremers at cloudbear.nl>
>>> ---
>>>    PVE/QemuServer/Cloudinit.pm | 20 +++++++++++++++++---
>>>    1 file changed, 17 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
>>> index f46f7fd..52c089f 100644
>>> --- a/PVE/QemuServer/Cloudinit.pm
>>> +++ b/PVE/QemuServer/Cloudinit.pm
>>> @@ -106,6 +106,10 @@ sub get_dns_conf {
>>>    }
>>>
>>>    sub cloudinit_userdata {
>>> +    return ""
>>> +}
>>> +
>>> +sub cloudinit_vendordata {
>>>        my ($conf, $vmid) = @_;
>>>
>>>        my ($hostname, $fqdn) = get_hostname_fqdn($conf, $vmid);
>>> @@ -216,8 +220,9 @@ EOF
>>>    sub generate_configdrive2 {
>>>        my ($conf, $vmid, $drive, $volname, $storeid) = @_;
>>>
>>> -    my ($user_data, $network_data, $meta_data) =
>>> get_custom_cloudinit_files($conf);
>>> +    my ($user_data, $vendor_data, $network_data, $meta_data) =
>>> get_custom_cloudinit_files($conf);
>>>        $user_data = cloudinit_userdata($conf, $vmid) if !defined($user_data);
>>> +    $vendor_data = cloudinit_vendordata($conf, $vmid) if
>>> !defined($vendor_data);
>>>        $network_data = configdrive2_network($conf) if !defined($network_data);
>>>
>>>        if (!defined($meta_data)) {
>>> @@ -228,6 +233,7 @@ sub generate_configdrive2 {
>>>        }
>>>        my $files = {
>>>     '/openstack/latest/user_data' => $user_data,
>>> + '/openstack/latest/vendor_data' => $vendor_data,
>>>     '/openstack/content/0000' => $network_data,
>>>     '/openstack/latest/meta_data.json' => $meta_data
>>>        };
>>> @@ -388,8 +394,9 @@ sub nocloud_metadata {
>>>    sub generate_nocloud {
>>>        my ($conf, $vmid, $drive, $volname, $storeid) = @_;
>>>
>>> -    my ($user_data, $network_data, $meta_data) =
>>> get_custom_cloudinit_files($conf);
>>> +    my ($user_data, $vendor_data, $network_data, $meta_data) =
>>> get_custom_cloudinit_files($conf);
>>>        $user_data = cloudinit_userdata($conf, $vmid) if !defined($user_data);
>>> +    $vendor_data = cloudinit_vendordata($conf, $vmid) if
>>> !defined($vendor_data);
>>>        $network_data = nocloud_network($conf) if !defined($network_data);
>>>
>>>        if (!defined($meta_data)) {
>>> @@ -401,6 +408,7 @@ sub generate_nocloud {
>>>
>>>        my $files = {
>>>     '/user-data' => $user_data,
>>> + '/vendor-data' => $vendor_data,
>>>     '/network-config' => $network_data,
>>>     '/meta-data' => $meta_data
>>>        };
>>> @@ -415,6 +423,7 @@ sub get_custom_cloudinit_files {
>>>
>>>        my $network_volid = $files->{network};
>>>        my $user_volid = $files->{user};
>>> +    my $vendor_volid = $files->{vendor};
>>>        my $meta_volid = $files->{meta};
>>>
>>>        my $storage_conf = PVE::Storage::config();
>>> @@ -429,12 +438,17 @@ sub get_custom_cloudinit_files {
>>>     $user_data = read_cloudinit_snippets_file($storage_conf, $user_volid);
>>>        }
>>>
>>> +    my $vendor_data;
>>> +    if ($vendor_volid) {
>>> +        $vendor_data = read_cloudinit_snippets_file($storage_conf,
>>> $vendor_volid);
>>> +    }
>>> +
>>>        my $meta_data;
>>>        if ($meta_volid) {
>>>     $meta_data = read_cloudinit_snippets_file($storage_conf, $meta_volid);
>>>        }
>>>
>>> -    return ($user_data, $network_data, $meta_data);
>>> +    return ($user_data, $vendor_data, $network_data, $meta_data);
>>>    }
>>>
>>>    sub read_cloudinit_snippets_file {
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel at pve.proxmox.com
>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>




More information about the pve-devel mailing list