Re: [PATCH proxmox v2 15/34] notify: migrate PBS's and PVE's default matcher to expression syntax

["Lukas Wagner" <l.wagner@proxmox.com>]

On Wed Oct 7, 2026 at 11:32 AM CEST, Michael Köppl wrote:
> On Thu Oct 1, 2026 at 10:36 AM CEST, Lukas Wagner wrote:
>
> [snip]
>
>> +
>> +            let expr: Expression<NotificationMatcher> = Expression::Constant(true);
>> +
>> +            let expr_str = serde_json::to_string(&expr)
>> +                .expect("failed serialize expression for 'default-matcher'");
>> +
>> +            config
>> +                .set_data(
>> +                    "default-matcher",
>> +                    MATCHER_TYPENAME,
>> +                    MatcherConfig {
>> +                        name: "default-matcher".into(),
>> +                        expression: Some(expr_str),
>
> If at least one of the nodes in a cluster is already upgraded and for
> ANY reason persists to /etc/pve/notifications.cfg, any node that is
> not upgraded will not be able to send any notifications at all until it
> is also upgraded, since it fails to parse this.
>
> IMO it's not really a problem since cluster nodes should be upgraded to
> the same version anyway, but wanted to point out that it might be worth
> highlighting to users when including this in a release, since this will
> stop nodes from sending notifications even if the new expressions are
> never used. Simply editing a target is enough to cause this.
>

Good catch!

After some discussions with you and Fiona (thanks a ton to both of you),
I've decided that the simplest option is to include a check in the
POST/PUT handlers for matchers - if 'expression' is set, we do a version
check to ensure that all (online) cluster nodes are recent enough. It's
not a perfect solution, but avoids creating a config that would fail to
parse on outdated nodes, leading to a failure to send notifications.

We can probably drop this check at the next major version.

>> +                        target: vec!["mail-to-root".into()],
>> +                        comment: Some("Route all notifications to mail-to-root".into()),
>> +                        ..Default::default()
>> +                    },
>> +                )
>> +                .expect("failed to set 'default-matcher' in default config");
>> +
>> +            config
>> +        })
>
> [snip]