* [pve-devel] [PATCH container/docs/ha-manager/manager/qemu-server 0/7] fix #6613: update HA rules upon resource deletion
@ 2025-09-17 11:53 Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH ha-manager 1/2] fix #6613: update rules containing the resource to be deleted Michael Köppl
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: Michael Köppl @ 2025-09-17 11:53 UTC (permalink / raw)
To: pve-devel
This patch series aims to fix #6613 [0]. Although an implementation was
proposed in the past, it was not applied since it was unclear how to
handle the case where the removed resource is the last resource in a
rule.
The approach in this patch series is the following:
- With the purge param activated (default), the
resource is removed from any rule referencing it.
- Without the purge param, rules are left untouched, making the removal
of resources from rules "opt-out", as was suggested in the previous
implementation [1].
- If the resource was the last resource in the rule *and* the purge
param is set, the rule will also be deleted.
- For guest destruction, the "Purge from job configurations" option will
also set the purge parameter for removing the HA resource
- The purge param is set by default for resource removal in the GUI and
CLI
After some back-and-forth, I settled in this approach as the most
intuitive and transparent. Alternative approaches considered were the
following:
- Updating rules even without the purge flag set, but failing and
showing an error to the user if a rule would be deleted, prompting the
user to enable the purge flag. This approach seems to be to
restrictive and IMO doesn't add any UX value.
- By default not setting the purge flag. This would by default leave
"zombie" rules with non-existent resources. This is IMO intransparent
if it happens by default. Users might be removing resources, not even
knowing they're leaving behind rules that still reference said
resources.
But I'm also asking for input here if someone has a different opinion or
suggestions for alternative approaches.
[0] https://bugzilla.proxmox.com/show_bug.cgi?id=6613
[1] https://lore.proxmox.com/pve-devel/c004b755-ba3c-46cd-b559-ee2c131d97fc@proxmox.com
pve-ha-manager:
Michael Köppl (2):
fix #6613: update rules containing the resource to be deleted
api: add purge parameter for resource deletion
src/PVE/API2/HA/Resources.pm | 10 +++++++++-
src/PVE/HA/Config.pm | 15 ++++++++++++++-
2 files changed, 23 insertions(+), 2 deletions(-)
qemu-server:
Michael Köppl (1):
fix #6613: pass purge param to delete_service_from_config
src/PVE/API2/Qemu.pm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
pve-container:
Michael Köppl (1):
fix #6613: pass purge param to delete_service_from_config
src/PVE/API2/LXC.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
pve-manager:
Michael Köppl (2):
ui: add SafeDestroyResource window
ui: use SafeDestroyResource window for removing resources
www/manager6/Makefile | 1 +
www/manager6/ha/Resources.js | 17 +++++++++---
www/manager6/window/SafeDestroyResource.js | 32 ++++++++++++++++++++++
3 files changed, 46 insertions(+), 4 deletions(-)
create mode 100644 www/manager6/window/SafeDestroyResource.js
pve-docs:
Michael Köppl (1):
add notes about effects of purge flag for resource and guest removal
ha-manager.adoc | 4 ++++
pct.adoc | 3 +++
qm.adoc | 3 +++
3 files changed, 10 insertions(+)
Summary over all repositories:
10 files changed, 83 insertions(+), 8 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] 8+ messages in thread
* [pve-devel] [PATCH ha-manager 1/2] fix #6613: update rules containing the resource to be deleted
2025-09-17 11:53 [pve-devel] [PATCH container/docs/ha-manager/manager/qemu-server 0/7] fix #6613: update HA rules upon resource deletion Michael Köppl
@ 2025-09-17 11:53 ` Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH ha-manager 2/2] api: add purge parameter for resource deletion Michael Köppl
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Michael Köppl @ 2025-09-17 11:53 UTC (permalink / raw)
To: pve-devel
If the purge param is set, the resource that is deleted is also removed
from any rules referencing it. In the case that a resource is removed
that is also the last resource in a rule, the rule is also removed if
the purge option is enabled. This avoids rules remaining in the rules
config if their resources no longer exist.
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
src/PVE/HA/Config.pm | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
index 301c62f..9a7ed0e 100644
--- a/src/PVE/HA/Config.pm
+++ b/src/PVE/HA/Config.pm
@@ -419,17 +419,30 @@ sub get_resource_motion_info {
# graceful, as long as locking + cfs_write works
sub delete_service_from_config {
- my ($sid) = @_;
+ my ($sid, $purge) = @_;
return 1 if !service_is_configured($sid);
my $res;
PVE::HA::Config::lock_ha_domain(
sub {
+
my $conf = read_resources_config();
$res = delete $conf->{ids}->{$sid};
write_resources_config($conf);
+ my $rules = read_rules_config();
+
+ if ($purge) {
+ for my $ruleid (keys $rules->{ids}->%*) {
+ my $rule_resources = $rules->{ids}->{$ruleid}->{resources} // {};
+
+ delete $rule_resources->{$sid};
+ delete $rules->{ids}->{$ruleid} if !%$rule_resources;
+ }
+ }
+
+ write_rules_config($rules);
},
"delete resource failed",
);
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH ha-manager 2/2] api: add purge parameter for resource deletion
2025-09-17 11:53 [pve-devel] [PATCH container/docs/ha-manager/manager/qemu-server 0/7] fix #6613: update HA rules upon resource deletion Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH ha-manager 1/2] fix #6613: update rules containing the resource to be deleted Michael Köppl
@ 2025-09-17 11:53 ` Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH qemu-server 1/1] fix #6613: pass purge param to delete_service_from_config Michael Köppl
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Michael Köppl @ 2025-09-17 11:53 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
src/PVE/API2/HA/Resources.pm | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/PVE/API2/HA/Resources.pm b/src/PVE/API2/HA/Resources.pm
index 894fe90..09fe954 100644
--- a/src/PVE/API2/HA/Resources.pm
+++ b/src/PVE/API2/HA/Resources.pm
@@ -284,6 +284,13 @@ __PACKAGE__->register_method({
'pve-ha-resource-or-vm-id',
{ completion => \&PVE::HA::Tools::complete_sid },
),
+ purge => {
+ type => 'boolean',
+ description =>
+ "Remove this resource from rules that reference it, deleting the rule if this resource is the only resource in the rule",
+ optional => 1,
+ default => 1,
+ },
},
},
returns => { type => 'null' },
@@ -291,12 +298,13 @@ __PACKAGE__->register_method({
my ($param) = @_;
my ($sid, $type, $name) = PVE::HA::Config::parse_sid(extract_param($param, 'sid'));
+ my $purge = extract_param($param, 'purge') // 1;
if (!PVE::HA::Config::service_is_configured($sid)) {
die "cannot delete service '$sid', not HA managed!\n";
}
- PVE::HA::Config::delete_service_from_config($sid);
+ PVE::HA::Config::delete_service_from_config($sid, $purge);
return undef;
},
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH qemu-server 1/1] fix #6613: pass purge param to delete_service_from_config
2025-09-17 11:53 [pve-devel] [PATCH container/docs/ha-manager/manager/qemu-server 0/7] fix #6613: update HA rules upon resource deletion Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH ha-manager 1/2] fix #6613: update rules containing the resource to be deleted Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH ha-manager 2/2] api: add purge parameter for resource deletion Michael Köppl
@ 2025-09-17 11:53 ` Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH container " Michael Köppl
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Michael Köppl @ 2025-09-17 11:53 UTC (permalink / raw)
To: pve-devel
This covers the case where users want to delete a VM that is also a HA
resource. If the purge param is set, the HA resource will also be
removed from the affinity rules referencing the resource. If the
affected rule only contains this one resource, the rule is also deleted.
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
src/PVE/API2/Qemu.pm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 34f615d8..46a6a06d 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -2760,7 +2760,9 @@ __PACKAGE__->register_method({
PVE::VZDump::Plugin::remove_vmid_from_backup_jobs($vmid);
if ($ha_managed) {
- PVE::HA::Config::delete_service_from_config("vm:$vmid");
+ PVE::HA::Config::delete_service_from_config(
+ "vm:$vmid", $param->{purge},
+ );
print "NOTE: removed VM $vmid from HA resource configuration.\n";
}
}
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH container 1/1] fix #6613: pass purge param to delete_service_from_config
2025-09-17 11:53 [pve-devel] [PATCH container/docs/ha-manager/manager/qemu-server 0/7] fix #6613: update HA rules upon resource deletion Michael Köppl
` (2 preceding siblings ...)
2025-09-17 11:53 ` [pve-devel] [PATCH qemu-server 1/1] fix #6613: pass purge param to delete_service_from_config Michael Köppl
@ 2025-09-17 11:53 ` Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH manager 1/2] ui: add SafeDestroyResource window Michael Köppl
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Michael Köppl @ 2025-09-17 11:53 UTC (permalink / raw)
To: pve-devel
This covers the case where users want to delete a CT that is also a HA
resource. If the purge param is set, the HA resource will also be
removed from the affinity rules referencing the resource. If the
affected rule only contains this one resource, the rule is also deleted.
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
src/PVE/API2/LXC.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 37b9d055..dc0ba5b2 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -879,7 +879,7 @@ __PACKAGE__->register_method({
PVE::VZDump::Plugin::remove_vmid_from_backup_jobs($vmid);
if ($ha_managed) {
- PVE::HA::Config::delete_service_from_config("ct:$vmid");
+ PVE::HA::Config::delete_service_from_config("ct:$vmid", $param->{purge});
print "NOTE: removed CT $vmid from HA resource configuration.\n";
}
}
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH manager 1/2] ui: add SafeDestroyResource window
2025-09-17 11:53 [pve-devel] [PATCH container/docs/ha-manager/manager/qemu-server 0/7] fix #6613: update HA rules upon resource deletion Michael Köppl
` (3 preceding siblings ...)
2025-09-17 11:53 ` [pve-devel] [PATCH container " Michael Köppl
@ 2025-09-17 11:53 ` Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH manager 2/2] ui: use SafeDestroyResource window for removing resources Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH docs 1/1] add notes about effects of purge flag for resource and guest removal Michael Köppl
6 siblings, 0 replies; 8+ messages in thread
From: Michael Köppl @ 2025-09-17 11:53 UTC (permalink / raw)
To: pve-devel
Add an extended removal dialog for HA resources, which also allows
setting a purge parameter through the checkbox. By default, the purge
option is enabled, updating rules referencing the resource and deleting
any rules that only have this resource left.
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
www/manager6/Makefile | 1 +
www/manager6/window/SafeDestroyResource.js | 32 ++++++++++++++++++++++
2 files changed, 33 insertions(+)
create mode 100644 www/manager6/window/SafeDestroyResource.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 85f9268d1..a8bea9413 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -127,6 +127,7 @@ JSSRC= \
window/Prune.js \
window/Restore.js \
window/SafeDestroyGuest.js \
+ window/SafeDestroyResource.js \
window/SafeDestroyStorage.js \
window/Settings.js \
window/Snapshot.js \
diff --git a/www/manager6/window/SafeDestroyResource.js b/www/manager6/window/SafeDestroyResource.js
new file mode 100644
index 000000000..988af7969
--- /dev/null
+++ b/www/manager6/window/SafeDestroyResource.js
@@ -0,0 +1,32 @@
+/*
+ * SafeDestroy window with additional checkboxes for removing resources
+ */
+Ext.define('PVE.window.SafeDestroyResource', {
+ extend: 'Proxmox.window.SafeDestroy',
+ alias: 'widget.pveSafeDestroyResource',
+
+ additionalItems: [
+ {
+ xtype: 'proxmoxcheckbox',
+ name: 'purge',
+ reference: 'purgeCheckbox',
+ boxLabel: gettext('Purge resource from referenced HA rules'),
+ checked: true,
+ autoEl: {
+ tag: 'div',
+ 'data-qtip': gettext(
+ 'Also removes resource from HA rules and removes rule if there are no other resources in it',
+ ),
+ },
+ },
+ ],
+
+ getParams: function () {
+ let me = this;
+
+ const purgeCheckbox = me.lookupReference('purgeCheckbox');
+ me.params.purge = purgeCheckbox.checked ? 1 : 0;
+
+ return me.callParent();
+ },
+});
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH manager 2/2] ui: use SafeDestroyResource window for removing resources
2025-09-17 11:53 [pve-devel] [PATCH container/docs/ha-manager/manager/qemu-server 0/7] fix #6613: update HA rules upon resource deletion Michael Köppl
` (4 preceding siblings ...)
2025-09-17 11:53 ` [pve-devel] [PATCH manager 1/2] ui: add SafeDestroyResource window Michael Köppl
@ 2025-09-17 11:53 ` Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH docs 1/1] add notes about effects of purge flag for resource and guest removal Michael Köppl
6 siblings, 0 replies; 8+ messages in thread
From: Michael Köppl @ 2025-09-17 11:53 UTC (permalink / raw)
To: pve-devel
This allows users to additionally choose whether they want to purge
referenced rules that only include the resource that is to be deleted.
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
www/manager6/ha/Resources.js | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/www/manager6/ha/Resources.js b/www/manager6/ha/Resources.js
index 65897bed2..8bac671d5 100644
--- a/www/manager6/ha/Resources.js
+++ b/www/manager6/ha/Resources.js
@@ -74,12 +74,21 @@ Ext.define('PVE.ha.ResourcesView', {
handler: run_editor,
},
{
- xtype: 'proxmoxStdRemoveButton',
+ xtype: 'proxmoxButton',
+ text: gettext('Remove'),
selModel: sm,
- getUrl: function (rec) {
- return `/cluster/ha/resources/${rec.get('sid')}`;
+ itemId: 'removeBtn',
+ handler: function (btn, e, rec) {
+ Ext.create('PVE.window.SafeDestroyResource', {
+ url: `/cluster/ha/resources/${rec.data.sid}`,
+ item: {
+ id: rec.data.sid,
+ formattedIdentifier: rec.data.sid,
+ },
+ taskName: 'Remove resource',
+ apiCallDone: () => me.rstore.load(),
+ }).show();
},
- callback: () => me.rstore.load(),
},
],
columns: [
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH docs 1/1] add notes about effects of purge flag for resource and guest removal
2025-09-17 11:53 [pve-devel] [PATCH container/docs/ha-manager/manager/qemu-server 0/7] fix #6613: update HA rules upon resource deletion Michael Köppl
` (5 preceding siblings ...)
2025-09-17 11:53 ` [pve-devel] [PATCH manager 2/2] ui: use SafeDestroyResource window for removing resources Michael Köppl
@ 2025-09-17 11:53 ` Michael Köppl
6 siblings, 0 replies; 8+ messages in thread
From: Michael Köppl @ 2025-09-17 11:53 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
ha-manager.adoc | 4 ++++
pct.adoc | 3 +++
qm.adoc | 3 +++
3 files changed, 10 insertions(+)
diff --git a/ha-manager.adoc b/ha-manager.adoc
index ea477cc0..5c17f115 100644
--- a/ha-manager.adoc
+++ b/ha-manager.adoc
@@ -220,6 +220,10 @@ the following command:
# ha-manager remove vm:100
----
+By default, this will also remove the resource from any rule that references it
+and will delete rules that only reference this resource. You can override this
+behavior using '--purge 0'.
+
NOTE: This does not start or stop the resource.
But all HA related tasks can be done in the GUI, so there is no need to
diff --git a/pct.adoc b/pct.adoc
index d6146ebe..b3695ac4 100644
--- a/pct.adoc
+++ b/pct.adoc
@@ -1044,6 +1044,9 @@ removes the firewall configuration of the container. You have to activate
'--purge', if you want to additionally remove the container from replication jobs,
backup jobs and HA resource configurations.
+NOTE: Activating purge will also remove the HA resource from any affinity rules
+referencing it and will remove rules that have only this one remaining resource.
+
----
# pct destroy 100 --purge
----
diff --git a/qm.adoc b/qm.adoc
index f798043a..e70064b1 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -2199,6 +2199,9 @@ removes the firewall configuration of the VM. You have to activate
'--purge', if you want to additionally remove the VM from replication jobs,
backup jobs and HA resource configurations.
+NOTE: Activating purge will also remove the HA resource from any affinity rules
+referencing it and will remove rules that have only this one remaining resource.
+
----
# qm destroy 300 --purge
----
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-09-17 11:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-17 11:53 [pve-devel] [PATCH container/docs/ha-manager/manager/qemu-server 0/7] fix #6613: update HA rules upon resource deletion Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH ha-manager 1/2] fix #6613: update rules containing the resource to be deleted Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH ha-manager 2/2] api: add purge parameter for resource deletion Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH qemu-server 1/1] fix #6613: pass purge param to delete_service_from_config Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH container " Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH manager 1/2] ui: add SafeDestroyResource window Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH manager 2/2] ui: use SafeDestroyResource window for removing resources Michael Köppl
2025-09-17 11:53 ` [pve-devel] [PATCH docs 1/1] add notes about effects of purge flag for resource and guest removal Michael Köppl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox