[PATCH qemu-server/pve-manager 0/3] add ci instance-id mode

[PATCH qemu-server/pve-manager 0/3] add ci instance-id mode

[Yuri Konotopov]

This series adds an explicit cloud-init instance-id mode for qemu vms.

Legacy mode (the default one) keeps the current generated id behavior.
Persistent mode stores the effective instance-id and reuses it until the
user changes or resets it. The ui exposes the mode and instance-id
editor while keeping the cloud-init overview limited to the selected
mode.

I hope to see persistent mode become the default in future pve releases.

The first qemu-server patch factors out the preparation of generated
cloud-init files so the new code can reuse it without duplicating the
logic. The second qemu-server patch adds the backend property and api.
The pve-manager patch adds the editor.

Relevant bug: https://bugzilla.proxmox.com/show_bug.cgi?id=1739

Yuri Konotopov (3):
  cloud-init: factor out generated file preparation
  fix #1739: cloud-init: add persistent instance-id mode
  fix #1739: ui: qemu: expose cloud-init instance-id mode

 qemu-server/src/PVE/API2/Qemu.pm                  |  38 +++++++
 qemu-server/src/PVE/QemuServer.pm                 |  54 ++++++++++
 qemu-server/src/PVE/QemuServer/Cloudinit.pm       | 119 +++++++++++++++------
 pve-manager/www/manager6/Makefile                 |   1 +
 pve-manager/www/manager6/qemu/CIInstanceIdEdit.js | 120 ++++++++++++++++++++++
 pve-manager/www/manager6/qemu/CloudInit.js        |  17 +++
 6 files changed, 323 insertions(+), 26 deletions(-)

-- 
2.47.3

[PATCH qemu-server 1/2] cloud-init: factor out generated file preparation

[Yuri Konotopov]

Signed-off-by: Yuri Konotopov <ykonotopov@gnome.org>
---
 src/PVE/QemuServer/Cloudinit.pm | 39 ++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/src/PVE/QemuServer/Cloudinit.pm b/src/PVE/QemuServer/Cloudinit.pm
index c1311da8..67f83d3a 100644
--- a/src/PVE/QemuServer/Cloudinit.pm
+++ b/src/PVE/QemuServer/Cloudinit.pm
@@ -245,24 +245,25 @@ sub configdrive2_metadata {
 EOF
 }
 
+sub configdrive2_network_generator {
+    my ($conf) = @_;
+
+    return PVE::QemuServer::Helpers::windows_version($conf->{ostype})
+        ? \&cloudbase_network_eni
+        : \&configdrive2_network;
+}
+
 sub generate_configdrive2 {
     my ($conf, $vmid, $drive, $volname, $storeid) = @_;
 
-    my ($user_data, $network_data, $meta_data, $vendor_data) = get_custom_cloudinit_files($conf);
+    my ($user_data, $network_data, $meta_data, $vendor_data) =
+        get_cloudinit_files($conf, $vmid, configdrive2_network_generator($conf));
     if (PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
-        $user_data = cloudinit_userdata($conf, $vmid) if !defined($user_data);
-        $network_data = cloudbase_network_eni($conf) if !defined($network_data);
-        $vendor_data = '' if !defined($vendor_data);
-
         if (!defined($meta_data)) {
             my $instance_id = cloudbase_gen_instance_id($user_data, $network_data);
             $meta_data = cloudbase_configdrive2_metadata($instance_id, $conf);
         }
     } else {
-        $user_data = cloudinit_userdata($conf, $vmid) if !defined($user_data);
-        $network_data = configdrive2_network($conf) if !defined($network_data);
-        $vendor_data = '' if !defined($vendor_data);
-
         if (!defined($meta_data)) {
             $meta_data = configdrive2_gen_metadata($user_data, $network_data);
         }
@@ -592,10 +593,8 @@ sub nocloud_gen_metadata {
 sub generate_nocloud {
     my ($conf, $vmid, $drive, $volname, $storeid) = @_;
 
-    my ($user_data, $network_data, $meta_data, $vendor_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);
-    $vendor_data = '' if !defined($vendor_data);
+    my ($user_data, $network_data, $meta_data, $vendor_data) =
+        get_cloudinit_files($conf, $vmid, \&nocloud_network);
 
     if (!defined($meta_data)) {
         $meta_data = nocloud_gen_metadata($user_data, $network_data);
@@ -616,6 +615,20 @@ sub generate_nocloud {
     commit_cloudinit_disk($conf, $vmid, $drive, $volname, $storeid, $files, 'cidata');
 }
 
+sub get_cloudinit_files {
+    my ($conf, $vmid, $network_generator) = @_;
+
+    my ($user_data, $network_data, $meta_data, $vendor_data);
+    if ($conf->{cicustom}) {
+        ($user_data, $network_data, $meta_data, $vendor_data) = get_custom_cloudinit_files($conf);
+    }
+    $user_data = cloudinit_userdata($conf, $vmid) if !defined($user_data);
+    $network_data = $network_generator->($conf) if !defined($network_data);
+    $vendor_data = '' if !defined($vendor_data);
+
+    return ($user_data, $network_data, $meta_data, $vendor_data);
+}
+
 sub get_custom_cloudinit_files {
     my ($conf) = @_;
 
-- 
2.47.3

[PATCH qemu-server 2/2] fix #1739: cloud-init: add persistent instance-id mode

[Yuri Konotopov]

Signed-off-by: Yuri Konotopov <ykonotopov@gnome.org>
---
 src/PVE/API2/Qemu.pm            | 38 ++++++++++++++++
 src/PVE/QemuServer.pm           | 54 ++++++++++++++++++++++
 src/PVE/QemuServer/Cloudinit.pm | 80 +++++++++++++++++++++++++++------
 3 files changed, 159 insertions(+), 13 deletions(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index dadc06b0..102b8d24 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -826,6 +826,7 @@ my $diskoptions = {
 
 my $cloudinitoptions = {
     cicustom => 1,
+    ciinstanceid => 1,
     cipassword => 1,
     citype => 1,
     ciuser => 1,
@@ -4544,6 +4545,7 @@ __PACKAGE__->register_method({
             if ($newconf->{vmgenid}) {
                 $newconf->{vmgenid} = PVE::QemuServer::generate_uuid();
             }
+            PVE::QemuServer::reset_ciinstanceid($newconf);
 
             delete $newconf->{template};
 
@@ -6509,6 +6511,42 @@ __PACKAGE__->register_method({
     },
 });
 
+__PACKAGE__->register_method({
+    name => 'cloudinit_instance_id',
+    path => '{vmid}/cloudinit/instance-id',
+    method => 'GET',
+    proxyto => 'node',
+    description => "Get the effective cloud-init instance-id.",
+    permissions => {
+        check => ['perm', '/vms/{vmid}', ['VM.Audit']],
+    },
+    parameters => {
+        additionalProperties => 0,
+        properties => {
+            node => get_standard_option('pve-node'),
+            vmid =>
+                get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
+        },
+    },
+    returns => {
+        type => 'object',
+        properties => {
+            id => {
+                type => 'string',
+                optional => 1,
+            },
+        },
+    },
+    code => sub {
+        my ($param) = @_;
+
+        my $conf = PVE::QemuConfig->load_current_config($param->{vmid});
+        my $id = PVE::QemuServer::Cloudinit::get_cloudinit_instance_id($conf, $param->{vmid});
+
+        return defined($id) ? { id => $id } : {};
+    },
+});
+
 __PACKAGE__->register_method({
     name => 'cloudinit_generated_config_dump',
     path => '{vmid}/cloudinit/dump',
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 2a469fff..c569f437 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -774,6 +774,25 @@ my $cicustom_fmt = {
 };
 PVE::JSONSchema::register_format('pve-qm-cicustom', $cicustom_fmt);
 
+my $ciinstanceid_fmt = {
+    mode => {
+        type => 'string',
+        enum => ['legacy', 'persistent'],
+        description => 'Controls how the cloud-init instance-id is generated.',
+        default => 'legacy',
+        optional => 1,
+    },
+    id => {
+        type => 'string',
+        pattern => '[A-Za-z0-9][A-Za-z0-9_.:-]{0,63}',
+        maxLength => 64,
+        format_description => 'instance-id',
+        description => 'Use this cloud-init instance-id in persistent mode.',
+        optional => 1,
+    },
+};
+PVE::JSONSchema::register_format('pve-qm-ciinstanceid', $ciinstanceid_fmt);
+
 # any new option might need to be added to $cloudinitoptions in PVE::API2::Qemu
 my $confdesc_cloudinit = {
     citype => {
@@ -805,6 +824,12 @@ my $confdesc_cloudinit = {
         description => 'cloud-init: do an automatic package upgrade after the first boot.',
         default => 1,
     },
+    ciinstanceid => {
+        optional => 1,
+        type => 'string',
+        description => 'cloud-init: Configure how the instance-id is generated.',
+        format => 'pve-qm-ciinstanceid',
+    },
     cicustom => {
         optional => 1,
         type => 'string',
@@ -1656,6 +1681,35 @@ sub parse_vga {
     return $res;
 }
 
+sub parse_ciinstanceid {
+    my ($value) = @_;
+
+    return {} if !$value;
+
+    my $res = eval { parse_property_string($ciinstanceid_fmt, $value) };
+    warn $@ if $@;
+    return $res // {};
+}
+
+sub print_ciinstanceid {
+    my ($ciinstanceid) = @_;
+
+    my $res = {%$ciinstanceid};
+    $res->{mode} = 'legacy' if !defined($res->{mode});
+
+    return PVE::JSONSchema::print_property_string($res, $ciinstanceid_fmt);
+}
+
+sub reset_ciinstanceid {
+    my ($conf) = @_;
+
+    my $ciinstanceid = parse_ciinstanceid($conf->{ciinstanceid});
+    return if ($ciinstanceid->{mode} // 'legacy') ne 'persistent';
+
+    delete $ciinstanceid->{id};
+    $conf->{ciinstanceid} = print_ciinstanceid($ciinstanceid);
+}
+
 sub qemu_created_version_fixups {
     my ($conf, $forcemachine, $kvmver) = @_;
 
diff --git a/src/PVE/QemuServer/Cloudinit.pm b/src/PVE/QemuServer/Cloudinit.pm
index 67f83d3a..587f7f3f 100644
--- a/src/PVE/QemuServer/Cloudinit.pm
+++ b/src/PVE/QemuServer/Cloudinit.pm
@@ -229,9 +229,9 @@ sub configdrive2_network {
 }
 
 sub configdrive2_gen_metadata {
-    my ($user, $network) = @_;
+    my ($conf, $vmid, $user, $network) = @_;
 
-    my $uuid_str = Digest::SHA::sha1_hex($user . $network);
+    my $uuid_str = get_cloudinit_instance_id_from_content($conf, $user, $network);
     return configdrive2_metadata($uuid_str);
 }
 
@@ -260,12 +260,12 @@ sub generate_configdrive2 {
         get_cloudinit_files($conf, $vmid, configdrive2_network_generator($conf));
     if (PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
         if (!defined($meta_data)) {
-            my $instance_id = cloudbase_gen_instance_id($user_data, $network_data);
+            my $instance_id = cloudbase_gen_instance_id($conf, $vmid, $user_data, $network_data);
             $meta_data = cloudbase_configdrive2_metadata($instance_id, $conf);
         }
     } else {
         if (!defined($meta_data)) {
-            $meta_data = configdrive2_gen_metadata($user_data, $network_data);
+            $meta_data = configdrive2_gen_metadata($conf, $vmid, $user_data, $network_data);
         }
     }
 
@@ -355,10 +355,9 @@ sub cloudbase_configdrive2_metadata {
 }
 
 sub cloudbase_gen_instance_id {
-    my ($user, $network) = @_;
+    my ($conf, $vmid, $user, $network) = @_;
 
-    my $uuid_str = Digest::SHA::sha1_hex($user . $network);
-    return $uuid_str;
+    return get_cloudinit_instance_id_from_content($conf, $user, $network);
 }
 
 sub generate_opennebula {
@@ -584,12 +583,57 @@ sub nocloud_metadata {
 }
 
 sub nocloud_gen_metadata {
-    my ($user, $network) = @_;
+    my ($conf, $vmid, $user, $network) = @_;
 
-    my $uuid_str = Digest::SHA::sha1_hex($user . $network);
+    my $uuid_str = get_cloudinit_instance_id_from_content($conf, $user, $network);
     return nocloud_metadata($uuid_str);
 }
 
+sub get_cloudinit_instance_id_from_content {
+    my ($conf, $user, $network) = @_;
+
+    my $ciinstanceid = PVE::QemuServer::parse_ciinstanceid($conf->{ciinstanceid});
+    return $ciinstanceid->{id}
+        if ($ciinstanceid->{mode} // 'legacy') eq 'persistent' && $ciinstanceid->{id};
+
+    return Digest::SHA::sha1_hex($user . $network);
+}
+
+sub get_cloudinit_instance_id {
+    my ($conf, $vmid) = @_;
+
+    my $format = get_cloudinit_format($conf);
+    return if $format eq 'opennebula';
+    return if has_custom_cloudinit_metadata($conf);
+
+    my $ciinstanceid = PVE::QemuServer::parse_ciinstanceid($conf->{ciinstanceid});
+    return $ciinstanceid->{id}
+        if ($ciinstanceid->{mode} // 'legacy') eq 'persistent' && $ciinstanceid->{id};
+
+    my $network_generator =
+        $format eq 'nocloud' ? \&nocloud_network : configdrive2_network_generator($conf);
+
+    my ($user, $network, $meta_data) = get_cloudinit_files($conf, $vmid, $network_generator);
+    return if defined($meta_data);
+
+    return get_cloudinit_instance_id_from_content($conf, $user, $network);
+}
+
+sub ensure_cloudinit_instance_id {
+    my ($conf, $vmid) = @_;
+
+    my $ciinstanceid = PVE::QemuServer::parse_ciinstanceid($conf->{ciinstanceid});
+    return if ($ciinstanceid->{mode} // 'legacy') ne 'persistent';
+    return if $ciinstanceid->{id};
+
+    my $instance_id = get_cloudinit_instance_id($conf, $vmid);
+    return if !defined($instance_id);
+
+    $ciinstanceid->{id} = $instance_id;
+    $conf->{ciinstanceid} = PVE::QemuServer::print_ciinstanceid($ciinstanceid);
+    return 1;
+}
+
 sub generate_nocloud {
     my ($conf, $vmid, $drive, $volname, $storeid) = @_;
 
@@ -597,7 +641,7 @@ sub generate_nocloud {
         get_cloudinit_files($conf, $vmid, \&nocloud_network);
 
     if (!defined($meta_data)) {
-        $meta_data = nocloud_gen_metadata($user_data, $network_data);
+        $meta_data = nocloud_gen_metadata($conf, $vmid, $user_data, $network_data);
     }
 
     # we always allocate a 4MiB disk for cloudinit and with the overhead of the ISO
@@ -615,6 +659,15 @@ sub generate_nocloud {
     commit_cloudinit_disk($conf, $vmid, $drive, $volname, $storeid, $files, 'cidata');
 }
 
+sub has_custom_cloudinit_metadata {
+    my ($conf) = @_;
+
+    return if !$conf->{cicustom};
+
+    my $files = PVE::JSONSchema::parse_property_string('pve-qm-cicustom', $conf->{cicustom});
+    return !!$files->{meta};
+}
+
 sub get_cloudinit_files {
     my ($conf, $vmid, $network_generator) = @_;
 
@@ -698,6 +751,7 @@ sub generate_cloudinit_config {
 
     my $format = get_cloudinit_format($conf);
 
+    my $instance_id_changed = ensure_cloudinit_instance_id($conf, $vmid);
     my $has_changes = has_changes($conf);
 
     PVE::QemuConfig->foreach_volume(
@@ -716,7 +770,7 @@ sub generate_cloudinit_config {
         },
     );
 
-    return $has_changes;
+    return $has_changes || $instance_id_changed;
 }
 
 sub apply_cloudinit_config {
@@ -751,10 +805,10 @@ sub dump_cloudinit_config {
         my $user = cloudinit_userdata($conf, $vmid);
         if ($format eq 'nocloud') {
             my $network = nocloud_network($conf);
-            return nocloud_gen_metadata($user, $network);
+            return nocloud_gen_metadata($conf, $vmid, $user, $network);
         } else {
             my $network = configdrive2_network($conf);
-            return configdrive2_gen_metadata($user, $network);
+            return configdrive2_gen_metadata($conf, $vmid, $user, $network);
         }
     }
 }
-- 
2.47.3

[PATCH pve-manager] fix #1739: ui: qemu: expose cloud-init instance-id mode

[Yuri Konotopov]

Signed-off-by: Yuri Konotopov <ykonotopov@gnome.org>
---
 www/manager6/Makefile                 |   1 +
 www/manager6/qemu/CIInstanceIdEdit.js | 120 ++++++++++++++++++++++++++
 www/manager6/qemu/CloudInit.js        |  17 ++++
 3 files changed, 138 insertions(+)
 create mode 100644 www/manager6/qemu/CIInstanceIdEdit.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index b506849d..510c5502 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -248,6 +248,7 @@ JSSRC= 							\
 	qemu/BootOrderEdit.js				\
 	qemu/CDEdit.js					\
 	qemu/CIDriveEdit.js				\
+	qemu/CIInstanceIdEdit.js				\
 	qemu/CloudInit.js				\
 	qemu/CmdMenu.js					\
 	qemu/Config.js					\
diff --git a/www/manager6/qemu/CIInstanceIdEdit.js b/www/manager6/qemu/CIInstanceIdEdit.js
new file mode 100644
index 00000000..71d42f93
--- /dev/null
+++ b/www/manager6/qemu/CIInstanceIdEdit.js
@@ -0,0 +1,120 @@
+Ext.define('PVE.qemu.CIInstanceIdInputPanel', {
+    extend: 'Proxmox.panel.InputPanel',
+    alias: 'widget.pveCIInstanceIdInputPanel',
+
+    effectiveInstanceId: undefined,
+
+    onGetValues: function (values) {
+        let data = {
+            mode: values.mode,
+            id: values.mode === 'persistent' ? values.id : undefined,
+        };
+
+        if (!data.id) {
+            delete data.id;
+        }
+
+        return {
+            ciinstanceid: PVE.Parser.printPropertyString(data),
+        };
+    },
+
+    setCiInstanceId: function (value, effectiveInstanceId) {
+        let data = value ? PVE.Parser.parsePropertyString(value) : {};
+        data.mode = data.mode || 'legacy';
+        data.id = data.id || effectiveInstanceId || '';
+
+        this.effectiveInstanceId = effectiveInstanceId;
+        this.setValues(data);
+        this.down('field[name=id]').setDisabled(data.mode === 'legacy');
+    },
+
+    items: [
+        {
+            xtype: 'proxmoxKVComboBox',
+            name: 'mode',
+            fieldLabel: gettext('Mode'),
+            comboItems: [
+                ['legacy', gettext('Legacy')],
+                ['persistent', gettext('Persistent')],
+            ],
+            value: 'legacy',
+            allowBlank: false,
+            listeners: {
+                change: function (field, value) {
+                    let ipanel = field.up('inputpanel');
+                    let idField = ipanel.down('field[name=id]');
+                    idField.setDisabled(value === 'legacy');
+                    if (value === 'persistent' && !idField.getValue()) {
+                        idField.setValue(ipanel.effectiveInstanceId || '');
+                    }
+                },
+            },
+        },
+        {
+            xtype: 'proxmoxtextfield',
+            name: 'id',
+            fieldLabel: gettext('Instance ID'),
+            allowBlank: true,
+            disabled: true,
+            validator: function (value) {
+                if (!value) {
+                    return true;
+                }
+                return /^[A-Za-z0-9][A-Za-z0-9_.:-]{0,63}$/.test(value)
+                    ? true
+                    : gettext('Invalid Instance ID');
+            },
+        },
+    ],
+});
+
+Ext.define('PVE.qemu.CIInstanceIdEdit', {
+    extend: 'Proxmox.window.Edit',
+
+    initComponent: function () {
+        let me = this;
+
+        let ipanel = Ext.create('PVE.qemu.CIInstanceIdInputPanel');
+        let configValue;
+        let effectiveInstanceId;
+        let loadedConfig = false;
+        let loadedInstanceId = false;
+
+        let applyValues = function () {
+            if (loadedConfig && loadedInstanceId) {
+                ipanel.setCiInstanceId(configValue, effectiveInstanceId);
+            }
+        };
+
+        Ext.apply(me, {
+            subject: gettext('Instance ID'),
+            width: 450,
+            items: [ipanel],
+        });
+
+        me.callParent();
+
+        me.load({
+            success: function (response) {
+                configValue = response.result.data.ciinstanceid;
+                loadedConfig = true;
+                applyValues();
+            },
+        });
+
+        Proxmox.Utils.API2Request({
+            url: me.baseurl + '/cloudinit/instance-id',
+            method: 'GET',
+            success: function (response) {
+                effectiveInstanceId = response.result.data.id;
+                loadedInstanceId = true;
+                applyValues();
+            },
+            failure: function () {
+                loadedInstanceId = true;
+                applyValues();
+            },
+        });
+    },
+});
diff --git a/www/manager6/qemu/CloudInit.js b/www/manager6/qemu/CloudInit.js
index b0304442..c8d2413e 100644
--- a/www/manager6/qemu/CloudInit.js
+++ b/www/manager6/qemu/CloudInit.js
@@ -166,6 +166,7 @@ Ext.define('PVE.qemu.CloudInit', {
         me.baseurl = '/api2/extjs/nodes/' + nodename + '/qemu/' + vmid;
         me.url = me.baseurl + '/pending';
         me.editorConfig.url = me.baseurl + '/config';
+        me.editorConfig.baseurl = me.baseurl;
         me.editorConfig.pveSelNode = me.pveSelNode;
 
         let caps_ci = caps.vms['VM.Config.Cloudinit'] || caps.vms['VM.Config.Network'];
@@ -287,6 +288,22 @@ Ext.define('PVE.qemu.CloudInit', {
                     },
                 },
             },
+            ciinstanceid: {
+                header: gettext('Instance ID'),
+                iconCls: 'fa fa-id-card-o',
+                renderer: function (value) {
+                    let data = value ? PVE.Parser.parsePropertyString(value) : {};
+                    let mode = data.mode || 'legacy';
+                    let modes = {
+                        legacy: gettext('Legacy'),
+                        persistent: gettext('Persistent'),
+                    };
+
+                    return modes[mode];
+                },
+                defaultValue: 'mode=legacy',
+                editor: caps_ci ? 'PVE.qemu.CIInstanceIdEdit' : undefined,
+            },
         };
         var i;
         var ipconfig_renderer = function (value, md, record, ri, ci, store, pending) {
-- 
2.47.3