Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-12 Thread Vincent Bernat
 ❦  8 mai 2019 13:44 +00, Veiko Kukk :

>> It was slightly modified to cleanly apply, because HAProxy's default
>> unit file does not include rsyslog.service as an 'After' dependency.
>> Also the subject line was modified to include the proper subsystem
>> and severity.
>
> I think, instead of After=rsyslog.service, it should be
> After=syslog.service, then any logger daemon could be used that has
> Alias=syslog.service.

After looking a bit more, in Debian, we are using rsyslog.service
explicitely to ensure the presence of a syslog socket inside HAProxy
chroot. As the configuration for this socket is done for rsyslog only,
we depend on rsyslog running for this aspect.
-- 
Make sure your code "does nothing" gracefully.
- The Elements of Programming Style (Kernighan & Plauger)



Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-08 Thread Vincent Bernat
 ❦  8 mai 2019 16:23 +02, Tim Düsterhus :

>> I think, instead of After=rsyslog.service, it should be
>> After=syslog.service, then any logger daemon could be used that has
>> Alias=syslog.service.
>> 
>> https://www.freedesktop.org/wiki/Software/systemd/syslog/
>> 
>
> The HAProxy provided unit file does not include a dependency at all.
> That's a Debian specific patch to include the dependency.

I think we didn't know about the alias stuff. We'll fix that on our side
on next upload.
-- 
Make it right before you make it faster.
- The Elements of Programming Style (Kernighan & Plauger)



Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-08 Thread Tim Düsterhus
Veiko,

Am 08.05.19 um 15:44 schrieb Veiko Kukk:
> I think, instead of After=rsyslog.service, it should be
> After=syslog.service, then any logger daemon could be used that has
> Alias=syslog.service.
> 
> https://www.freedesktop.org/wiki/Software/systemd/syslog/
> 

The HAProxy provided unit file does not include a dependency at all.
That's a Debian specific patch to include the dependency.

Best regards
Tim Düsterhus



Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-08 Thread Veiko Kukk

On 2019-05-06 11:00, Tim Duesterhus wrote:

From: Apollon Oikonomopoulos 

This will allow seamless upgrades from the sysvinit system while 
respecting
any changes the users may have made. It will also make local 
configuration

easier than overriding the systemd unit file.

Note by Tim:

This GPL-2 licensed patch was taken from the Debian project at [1].

It was slightly modified to cleanly apply, because HAProxy's default 
unit
file does not include rsyslog.service as an 'After' dependency. Also 
the

subject line was modified to include the proper subsystem and severity.


I think, instead of After=rsyslog.service, it should be 
After=syslog.service, then any logger daemon could be used that has 
Alias=syslog.service.


https://www.freedesktop.org/wiki/Software/systemd/syslog/

Regards,
Veiko



Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-07 Thread William Lallemand
On Mon, May 06, 2019 at 04:07:47PM +0200, William Lallemand wrote:
> On Mon, May 06, 2019 at 02:20:32PM +0200, Vincent Bernat wrote:
> > However, many people prefer /etc/default and /etc/sysconfig to systemd
> > overrides. And for distribution, it enables a smoother transition. For
> > Debian, we would still add the EnvironmentFile directive. You could
> > still be compatible with both styles of distribution with:
> > 
> > EnvironmentFile=-/etc/default/haproxy
> > EnvironmentFile=-/etc/sysconfig/haproxy
> 
> Oh that's right, I forgot that the - was checking if the file exists, looks 
> like
> a good solution.
> 

Just pushed in master the 2 previous patches + a patch which add 
/etc/sysconfig/haproxy.

Thanks everyone.

-- 
William Lallemand



Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-06 Thread William Lallemand
On Mon, May 06, 2019 at 02:20:32PM +0200, Vincent Bernat wrote:
> However, many people prefer /etc/default and /etc/sysconfig to systemd
> overrides. And for distribution, it enables a smoother transition. For
> Debian, we would still add the EnvironmentFile directive. You could
> still be compatible with both styles of distribution with:
> 
> EnvironmentFile=-/etc/default/haproxy
> EnvironmentFile=-/etc/sysconfig/haproxy

Oh that's right, I forgot that the - was checking if the file exists, looks like
a good solution.

-- 
William Lallemand



Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-06 Thread Vincent Bernat
 ❦  6 mai 2019 13:46 +02, William Lallemand :

>> /etc/default is a debianism. Other distros use different directories, 
>> such as RedHat which uses /etc/sysconfig
>> 
>> -Patrick
>
> Hi Patrick,
>
> I don't think that's a problem, most distribution use their own unit file
> anyway, people should edit this file before using it.

One of the promise of systemd was to be able to share unit files accross
distribution. For this, systemd wants to discourage the use of
/etc/default and /etc/sysconfig (there was even a discussion about
deprecating EnvironmentFile). You can still use the EXTRAOPTS approach
by using:

ExecStart=/usr/sbin/haproxy -Ws -f $CONFIG $EXTRAOPTS

And let people override EXTRAOPTS with an override:

Environment=EXTRAOPTS=somethingmore

However, many people prefer /etc/default and /etc/sysconfig to systemd
overrides. And for distribution, it enables a smoother transition. For
Debian, we would still add the EnvironmentFile directive. You could
still be compatible with both styles of distribution with:

EnvironmentFile=-/etc/default/haproxy
EnvironmentFile=-/etc/sysconfig/haproxy
-- 
Use the "telephone test" for readability.
- The Elements of Programming Style (Kernighan & Plauger)


signature.asc
Description: PGP signature


Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-06 Thread William Lallemand
On Mon, May 06, 2019 at 07:15:11AM -0400, Patrick Hemmer wrote:
> 
> /etc/default is a debianism. Other distros use different directories, 
> such as RedHat which uses /etc/sysconfig
> 
> -Patrick

Hi Patrick,

I don't think that's a problem, most distribution use their own unit file
anyway, people should edit this file before using it.

-- 
William Lallemand



Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-06 Thread Patrick Hemmer




*From:* Tim Duesterhus [mailto:t...@bastelstu.be]
*Sent:* Monday, May 6, 2019, 07:00 EDT
*To:* haproxy@formilux.org
*Cc:* Apollon Oikonomopoulos , 
wlallem...@haproxy.com, w...@1wt.eu, ber...@debian.org, Tim Duesterhus 

*Subject:* [PATCH v2 1/2] MINOR: systemd: Use the variables from 
/etc/default/haproxy



From: Apollon Oikonomopoulos 

This will allow seamless upgrades from the sysvinit system while respecting
any changes the users may have made. It will also make local configuration
easier than overriding the systemd unit file.

Note by Tim:

This GPL-2 licensed patch was taken from the Debian project at [1].

It was slightly modified to cleanly apply, because HAProxy's default unit
file does not include rsyslog.service as an 'After' dependency. Also the
subject line was modified to include the proper subsystem and severity.

This patch may be backported to 1.9.

[1] 
https://salsa.debian.org/haproxy-team/haproxy/blob/master/debian/patches/haproxy.service-use-environment-variables.patch

Co-authored-by: Tim Duesterhus 
---
  contrib/systemd/haproxy.service.in | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/contrib/systemd/haproxy.service.in 
b/contrib/systemd/haproxy.service.in
index 74e66e302..138701223 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -3,10 +3,11 @@ Description=HAProxy Load Balancer
  After=network.target
  
  [Service]

+EnvironmentFile=-/etc/default/haproxy
  Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
-ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q
-ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE
-ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
+ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q $EXTRAOPTS
+ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE $EXTRAOPTS
+ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q $EXTRAOPTS
  ExecReload=/bin/kill -USR2 $MAINPID
  KillMode=mixed
  Restart=always


/etc/default is a debianism. Other distros use different directories, 
such as RedHat which uses /etc/sysconfig


-Patrick


[PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-06 Thread Tim Duesterhus
From: Apollon Oikonomopoulos 

This will allow seamless upgrades from the sysvinit system while respecting
any changes the users may have made. It will also make local configuration
easier than overriding the systemd unit file.

Note by Tim:

This GPL-2 licensed patch was taken from the Debian project at [1].

It was slightly modified to cleanly apply, because HAProxy's default unit
file does not include rsyslog.service as an 'After' dependency. Also the
subject line was modified to include the proper subsystem and severity.

This patch may be backported to 1.9.

[1] 
https://salsa.debian.org/haproxy-team/haproxy/blob/master/debian/patches/haproxy.service-use-environment-variables.patch

Co-authored-by: Tim Duesterhus 
---
 contrib/systemd/haproxy.service.in | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/contrib/systemd/haproxy.service.in 
b/contrib/systemd/haproxy.service.in
index 74e66e302..138701223 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -3,10 +3,11 @@ Description=HAProxy Load Balancer
 After=network.target
 
 [Service]
+EnvironmentFile=-/etc/default/haproxy
 Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
-ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q
-ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE
-ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
+ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q $EXTRAOPTS
+ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE $EXTRAOPTS
+ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q $EXTRAOPTS
 ExecReload=/bin/kill -USR2 $MAINPID
 KillMode=mixed
 Restart=always
-- 
2.21.0