Re: [PATCH pve-manager] api: nodes: dns: restart postfix after changing name servers

[Dominik Csapak <d.csapak@proxmox.com>]

Hi,

first, does postfix really need a restart or would a 'reload' be enough?

if so, would it be possible to use try-reload-or-restart instead?

some comments inline:

On 11/9/26 11:51 AM, Jonas Theisen wrote:
> Postfix runs its SMTP client in a chroot and copies /etc/resolv.conf
> into it only when the instance starts. A running postfix therefore
> keeps using the old name servers after a DNS settings change,
> which can make mail delivery fail until the next restart.
> 
> Restart the postfix service after writing the new settings, but only
> if it is currently active.
> A failed restart only warns, since the DNS settings were already
> written.
> 
> Came up in: https://forum.proxmox.com/threads/187442
> 
> Signed-off-by: Jonas Theisen <j.theisen@proxmox.com>
> ---
>   PVE/API2/Nodes.pm | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
> index df0b2c2d..35078d3b 100644
> --- a/PVE/API2/Nodes.pm
> +++ b/PVE/API2/Nodes.pm
> @@ -1671,6 +1671,17 @@ __PACKAGE__->register_method({
>   
>           PVE::INotify::update_file('resolvconf', $param);
>   
> +        # postfix copies resolv.conf into its chroot only on start, so a running instance needs a
> +        # restart to use the new name servers
> +        eval {
> +            my $active = PVE::Tools::run_command(

please use `PVE::Cmd::run` for new code instead of `run_command`
(we recently moved that code into it's own perl module)
interface is identical

> +                ['systemctl', 'is-active', '--quiet', 'postfix@-'],
> +                noerr => 1,
> +            );

on several machines of mine this does return an error:

```
root@pve:~# systemctl is-active --quiet postfix@-
root@pve:~# echo $?
3
```

sure it's the right unit to check and restart?

also, '$active' sounds like must be set to >0 if the unit is active
and code below looks like we restart it if it's not running

better name could be

$is_active_error

(or something like that, i'm not the best at naming either ;) )

> +            PVE::Tools::run_command(['systemctl', 'restart', 'postfix@-']) if $active == 0;
> +        };
> +        warn "failed to restart postfix - $@" if $@;
> +
>           return;
>       },
>   });