public inbox for [email protected]
help / color / mirror / Atom feedsystemd service files vs. postgresql9x-setup
7+ messages / 4 participants
[nested] [flat]
* systemd service files vs. postgresql9x-setup
@ 2015-08-12 14:18 Bernd Helmle <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Bernd Helmle @ 2015-08-12 14:18 UTC (permalink / raw)
To: pgsql-pkg-yum
Hi,
Looks like postgresql9[45]-setup is careless when someone tries to use them
in customized
systemd environments (e.g. via drop-in configurations[1]). Currently we do
this:
# this parsing technique fails for PGDATA pathnames containing spaces,
# but there's not much I can do about it given systemctl's output format...
PGDATA=`systemctl show -p Environment "${SERVICE_NAME}.service" |
sed 's/^Environment=//' | tr ' ' '\n' |
sed -n 's/^PGDATA=//p' | tail -n 1`
[...some more code later...]
# Get data directory from the service file
PGDATA=`sed -n 's/Environment=PGDATA=//p' "${SERVICE_FILE}"`
So we obviously overwrite any PGDATA setting returned by 'systemctl show'
earlier. If someone uses service files only, this doesn't heavily matter,
since SERVICE_FILE is tested against multiple locations. However, drop-in
configurations are ignored/overwritten with this method. I don't
understand, why we do the sed approach anyways, since 'systemctl show'
already covers all cases, afaics. So i suggest to get rid of handling the
SERVICE_FILE directly and leave 'systemctl show' alone, patch attached.
Opinions?
[1] <http://www.freedesktop.org/software/systemd/man/systemd.unit.html;
--
Thanks
Bernd
--
Sent via pgsql-pkg-yum mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-pkg-yum
Attachments:
[application/octet-stream] 0001-Get-rid-of-sed-magic-to-retrieve-PGDATA-directly-fro.patch (2.7K, 2-0001-Get-rid-of-sed-magic-to-retrieve-PGDATA-directly-fro.patch)
download | inline diff:
From a5a8f68ebcf5d2c2b4e680521c6fa916803e21a8 Mon Sep 17 00:00:00 2001
From: Bernd Helmle <[email protected]>
Date: Wed, 12 Aug 2015 15:28:51 +0200
Subject: [PATCH] Get rid of sed magic to retrieve PGDATA directly from systemd
service files.
Instead rely on systemctl only, which also allows to use systemd drop-in
configuration directives.
The former coding also contains a bug, where PGDATA is always overwritten
by settings derived from service files directly.
---
rpm/redhat/9.4/postgresql/EL-7/postgresql94-setup | 15 ---------------
rpm/redhat/9.5/postgresql/EL-7/postgresql95-setup | 15 ---------------
2 files changed, 30 deletions(-)
diff --git a/rpm/redhat/9.4/postgresql/EL-7/postgresql94-setup b/rpm/redhat/9.4/postgresql/EL-7/postgresql94-setup
index 1f0cb0f..3a745be 100644
--- a/rpm/redhat/9.4/postgresql/EL-7/postgresql94-setup
+++ b/rpm/redhat/9.4/postgresql/EL-7/postgresql94-setup
@@ -83,26 +83,11 @@ if [ x"$PGDATA" = x ]; then
exit 1
fi
-# Find the unit file for new version.
-if [ -f "/etc/systemd/system/${SERVICE_NAME}.service" ]
-then
- SERVICE_FILE="/etc/systemd/system/${SERVICE_NAME}.service"
-elif [ -f "/lib/systemd/system/${SERVICE_NAME}.service" ]
-then
- SERVICE_FILE="/lib/systemd/system/${SERVICE_NAME}.service"
-else
- echo "Could not find systemd unit file ${SERVICE_NAME}.service"
- exit 1
-fi
-
# Log file for pg_upgrade
PGUPLOG=/var/lib/pgsql/$PGMAJORVERSION/pgupgrade.log
# Log file for initdb
PGLOG=/var/lib/pgsql/9.4/initdb.log
-# Get data directory from the service file
-PGDATA=`sed -n 's/Environment=PGDATA=//p' "${SERVICE_FILE}"`
-
export PGDATA
# For SELinux we need to use 'runuser' not 'su'
diff --git a/rpm/redhat/9.5/postgresql/EL-7/postgresql95-setup b/rpm/redhat/9.5/postgresql/EL-7/postgresql95-setup
index 697e0b4..0ede51e 100644
--- a/rpm/redhat/9.5/postgresql/EL-7/postgresql95-setup
+++ b/rpm/redhat/9.5/postgresql/EL-7/postgresql95-setup
@@ -83,26 +83,11 @@ if [ x"$PGDATA" = x ]; then
exit 1
fi
-# Find the unit file for new version.
-if [ -f "/etc/systemd/system/${SERVICE_NAME}.service" ]
-then
- SERVICE_FILE="/etc/systemd/system/${SERVICE_NAME}.service"
-elif [ -f "/lib/systemd/system/${SERVICE_NAME}.service" ]
-then
- SERVICE_FILE="/lib/systemd/system/${SERVICE_NAME}.service"
-else
- echo "Could not find systemd unit file ${SERVICE_NAME}.service"
- exit 1
-fi
-
# Log file for pg_upgrade
PGUPLOG=/var/lib/pgsql/$PGMAJORVERSION/pgupgrade.log
# Log file for initdb
PGLOG=/var/lib/pgsql/9.5/initdb.log
-# Get data directory from the service file
-PGDATA=`sed -n 's/Environment=PGDATA=//p' "${SERVICE_FILE}"`
-
export PGDATA
# For SELinux we need to use 'runuser' not 'su'
--
1.8.3.1
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: systemd service files vs. postgresql9x-setup
@ 2015-08-12 23:54 Jeff Frost <[email protected]>
parent: Bernd Helmle <[email protected]>
0 siblings, 2 replies; 7+ messages in thread
From: Jeff Frost @ 2015-08-12 23:54 UTC (permalink / raw)
To: Bernd Helmle <[email protected]>; +Cc: pgsql-pkg-yum
>
> On Aug 12, 2015, at 7:18 AM, Bernd Helmle <[email protected]> wrote:
>
> Hi,
>
> Looks like postgresql9[45]-setup is careless when someone tries to use them
> in customized
> systemd environments (e.g. via drop-in configurations[1]). Currently we do
> this:
>
> # this parsing technique fails for PGDATA pathnames containing spaces,
> # but there's not much I can do about it given systemctl's output format...
> PGDATA=`systemctl show -p Environment "${SERVICE_NAME}.service" |
> sed 's/^Environment=//' | tr ' ' '\n' |
> sed -n 's/^PGDATA=//p' | tail -n 1`
>
> [...some more code later...]
>
> # Get data directory from the service file
> PGDATA=`sed -n 's/Environment=PGDATA=//p' "${SERVICE_FILE}"`
>
> So we obviously overwrite any PGDATA setting returned by 'systemctl show'
> earlier. If someone uses service files only, this doesn't heavily matter,
> since SERVICE_FILE is tested against multiple locations. However, drop-in
> configurations are ignored/overwritten with this method. I don't
> understand, why we do the sed approach anyways, since 'systemctl show'
> already covers all cases, afaics. So i suggest to get rid of handling the
> SERVICE_FILE directly and leave 'systemctl show' alone, patch attached.
>
> Opinions?
>
> [1] <http://www.freedesktop.org/software/systemd/man/systemd.unit.html;
Unfortunately, I’m not that familiar with systemd yet and Devrim is out for a few weeks, so we might not get to this very quickly.
--
Sent via pgsql-pkg-yum mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-pkg-yum
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: systemd service files vs. postgresql9x-setup
@ 2015-08-14 08:28 Bernd Helmle <[email protected]>
parent: Jeff Frost <[email protected]>
1 sibling, 0 replies; 7+ messages in thread
From: Bernd Helmle @ 2015-08-14 08:28 UTC (permalink / raw)
To: Jeff Frost <[email protected]>; +Cc: pgsql-pkg-yum
--On 12. August 2015 16:54:41 -0700 Jeff Frost <[email protected]> wrote:
> Unfortunately, I’m not that familiar with systemd yet and Devrim is out
> for a few weeks, so we might not get to this very quickly.
No worries.
--
Thanks
Bernd
--
Sent via pgsql-pkg-yum mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-pkg-yum
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: systemd service files vs. postgresql9x-setup
@ 2015-09-17 12:49 Bernd Helmle <[email protected]>
parent: Jeff Frost <[email protected]>
1 sibling, 1 reply; 7+ messages in thread
From: Bernd Helmle @ 2015-09-17 12:49 UTC (permalink / raw)
To: Jeff Frost <[email protected]>; +Cc: pgsql-pkg-yum
--On 12. August 2015 16:54:41 -0700 Jeff Frost <[email protected]> wrote:
>> Opinions?
>>
>> [1] <http://www.freedesktop.org/software/systemd/man/systemd.unit.html;
>
> Unfortunately, I’m not that familiar with systemd yet and Devrim is out
> for a few weeks, so we might not get to this very quickly.
So i'd like to step in again into this patch.
We're working with a RHEL7 environment where those drop-in configurations
are heavily used to move PGDATA to different places and i'd like to
preserve that behavior.
Recap:
postgresql9[45]-setup happily overwrites any PGDATA setting derived from
drop-in configurations by examining the service file directly and ignoring
the previous retrieved setting from systemctl.
So this looks worth to fix for any setups, even when other ways to setup
PGDATA are used.
--
Thanks
Bernd
--
Sent via pgsql-pkg-yum mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-pkg-yum
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: systemd service files vs. postgresql9x-setup
@ 2015-09-18 13:48 Devrim GÜNDÜZ <[email protected]>
parent: Bernd Helmle <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Devrim GÜNDÜZ @ 2015-09-18 13:48 UTC (permalink / raw)
To: Bernd Helmle <[email protected]>; Jeff Frost <[email protected]>; +Cc: pgsql-pkg-yum
Hi Bernd,
First of all: Thanks for the patch!
The next set of new minor releases are due in a few weeks. I think we
can apply this patch then.
(We need a issue tracker!)
Regards, Devrim
On Thu, 2015-09-17 at 14:49 +0200, Bernd Helmle wrote:
>
> --On 12. August 2015 16:54:41 -0700 Jeff Frost <[email protected]>
> wrote:
>
> > > Opinions?
> > >
> > > [1] <
> > > http://www.freedesktop.org/software/systemd/man/systemd.unit.html
> > > >
> >
> > Unfortunately, I’m not that familiar with systemd yet and Devrim is
> > out
> > for a few weeks, so we might not get to this very quickly.
>
> So i'd like to step in again into this patch.
>
> We're working with a RHEL7 environment where those drop-in
> configurations
> are heavily used to move PGDATA to different places and i'd like to
> preserve that behavior.
>
> Recap:
>
> postgresql9[45]-setup happily overwrites any PGDATA setting derived
> from
> drop-in configurations by examining the service file directly and
> ignoring
> the previous retrieved setting from systemctl.
>
> So this looks worth to fix for any setups, even when other ways to
> setup
> PGDATA are used.
>
> --
> Thanks
>
> Bernd
>
>
--
Devrim GÜNDÜZ
Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com
PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR
--
Sent via pgsql-pkg-yum mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-pkg-yum
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: systemd service files vs. postgresql9x-setup
@ 2016-03-11 12:01 Bernd Helmle <[email protected]>
parent: Devrim GÜNDÜZ <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Bernd Helmle @ 2016-03-11 12:01 UTC (permalink / raw)
To: Devrim GÜNDÜZ <[email protected]>; Jeff Frost <[email protected]>; +Cc: pgsql-pkg-yum
--On 18. September 2015 16:48:09 +0300 Devrim GÜNDÜZ <[email protected]>
wrote:
> First of all: Thanks for the patch!
>
> The next set of new minor releases are due in a few weeks. I think we
> can apply this patch then.
>
> (We need a issue tracker!)
Hmm, i've totally forgotten that. I see that it's not committed yet, so i
just go forward and ask if there's something missing we need to address?
--
Thanks
Bernd
--
Sent via pgsql-pkg-yum mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-pkg-yum
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: systemd service files vs. postgresql9x-setup
@ 2016-05-18 15:06 Strahinja Kustudić <[email protected]>
parent: Bernd Helmle <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Strahinja Kustudić @ 2016-05-18 15:06 UTC (permalink / raw)
To: Bernd Helmle <[email protected]>; +Cc: Devrim GÜNDÜZ <[email protected]>; Jeff Frost <[email protected]>; pgsql-pkg-yum
I propose the same patch with a modification to work with spaces in the
path, since systemctl prints spaces as '\x20', so we just needed one more
sed to replace that with a space.
Would also like to see this fixed in the next minor release, since it makes
it really hard to change PGDATA the correct way, which is by droping a
.conf file into /etc/systemd/system/postgresql-9.x.service.d/ directory.
Also would be really cool if the postgres rpm created the
/etc/systemd/system/postgresql-9.x.service.d/ directory, but it's not that
important as the setup scripts working correctly.
Strahinja Kustudić | Lead System Engineer | Nordeus
On Fri, Mar 11, 2016 at 1:01 PM, Bernd Helmle <[email protected]> wrote:
>
>
> --On 18. September 2015 16:48:09 +0300 Devrim GÜNDÜZ <[email protected]>
> wrote:
>
> > First of all: Thanks for the patch!
> >
> > The next set of new minor releases are due in a few weeks. I think we
> > can apply this patch then.
> >
> > (We need a issue tracker!)
>
> Hmm, i've totally forgotten that. I see that it's not committed yet, so i
> just go forward and ask if there's something missing we need to address?
>
> --
> Thanks
>
> Bernd
>
>
> --
> Sent via pgsql-pkg-yum mailing list ([email protected])
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-pkg-yum
>
--
Sent via pgsql-pkg-yum mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-pkg-yum
Attachments:
[text/x-patch] read-PGDATA-from-systemctl-and-space-in-path.patch (3.0K, 3-read-PGDATA-from-systemctl-and-space-in-path.patch)
download | inline diff:
diff --git a/rpm/redhat/9.4/postgresql/EL-7/postgresql94-setup b/rpm/redhat/9.4/postgresql/EL-7/postgresql94-setup
index 1f0cb0f..44cc090 100644
--- a/rpm/redhat/9.4/postgresql/EL-7/postgresql94-setup
+++ b/rpm/redhat/9.4/postgresql/EL-7/postgresql94-setup
@@ -73,36 +73,20 @@ case "$1" in
;;
esac
-# this parsing technique fails for PGDATA pathnames containing spaces,
-# but there's not much I can do about it given systemctl's output format...
PGDATA=`systemctl show -p Environment "${SERVICE_NAME}.service" |
sed 's/^Environment=//' | tr ' ' '\n' |
- sed -n 's/^PGDATA=//p' | tail -n 1`
+ sed -n 's/^PGDATA=//p' | tail -n 1 |
+ sed 's/\\x20/ /g'` # replace '\x20' with space
if [ x"$PGDATA" = x ]; then
echo "failed to find PGDATA setting in ${SERVICE_NAME}.service"
exit 1
fi
-# Find the unit file for new version.
-if [ -f "/etc/systemd/system/${SERVICE_NAME}.service" ]
-then
- SERVICE_FILE="/etc/systemd/system/${SERVICE_NAME}.service"
-elif [ -f "/lib/systemd/system/${SERVICE_NAME}.service" ]
-then
- SERVICE_FILE="/lib/systemd/system/${SERVICE_NAME}.service"
-else
- echo "Could not find systemd unit file ${SERVICE_NAME}.service"
- exit 1
-fi
-
# Log file for pg_upgrade
PGUPLOG=/var/lib/pgsql/$PGMAJORVERSION/pgupgrade.log
# Log file for initdb
PGLOG=/var/lib/pgsql/9.4/initdb.log
-# Get data directory from the service file
-PGDATA=`sed -n 's/Environment=PGDATA=//p' "${SERVICE_FILE}"`
-
export PGDATA
# For SELinux we need to use 'runuser' not 'su'
diff --git a/rpm/redhat/9.5/postgresql/EL-7/postgresql95-setup b/rpm/redhat/9.5/postgresql/EL-7/postgresql95-setup
index 697e0b4..c68a1a3 100644
--- a/rpm/redhat/9.5/postgresql/EL-7/postgresql95-setup
+++ b/rpm/redhat/9.5/postgresql/EL-7/postgresql95-setup
@@ -73,36 +73,20 @@ case "$1" in
;;
esac
-# this parsing technique fails for PGDATA pathnames containing spaces,
-# but there's not much I can do about it given systemctl's output format...
PGDATA=`systemctl show -p Environment "${SERVICE_NAME}.service" |
sed 's/^Environment=//' | tr ' ' '\n' |
- sed -n 's/^PGDATA=//p' | tail -n 1`
+ sed -n 's/^PGDATA=//p' | tail -n 1 |
+ sed 's/\\x20/ /g'` # replace '\x20' with space
if [ x"$PGDATA" = x ]; then
echo "failed to find PGDATA setting in ${SERVICE_NAME}.service"
exit 1
fi
-# Find the unit file for new version.
-if [ -f "/etc/systemd/system/${SERVICE_NAME}.service" ]
-then
- SERVICE_FILE="/etc/systemd/system/${SERVICE_NAME}.service"
-elif [ -f "/lib/systemd/system/${SERVICE_NAME}.service" ]
-then
- SERVICE_FILE="/lib/systemd/system/${SERVICE_NAME}.service"
-else
- echo "Could not find systemd unit file ${SERVICE_NAME}.service"
- exit 1
-fi
-
# Log file for pg_upgrade
PGUPLOG=/var/lib/pgsql/$PGMAJORVERSION/pgupgrade.log
# Log file for initdb
PGLOG=/var/lib/pgsql/9.5/initdb.log
-# Get data directory from the service file
-PGDATA=`sed -n 's/Environment=PGDATA=//p' "${SERVICE_FILE}"`
-
export PGDATA
# For SELinux we need to use 'runuser' not 'su'
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2016-05-18 15:06 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2015-08-12 14:18 systemd service files vs. postgresql9x-setup Bernd Helmle <[email protected]>
2015-08-12 23:54 ` Jeff Frost <[email protected]>
2015-08-14 08:28 ` Bernd Helmle <[email protected]>
2015-09-17 12:49 ` Bernd Helmle <[email protected]>
2015-09-18 13:48 ` Devrim GÜNDÜZ <[email protected]>
2016-03-11 12:01 ` Bernd Helmle <[email protected]>
2016-05-18 15:06 ` Strahinja Kustudić <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox