public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH manager 0/2] refresh affinity rules on edit to display conflicts
@ 2025-08-01 16:58 Michael Köppl
  2025-08-01 16:58 ` [pve-devel] [PATCH manager 1/2] ui: ha: refresh rules lists on edit to display conflicts immediately Michael Köppl
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Michael Köppl @ 2025-08-01 16:58 UTC (permalink / raw)
  To: pve-devel

The goal of this is to refresh both affinity rules stores to
immediately display any conflicts. Without this change, users have to
refresh the page to see conflicts on resource affinity rules after they
added a node affinity rule that was in conflict (and vice-versa).

The second patch is formatting through `make tidy`.

pve-manager:

Michael Köppl (2):
  ui: ha: refresh rules lists on edit to display conflicts immediately
  run make tidy

 www/manager6/ha/Rules.js | 118 ++++++++++++++++++++++++---------------
 1 file changed, 73 insertions(+), 45 deletions(-)


Summary over all repositories:
  1 files changed, 73 insertions(+), 45 deletions(-)

-- 
Generated by git-murpp 0.8.0


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] [PATCH manager 1/2] ui: ha: refresh rules lists on edit to display conflicts immediately
  2025-08-01 16:58 [pve-devel] [PATCH manager 0/2] refresh affinity rules on edit to display conflicts Michael Köppl
@ 2025-08-01 16:58 ` Michael Köppl
  2025-08-01 16:58 ` [pve-devel] [PATCH manager 2/2] run make tidy Michael Köppl
  2025-08-02 17:31 ` [pve-devel] [PATCH manager 0/2] refresh affinity rules on edit to display conflicts Thomas Lamprecht
  2 siblings, 0 replies; 5+ messages in thread
From: Michael Köppl @ 2025-08-01 16:58 UTC (permalink / raw)
  To: pve-devel

On each add, edit, remove of a rule, refresh both affinity rule stores
to immediately display any conflicts between the rules. Before this
change, users would have to refresh the page to see conflicts on
resource affinity rules after adding node affinity rules and vice-versa.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
Note: I moved the stores from the RulesBaseView outside to the
RulesView, which encapsulates the NodeAffinityRulesView and the
ResourceAffinityRulesView. I did this to make it easier to refresh both
stores when the datachanged event is fired. Open to suggestions, though.

 www/manager6/ha/Rules.js | 143 ++++++++++++++++++++++++---------------
 1 file changed, 87 insertions(+), 56 deletions(-)

diff --git a/www/manager6/ha/Rules.js b/www/manager6/ha/Rules.js
index 1799d25fb..c8d4c1dac 100644
--- a/www/manager6/ha/Rules.js
+++ b/www/manager6/ha/Rules.js
@@ -8,18 +8,9 @@ Ext.define('PVE.ha.RulesBaseView', {
             throw 'no rule type given';
         }
 
-        let store = new Ext.data.Store({
-            model: 'pve-ha-rules',
-            autoLoad: true,
-            filters: [
-                {
-                    property: 'type',
-                    value: me.ruleType,
-                },
-            ],
-        });
-
-        let reloadStore = () => store.load();
+        let reloadStore = () => {
+            me.fireEvent('datachanged', me);
+        };
 
         let sm = Ext.create('Ext.selection.RowModel', {});
 
@@ -63,7 +54,7 @@ Ext.define('PVE.ha.RulesBaseView', {
         });
 
         Ext.apply(me, {
-            store: store,
+            store: me.store,
             selModel: sm,
             viewConfig: {
                 trackOver: false,
@@ -160,49 +151,89 @@ Ext.define(
             align: 'stretch',
         },
 
-        items: [
-            {
-                title: gettext('HA Node Affinity Rules'),
-                xtype: 'pveHANodeAffinityRulesView',
-                flex: 1,
-                border: 0,
-            },
-            {
-                xtype: 'splitter',
-                collapsible: false,
-                performCollapse: false,
-            },
-            {
-                title: gettext('HA Resource Affinity Rules'),
-                xtype: 'pveHAResourceAffinityRulesView',
-                flex: 1,
-                border: 0,
-            },
-        ],
-    },
-    function () {
-        Ext.define('pve-ha-rules', {
-            extend: 'Ext.data.Model',
-            fields: [
-                'rule',
-                'type',
-                'nodes',
-                'digest',
-                'errors',
-                'disable',
-                'comment',
-                'affinity',
-                'resources',
+        initComponent: function () {
+            var me = this;
+
+            Ext.define('pve-ha-rules', {
+                extend: 'Ext.data.Model',
+                fields: [
+                    'rule',
+                    'type',
+                    'nodes',
+                    'digest',
+                    'errors',
+                    'disable',
+                    'comment',
+                    'affinity',
+                    'resources',
+                    {
+                        name: 'strict',
+                        type: 'boolean',
+                    },
+                ],
+                proxy: {
+                    type: 'proxmox',
+                    url: '/api2/json/cluster/ha/rules',
+                },
+                idProperty: 'rule',
+            });
+
+            let nodeAffinityStore = new Ext.data.Store({
+                model: 'pve-ha-rules',
+                autoLoad: true,
+                filters: [
+                    {
+                        property: 'type',
+                        value: 'node-affinity',
+                    },
+                ],
+            });
+
+            let resourceAffinityStore = new Ext.data.Store({
+                model: 'pve-ha-rules',
+                autoLoad: true,
+                filters: [
+                    {
+                        property: 'type',
+                        value: 'resource-affinity',
+                    },
+                ],
+            });
+
+            let reloadStores = () => {
+                nodeAffinityStore.load();
+                resourceAffinityStore.load();
+            };
+
+            me.items = [
                 {
-                    name: 'strict',
-                    type: 'boolean',
+                    title: gettext('HA Node Affinity Rules'),
+                    xtype: 'pveHANodeAffinityRulesView',
+                    flex: 1,
+                    border: 0,
+                    store: nodeAffinityStore,
+                    listeners: {
+                        datachanged: reloadStores,
+                    },
                 },
-            ],
-            proxy: {
-                type: 'proxmox',
-                url: '/api2/json/cluster/ha/rules',
-            },
-            idProperty: 'rule',
-        });
-    },
+                {
+                    xtype: 'splitter',
+                    collapsible: false,
+                    performCollapse: false,
+                },
+                {
+                    title: gettext('HA Resource Affinity Rules'),
+                    xtype: 'pveHAResourceAffinityRulesView',
+                    flex: 1,
+                    border: 0,
+                    store: resourceAffinityStore,
+                    listeners: {
+                        datachanged: reloadStores,
+                    },
+                },
+            ];
+
+            me.callParent();
+        },
+    }
 );
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] [PATCH manager 2/2] run make tidy
  2025-08-01 16:58 [pve-devel] [PATCH manager 0/2] refresh affinity rules on edit to display conflicts Michael Köppl
  2025-08-01 16:58 ` [pve-devel] [PATCH manager 1/2] ui: ha: refresh rules lists on edit to display conflicts immediately Michael Köppl
@ 2025-08-01 16:58 ` Michael Köppl
  2025-08-02 12:51   ` Thomas Lamprecht
  2025-08-02 17:31 ` [pve-devel] [PATCH manager 0/2] refresh affinity rules on edit to display conflicts Thomas Lamprecht
  2 siblings, 1 reply; 5+ messages in thread
From: Michael Köppl @ 2025-08-01 16:58 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 www/manager6/ha/Rules.js | 175 +++++++++++++++++++--------------------
 1 file changed, 86 insertions(+), 89 deletions(-)

diff --git a/www/manager6/ha/Rules.js b/www/manager6/ha/Rules.js
index c8d4c1dac..5fd02b7ba 100644
--- a/www/manager6/ha/Rules.js
+++ b/www/manager6/ha/Rules.js
@@ -138,102 +138,99 @@ Ext.define('PVE.ha.RulesBaseView', {
     },
 });
 
-Ext.define(
-    'PVE.ha.RulesView',
-    {
-        extend: 'Ext.panel.Panel',
-        alias: 'widget.pveHARulesView',
-
-        onlineHelp: 'ha_manager_rules',
-
-        layout: {
-            type: 'vbox',
-            align: 'stretch',
-        },
-
-        initComponent: function () {
-            var me = this;
-
-            Ext.define('pve-ha-rules', {
-                extend: 'Ext.data.Model',
-                fields: [
-                    'rule',
-                    'type',
-                    'nodes',
-                    'digest',
-                    'errors',
-                    'disable',
-                    'comment',
-                    'affinity',
-                    'resources',
-                    {
-                        name: 'strict',
-                        type: 'boolean',
-                    },
-                ],
-                proxy: {
-                    type: 'proxmox',
-                    url: '/api2/json/cluster/ha/rules',
-                },
-                idProperty: 'rule',
-            });
+Ext.define('PVE.ha.RulesView', {
+    extend: 'Ext.panel.Panel',
+    alias: 'widget.pveHARulesView',
 
-            let nodeAffinityStore = new Ext.data.Store({
-                model: 'pve-ha-rules',
-                autoLoad: true,
-                filters: [
-                    {
-                        property: 'type',
-                        value: 'node-affinity',
-                    },
-                ],
-            });
-
-            let resourceAffinityStore = new Ext.data.Store({
-                model: 'pve-ha-rules',
-                autoLoad: true,
-                filters: [
-                    {
-                        property: 'type',
-                        value: 'resource-affinity',
-                    },
-                ],
-            });
+    onlineHelp: 'ha_manager_rules',
 
-            let reloadStores = () => {
-                nodeAffinityStore.load();
-                resourceAffinityStore.load();
-            };
+    layout: {
+        type: 'vbox',
+        align: 'stretch',
+    },
 
-            me.items = [
+    initComponent: function () {
+        var me = this;
+
+        Ext.define('pve-ha-rules', {
+            extend: 'Ext.data.Model',
+            fields: [
+                'rule',
+                'type',
+                'nodes',
+                'digest',
+                'errors',
+                'disable',
+                'comment',
+                'affinity',
+                'resources',
                 {
-                    title: gettext('HA Node Affinity Rules'),
-                    xtype: 'pveHANodeAffinityRulesView',
-                    flex: 1,
-                    border: 0,
-                    store: nodeAffinityStore,
-                    listeners: {
-                        datachanged: reloadStores,
-                    },
+                    name: 'strict',
+                    type: 'boolean',
                 },
+            ],
+            proxy: {
+                type: 'proxmox',
+                url: '/api2/json/cluster/ha/rules',
+            },
+            idProperty: 'rule',
+        });
+
+        let nodeAffinityStore = new Ext.data.Store({
+            model: 'pve-ha-rules',
+            autoLoad: true,
+            filters: [
                 {
-                    xtype: 'splitter',
-                    collapsible: false,
-                    performCollapse: false,
+                    property: 'type',
+                    value: 'node-affinity',
                 },
+            ],
+        });
+
+        let resourceAffinityStore = new Ext.data.Store({
+            model: 'pve-ha-rules',
+            autoLoad: true,
+            filters: [
                 {
-                    title: gettext('HA Resource Affinity Rules'),
-                    xtype: 'pveHAResourceAffinityRulesView',
-                    flex: 1,
-                    border: 0,
-                    store: resourceAffinityStore,
-                    listeners: {
-                        datachanged: reloadStores,
-                    },
+                    property: 'type',
+                    value: 'resource-affinity',
                 },
-            ];
+            ],
+        });
 
-            me.callParent();
-        },
-    }
-);
+        let reloadStores = () => {
+            nodeAffinityStore.load();
+            resourceAffinityStore.load();
+        };
+
+        me.items = [
+            {
+                title: gettext('HA Node Affinity Rules'),
+                xtype: 'pveHANodeAffinityRulesView',
+                flex: 1,
+                border: 0,
+                store: nodeAffinityStore,
+                listeners: {
+                    datachanged: reloadStores,
+                },
+            },
+            {
+                xtype: 'splitter',
+                collapsible: false,
+                performCollapse: false,
+            },
+            {
+                title: gettext('HA Resource Affinity Rules'),
+                xtype: 'pveHAResourceAffinityRulesView',
+                flex: 1,
+                border: 0,
+                store: resourceAffinityStore,
+                listeners: {
+                    datachanged: reloadStores,
+                },
+            },
+        ];
+
+        me.callParent();
+    },
+});
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [pve-devel] [PATCH manager 2/2] run make tidy
  2025-08-01 16:58 ` [pve-devel] [PATCH manager 2/2] run make tidy Michael Köppl
@ 2025-08-02 12:51   ` Thomas Lamprecht
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2025-08-02 12:51 UTC (permalink / raw)
  To: Proxmox VE development discussion, Michael Köppl

Am 01.08.25 um 18:58 schrieb Michael Köppl:
> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
> ---
>  www/manager6/ha/Rules.js | 175 +++++++++++++++++++--------------------
>  1 file changed, 86 insertions(+), 89 deletions(-)
> 

This seems off to me. The master state is clean w.r.t. formatting, so the formatting
fixes here get introduced by your first patch of this series, so rather you should
squash them together to a single cleanly formatted patch.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [pve-devel] [PATCH manager 0/2] refresh affinity rules on edit to display conflicts
  2025-08-01 16:58 [pve-devel] [PATCH manager 0/2] refresh affinity rules on edit to display conflicts Michael Köppl
  2025-08-01 16:58 ` [pve-devel] [PATCH manager 1/2] ui: ha: refresh rules lists on edit to display conflicts immediately Michael Köppl
  2025-08-01 16:58 ` [pve-devel] [PATCH manager 2/2] run make tidy Michael Köppl
@ 2025-08-02 17:31 ` Thomas Lamprecht
  2 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2025-08-02 17:31 UTC (permalink / raw)
  To: Proxmox VE development discussion, Michael Köppl

Am 01.08.25 um 18:58 schrieb Michael Köppl:
> The goal of this is to refresh both affinity rules stores to
> immediately display any conflicts. Without this change, users have to
> refresh the page to see conflicts on resource affinity rules after they
> added a node affinity rule that was in conflict (and vice-versa).


I went a bit of another route[0][1] that has also the advantage of
not loading the same data twice which in turn also avoids not being
potentially out-of-sync due to those twice loads returning a
potentially different state.

Thanks nonetheless for shining light to this issue, that was very
helpful to me and allowed for some more code clean-ups–the original
HA groups UI was a bit dated as it was added in 2015 where we did
not used the more modern capabilities from ExtJS that allow a bit
more declarative coding style by, e.g., encapsulating more logic
to view controllers.

[0]: https://git.proxmox.com/?p=pve-manager.git;a=commitdiff;h=566ec7fdb80887cc59cd5b9946b44a7e747e2ad3
[1]: https://lore.proxmox.com/pve-devel/20250802172619.3692858-2-t.lamprecht@proxmox.com/T/#u


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-08-02 17:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-01 16:58 [pve-devel] [PATCH manager 0/2] refresh affinity rules on edit to display conflicts Michael Köppl
2025-08-01 16:58 ` [pve-devel] [PATCH manager 1/2] ui: ha: refresh rules lists on edit to display conflicts immediately Michael Köppl
2025-08-01 16:58 ` [pve-devel] [PATCH manager 2/2] run make tidy Michael Köppl
2025-08-02 12:51   ` Thomas Lamprecht
2025-08-02 17:31 ` [pve-devel] [PATCH manager 0/2] refresh affinity rules on edit to display conflicts Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal