Re: [pve-devel] [PATCH ksm-control-daemon] ksmtuned: fix large number processing
Hi, Am 28/02/2024 um 23:47 schrieb Roland: > any reason why this did not get a response ? (i do not see rejection of > this ,nor did it appear in > https://git.proxmox.com/?p=ksm-control-daemon.git;a=summary ) No reason, but even if this looks pretty straight forward, positive feedback would still help to speed this up – did you test this successfully? Then I could apply it with a Tested-by: name trailer. > > and, while we are at ksmtuned, i think it's is broken, especially when > run on ZFS based installations, as it's totally mis-calculating ram > ressources. > > https://forum.proxmox.com/threads/ksm-is-needlessly-burning-cpu-because-of-using-vzs-and-ignoring-arcsize.142397/ Yeah KSM could definitively do with some more love, lets see if we can allocate some dev time for this. The RSS (which rsz is an alias for) vs. VSS (vsz aliased) looks interesting, and VSS really seems to be the wrong thing to look at to me (albeit without deeper inspection of the matter). FWIW, depending on how the sum is used it might actually make even more sense to use PSS, i.e., the proportional set size which better accounts for shared memory by dividing that part between all its users, as if e.g. 10 QEMU processes have 100 MB of shared code and what not in their RSS, using RSS one would sum up 900 MB to much compared using PSS, but what's the correct one here is then depending on how they result is used. @Stefan, as you checked this out, would you care checking out the VSS vs. RSS vs. PSS matter too? I.e. checking what should make more sense to use and actually testing that out in a somewhat defined workload. The ZFS ARC thing is something else and might be a bit more complicated, so I'd focus first one above at that seems to provide better improvements for less work, or at least with less potential to build an unstable control system. thanks, Thomas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH ksm-control-daemon] ksmtuned: fix large number processing
--- Begin Message ---
hi,
any reason why this did not get a response ? (i do not see rejection of
this ,nor did it appear in
https://git.proxmox.com/?p=ksm-control-daemon.git;a=summary )
and, while we are at ksmtuned, i think it's is broken, especially when
run on ZFS based installations, as it's totally mis-calculating ram
ressources.
https://forum.proxmox.com/threads/ksm-is-needlessly-burning-cpu-because-of-using-vzs-and-ignoring-arcsize.142397/
regards
roland
Am 25.01.24 um 11:56 schrieb Stefan Lendl:
awk internally uses float for every calculation, printing a large float
with awk results in 1.233e+09 format which causes the script to fail afterwards.
Instead I am printing the float without decimals.
Signed-off-by: Stefan Lendl
---
debian/patches/awk-printf.diff | 16
debian/patches/series | 1 +
2 files changed, 17 insertions(+)
create mode 100644 debian/patches/awk-printf.diff
diff --git a/debian/patches/awk-printf.diff b/debian/patches/awk-printf.diff
new file mode 100644
index 000..11a957f
--- /dev/null
+++ b/debian/patches/awk-printf.diff
@@ -0,0 +1,16 @@
+--- ksm-control-scripts/ksmtuned 2024-01-25 11:33:03.485039813 +0100
ksm-control-scripts.new/ksmtuned 2024-01-25 11:37:40.544751316 +0100
+@@ -72,11 +72,11 @@
+ # calculate how much memory is committed to running qemu processes
+ local progname
+ progname=${1:-kvm}
+-ps -C "$progname" -o vsz= | awk '{ sum += $1 }; END { print sum }'
++ps -C "$progname" -o vsz= | awk '{ sum += $1 }; END { printf ("%.0f",
sum) }'
+ }
+
+ free_memory () {
+-awk '/^(MemFree|Buffers|Cached):/ {free += $2}; END {print free}' \
++awk '/^(MemFree|Buffers|Cached):/ {free += $2}; END { printf ("%.0f",
free) }' \
+ /proc/meminfo
+ }
+
diff --git a/debian/patches/series b/debian/patches/series
index 7aaec2c..63aba40 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -2,3 +2,4 @@ init-script.diff
ksmtuned.diff
adjust-ksm-slepp.diff
use-vsz-instead-of-rsz.diff
+awk-printf.diff
--- End Message ---
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH pmg-docs] installation: fix codeblock rendering in zfs performance tips section
Thanks for the catch and fix! applied it!
(nit: pmg-de...@list.proxmox.com is preferred for pmg-docs patches)
On Wed, 28 Feb 2024 19:08:21 +0100
Christoph Heiss wrote:
> That slipped through, asciidoc uses 4 not 3 dashes for that.
>
> Fixes: c8be3f0 ("installation: align zfs performance tip with PVE
> documentation")
> Signed-off-by: Christoph Heiss
> ---
> pmg-installation.adoc | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/pmg-installation.adoc b/pmg-installation.adoc
> index 326fe5b..25d16a7 100644
> --- a/pmg-installation.adoc
> +++ b/pmg-installation.adoc
> @@ -284,9 +284,9 @@ ZFS can use a dedicated drive as write cache, called the
> ZFS Intent Log (ZIL).
> Use a fast drive (SSD) for it. It can be added after installation with the
> following command:
>
>
> +
> # zpool add log
>
> +
>
> Adding the `nomodeset` Kernel Parameter
> ~
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 access-control] fix #5136: ldap: Decode non-ASCII characters in attributes
On Wed, 28 Feb 2024 16:00:48 +0100
Fiona Ebner wrote:
> Am 28.02.24 um 15:41 schrieb Thomas Lamprecht:
> > Am 09/01/2024 um 14:35 schrieb Filip Schauer:
> >> UTF8 decode non-ASCII characters when syncing user attributes, since
> >> those will be encoded later on. Without this fix the attributes were
> >> encoded twice, resulting in cases such as 'ü' turning into 'ü'.
> >>
> >> Signed-off-by: Filip Schauer
> >> ---
> >> Changes since v1:
> >> * Do not try to URI unescape the user attributes, since we do that later
> >> in PVE::AccessControl::parse_user_config anyways.
> >>
> >> src/PVE/Auth/LDAP.pm | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/src/PVE/Auth/LDAP.pm b/src/PVE/Auth/LDAP.pm
> >> index b958f2b..06177db 100755
> >> --- a/src/PVE/Auth/LDAP.pm
> >> +++ b/src/PVE/Auth/LDAP.pm
> >> @@ -301,7 +301,7 @@ sub get_users {
> >>
> >>foreach my $attr (keys %$user_attributes) {
> >>if (my $ours = $ldap_attribute_map->{$attr}) {
> >> - $ret->{$username}->{$ours} = $user_attributes->{$attr}->[0];
> >> + $ret->{$username}->{$ours} = Encode::decode('utf8',
> >> $user_attributes->{$attr}->[0]);
>
> Note: missing use Encode; at the beginning of the file.
>
> >>}
> >>}
> >>
> >
> > this would need a rebase, oh, and would be great if the original testers
> > could reconfirm the v2 approach of doing utf-8 decoding only.
> >
>
> Gave it a quick test and fixes issues with special characters for me.
> Don't forget to also use the latest master of pve-cluster, otherwise
> writing the user config will still do the wrong thing [0]! Both are
> needed to fix the issue here. I'm just wondering if we are guaranteed
> that the LDAP server sends UTF-8 encoded data?
sadly (or luckily) not too much experience with validity of LDAP data out
in the wild. Quickly searched online and went through the rfc-chain until
there was not Link to "Obsoleted by" anymore (and then going through all
RFC indexed there [0]:
The (~18 year old) standard indicates that strings used should be UTF-8
encoded:
https://datatracker.ietf.org/doc/html/rfc4511#section-4.1.2
(and pointed out the (by now probably not significant difference between
unicode and ISO10646 - see [1]).
However, probably with any protocol that has been around for 30+ years -
guarantees are hard to come by:
https://datatracker.ietf.org/doc/html/rfc4512#section-7.2
anyways - iiuc we can just skip the syncing of the attribute in this part?
- if we add a warning to the log it sounds ok to me (but I only very
quickly skimmed through what the code does)
[0] https://datatracker.ietf.org/doc/html/rfc4510
[1] https://www.unicode.org/versions/Unicode15.0.0/appC.pdf
>
> [0]:
> https://git.proxmox.com/?p=pve-cluster.git;a=commit;h=2e276ccd9beb2004ddd72396b2a9b72a288771d8
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pmg-docs] installation: fix codeblock rendering in zfs performance tips section
That slipped through, asciidoc uses 4 not 3 dashes for that.
Fixes: c8be3f0 ("installation: align zfs performance tip with PVE
documentation")
Signed-off-by: Christoph Heiss
---
pmg-installation.adoc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pmg-installation.adoc b/pmg-installation.adoc
index 326fe5b..25d16a7 100644
--- a/pmg-installation.adoc
+++ b/pmg-installation.adoc
@@ -284,9 +284,9 @@ ZFS can use a dedicated drive as write cache, called the
ZFS Intent Log (ZIL).
Use a fast drive (SSD) for it. It can be added after installation with the
following command:
+
# zpool add log
+
Adding the `nomodeset` Kernel Parameter
~
--
2.43.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 access-control] fix #5136: ldap: Decode non-ASCII characters in attributes
Am 28.02.24 um 15:41 schrieb Thomas Lamprecht:
> Am 09/01/2024 um 14:35 schrieb Filip Schauer:
>> UTF8 decode non-ASCII characters when syncing user attributes, since
>> those will be encoded later on. Without this fix the attributes were
>> encoded twice, resulting in cases such as 'ü' turning into 'ü'.
>>
>> Signed-off-by: Filip Schauer
>> ---
>> Changes since v1:
>> * Do not try to URI unescape the user attributes, since we do that later
>> in PVE::AccessControl::parse_user_config anyways.
>>
>> src/PVE/Auth/LDAP.pm | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/Auth/LDAP.pm b/src/PVE/Auth/LDAP.pm
>> index b958f2b..06177db 100755
>> --- a/src/PVE/Auth/LDAP.pm
>> +++ b/src/PVE/Auth/LDAP.pm
>> @@ -301,7 +301,7 @@ sub get_users {
>>
>> foreach my $attr (keys %$user_attributes) {
>> if (my $ours = $ldap_attribute_map->{$attr}) {
>> -$ret->{$username}->{$ours} = $user_attributes->{$attr}->[0];
>> +$ret->{$username}->{$ours} = Encode::decode('utf8',
>> $user_attributes->{$attr}->[0]);
Note: missing use Encode; at the beginning of the file.
>> }
>> }
>>
>
> this would need a rebase, oh, and would be great if the original testers
> could reconfirm the v2 approach of doing utf-8 decoding only.
>
Gave it a quick test and fixes issues with special characters for me.
Don't forget to also use the latest master of pve-cluster, otherwise
writing the user config will still do the wrong thing [0]! Both are
needed to fix the issue here. I'm just wondering if we are guaranteed
that the LDAP server sends UTF-8 encoded data?
[0]:
https://git.proxmox.com/?p=pve-cluster.git;a=commit;h=2e276ccd9beb2004ddd72396b2a9b72a288771d8
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied-series: fix #5254: add separate Sys.AccessNetwork privilege
Am 19/02/2024 um 18:14 schrieb Thomas Lamprecht:
> Adds a new Sys.AccessNetwork privilege that can be used to guard API
> endpoints that can do outgoing network requests with (some) user control
> over said requests, like e.g. the "download URL to storage" one.
>
> ## Backstory:
>
> This stems from an user request [0] w.r.t. the "download image through
> and URL directly to a storage" functionality and their use case of that
> through automation while wanting to adhere to the principle of least
> privilege.
>
> Because before this series the access to the required endpoints was
> guarded by the more powerful Sys.Modify and Sys.Audit privilege
> requirement on the / root ACL object path.
> So, if anybody wants to set up an API token so that automation can
> handle image downloads they'd need to give that API token very powerful
> permissions to make it work.
>
> A more specialized privilege seems warranted now, so add the
> Sys.AccessNetwork one and adapt the /nodes/{node}/query-url-metadata and
> the related /nodes/{node}/storage/{storage}/download-url API endpoints
> for now.
>
> ## Testing:
>
> Tested by creating a new custom role with the privileges
> `Datastore.Audit,Datastore.AllocateTemplate,Sys.AccessNetwork`, then
> created a user that gets a permission with above role for a specific
> node and a storage and then try querying and downloading an image, with
> and without this patch series applied.
>
> ## Future Work
>
> We could this even re-use for other endpoints, like adding storages that
> are accessed through the network, as that provides a (limited) side
> channel too.
>
> access-control:
>
> Thomas Lamprecht (1):
> add Sys.AccessNetwork privilege
>
> src/PVE/AccessControl.pm | 1 +
> src/test/perm-test1.pl | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> storage:
>
> Thomas Lamprecht (1):
> fix #5254: api: allow usage of download-url with Sys.AccessNetwork
>
> src/PVE/API2/Storage/Status.pm | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> manager:
>
> Thomas Lamprecht (2):
> api: nodes: allow usage of query url metadata with Sys.AccessNetwork
> ui: storage: enable download-url button with Sys.AccessNetwork
> capability
>
> PVE/API2/Nodes.pm | 5 -
> www/manager6/storage/Browser.js | 5 -
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
applied series with Hannes' T-b and Fabians' R-b, thanks for the test/review!
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 access-control] fix #5136: ldap: Decode non-ASCII characters in attributes
Am 09/01/2024 um 14:35 schrieb Filip Schauer:
> UTF8 decode non-ASCII characters when syncing user attributes, since
> those will be encoded later on. Without this fix the attributes were
> encoded twice, resulting in cases such as 'ü' turning into 'ü'.
>
> Signed-off-by: Filip Schauer
> ---
> Changes since v1:
> * Do not try to URI unescape the user attributes, since we do that later
> in PVE::AccessControl::parse_user_config anyways.
>
> src/PVE/Auth/LDAP.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/PVE/Auth/LDAP.pm b/src/PVE/Auth/LDAP.pm
> index b958f2b..06177db 100755
> --- a/src/PVE/Auth/LDAP.pm
> +++ b/src/PVE/Auth/LDAP.pm
> @@ -301,7 +301,7 @@ sub get_users {
>
> foreach my $attr (keys %$user_attributes) {
> if (my $ours = $ldap_attribute_map->{$attr}) {
> - $ret->{$username}->{$ours} = $user_attributes->{$attr}->[0];
> + $ret->{$username}->{$ours} = Encode::decode('utf8',
> $user_attributes->{$attr}->[0]);
> }
> }
>
this would need a rebase, oh, and would be great if the original testers
could reconfirm the v2 approach of doing utf-8 decoding only.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH manager] appliance index: fix precedence in size check for log rotation
Am 28/02/2024 um 13:07 schrieb Fiona Ebner:
> In Perl, > takes precedence over ||, see perldoc perlop, so currently
> the check will trigger with any size.
>
> Fixes: 805cae93 ("appliance index: rotate update log if bigger than 256 KiB")
> Signed-off-by: Fiona Ebner
> ---
> PVE/APLInfo.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
applied, thanks!
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager] appliance index: fix precedence in size check for log rotation
In Perl, > takes precedence over ||, see perldoc perlop, so currently
the check will trigger with any size.
Fixes: 805cae93 ("appliance index: rotate update log if bigger than 256 KiB")
Signed-off-by: Fiona Ebner
---
PVE/APLInfo.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/PVE/APLInfo.pm b/PVE/APLInfo.pm
index a4acf490..07542743 100644
--- a/PVE/APLInfo.pm
+++ b/PVE/APLInfo.pm
@@ -213,7 +213,7 @@ sub get_apl_sources {
sub update {
my ($proxy) = @_;
-if (-s $logfile || 0 > 1024 * 256) {
+if ((-s $logfile || 0) > 1024 * 256) {
rename($logfile, "$logfile.0");
}
my $logfd = IO::File->new (">>$logfile");
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-cluster] docs: update http_proxy option description
Signed-off-by: Hannes Laimer
---
src/PVE/DataCenterConfig.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PVE/DataCenterConfig.pm b/src/PVE/DataCenterConfig.pm
index c80872c..1f1291c 100644
--- a/src/PVE/DataCenterConfig.pm
+++ b/src/PVE/DataCenterConfig.pm
@@ -343,7 +343,7 @@ my $datacenter_schema = {
http_proxy => {
optional => 1,
type => 'string',
- description => "Specify external http proxy which is used for
downloads (example: 'http://username:password\@host:port/')",
+ description => "Specify external http proxy which is used for
downloads and ACME DNS-Challenges (example:
'http://username:password\@host:port/')",
pattern => "http://.*;,
},
# FIXME: remove with 8.0 (add check to pve7to8!), merged into
"migration" since 4.3
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-acme/pve-cluster 0/2] Add support to use a http proxy for acme dns challenges
Use the http_proxy configured in the datacenter also for dns challenges, I don't think there is a use-case to configure different proxies for downloads and dns challenges. This is based on the reverted patch [1] of Stoiko a while ago. [1] https://git.proxmox.com/?p=proxmox-acme.git;a=commitdiff;h=4ed79f7b4cd3e77ec9764f6233ce83098ace60d9;hp=e1088f616ffc73a96ee3433f0ea07639ef7513e7 - proxmox-acme Hannes Laimer (1): dns-challenge: use configured datacenter http_proxy for acme dns challenges src/PVE/ACME/DNSChallenge.pm | 4 1 file changed, 4 insertions(+) - pve-cluster Hannes Laimer (1): docs: update http_proxy option description src/PVE/DataCenterConfig.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-acme 1/1] dns-challenge: use configured datacenter http_proxy for acme dns challenges
the proxy is added to the plugin config so the `proxmox-acme` script
exports it and it'll be used by curl when requests are made.
Based on e1088f616ffc73a96ee3433f0ea07639ef7513e7 (reverted).
Signed-off-by: Hannes Laimer
---
src/PVE/ACME/DNSChallenge.pm | 4
1 file changed, 4 insertions(+)
diff --git a/src/PVE/ACME/DNSChallenge.pm b/src/PVE/ACME/DNSChallenge.pm
index 7214d88..29b741e 100644
--- a/src/PVE/ACME/DNSChallenge.pm
+++ b/src/PVE/ACME/DNSChallenge.pm
@@ -85,6 +85,9 @@ my $proxmox_acme_command = sub {
my $dnsplugin = $data->{plugin}->{api};
my $plugin_conf_string = $data->{plugin}->{data};
+my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
+my $proxy = $dccfg->{http_proxy};
+
# for security reasons, we execute the command as nobody
# we can't verify that the code of the DNSPlugins are harmless.
my $cmd = ["setpriv", "--reuid", "nobody", "--regid", "nogroup",
"--clear-groups", "--reset-env", "--"];
@@ -99,6 +102,7 @@ my $proxmox_acme_command = sub {
}
my $input = "$txtvalue\n";
$input .= "$plugin_conf_string\n" if $plugin_conf_string;
+$input .= "https_proxy=$proxy\nhttp_proxy=$proxy\n" if $proxy;
PVE::Tools::run_command($cmd, input => $input);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH widget-toolkit v4 11/18] utils: add extendable, translatable notifiction event descriptions
Am 28/02/2024 um 11:00 schrieb Lukas Wagner: > Signed-off-by: Lukas Wagner > --- > src/Utils.js | 31 +++ > 1 file changed, 31 insertions(+) > > applied, with adding to the commit message that this is oriented on the existing task description add/override mechanism, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH widget-toolkit v4 10/18] combogrid: add 'showClearTrigger' config
Am 28/02/2024 um 11:00 schrieb Lukas Wagner:
> This allows one configure the clear trigger to be shown, even if
> 'allowBlank' is set false. This can be useful if one has a
> non-editable combogrid where the value is set to something not
> present in the store. Example: Match rule editing, one selects
> a backup job to be match. If the backup job is removed and the match
> rule edit window is opened again, then the old, deleted value cannot
> be removed from the combogrid if there is no clear trigger.
>
> Signed-off-by: Lukas Wagner
> ---
> src/form/ComboGrid.js | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>
applied, thanks!
I made a follow up though to clarify that the showClearTrigger cannot be
used to hide it, and to simplify the boolean expression.
diff --git a/src/form/ComboGrid.js b/src/form/ComboGrid.js
index 3aaa717..cca92b1 100644
--- a/src/form/ComboGrid.js
+++ b/src/form/ComboGrid.js
@@ -32,6 +32,8 @@ Ext.define('Proxmox.form.ComboGrid', {
notFoundIsValid: false,
deleteEmpty: false,
errorHeight: 100,
+ // NOTE: the trigger will always be shown if allowBlank is true,
setting showClearTrigger
+ // to false cannot change that
showClearTrigger: false,
},
@@ -55,10 +57,7 @@ Ext.define('Proxmox.form.ComboGrid', {
setValue: function(value) {
let me = this;
let empty = Ext.isArray(value) ? !value.length : !value;
- me.triggers.clear.setVisible(
- (!empty && me.allowBlank) ||
- (!empty && me.showClearTrigger),
- );
+ me.triggers.clear.setVisible(!empty && (me.allowBlank ||
me.showClearTrigger));
return me.callParent([value]);
},
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH widget-toolkit v4 13/18] notification: matcher: move match-field formulas to local viewModel
This should make the code more cohesive and easier to follow.
No functional changes.
Signed-off-by: Lukas Wagner
Tested-by: Maximiliano Sandoval
---
src/window/NotificationMatcherEdit.js | 186 +-
1 file changed, 92 insertions(+), 94 deletions(-)
diff --git a/src/window/NotificationMatcherEdit.js
b/src/window/NotificationMatcherEdit.js
index fa42e1e..9d0a6bd 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -380,15 +380,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
}
return !record.isRoot();
},
- typeIsMatchField: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- get: function(record) {
- return record?.get('type') === 'match-field';
- },
- },
typeIsMatchSeverity: {
bind: {
bindTo: '{selectedRecord}',
@@ -407,89 +398,13 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
return record?.get('type') === 'match-calendar';
},
},
- matchFieldType: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
- let currentData = record.get('data');
-
- let newValue = [];
-
- // Build equivalent regular expression if switching
- // to 'regex' mode
- if (value === 'regex') {
- let regexVal = "^(";
- regexVal += currentData.value.join('|') + ")$";
- newValue.push(regexVal);
- }
-
- record.set({
- data: {
- ...currentData,
- type: value,
- value: newValue,
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.type;
- },
- },
- matchFieldField: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
- let currentData = record.get('data');
-
- record.set({
- data: {
- ...currentData,
- field: value,
- // Reset value if field changes
- value: [],
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.field;
- },
- },
- matchFieldValue: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
- let currentData = record.get('data');
- record.set({
- data: {
- ...currentData,
- value: value,
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.value;
- },
- },
matchSeverityValue: {
bind: {
bindTo: '{selectedRecord}',
deep: true,
},
set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
+ let record = this.get('selectedRecord');
let currentData = record.get('data');
record.set({
data: {
@@ -1137,7 +1052,95 @@ Ext.define('Proxmox.panel.MatchFieldSettings', {
},
},
},
+viewModel: {
+ // parent is set in `initComponents`
+ formulas: {
+ typeIsMatchField: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ return record?.get('type') === 'match-field';
+ },
+ },
+ isRegex: function(get) {
+ return get('matchFieldType') === 'regex';
+ },
+ matchFieldType: {
+ bind: {
+ bindTo:
[pve-devel] [PATCH docs v4 18/18] notifications: match-field 'exact'-mode can now match multiple values
Signed-off-by: Lukas Wagner --- notifications.adoc | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/notifications.adoc b/notifications.adoc index 0eeed6a..912b048 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -221,11 +221,16 @@ configurable schedule. Field Matching Rules Notifications have a selection of metadata fields that can be matched. +When using `exact` as a matching mode, a `,` can be used as a separator. +The matching rule then matches if the metadata field has *any* of the specified +values. * `match-field exact:type=vzdump` Only match notifications about backups. +* `match-field exact:type=replication,fencing` Match `replication` and `fencing` notifications. * `match-field regex:hostname=^.+\.example\.com$` Match the hostname of the node. + If a matched metadata field does not exist, the notification will not be matched. For instance, a `match-field regex:hostname=.*` directive will only match @@ -267,18 +272,7 @@ matcher: backup-failures comment Send notifications about backup failures to one group of admins matcher: cluster-failures -match-field exact:type=replication -match-field exact:type=fencing -mode any -target cluster-admins -comment Send cluster-related notifications to other group of admins - - -The last matcher could also be rewritten using a field matcher with a regular -expression: - -matcher: cluster-failures -match-field regex:type=^(replication|fencing)$ +match-field exact:type=replication,fencing target cluster-admins comment Send cluster-related notifications to other group of admins -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v4 17/18] notifications: describe new notification metadata fields
Signed-off-by: Lukas Wagner --- notifications.adoc | 21 - 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/notifications.adoc b/notifications.adoc index 57053c8..0eeed6a 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -289,19 +289,22 @@ Notification Events [width="100%",options="header"] |=== -| Event| `type`| Severity | Metadata fields (in addition to `type`) -| System updates available |`package-updates` | `info` | `hostname` -| Cluster node fenced |`fencing` | `error` | `hostname` -| Storage replication failed |`replication` | `error` | - -| Backup finished |`vzdump` | `info` (`error` on failure) | `hostname` -| Mail for root|`system-mail` | `unknown`| - +| Event| `type`| Severity | Metadata fields (in addition to `type`) +| System updates available |`package-updates` | `info` | `hostname` +| Cluster node fenced |`fencing` | `error` | `hostname` +| Storage replication job failed |`replication` | `error` | `hostname`, `replication-job` +| Backup succeeded |`vzdump` | `info` | `hostname`, `backup-job` (only for backup jobs) +| Backup failed|`vzdump` | `error` | `hostname`, `backup-job` (only for backup jobs) +| Mail for root|`system-mail` | `unknown`| `hostname` |=== [width="100%",options="header"] |=== -| Field name | Description -| `type` | Type of the notifcation -| `hostname` | Hostname, without domain (e.g. `pve1`) +| Field name| Description +| `type`| Type of the notifcation +| `hostname`| Hostname, without domain (e.g. `pve1`) +| `backup-job` | Backup job ID +| `replication-job` | Replication job ID |=== System Mail Forwarding -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v4 16/18] notification: clarify that 'hostname' does not include the domain
This was a bit inconsistent between the different notification types: - APT/VZDump included the domain part - fence notifications did not A decision has been made to unify this by removing the domain part from APT/VZDump notifications. Signed-off-by: Lukas Wagner --- notifications.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notifications.adoc b/notifications.adoc index 46aff6a..57053c8 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -301,7 +301,7 @@ Notification Events |=== | Field name | Description | `type` | Type of the notifcation -| `hostname` | Hostname, including domain (e.g. `pve1.example.com`) +| `hostname` | Hostname, without domain (e.g. `pve1`) |=== System Mail Forwarding -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH widget-toolkit v4 10/18] combogrid: add 'showClearTrigger' config
This allows one configure the clear trigger to be shown, even if
'allowBlank' is set false. This can be useful if one has a
non-editable combogrid where the value is set to something not
present in the store. Example: Match rule editing, one selects
a backup job to be match. If the backup job is removed and the match
rule edit window is opened again, then the old, deleted value cannot
be removed from the combogrid if there is no clear trigger.
Signed-off-by: Lukas Wagner
---
src/form/ComboGrid.js | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/form/ComboGrid.js b/src/form/ComboGrid.js
index 6338a3a..3aaa717 100644
--- a/src/form/ComboGrid.js
+++ b/src/form/ComboGrid.js
@@ -32,6 +32,7 @@ Ext.define('Proxmox.form.ComboGrid', {
notFoundIsValid: false,
deleteEmpty: false,
errorHeight: 100,
+ showClearTrigger: false,
},
// needed to trigger onKeyUp etc.
@@ -54,7 +55,10 @@ Ext.define('Proxmox.form.ComboGrid', {
setValue: function(value) {
let me = this;
let empty = Ext.isArray(value) ? !value.length : !value;
- me.triggers.clear.setVisible(!empty && me.allowBlank);
+ me.triggers.clear.setVisible(
+ (!empty && me.allowBlank) ||
+ (!empty && me.showClearTrigger),
+ );
return me.callParent([value]);
},
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v4 07/18] api: replication: include 'hostname' field for notifications
The field contains the hostname of the host (without any domain part)
which sends the notification. This field can be used in match-field
match rules.
Signed-off-by: Lukas Wagner
---
PVE/API2/Replication.pm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index 703640f5..192b8af3 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -142,6 +142,8 @@ my sub _handle_job_err {
my $metadata_fields = {
type => "replication",
"replication-job" => $job->{id},
+ # Hostname (without domain part)
+ hostname => PVE::INotify::nodename(),
};
eval {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v4 05/18] api: replication: add 'replication-job' to notification metadata
This allows users to create notification match rules for specific
replication jobs, if they so desire.
Signed-off-by: Lukas Wagner
---
PVE/API2/Replication.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index 0dc944c9..703640f5 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -140,8 +140,8 @@ my sub _handle_job_err {
};
my $metadata_fields = {
- # TODO: Add job-id?
type => "replication",
+ "replication-job" => $job->{id},
};
eval {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v4 04/18] ui: dc: backup: allow to set custom job id in advanced settings
This might be useful if somebody wants to match on the new
'backup-job' field in a notification match rule.
Signed-off-by: Lukas Wagner
---
www/manager6/Utils.js | 4
www/manager6/dc/Backup.js | 11 +++
2 files changed, 15 insertions(+)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index a8d5786d..4c8bf9d3 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1942,6 +1942,10 @@ Ext.define('PVE.Utils', {
singleton: true,
constructor: function() {
var me = this;
+
+ // Same regex as 'pve-configid
+ me.CONFIGID_RE = /^[A-Za-z][A-Za-z0-9_-]+$/;
+
Ext.apply(me, me.utilities);
Proxmox.Utils.override_task_descriptions({
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 4beb84c0..5b6f6688 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -397,6 +397,17 @@ Ext.define('PVE.dc.BackupEdit', {
},
],
advancedColumn1: [
+ {
+ xtype: 'pmxDisplayEditField',
+ fieldLabel: gettext('Job ID'),
+ emptyText: gettext('Autogenerate'),
+ name: 'id',
+ allowBlank: true,
+ regex: PVE.Utils.CONFIGID_RE,
+ cbind: {
+ editable: '{isCreate}',
+ },
+ },
{
xtype: 'proxmoxcheckbox',
fieldLabel: gettext('Repeat missed'),
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v4 06/18] vzdump: apt: notification: do not include domain in 'hostname' field
- The man page warns about the usage of `hostname -f`, since a host
may have multiple domains (or none at all)
- The fallback PVE::INotify::nodename() already only returned the
hostname without the domain part
- Fencing notifications didn't include the domain part anyway
This may result in soft-breakage for any users who have already relied
on the domain being present. If there is need for it, it could include
a fqdn metadata field.
The hostname property used for rendering the notification template
is unaffected for now.
Signed-off-by: Lukas Wagner
---
PVE/API2/APT.pm | 3 ++-
PVE/VZDump.pm | 8
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index 54121ec2..71c83581 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -354,7 +354,8 @@ __PACKAGE__->register_method({
# matchers.
my $metadata_fields = {
type => 'package-updates',
- hostname => $hostname,
+ # Hostname (without domain part)
+ hostname => PVE::INotify::nodename(),
};
PVE::Notify::info(
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 88f42962..c24ff38e 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -487,10 +487,9 @@ sub send_notification {
"See Task History for details!\n";
};
-my $hostname = get_hostname();
-
my $notification_props = {
- "hostname" => $hostname,
+ # Hostname, might include domain part
+ "hostname" => get_hostname(),
"error-message" => $err,
"guest-table" => build_guest_table($tasklist),
"logs" => $text_log_part,
@@ -501,7 +500,8 @@ sub send_notification {
my $fields = {
type => "vzdump",
- hostname => $hostname,
+ # Hostname (without domain part)
+ hostname => PVE::INotify::nodename(),
};
# Add backup-job metadata field in case this is a backup job.
$fields->{'backup-job'} = $job_id if $job_id;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v4 03/18] ui: dc: backup: send 'id' property when starting a backup job manually
Signed-off-by: Lukas Wagner
---
www/manager6/dc/Backup.js | 1 -
1 file changed, 1 deletion(-)
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 70903bdc..4beb84c0 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -569,7 +569,6 @@ Ext.define('PVE.dc.BackupView', {
delete job.enabled;
delete job.starttime;
delete job.dow;
- delete job.id;
delete job.schedule;
delete job.type;
delete job.node;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH widget-toolkit v4 12/18] notification: matcher: match-field: show known fields/values
These changes introduce combogrid pickers for the 'field' and 'value'
form elements for 'match-field' match rules. The 'field' picker shows
a list of all known metadata fields, while the 'value' picker shows a
list of all known values, filtered depending on the current value of
'field'.
The list of known fields/values is retrieved from new API endpoints.
Some values are marked 'internal' by the backend. This means that the
'value' field was not user-created (counter example: backup job
IDs) and can therefore be used as a base for translations.
Signed-off-by: Lukas Wagner
Tested-by: Maximiliano Sandoval
---
src/data/model/NotificationConfig.js | 12 ++
src/window/NotificationMatcherEdit.js | 300 +-
2 files changed, 256 insertions(+), 56 deletions(-)
diff --git a/src/data/model/NotificationConfig.js
b/src/data/model/NotificationConfig.js
index e8ebf28..a2c365b 100644
--- a/src/data/model/NotificationConfig.js
+++ b/src/data/model/NotificationConfig.js
@@ -15,3 +15,15 @@ Ext.define('proxmox-notification-matchers', {
},
idProperty: 'name',
});
+
+Ext.define('proxmox-notification-fields', {
+extend: 'Ext.data.Model',
+fields: ['name', 'description'],
+idProperty: 'name',
+});
+
+Ext.define('proxmox-notification-field-values', {
+extend: 'Ext.data.Model',
+fields: ['value', 'comment', 'internal', 'field'],
+idProperty: 'value',
+});
diff --git a/src/window/NotificationMatcherEdit.js
b/src/window/NotificationMatcherEdit.js
index f88576a..fa42e1e 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -79,7 +79,7 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', {
labelWidth: 120,
},
-width: 700,
+width: 800,
initComponent: function() {
let me = this;
@@ -416,10 +416,22 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
let me = this;
let record = me.get('selectedRecord');
let currentData = record.get('data');
+
+ let newValue = [];
+
+ // Build equivalent regular expression if switching
+ // to 'regex' mode
+ if (value === 'regex') {
+ let regexVal = "^(";
+ regexVal += currentData.value.join('|') + ")$";
+ newValue.push(regexVal);
+ }
+
record.set({
data: {
...currentData,
type: value,
+ value: newValue,
},
});
},
@@ -441,6 +453,8 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
data: {
...currentData,
field: value,
+ // Reset value if field changes
+ value: [],
},
});
},
@@ -549,6 +563,9 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
column2: [
{
xtype: 'pmxNotificationMatchRuleSettings',
+ cbind: {
+ baseUrl: '{baseUrl}',
+ },
},
],
@@ -601,7 +618,7 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
let value = data.value;
text = Ext.String.format(gettext("Match field: {0}={1}"),
field, value);
iconCls = 'fa fa-square-o';
- if (!field || !value) {
+ if (!field || !value || (Ext.isArray(value) && !value.length)) {
iconCls += ' internal-error';
}
} break;
@@ -821,6 +838,11 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
if (type === undefined) {
type = "exact";
}
+
+ if (type === 'exact') {
+ matchedValue = matchedValue.split(',');
+ }
+
return {
type: 'match-field',
data: {
@@ -982,7 +1004,9 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
extend: 'Ext.panel.Panel',
xtype: 'pmxNotificationMatchRuleSettings',
+mixins: ['Proxmox.Mixin.CBind'],
border: false,
+layout: 'anchor',
items: [
{
@@ -1000,6 +1024,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings',
{
['notall', gettext('At least one rule does not match')],
['notany', gettext('No rule matches')],
],
+ // Hide initially to avoid glitches when opening the window
+ hidden: true,
bind: {
hidden: '{!showMatchingMode}',
disabled: '{!showMatchingMode}',
@@ -1011,7 +1037,8 @@
[pve-devel] [PATCH widget-toolkit v4 15/18] notification: matcher: move match-severity fields to panel
Also introduce a local viewModel that is linked to a parent viewModel,
allowing us to move the formulas to the panel.
This should make the code more cohesive and easier to follow.
No functional changes.
Signed-off-by: Lukas Wagner
Tested-by: Maximiliano Sandoval
---
src/window/NotificationMatcherEdit.js | 129 --
1 file changed, 80 insertions(+), 49 deletions(-)
diff --git a/src/window/NotificationMatcherEdit.js
b/src/window/NotificationMatcherEdit.js
index 47f6e18..3e2449f 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -380,34 +380,7 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
}
return !record.isRoot();
},
- typeIsMatchSeverity: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- get: function(record) {
- return record?.get('type') === 'match-severity';
- },
- },
- matchSeverityValue: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let record = this.get('selectedRecord');
- let currentData = record.get('data');
- record.set({
- data: {
- ...currentData,
- value: value,
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.value;
- },
- },
+
rootMode: {
bind: {
bindTo: '{selectedRecord}',
@@ -944,27 +917,7 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
},
},
{
- xtype: 'proxmoxKVComboBox',
- fieldLabel: gettext('Severities to match'),
- isFormField: false,
- allowBlank: true,
- multiSelect: true,
- field: 'value',
- // Hide initially to avoid glitches when opening the window
- hidden: true,
- bind: {
- value: '{matchSeverityValue}',
- hidden: '{!typeIsMatchSeverity}',
- disabled: '{!typeIsMatchSeverity}',
- },
-
- comboItems: [
- ['info', gettext('Info')],
- ['notice', gettext('Notice')],
- ['warning', gettext('Warning')],
- ['error', gettext('Error')],
- ['unknown', gettext('Unknown')],
- ],
+ xtype: 'pmxNotificationMatchSeveritySettings',
},
{
xtype: 'pmxNotificationMatchCalendarSettings',
@@ -1047,6 +1000,84 @@ Ext.define('Proxmox.panel.MatchCalendarSettings', {
},
});
+Ext.define('Proxmox.panel.MatchSeveritySettings', {
+extend: 'Ext.panel.Panel',
+xtype: 'pmxNotificationMatchSeveritySettings',
+border: false,
+layout: 'anchor',
+// Hide initially to avoid glitches when opening the window
+hidden: true,
+bind: {
+ hidden: '{!typeIsMatchSeverity}',
+},
+viewModel: {
+ // parent is set in `initComponents`
+ formulas: {
+ typeIsMatchSeverity: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ return record?.get('type') === 'match-severity';
+ },
+ },
+ matchSeverityValue: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ set: function(value) {
+ let record = this.get('selectedRecord');
+ let currentData = record.get('data');
+ record.set({
+ data: {
+ ...currentData,
+ value: value,
+ },
+ });
+ },
+ get: function(record) {
+ return record?.get('data')?.value;
+ },
+ },
+ },
+},
+items: [
+ {
+ xtype: 'proxmoxKVComboBox',
+ fieldLabel: gettext('Severities to match'),
+ isFormField: false,
+ allowBlank: true,
+ multiSelect: true,
+ field: 'value',
+ // Hide initially to avoid glitches when opening the window
+ hidden: true,
+ bind: {
+ value: '{matchSeverityValue}',
+ hidden: '{!typeIsMatchSeverity}',
+ disabled: '{!typeIsMatchSeverity}',
+ },
+
+ comboItems: [
+ ['info', gettext('Info')],
+ ['notice', gettext('Notice')],
+
[pve-devel] [PATCH widget-toolkit v4 14/18] notification: matcher: move match-calendar fields to panel
Also introduce a local viewModel that is linked to a parent viewModel,
allowing us to move the formulas to the panel.
This should make the code more cohesive and easier to follow.
No functional changes.
Signed-off-by: Lukas Wagner
Tested-by: Maximiliano Sandoval
---
src/window/NotificationMatcherEdit.js | 92 +--
1 file changed, 60 insertions(+), 32 deletions(-)
diff --git a/src/window/NotificationMatcherEdit.js
b/src/window/NotificationMatcherEdit.js
index 9d0a6bd..47f6e18 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -389,15 +389,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
return record?.get('type') === 'match-severity';
},
},
- typeIsMatchCalendar: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- get: function(record) {
- return record?.get('type') === 'match-calendar';
- },
- },
matchSeverityValue: {
bind: {
bindTo: '{selectedRecord}',
@@ -417,26 +408,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
return record?.get('data')?.value;
},
},
- matchCalendarValue: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
- let currentData = record.get('data');
- record.set({
- data: {
- ...currentData,
- value: value,
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.value;
- },
- },
rootMode: {
bind: {
bindTo: '{selectedRecord}',
@@ -995,6 +966,58 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
['unknown', gettext('Unknown')],
],
},
+ {
+ xtype: 'pmxNotificationMatchCalendarSettings',
+ },
+],
+});
+
+Ext.define('Proxmox.panel.MatchCalendarSettings', {
+extend: 'Ext.panel.Panel',
+xtype: 'pmxNotificationMatchCalendarSettings',
+border: false,
+layout: 'anchor',
+// Hide initially to avoid glitches when opening the window
+hidden: true,
+bind: {
+ hidden: '{!typeIsMatchCalendar}',
+},
+viewModel: {
+ // parent is set in `initComponents`
+ formulas: {
+ typeIsMatchCalendar: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ return record?.get('type') === 'match-calendar';
+ },
+ },
+
+ matchCalendarValue: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ set: function(value) {
+ let me = this;
+ let record = me.get('selectedRecord');
+ let currentData = record.get('data');
+ record.set({
+ data: {
+ ...currentData,
+ value: value,
+ },
+ });
+ },
+ get: function(record) {
+ return record?.get('data')?.value;
+ },
+ },
+ },
+},
+items: [
{
xtype: 'proxmoxKVComboBox',
fieldLabel: gettext('Timespan to match'),
@@ -1003,11 +1026,8 @@
Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
editable: true,
displayField: 'key',
field: 'value',
- // Hide initially to avoid glitches when opening the window
- hidden: true,
bind: {
value: '{matchCalendarValue}',
- hidden: '{!typeIsMatchCalendar}',
disabled: '{!typeIsMatchCalender}',
},
@@ -1017,6 +1037,14 @@
Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
],
},
],
+
+initComponent: function() {
+ let me = this;
+ Ext.apply(me.viewModel, {
+ parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(),
+ });
+ me.callParent();
+},
});
Ext.define('Proxmox.panel.MatchFieldSettings', {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v4 08/18] api: notification: add API for getting known metadata fields/values
This new API route returns known notification metadata fields and
a list of known possible values. This will be used by the UI to
provide suggestions when adding/modifying match rules.
Signed-off-by: Lukas Wagner
---
PVE/API2/Cluster/Notifications.pm | 152 ++
1 file changed, 152 insertions(+)
diff --git a/PVE/API2/Cluster/Notifications.pm
b/PVE/API2/Cluster/Notifications.pm
index 68fdda2a..16548bec 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -79,12 +79,164 @@ __PACKAGE__->register_method ({
{ name => 'endpoints' },
{ name => 'matchers' },
{ name => 'targets' },
+ { name => 'fields' },
+ { name => 'values' },
];
return $result;
}
});
+__PACKAGE__->register_method ({
+name => 'get_fields',
+path => 'fields',
+method => 'GET',
+description => 'Returns known notification metadata fields and their known
values',
+permissions => {
+ check => ['or',
+ ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ ['perm', '/mapping/notifications', ['Mapping.Audit']],
+ ],
+},
+protected => 1,
+parameters => {
+ additionalProperties => 0,
+ properties => {},
+},
+returns => {
+ type => 'array',
+ items => {
+ type => 'object',
+ properties => {
+ name => {
+ description => 'Name of the field.',
+ type => 'string',
+ },
+ },
+ },
+ links => [ { rel => 'child', href => '{name}' } ],
+},
+code => sub {
+ # TODO: Adapt this API handler once we have a 'notification registry'
+
+ my $result = [
+ { name => 'type' },
+ { name => 'hostname' },
+ { name => 'backup-job' },
+ { name => 'replication-job' },
+ ];
+
+ return $result;
+}
+});
+
+__PACKAGE__->register_method ({
+name => 'get_field_values',
+path => 'values',
+method => 'GET',
+description => 'Returns known notification metadata fields and their known
values',
+permissions => {
+ check => ['or',
+ ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ ['perm', '/mapping/notifications', ['Mapping.Audit']],
+ ],
+},
+protected => 1,
+parameters => {
+ additionalProperties => 0,
+},
+returns => {
+ type => 'array',
+ items => {
+ type => 'object',
+ properties => {
+ 'value' => {
+ description => 'Notification metadata value known by the
system.',
+ type => 'string'
+ },
+ 'comment' => {
+ description => 'Additional comment for this value.',
+ type => 'string',
+ optional => 1,
+ },
+ 'field' => {
+ description => 'Field this value belongs to.',
+ type => 'string',
+ optional => 1,
+ },
+ 'internal' => {
+ description => 'Set if "value" was generated by the system
and can'
+ . ' safely be used as base for translations.',
+ type => 'boolean',
+ optional => 1,
+ },
+ },
+ },
+},
+code => sub {
+ # TODO: Adapt this API handler once we have a 'notification registry'
+ my $rpcenv = PVE::RPCEnvironment::get();
+ my $user = $rpcenv->get_user();
+
+ my $values = [
+ {
+ value => 'package-updates',
+ internal => 1,
+ field => 'type',
+ },
+ {
+ value => 'fencing',
+ internal => 1,
+ field => 'type',
+ },
+ {
+ value => 'replication',
+ internal => 1,
+ field => 'type',
+ },
+ {
+ value => 'vzdump',
+ internal => 1,
+ field => 'type',
+ },
+ {
+ value => 'system-mail',
+ internal => 1,
+ field => 'type',
+ },
+ ];
+
+ # Here we need a manual permission check.
+ if ($rpcenv->check($user, "/", ["Sys.Audit"], 1)) {
+ for my $backup_job (@{PVE::API2::Backup->index({})}) {
+ push @$values, {
+ value => $backup_job->{id},
+ comment => $backup_job->{comment},
+ field => 'backup-job'
+ };
+ }
+ }
+ # The API call returns only returns jobs for which the user
+ # has adequate permissions.
+ for my $sync_job (@{PVE::API2::ReplicationConfig->index({})}) {
+ push @$values, {
+ value => $sync_job->{id},
+
[pve-devel] [PATCH widget-toolkit v4 11/18] utils: add extendable, translatable notifiction event descriptions
Signed-off-by: Lukas Wagner
---
src/Utils.js | 31 +++
1 file changed, 31 insertions(+)
diff --git a/src/Utils.js b/src/Utils.js
index 009e222..ff7c1a7 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -647,6 +647,37 @@ utilities: {
Proxmox.Utils.unknownText;
},
+// Only add product-agnostic fields here!
+notificationFieldName: {
+ 'type': gettext('Notification type'),
+ 'hostname': gettext('Hostname'),
+},
+
+formatNotificationFieldName: (value) =>
+ Proxmox.Utils.notificationFieldName[value] || value,
+
+// to add or change existing for product specific ones
+overrideNotificationFieldName: function(extra) {
+ for (const [key, value] of Object.entries(extra)) {
+ Proxmox.Utils.notificationFieldName[key] = value;
+ }
+},
+
+// Only add product-agnostic fields here!
+notificationFieldValue: {
+ 'system-mail': gettext('Forwarded mails to the local root user'),
+},
+
+formatNotificationFieldValue: (value) =>
+ Proxmox.Utils.notificationFieldValue[value] || value,
+
+// to add or change existing for product specific ones
+overrideNotificationFieldValue: function(extra) {
+ for (const [key, value] of Object.entries(extra)) {
+ Proxmox.Utils.notificationFieldValue[key] = value;
+ }
+},
+
// NOTE: only add general, product agnostic, ones here! Else use override
helper in product repos
task_desc_table: {
aptupdate: ['', gettext('Update package database')],
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v4 09/18] ui: utils: add overrides for translatable notification fields/values
Signed-off-by: Lukas Wagner
---
www/manager6/Utils.js | 12
1 file changed, 12 insertions(+)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 4c8bf9d3..80c8f0f6 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -2042,6 +2042,18 @@ Ext.define('PVE.Utils', {
zfscreate: [gettext('ZFS Storage'), gettext('Create')],
zfsremove: ['ZFS Pool', gettext('Remove')],
});
+
+ Proxmox.Utils.overrideNotificationFieldName({
+ 'backup-job': gettext('Backup job ID'),
+ 'replication-job': gettext('Replication job ID'),
+ });
+
+ Proxmox.Utils.overrideNotificationFieldValue({
+ 'package-updates': gettext('Package updates are available'),
+ 'vzdump': gettext('Backup notifications'),
+ 'replication': gettext('Replication job notifications'),
+ 'fencing': gettext('Node fencing notifications'),
+ });
},
});
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v4 02/18] api: jobs: vzdump: pass job 'id' parameter
This allows us to access us the backup job id in the send_notification
function, where we can set it as metadata for the notification.
Signed-off-by: Lukas Wagner
---
PVE/API2/VZDump.pm | 8
PVE/Jobs/VZDump.pm | 4 +++-
PVE/VZDump.pm | 6 +++---
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index f66fc740..6bc0b792 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -52,6 +52,14 @@ __PACKAGE__->register_method ({
parameters => {
additionalProperties => 0,
properties => PVE::VZDump::Common::json_config_properties({
+ id => {
+ description => "The ID of the backup job. If set, the
'backup-job' metadata field"
+ . " of the backup notification will be set to this value.",
+ type => 'string',
+ format => 'pve-configid',
+ maxLength => 64,
+ optional => 1,
+ },
stdout => {
type => 'boolean',
description => "Write tar to stdout, not to a file.",
diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
index b8e57945..2f7c72ab 100644
--- a/PVE/Jobs/VZDump.pm
+++ b/PVE/Jobs/VZDump.pm
@@ -12,7 +12,7 @@ use PVE::API2::VZDump;
use base qw(PVE::VZDump::JobBase);
sub run {
-my ($class, $conf) = @_;
+my ($class, $conf, $id) = @_;
my $props = $class->properties();
# remove all non vzdump related options
@@ -20,6 +20,8 @@ sub run {
delete $conf->{$opt} if !defined($props->{$opt});
}
+$conf->{id} = $id;
+
# Required as string parameters # FIXME why?! we could just check ref()
for my $key (keys $PVE::VZDump::Common::PROPERTY_STRINGS->%*) {
if ($conf->{$key} && ref($conf->{$key}) eq 'HASH') {
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 152eb3e5..88f42962 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -453,6 +453,7 @@ sub send_notification {
my ($self, $tasklist, $total_time, $err, $detail_pre, $detail_post) = @_;
my $opts = $self->{opts};
+my $job_id = $opts->{id};
my $mailto = $opts->{mailto};
my $cmdline = $self->{cmdline};
my $policy = $opts->{mailnotification} // 'always';
@@ -499,12 +500,11 @@ sub send_notification {
};
my $fields = {
- # TODO: There is no straight-forward way yet to get the
- # backup job id here... (I think pvescheduler would need
- # to pass that to the vzdump call?)
type => "vzdump",
hostname => $hostname,
};
+# Add backup-job metadata field in case this is a backup job.
+$fields->{'backup-job'} = $job_id if $job_id;
my $severity = $failed ? "error" : "info";
my $email_configured = $mailto && scalar(@$mailto);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v4 01/18] api: notifications: add 'smtp' to target index
Signed-off-by: Lukas Wagner
---
PVE/API2/Cluster/Notifications.pm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/PVE/API2/Cluster/Notifications.pm
b/PVE/API2/Cluster/Notifications.pm
index 7047f0b1..68fdda2a 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -107,6 +107,7 @@ __PACKAGE__->register_method ({
my $result = [
{ name => 'gotify' },
{ name => 'sendmail' },
+ { name => 'smtp' },
];
return $result;
@@ -143,7 +144,7 @@ __PACKAGE__->register_method ({
'type' => {
description => 'Type of the target.',
type => 'string',
- enum => [qw(sendmail gotify)],
+ enum => [qw(sendmail gotify smtp)],
},
'comment' => {
description => 'Comment',
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager/widget-toolkit/docs v4 00/18] notifications: notification metadata matching improvements
This patch series attempts to improve the user experience when creating notification matchers. Some of the noteworthy changes: - Fixup inconsistent 'hostname' field. Some notification events sent the hostname including a domain, while other did not. This series unifies the behavior, now the field only includes the hostname without a domain. Also updated the docs to reflect this change. - Allow setting a custom backup job ID, similar how we handle it for sync/prune jobs in PBS (to allow recognizable names used in matchers) - Adding columns for backup job ID/replication job ID in the UI - New metadata fields: - backup-job: ID of the backup job (set for backups, unless they are 'ad-hoc' backups) - replication-job: ID of the replication job - Add an API that enumerates known notification metadata fields/values - Suggest known fields/values in match rule window - Some code clean up for match rule edit window - Extended the 'exact' match-field mode - it now allows setting multiple allowed values, separated via ',': e.g. `match-field exact:type=replication,fencing Originally, I created a separate 'list' match type for this, but since the semantics for a list with one value and 'exact' mode are identical, I decided to just extend 'exact'. This is a safe change since there are are no values where a ',' makes any sense (config IDs, hostnames) Inter-Dependencies: - widget-toolkit relies on a new API endpoint provided by pve-manager - pve-manager needs the "utils: add extendable, translatable notifiction event descriptions" patch from the widget-toolkit-part - can be applied ahead of time - the changes in the backend for matching multiple values in "exact" mode have already been rolled out as of libpve-rs-perl 0.8.8 --> this means that libpve-notify-perl (which pulls in libpve-rs-perl) must be bumped as well, in order for the GUI changes to make sense Changelog: - v4: - widget-toolkit: break out changes for the utils module so that they can be applied ahead of time to ease dep bumping - don't show Job IDs in the backup/replication job columns - v3: - Drop already applied patches for `proxmox` - Rebase onto latest master - minor conflict resolution was needed - v2: - include 'type' metadata field for forwarded mails --> otherwise it's not possible to match them - include Maximilliano's T-b trailer in UI patches pve-manager: Lukas Wagner (9): api: notifications: add 'smtp' to target index api: jobs: vzdump: pass job 'id' parameter ui: dc: backup: send 'id' property when starting a backup job manually ui: dc: backup: allow to set custom job id in advanced settings api: replication: add 'replication-job' to notification metadata vzdump: apt: notification: do not include domain in 'hostname' field api: replication: include 'hostname' field for notifications api: notification: add API for getting known metadata fields/values ui: utils: add overrides for translatable notification fields/values PVE/API2/APT.pm | 3 +- PVE/API2/Cluster/Notifications.pm | 155 +- PVE/API2/Replication.pm | 4 +- PVE/API2/VZDump.pm| 8 ++ PVE/Jobs/VZDump.pm| 4 +- PVE/VZDump.pm | 14 +-- www/manager6/Utils.js | 16 +++ www/manager6/dc/Backup.js | 12 ++- 8 files changed, 204 insertions(+), 12 deletions(-) proxmox-widget-toolkit: Lukas Wagner (6): combogrid: add 'showClearTrigger' config utils: add extendable, translatable notifiction event descriptions notification: matcher: match-field: show known fields/values notification: matcher: move match-field formulas to local viewModel notification: matcher: move match-calendar fields to panel notification: matcher: move match-severity fields to panel src/Utils.js | 31 ++ src/data/model/NotificationConfig.js | 12 + src/form/ComboGrid.js | 6 +- src/window/NotificationMatcherEdit.js | 613 ++ 4 files changed, 477 insertions(+), 185 deletions(-) pve-docs: Lukas Wagner (3): notification: clarify that 'hostname' does not include the domain notifications: describe new notification metadata fields notifications: match-field 'exact'-mode can now match multiple values notifications.adoc | 39 ++- 1 file changed, 18 insertions(+), 21 deletions(-) Summary over all repositories: 13 files changed, 699 insertions(+), 218 deletions(-) -- Generated by git-murpp 0.5.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH proxmox-i18n] update Italian translations
Am 27/02/2024 um 18:12 schrieb Christian Ebner: > Signed-off-by: Christian Ebner > --- > it.po | 1028 - > 1 file changed, 351 insertions(+), 677 deletions(-) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 firewall 6/6] simulator: use new bridge naming scheme
Am 27/02/2024 um 13:35 schrieb Stefan Hanreich:
> On Mon, Feb 26, 2024 at 04:36:59PM +0100, Thomas Lamprecht wrote:
>> Am 26/02/2024 um 11:51 schrieb DERUMIER, Alexandre via pve-devel:
>>> hi,I think you should limit to 8 characters like for sdn vnet,
>>>
>>> as we need to space to vlan tag for example (vmbrY.), or other sdn
>>> construct.
>>
>> alternatively just show a hint in the UI if longer than 8 characters
>> and, if possible, error out with a clear message when one sets up
>> something that cannot work any more.
>> [...]
>> That said, starting out with a 8 characters max length limit is quicker
>> to implement and would be fine for me.
>
> When creating a VNet with this patch, the Web UI should validate that
> the bridge name isn't longer than 10 characters, so it should be fine
> since . is at most 5 characters - or am I missing something?
>
> Should be no problem to switch from 10 to 8 though, if this is solely
> for possible future additions that might require more than 5
> characters.
>
> Might be a bit awkward if a user creates a bridge with >10 characters
> and then notices he cannot use it as a bridge in SDN.
yeah that's why I'd show a hint that makes users aware that their
current name length is not ideal for maximum feature compat, but as
said, no hard feelings going for just 10 as maximum is fine; I for
one often want shorter names anyway (brX instead of vmbrX ^^
>> btw. one could also lift the strict naming scheme for bonds using
>> the 'bond-mode' flag to detect them.
>
> Yes, definitely something I could introduce but we would need some
> solution for the pve-firewall simulator, since it only goes off of
> naming schemes rather than the interfaces file.
meh, that's really limited... either check the real world, e.g.,
for bridges if /sys/class/net/$iface/bridge exists or via ethtool,
or make the user provide that info, best would be probably a mix
of both, existing interfaces are detected and others need to be
manually specified (e.g., via CLI option map --iface $name:$type
or the like)
>> Oh, and fwiw, having some awareness safety net like:
>>
>> warn "..." if !defined $d->{'bridge_ports'} && $iface =~ m/^vmbr\d+$/;
>
> Sounds good, you mean in the parsing of the interface file - I assume?
>
yes, while that will be noisy, it's fine in this case.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH proxmox-i18n] ja: Improve translation
Am 24/01/2024 um 10:36 schrieb ribbon: > On Wed, Jan 24, 2024 at 04:13:47PM +0900, ribbon wrote: >> I have reviewed the whole Japanese translation. > > Attachment file deleted? Yes, the list never allowed attachements to avoid bloating up the mailing list ize and for security reasons. I adapted the diff to a git commit and rebased it on current master, so: applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
