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

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

[Fabian Ebner]

The only thing remaining from v1 is adding a cluster-wide storage
selector for the cluster-wide backup job editor.

The new approach is to re-use the existing storage selector and add
a config option to enable cluster-wide selection.


Changes from v1:
    * Dropped already applied patches.
    * Extend existing storage selector to also support a cluster view
      instead of adding a new storage selector.


pve-manager:

Fabian Ebner (4):
  ui: storage selector: only check for nodename before loading storage
  ui: storage selector: add new clusterView option
  ui: cluster backup: only restrict storages to node when it's selected
  ui: storage selector: use storage type renderer

 www/manager6/dc/Backup.js            |   4 +-
 www/manager6/form/StorageSelector.js | 109 +++++++++++++++++++++------
 2 files changed, 88 insertions(+), 25 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH v2 manager 1/4] ui: storage selector: only check for nodename before loading storage

[Fabian Ebner]

but allow an empty nodename to be set. This is in preparation for
allowing a cluster view as well, where an empty node name will
indicate that no node is selected.

Since the reloadStorageList function still returns early if there is
no nodename, this should not lead to any issues.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v2.

 www/manager6/form/StorageSelector.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/www/manager6/form/StorageSelector.js b/www/manager6/form/StorageSelector.js
index 631eb378..bb56f159 100644
--- a/www/manager6/form/StorageSelector.js
+++ b/www/manager6/form/StorageSelector.js
@@ -74,7 +74,9 @@ Ext.define('PVE.form.StorageSelector', {
     setNodename: function(nodename) {
 	var me = this;
 
-	if (!nodename || me.nodename === nodename) {
+	nodename = nodename || '';
+
+	if (me.nodename === nodename) {
 	    return;
 	}
 
@@ -103,9 +105,7 @@ Ext.define('PVE.form.StorageSelector', {
 
 	me.callParent();
 
-	if (nodename) {
-	    me.setNodename(nodename);
-	}
+	me.setNodename(nodename);
     },
 }, function() {
     Ext.define('pve-storage-status', {
-- 
2.30.2

[pve-devel] [PATCH v2 manager 2/4] ui: storage selector: add new clusterView option

[Fabian Ebner]

which allows selecting storages available in the whole cluster and
controls whether usage information or node/shared information is
displayed. It is still possible to filter by node and content type.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * Extend existing storage selector to also support a cluster view
      instead of adding a new storage selector.

 www/manager6/form/StorageSelector.js | 98 +++++++++++++++++++++++-----
 1 file changed, 80 insertions(+), 18 deletions(-)

diff --git a/www/manager6/form/StorageSelector.js b/www/manager6/form/StorageSelector.js
index bb56f159..5a394538 100644
--- a/www/manager6/form/StorageSelector.js
+++ b/www/manager6/form/StorageSelector.js
@@ -1,11 +1,19 @@
 Ext.define('PVE.form.StorageSelector', {
     extend: 'Proxmox.form.ComboGrid',
     alias: 'widget.pveStorageSelector',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    cbindData: {
+	clusterView: false,
+    },
 
     allowBlank: false,
     valueField: 'storage',
     displayField: 'storage',
     listConfig: {
+	cbind: {
+	    clusterView: '{clusterView}',
+	},
 	width: 450,
 	columns: [
 	    {
@@ -24,37 +32,87 @@ Ext.define('PVE.form.StorageSelector', {
 		width: 90,
 		dataIndex: 'avail',
 		renderer: Proxmox.Utils.format_size,
+		cbind: {
+		    hidden: '{clusterView}',
+		},
 	    },
 	    {
 		header: gettext('Capacity'),
 		width: 90,
 		dataIndex: 'total',
 		renderer: Proxmox.Utils.format_size,
+		cbind: {
+		    hidden: '{clusterView}',
+		},
+	    },
+	    {
+		header: gettext('Nodes'),
+		width: 120,
+		dataIndex: 'nodes',
+		renderer: (value) => value ? value : '-- ' + gettext('All') + ' --',
+		cbind: {
+		    hidden: '{!clusterView}',
+		},
+	    },
+	    {
+		header: gettext('Shared'),
+		width: 70,
+		dataIndex: 'shared',
+		renderer: Proxmox.Utils.format_boolean,
+		cbind: {
+		    hidden: '{!clusterView}',
+		},
 	    },
 	],
     },
 
     reloadStorageList: function() {
 	let me = this;
-	if (!me.nodename) {
-	    return;
-	}
 
-	let params = {
-	    format: 1,
-	};
-	if (me.storageContent) {
-	    params.content = me.storageContent;
-	}
-	if (me.targetNode) {
-	    params.target = me.targetNode;
-	    params.enabled = 1; // skip disabled storages
+	if (me.clusterView) {
+	    me.getStore().setProxy({
+		type: 'proxmox',
+		url: `/api2/json/storage`,
+	    });
+
+	    // filter here, back-end does not support it currently
+	    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);
+	} else {
+	    if (!me.nodename) {
+		return;
+	    }
+
+	    let params = {
+		format: 1,
+	    };
+	    if (me.storageContent) {
+		params.content = me.storageContent;
+	    }
+	    if (me.targetNode) {
+		params.target = me.targetNode;
+		params.enabled = 1; // skip disabled storages
+	    }
+	    me.store.setProxy({
+		type: 'proxmox',
+		url: `/api2/json/nodes/${me.nodename}/storage`,
+		extraParams: params,
+	    });
 	}
-	me.store.setProxy({
-	    type: 'proxmox',
-	    url: `/api2/json/nodes/${me.nodename}/storage`,
-	    extraParams: params,
-	});
 
 	me.store.load(() => me.validate());
     },
@@ -66,6 +124,10 @@ Ext.define('PVE.form.StorageSelector', {
 	    return;
 	}
 
+	if (me.clusterView) {
+	    throw "setting targetNode with clusterView is not implemented";
+	}
+
 	me.targetNode = targetNode;
 
 	me.reloadStorageList();
@@ -110,7 +172,7 @@ Ext.define('PVE.form.StorageSelector', {
 }, function() {
     Ext.define('pve-storage-status', {
 	extend: 'Ext.data.Model',
-	fields: ['storage', 'active', 'type', 'avail', 'total'],
+	fields: ['storage', 'active', 'type', 'avail', 'total', 'nodes', 'shared'],
 	idProperty: 'storage',
     });
 });
-- 
2.30.2

[pve-devel] [PATCH v2 manager 3/4] ui: cluster backup: only restrict storages to node when it's selected

[Fabian Ebner]

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.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * Use a separate patch for this.
    * Use the clusterView option.

 www/manager6/dc/Backup.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 0293c099..adefc5f4 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -56,7 +56,7 @@ Ext.define('PVE.dc.BackupEdit', {
 
 	let storagesel = Ext.create('PVE.form.StorageSelector', {
 	    fieldLabel: gettext('Storage'),
-	    nodename: 'localhost',
+	    clusterView: true,
 	    storageContent: 'backup',
 	    allowBlank: false,
 	    name: 'storage',
@@ -159,7 +159,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) {
-- 
2.30.2

[pve-devel] [PATCH v2 manager 4/4] ui: storage selector: use storage type renderer

[Fabian Ebner]

and make the column wider, so that "Proxmox Backup Server" fits.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * This was part of the proposed ClusterStorageSelector in v1.
    * Instead of 140 which lead to "Proxmox Backup ...", use 160
      so it fully fits.

 www/manager6/form/StorageSelector.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/www/manager6/form/StorageSelector.js b/www/manager6/form/StorageSelector.js
index 5a394538..6edac042 100644
--- a/www/manager6/form/StorageSelector.js
+++ b/www/manager6/form/StorageSelector.js
@@ -24,8 +24,9 @@ Ext.define('PVE.form.StorageSelector', {
 	    },
 	    {
 		header: gettext('Type'),
-		width: 75,
+		width: 160,
 		dataIndex: 'type',
+		renderer: PVE.Utils.format_storage_type,
 	    },
 	    {
 		header: gettext('Avail'),
-- 
2.30.2

[pve-devel] applied-series-partially: [PATCH-SERIES v2 manager] ui: some backup job improvements

[Thomas Lamprecht]

On 21.09.21 13:22, Fabian Ebner wrote:
> The only thing remaining from v1 is adding a cluster-wide storage
> selector for the cluster-wide backup job editor.
> 
> The new approach is to re-use the existing storage selector and add
> a config option to enable cluster-wide selection.
> 
> 
> Changes from v1:
>     * Dropped already applied patches.
>     * Extend existing storage selector to also support a cluster view
>       instead of adding a new storage selector.
> 
> 
> pve-manager:
> 
> Fabian Ebner (4):
>   ui: storage selector: only check for nodename before loading storage
>   ui: storage selector: add new clusterView option
>   ui: cluster backup: only restrict storages to node when it's selected
>   ui: storage selector: use storage type renderer
> 
>  www/manager6/dc/Backup.js            |   4 +-
>  www/manager6/form/StorageSelector.js | 109 +++++++++++++++++++++------
>  2 files changed, 88 insertions(+), 25 deletions(-)
> 



applied all but the last one, I get where you come from but this is just to much
wasted space most of the time IMO, especially in the non-backup job cases, as there
we can never have a PBS and the other storage type names are all quite a bit shorter..

anyhow, thanks for the other stuff!