thias / puppet-postfix

Puppet Postfix module
Other
17 stars 117 forks source link

Service should restart after changing listening interfaces #11

Closed nicwaller closed 10 years ago

nicwaller commented 11 years ago

When changing the listening interface, the service should restart instead of just doing a reload. Otherwise the changes won't take effect without manual intervention or a restart.

Consider separating config lines that require a restart into a separate file, then when Puppet sees changes to that file it should notify the service to restart.

thias commented 11 years ago

I think trying to implement both reload and restart is overkill. What about changing to "restart" by default, while still letting people choose to use "reload" if they know they're never going to change anything that requires a full restart? Postfix restarts very fast and cleanly, and SMTP sessions are short and retried when they get interrupted, so defaulting to a full restart shouldn't be an issue.

BTW, it's currently the other way around, and easy to override in the same way :

class { '::postfix::server':
  service_restart => '/sbin/service postfix restart',
  [...]
}

Thoughts?

nicwaller commented 11 years ago

At first I wanted to agree, but I think it would be a step backward to restart postfix for every config change. The man pages are fairly specific about what changes need a restart and encourage people to avoid doing start stop unless it is mandatory.

Although you are right that a well behaved SMTP client will reattempt transmission, there is at least one case where that is not true. Consider a naive PHP application that sends SMTP mail to Postfix - it will not retry, and will likely fail with an exception. In a moderately large site, Postfix is likely to have active sessions at any given moment.

It seems to me that not dropping mail should be the first priority for a mail host. Since reload is safer, it should remain as the default.

Perhaps add a note about this to the docs instead?

Nic Waller

On Mon, Oct 7, 2013 at 2:19 AM, Matthias Saou notifications@github.com wrote:

I think trying to implement both reload and restart is overkill. What about changing to "restart" by default, while still letting people choose to use "reload" if they know they're never going to change anything that requires a full restart? Postfix restarts very fast and cleanly, and SMTP sessions are short and retried when they get interrupted, so defaulting to a full restart shouldn't be an issue. BTW, it's currently the other way around, and easy to override in the same way :

class { '::postfix::server':
  service_restart => '/sbin/service postfix restart',
  [...]
}

Thoughts?

Reply to this email directly or view it on GitHub: https://github.com/thias/puppet-postfix/issues/11#issuecomment-25794560

thias commented 10 years ago

Is the change in bb2a423be62140ee601740cab69f0937f8408f4d to add a 'Limitations' section to the README enough for you?

thias commented 10 years ago

I'm guessing it is, closing :-)

nicwaller commented 10 years ago

Yep that is great.