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
off...@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.limb...@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 <mcrem...@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@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
