[pve-devel] [PATCH-SERIES manager] ui: some backup job improvements

[pve-devel] [PATCH-SERIES manager] ui: some backup job improvements

[Fabian Ebner]

The first patch fixes the 'Run now' button for jobs with existing
retention settings.

The next three patches prepare and implement handling of retention
settings for backup jobs in the UI, fixing bug #1803.

Patch #5 also adds the information about retention to the 'Job Detail'
view.

Finally, patch #6 is not related to retention, but tries to improve the
storage selection. Previously, the current node's storages were
displayed, but we're in a cluster context here.

Fabian Ebner (6):
  ui: cluster backup: fix running backup with prune settings
  ui: factor out input panel for editing prune settings
  ui: prune edit: prepare for being re-used for backup jobs
  fix #1803: ui: cluster backup: handle job-specific retention options
  ui: backup job detail: also show retention settings
  ui: cluster backup: use cluster-wide storage selector

 www/manager6/Makefile                       |   2 +
 www/manager6/dc/Backup.js                   | 170 +++++++++++++++++++-
 www/manager6/form/ClusterStorageSelector.js |  96 +++++++++++
 www/manager6/panel/EditPruneInputPanel.js   |  99 ++++++++++++
 www/manager6/storage/Base.js                | 100 +-----------
 5 files changed, 367 insertions(+), 100 deletions(-)
 create mode 100644 www/manager6/form/ClusterStorageSelector.js
 create mode 100644 www/manager6/panel/EditPruneInputPanel.js

-- 
2.30.2

[pve-devel] [PATCH manager 1/6] ui: cluster backup: fix running backup with prune settings

[Fabian Ebner]

The API expects a property string for retention settings, so make sure
that's what's passed in.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/dc/Backup.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index f06c25ff..1b7b1aa1 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -880,6 +880,10 @@ Ext.define('PVE.dc.BackupView', {
 	    delete job.node;
 	    job.all = job.all === true ? 1 : 0;
 
+	    if (job['prune-backups']) {
+		job['prune-backups'] = PVE.Parser.printPropertyString(job['prune-backups']);
+	    }
+
 	    let allNodes = PVE.data.ResourceStore.getNodes();
 	    let nodes = allNodes.filter(node => node.status === 'online').map(node => node.node);
 	    let errors = [];
-- 
2.30.2

[pve-devel] applied: [PATCH manager 1/6] ui: cluster backup: fix running backup with prune settings

[Thomas Lamprecht]

On 06.09.21 13:32, Fabian Ebner wrote:
> The API expects a property string for retention settings, so make sure
> that's what's passed in.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  www/manager6/dc/Backup.js | 4 ++++
>  1 file changed, 4 insertions(+)
> 
>

applied, thanks!

[pve-devel] [PATCH manager 2/6] ui: factor out input panel for editing prune settings

[Fabian Ebner]

To be re-used for configuring job-specific retention options for
backups.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/Makefile                     |  1 +
 www/manager6/panel/EditPruneInputPanel.js | 97 +++++++++++++++++++++++
 www/manager6/storage/Base.js              | 96 +---------------------
 3 files changed, 99 insertions(+), 95 deletions(-)
 create mode 100644 www/manager6/panel/EditPruneInputPanel.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 75d355a5..3f81d9c4 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -79,6 +79,7 @@ JSSRC= 							\
 	grid/Replication.js				\
 	grid/ResourceGrid.js				\
 	panel/ConfigPanel.js				\
+	panel/EditPruneInputPanel.js			\
 	panel/HealthWidget.js				\
 	panel/IPSet.js					\
 	panel/NotesView.js				\
diff --git a/www/manager6/panel/EditPruneInputPanel.js b/www/manager6/panel/EditPruneInputPanel.js
new file mode 100644
index 00000000..d58db208
--- /dev/null
+++ b/www/manager6/panel/EditPruneInputPanel.js
@@ -0,0 +1,97 @@
+/*
+ * Input panel for prune settings with a keep-all option intended to be used as
+ * part of an edit/create window.
+ */
+Ext.define('PVE.panel.EditPruneInputPanel', {
+    extend: 'Proxmox.panel.PruneInputPanel',
+    xtype: 'pveEditPruneInputPanel',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    onlineHelp: 'vzdump_retention',
+
+    onGetValues: function(formValues) {
+	if (this.needMask) { // isMasked() may not yet be true if not rendered once
+	    return {};
+	} else if (this.isCreate && !this.rendered) {
+	    return { 'prune-backups': 'keep-all=1' };
+	}
+	delete formValues.delete;
+	let retention = PVE.Parser.printPropertyString(formValues);
+	if (retention === '') {
+	    if (this.isCreate) {
+		return {};
+	    }
+	    // always delete old 'maxfiles' on edit, we map it to keep-last on window load
+	    return {
+		'delete': ['prune-backups', 'maxfiles'],
+	    };
+	}
+	let options = { 'prune-backups': retention };
+	if (!this.isCreate) {
+	    options.delete = 'maxfiles';
+	}
+	return options;
+    },
+
+    updateComponents: function() {
+	let me = this;
+
+	let keepAll = me.down('proxmoxcheckbox[name=keep-all]').getValue();
+	let anyValue = false;
+	me.query('pmxPruneKeepField').forEach(field => {
+	    anyValue = anyValue || field.getValue() !== null;
+	    field.setDisabled(keepAll);
+	});
+	me.down('component[name=no-keeps-hint]').setHidden(anyValue || keepAll);
+    },
+
+    listeners: {
+	afterrender: function(panel) {
+	    if (panel.needMask) {
+		panel.down('component[name=no-keeps-hint]').setHtml('');
+		panel.mask(
+		    gettext('Backup content type not available for this storage.'),
+		);
+	    } else if (panel.isCreate) {
+		panel.down('proxmoxcheckbox[name=keep-all]').setValue(true);
+	    }
+	    panel.down('component[name=pbs-hint]').setHidden(!panel.isPBS);
+
+	    panel.query('pmxPruneKeepField').forEach(field => {
+		field.on('change', panel.updateComponents, panel);
+	    });
+	    panel.updateComponents();
+	},
+    },
+
+    columnT: {
+	xtype: 'proxmoxcheckbox',
+	name: 'keep-all',
+	boxLabel: gettext('Keep all backups'),
+	listeners: {
+	    change: function(field, newValue) {
+		let panel = field.up('pveEditPruneInputPanel');
+		panel.updateComponents();
+	    },
+	},
+    },
+
+    columnB: [
+	{
+	    xtype: 'component',
+	    userCls: 'pmx-hint',
+	    name: 'no-keeps-hint',
+	    hidden: true,
+	    padding: '5 1',
+	    html: gettext('Without any keep option, the node\'s vzdump.conf or `keep-all` is used as fallback for backup jobs'),
+	},
+	{
+	    xtype: 'component',
+	    userCls: 'pmx-hint',
+	    name: 'pbs-hint',
+	    hidden: true,
+	    padding: '5 1',
+	    html: gettext("It's preferred to configure backup retention directly on the Proxmox Backup Server."),
+	},
+    ],
+});
diff --git a/www/manager6/storage/Base.js b/www/manager6/storage/Base.js
index ee8b54e8..862c981b 100644
--- a/www/manager6/storage/Base.js
+++ b/www/manager6/storage/Base.js
@@ -55,100 +55,6 @@ Ext.define('PVE.panel.StorageBase', {
     },
 });
 
-Ext.define('PVE.panel.StoragePruneInputPanel', {
-    extend: 'Proxmox.panel.PruneInputPanel',
-    xtype: 'pveStoragePruneInputPanel',
-    mixins: ['Proxmox.Mixin.CBind'],
-
-    onlineHelp: 'vzdump_retention',
-
-    onGetValues: function(formValues) {
-	if (this.needMask) { // isMasked() may not yet be true if not rendered once
-	    return {};
-	} else if (this.isCreate && !this.rendered) {
-	    return { 'prune-backups': 'keep-all=1' };
-	}
-	delete formValues.delete;
-	let retention = PVE.Parser.printPropertyString(formValues);
-	if (retention === '') {
-	    if (this.isCreate) {
-		return {};
-	    }
-	    // always delete old 'maxfiles' on edit, we map it to keep-last on window load
-	    return {
-		'delete': ['prune-backups', 'maxfiles'],
-	    };
-	}
-	let options = { 'prune-backups': retention };
-	if (!this.isCreate) {
-	    options.delete = 'maxfiles';
-	}
-	return options;
-    },
-
-    updateComponents: function() {
-	let me = this;
-
-	let keepAll = me.down('proxmoxcheckbox[name=keep-all]').getValue();
-	let anyValue = false;
-	me.query('pmxPruneKeepField').forEach(field => {
-	    anyValue = anyValue || field.getValue() !== null;
-	    field.setDisabled(keepAll);
-	});
-	me.down('component[name=no-keeps-hint]').setHidden(anyValue || keepAll);
-    },
-
-    listeners: {
-	afterrender: function(panel) {
-	    if (panel.needMask) {
-		panel.down('component[name=no-keeps-hint]').setHtml('');
-		panel.mask(
-		    gettext('Backup content type not available for this storage.'),
-		);
-	    } else if (panel.isCreate) {
-		panel.down('proxmoxcheckbox[name=keep-all]').setValue(true);
-	    }
-	    panel.down('component[name=pbs-hint]').setHidden(!panel.isPBS);
-
-	    panel.query('pmxPruneKeepField').forEach(field => {
-		field.on('change', panel.updateComponents, panel);
-	    });
-	    panel.updateComponents();
-	},
-    },
-
-    columnT: {
-	xtype: 'proxmoxcheckbox',
-	name: 'keep-all',
-	boxLabel: gettext('Keep all backups'),
-	listeners: {
-	    change: function(field, newValue) {
-		let panel = field.up('pveStoragePruneInputPanel');
-		panel.updateComponents();
-	    },
-	},
-    },
-
-    columnB: [
-	{
-	    xtype: 'component',
-	    userCls: 'pmx-hint',
-	    name: 'no-keeps-hint',
-	    hidden: true,
-	    padding: '5 1',
-	    html: gettext('Without any keep option, the node\'s vzdump.conf or `keep-all` is used as fallback for backup jobs'),
-	},
-	{
-	    xtype: 'component',
-	    userCls: 'pmx-hint',
-	    name: 'pbs-hint',
-	    hidden: true,
-	    padding: '5 1',
-	    html: gettext("It's preferred to configure backup retention directly on the Proxmox Backup Server."),
-	},
-    ],
-});
-
 Ext.define('PVE.storage.BaseEdit', {
     extend: 'Proxmox.window.Edit',
 
@@ -191,7 +97,7 @@ Ext.define('PVE.storage.BaseEdit', {
 		items: [
 		    me.ipanel,
 		    {
-			xtype: 'pveStoragePruneInputPanel',
+			xtype: 'pveEditPruneInputPanel',
 			title: gettext('Backup Retention'),
 			isCreate: me.isCreate,
 			isPBS: me.ipanel.isPBS,
-- 
2.30.2

[pve-devel] applied: [PATCH manager 2/6] ui: factor out input panel for editing prune settings

[Thomas Lamprecht]

On 06.09.21 13:32, Fabian Ebner wrote:
> To be re-used for configuring job-specific retention options for
> backups.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  www/manager6/Makefile                     |  1 +
>  www/manager6/panel/EditPruneInputPanel.js | 97 +++++++++++++++++++++++
>  www/manager6/storage/Base.js              | 96 +---------------------
>  3 files changed, 99 insertions(+), 95 deletions(-)
>  create mode 100644 www/manager6/panel/EditPruneInputPanel.js
> 
>

applied, but renamed file to BackupJobPrune.js and widget xtype to pveBackupJobPrunePanel,
as normally each InputPanel is "Edit" and it's not common to encode the parent class type
in the file name either.

any how, thanks!

[pve-devel] [PATCH manager 3/6] ui: prune edit: prepare for being re-used for backup jobs

[Fabian Ebner]

1. Rename the isPBS variable to match the actual purpose. When editing
a backup job, the hint should not be shown, even if the storage is
PBS.

2. Allow specifying the text used for the fallback hint. For backup
jobs, the first fallback is the storage configuration, so the text
needs to be different there.

3. Make using keep-all=1 by default upon creation optional. For backup
jobs, the default should be "use fallback", i.e. no settings.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/panel/EditPruneInputPanel.js | 10 ++++++----
 www/manager6/storage/Base.js              |  4 +++-
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/www/manager6/panel/EditPruneInputPanel.js b/www/manager6/panel/EditPruneInputPanel.js
index d58db208..547daecc 100644
--- a/www/manager6/panel/EditPruneInputPanel.js
+++ b/www/manager6/panel/EditPruneInputPanel.js
@@ -13,7 +13,7 @@ Ext.define('PVE.panel.EditPruneInputPanel', {
 	if (this.needMask) { // isMasked() may not yet be true if not rendered once
 	    return {};
 	} else if (this.isCreate && !this.rendered) {
-	    return { 'prune-backups': 'keep-all=1' };
+	    return this.keepAllDefaultForCreate ? { 'prune-backups': 'keep-all=1' } : {};
 	}
 	delete formValues.delete;
 	let retention = PVE.Parser.printPropertyString(formValues);
@@ -52,10 +52,10 @@ Ext.define('PVE.panel.EditPruneInputPanel', {
 		panel.mask(
 		    gettext('Backup content type not available for this storage.'),
 		);
-	    } else if (panel.isCreate) {
+	    } else if (panel.isCreate && panel.keepAllDefaultForCreate) {
 		panel.down('proxmoxcheckbox[name=keep-all]').setValue(true);
 	    }
-	    panel.down('component[name=pbs-hint]').setHidden(!panel.isPBS);
+	    panel.down('component[name=pbs-hint]').setHidden(!panel.showPBSHint);
 
 	    panel.query('pmxPruneKeepField').forEach(field => {
 		field.on('change', panel.updateComponents, panel);
@@ -83,7 +83,9 @@ Ext.define('PVE.panel.EditPruneInputPanel', {
 	    name: 'no-keeps-hint',
 	    hidden: true,
 	    padding: '5 1',
-	    html: gettext('Without any keep option, the node\'s vzdump.conf or `keep-all` is used as fallback for backup jobs'),
+	    cbind: {
+		html: '{fallbackHintHtml}',
+	    },
 	},
 	{
 	    xtype: 'component',
diff --git a/www/manager6/storage/Base.js b/www/manager6/storage/Base.js
index 862c981b..ffafc87e 100644
--- a/www/manager6/storage/Base.js
+++ b/www/manager6/storage/Base.js
@@ -100,7 +100,9 @@ Ext.define('PVE.storage.BaseEdit', {
 			xtype: 'pveEditPruneInputPanel',
 			title: gettext('Backup Retention'),
 			isCreate: me.isCreate,
-			isPBS: me.ipanel.isPBS,
+			keepAllDefaultForCreate: true,
+			showPBSHint: me.ipanel.isPBS,
+			fallbackHintHtml: gettext('Without any keep option, the node\'s vzdump.conf or `keep-all` is used as fallback for backup jobs'),
 		    },
 		],
 	    },
-- 
2.30.2

[pve-devel] applied: [PATCH manager 3/6] ui: prune edit: prepare for being re-used for backup jobs

[Thomas Lamprecht]

On 06.09.21 13:32, Fabian Ebner wrote:
> 1. Rename the isPBS variable to match the actual purpose. When editing
> a backup job, the hint should not be shown, even if the storage is
> PBS.
> 
> 2. Allow specifying the text used for the fallback hint. For backup
> jobs, the first fallback is the storage configuration, so the text
> needs to be different there.
> 
> 3. Make using keep-all=1 by default upon creation optional. For backup
> jobs, the default should be "use fallback", i.e. no settings.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  www/manager6/panel/EditPruneInputPanel.js | 10 ++++++----
>  www/manager6/storage/Base.js              |  4 +++-
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
>

applied, thanks!

[pve-devel] [PATCH manager 4/6] fix #1803: ui: cluster backup: handle job-specific retention options

[Fabian Ebner]

This was already possible via API (or manually editing vzdump.cron)
for a long time, but the settings were not visible in the UI at all.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/dc/Backup.js | 46 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 1b7b1aa1..ff52e382 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -346,7 +346,39 @@ Ext.define('PVE.dc.BackupEdit', {
 	    subject: gettext("Backup Job"),
 	    url: url,
 	    method: method,
-	    items: [ipanel, vmgrid],
+	    bodyPadding: 0,
+	    items: [
+		{
+		    xtype: 'tabpanel',
+		    region: 'center',
+		    layout: 'fit',
+		    bodyPadding: 10,
+		    items: [
+			{
+			    xtype: 'container',
+			    title: gettext('General'),
+			    region: 'center',
+			    layout: {
+				type: 'vbox',
+				align: 'stretch',
+			    },
+			    items: [
+				ipanel,
+				vmgrid,
+			    ],
+			},
+			{
+			    xtype: 'pveEditPruneInputPanel',
+			    title: gettext('Retention'),
+			    isCreate: me.isCreate,
+			    keepAllDefaultForCreate: false,
+			    showPBSHint: false,
+			    fallbackHintHtml: gettext('Without any keep option, the storage\'s configuration or node\'s vzdump.conf is used as fallback'),
+			},
+		    ],
+		},
+	    ],
+
 	});
 
 	me.callParent();
@@ -375,6 +407,18 @@ Ext.define('PVE.dc.BackupEdit', {
 			data.selMode = 'include';
 		    }
 
+		    if (data['prune-backups']) {
+			Object.assign(data, data['prune-backups']);
+			delete data['prune-backups'];
+		    } else if (data.maxfiles !== undefined) {
+			if (data.maxfiles > 0) {
+			    data['keep-last'] = data.maxfiles;
+			} else {
+			    data['keep-all'] = 1;
+			}
+			delete data.maxfiles;
+		    }
+
 		    me.setValues(data);
 		},
 	    });
-- 
2.30.2

[pve-devel] applied: [PATCH manager 4/6] fix #1803: ui: cluster backup: handle job-specific retention options

[Thomas Lamprecht]

On 06.09.21 13:32, Fabian Ebner wrote:
> This was already possible via API (or manually editing vzdump.cron)
> for a long time, but the settings were not visible in the UI at all.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  www/manager6/dc/Backup.js | 46 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 
>

applied, thanks!

[pve-devel] [PATCH manager 5/6] ui: backup job detail: also show retention settings

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/dc/Backup.js | 115 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index ff52e382..68f67811 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -589,6 +589,16 @@ Ext.define('PVE.dc.BackupInfo', {
     extend: 'Proxmox.panel.InputPanel',
     alias: 'widget.pveBackupInfo',
 
+    viewModel: {
+	data: {
+	    retentionType: 'none',
+	},
+	formulas: {
+	    hasRetention: (get) => get('retentionType') !== 'none',
+	    retentionKeepAll: (get) => get('retentionType') === 'all',
+	},
+    },
+
     padding: '5 0 5 10',
 
     column1: [
@@ -693,8 +703,81 @@ Ext.define('PVE.dc.BackupInfo', {
 	},
     ],
 
+    columnB: [
+	{
+	    xtype: 'label',
+	    name: 'pruneLabel',
+	    text: gettext('Retention Configuration') + ':',
+	    bind: {
+		hidden: '{!hasRetention}',
+	    },
+	},
+	{
+	    layout: 'hbox',
+	    border: false,
+	    defaults: {
+		border: false,
+		layout: 'anchor',
+		flex: 1,
+	    },
+	    items: [
+		{
+		    padding: '0 10 0 0',
+		    defaults: {
+			labelWidth: 110,
+		    },
+		    items: [{
+			xtype: 'displayfield',
+			name: 'keep-all',
+			fieldLabel: gettext('Keep All'),
+			renderer: Proxmox.Utils.format_boolean,
+			bind: {
+			    hidden: '{!retentionKeepAll}',
+			},
+		    }].concat(
+			[
+			    ['keep-last', gettext('Keep Last')],
+			    ['keep-daily', gettext('Keep Daily')],
+			    ['keep-monthly', gettext('Keep Monthly')],
+			].map(
+			    name => ({
+				xtype: 'displayfield',
+				name: name[0],
+				fieldLabel: name[1],
+				bind: {
+				    hidden: '{!hasRetention || retentionKeepAll}',
+				},
+			    }),
+			),
+		    ),
+		},
+		{
+		    padding: '0 0 0 10',
+		    defaults: {
+			labelWidth: 110,
+		    },
+		    items: [
+			['keep-hourly', gettext('Keep Hourly')],
+			['keep-weekly', gettext('Keep Weekly')],
+			['keep-yearly', gettext('Keep Yearly')],
+		    ].map(
+			name => ({
+			    xtype: 'displayfield',
+			    name: name[0],
+			    fieldLabel: name[1],
+			    bind: {
+				hidden: '{!hasRetention || retentionKeepAll}',
+			    },
+			}),
+		    ),
+		},
+	    ],
+	},
+    ],
+
     setValues: function(values) {
 	var me = this;
+	let vm = me.getViewModel();
 
         Ext.iterate(values, function(fieldId, val) {
 	    let field = me.query('[isFormField][name=' + fieldId + ']')[0];
@@ -703,6 +786,38 @@ Ext.define('PVE.dc.BackupInfo', {
             }
 	});
 
+	if (values['prune-backups'] || values.maxfiles !== undefined) {
+	    const keepNames = [
+		'keep-all',
+		'keep-last',
+		'keep-hourly',
+		'keep-daily',
+		'keep-weekly',
+		'keep-monthly',
+		'keep-yearly',
+	    ];
+
+	    let keepValues;
+	    if (values['prune-backups']) {
+		keepValues = values['prune-backups'];
+	    } else if (values.maxfiles > 0) {
+		keepValues = { 'keep-last': values.maxfiles };
+	    } else {
+		keepValues = { 'keep-all': 1 };
+	    }
+
+	    vm.set('retentionType', keepValues['keep-all'] ? 'all' : 'other');
+
+	    keepNames.forEach(function(name) {
+		let field = me.query('[isFormField][name=' + name + ']')[0];
+		if (field) {
+		    field.setValue(keepValues[name]);
+		}
+	    });
+	} else {
+	    vm.set('retentionType', 'none');
+	}
+
 	// selection Mode depends on the presence/absence of several keys
 	let selModeField = me.query('[isFormField][name=selMode]')[0];
 	let selMode = 'none';
-- 
2.30.2

Re: [pve-devel] [PATCH manager 5/6] ui: backup job detail: also show retention settings

[Thomas Lamprecht]

On 06.09.21 13:32, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  www/manager6/dc/Backup.js | 115 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 115 insertions(+)
> 

IMO now the space is available is not ideally used, we have not much vertical space
available for the actual guest list but I see lots of empty space in that view.

I'll need to try a bit around, maybe increasing columns from two now to three can
already solve that good enough..

[pve-devel] applied: [PATCH manager 5/6] ui: backup job detail: also show retention settings

[Thomas Lamprecht]

On 06.09.21 13:32, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  www/manager6/dc/Backup.js | 115 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 115 insertions(+)
> 
>

applied now, but moved to a three column layout for the keep-X fields + some general
refactoring in that file, thanks!

[pve-devel] [PATCH manager 6/6] ui: cluster backup: use cluster-wide storage selector

[Fabian Ebner]

adapted from the existing storage selector.

Previously, only the storages for the local node would be shown, which
prevented configuring a job for remote nodes when the storage is not
available on the local node.

In contrast to the existing storage selector, no usage information is
displayed. It's not readily available, and while it could be extracted
from the global resource store, that's a bit messy, and in case a
local storage is available on multiple nodes, there are multiple
values to deal with. Instead, show the list of nodes where the storage
is available and whether it is shared, which is relevant when seen
from a cluster perspective.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/Makefile                       |  1 +
 www/manager6/dc/Backup.js                   |  5 +-
 www/manager6/form/ClusterStorageSelector.js | 96 +++++++++++++++++++++
 3 files changed, 99 insertions(+), 3 deletions(-)
 create mode 100644 www/manager6/form/ClusterStorageSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 3f81d9c4..17d30177 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -26,6 +26,7 @@ JSSRC= 							\
 	form/CacheTypeSelector.js			\
 	form/CalendarEvent.js				\
 	form/CephPoolSelector.js			\
+	form/ClusterStorageSelector.js			\
 	form/CompressionSelector.js			\
 	form/ContentTypeSelector.js			\
 	form/ControllerSelector.js			\
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 68f67811..7d4db0f0 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -54,9 +54,8 @@ Ext.define('PVE.dc.BackupEdit', {
 	    },
 	});
 
-	let storagesel = Ext.create('PVE.form.StorageSelector', {
+	let storagesel = Ext.create('PVE.form.ClusterStorageSelector', {
 	    fieldLabel: gettext('Storage'),
-	    nodename: 'localhost',
 	    storageContent: 'backup',
 	    allowBlank: false,
 	    name: 'storage',
@@ -159,7 +158,7 @@ Ext.define('PVE.dc.BackupEdit', {
 	    emptyText: '-- ' + gettext('All') + ' --',
 	    listeners: {
 		change: function(f, value) {
-		    storagesel.setNodename(value || 'localhost');
+		    storagesel.setNodename(value);
 		    let mode = selModeField.getValue();
 		    store.clearFilter();
 		    store.filterBy(function(rec) {
diff --git a/www/manager6/form/ClusterStorageSelector.js b/www/manager6/form/ClusterStorageSelector.js
new file mode 100644
index 00000000..8207549c
--- /dev/null
+++ b/www/manager6/form/ClusterStorageSelector.js
@@ -0,0 +1,96 @@
+Ext.define('PVE.form.ClusterStorageSelector', {
+    extend: 'Proxmox.form.ComboGrid',
+    alias: 'widget.pveClusterStorageSelector',
+
+    allowBlank: false,
+    valueField: 'storage',
+    displayField: 'storage',
+
+    listConfig: {
+	width: 450,
+	columns: [
+	    {
+		header: gettext('Name'),
+		dataIndex: 'storage',
+		hideable: false,
+		flex: 1,
+	    },
+	    {
+		header: gettext('Type'),
+		width: 140,
+		dataIndex: 'type',
+		renderer: PVE.Utils.format_storage_type,
+	    },
+	    {
+		header: gettext('Nodes'),
+		width: 120,
+		dataIndex: 'nodes',
+		renderer: (value) => value ? value : '-- ' + gettext('All') + ' --',
+	    },
+	    {
+		header: gettext('Shared'),
+		width: 70,
+		dataIndex: 'shared',
+		renderer: Proxmox.Utils.format_boolean,
+	    },
+	],
+    },
+
+    store: {
+	xclass: 'Ext.data.Store',
+	model: 'pve-cluster-storage',
+	proxy: {
+	    type: 'proxmox',
+	    url: `/api2/json/storage`,
+	},
+	sorters: [{
+	    property: 'storage',
+	    order: 'DESC',
+	}],
+	autoLoad: true,
+    },
+
+    updateFilters: function() {
+	let me = this;
+
+	let filters = [
+	    (storage) => !storage.data.disable,
+	];
+
+	if (me.storageContent) {
+	    filters.push(
+		(storage) => storage.data.content.split(',').includes(me.storageContent),
+	    );
+	}
+
+	if (me.nodename) {
+	    filters.push(
+		(storage) => !storage.data.nodes || storage.data.nodes.includes(me.nodename),
+	    );
+	}
+
+	me.getStore().clearFilter();
+	me.getStore().setFilters(filters);
+    },
+
+    setNodename: function(nodename) {
+	let me = this;
+
+	me.nodename = nodename;
+	me.updateFilters();
+	me.validate();
+    },
+
+    listeners: {
+	beforerender: function() {
+	    let me = this;
+	    me.updateFilters();
+	},
+    },
+}, function() {
+    Ext.define('pve-cluster-storage', {
+	extend: 'Ext.data.Model',
+	fields: ['storage', 'type', 'nodes', 'shared'],
+	idProperty: 'storage',
+    });
+});
-- 
2.30.2

Re: [pve-devel] [PATCH manager 6/6] ui: cluster backup: use cluster-wide storage selector

[Thomas Lamprecht]

On 06.09.21 13:32, Fabian Ebner wrote:
> adapted from the existing storage selector.
> 
> Previously, only the storages for the local node would be shown, which
> prevented configuring a job for remote nodes when the storage is not
> available on the local node.
> 
> In contrast to the existing storage selector, no usage information is
> displayed. It's not readily available, and while it could be extracted
> from the global resource store, that's a bit messy, and in case a
> local storage is available on multiple nodes, there are multiple
> values to deal with. Instead, show the list of nodes where the storage
> is available and whether it is shared, which is relevant when seen
> from a cluster perspective.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  www/manager6/Makefile                       |  1 +
>  www/manager6/dc/Backup.js                   |  5 +-
>  www/manager6/form/ClusterStorageSelector.js | 96 +++++++++++++++++++++
>  3 files changed, 99 insertions(+), 3 deletions(-)
>  create mode 100644 www/manager6/form/ClusterStorageSelector.js
> 

not to happy with that one in regards to code reuse.

We either need to make this the parent of the node-specific one and hide combo-grid
columns depending on settings, or vice versa (this extends from the single one).
The former seems a bit more intuitive to me from a quick check, but no hard feelings
on that..

Re: [pve-devel] [PATCH manager 6/6] ui: cluster backup: use cluster-wide storage selector

[Fabian Ebner]

Am 08.09.21 um 13:06 schrieb Thomas Lamprecht:
> On 06.09.21 13:32, Fabian Ebner wrote:
>> adapted from the existing storage selector.
>>
>> Previously, only the storages for the local node would be shown, which
>> prevented configuring a job for remote nodes when the storage is not
>> available on the local node.
>>
>> In contrast to the existing storage selector, no usage information is
>> displayed. It's not readily available, and while it could be extracted
>> from the global resource store, that's a bit messy, and in case a
>> local storage is available on multiple nodes, there are multiple
>> values to deal with. Instead, show the list of nodes where the storage
>> is available and whether it is shared, which is relevant when seen
>> from a cluster perspective.
>>
>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>> ---
>>   www/manager6/Makefile                       |  1 +
>>   www/manager6/dc/Backup.js                   |  5 +-
>>   www/manager6/form/ClusterStorageSelector.js | 96 +++++++++++++++++++++
>>   3 files changed, 99 insertions(+), 3 deletions(-)
>>   create mode 100644 www/manager6/form/ClusterStorageSelector.js
>>
> 
> not to happy with that one in regards to code reuse.
> 
> We either need to make this the parent of the node-specific one and hide combo-grid
> columns depending on settings, or vice versa (this extends from the single one).
> The former seems a bit more intuitive to me from a quick check, but no hard feelings
> on that..
> 

Ok, I'll try to do one of those or maybe even use a single class and 
simply switch based on the presence of nodename.