Bug 840260 - RFE: Environment variable resolving in PIDFile=
RFE: Environment variable resolving in PIDFile=
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: systemd (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: systemd-maint
Fedora Extras Quality Assurance
:
Depends On:
Blocks: systemd-RFE
  Show dependency treegraph
 
Reported: 2012-07-15 00:28 EDT by Tom Lane
Modified: 2015-03-12 12:22 EDT (History)
12 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-04-18 10:06:04 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Tom Lane 2012-07-15 00:28:05 EDT
Description of problem:
In order to not have to rely on systemd "guessing" the main PID of the postgresql service, I tried to add
PIDFile=${PGDATA}/postmaster.pid
to postgresql.service, relying on the earlier line
Environment=PGDATA=/var/lib/pgsql/data

This results in bleats like the following in /var/log/messages:
Jul 14 23:30:42 rh3 systemd[1]: [/lib/systemd/system/postgresql.service:56] Not an absolute path, ignoring: ${PGDATA}/postmaster.pid
Shouldn't this work, and if not, why not?

Version-Release number of selected component (if applicable):
systemd-37-25.fc16.x86_64

How reproducible:
100%

Steps to Reproduce:
as above
Comment 1 Jóhann B. Guðmundsson 2012-07-15 07:17:53 EDT
You skip the PIDFile reference and only have 

Type=forking
GuessMainPID=no
ExecStart=/path/to/foo 

if you dont want systemd "guessing" the mainpid...

"Not an absolute path." is just that telling you that you need to provide an absolute path to the relevant systemd field ( there are few which you cant use Environment variables in like you are doing there ).

Out of my own administration curiosity why are you structuring the unit file like this ( using Environments that later gets used and is only used in the unit file itself instead of just providing absolute path always?

What's the use case for doing it like this?
Comment 2 Tom Lane 2012-07-15 10:53:45 EDT
(In reply to comment #1)
> What's the use case for doing it like this?

So that the same unit file can be used as the basis for running multiple postgresql installations on the same machine. The typical setup for an alternate service would require overriding PGPORT and PGDATA:

.include /lib/systemd/system/postgresql.service
[Service]
Environment=PGPORT=5433
Environment=PGDATA=/path/to/someplace

I want people to do it like that, and not copy-and-paste the whole unit file, because otherwise I can't make fixes in the base unit file that will do them any good.

So I can't hard-wire the path to the data directory; all references to it have to be conditioned off a variable.  I'm surprised that systemd has not got the concept of a variable that can be plugged into multiple settings, because surely that would come up in more cases than just postgres.

(And yes, this is a real feature that's actively used.  Note for instance in the recent thread here:
http://lists.fedoraproject.org/pipermail/devel/2012-June/169411.html
how there were immediately people asking how the legacy-scripts feature would interact with multiple postgresql services.)
Comment 3 Jóhann B. Guðmundsson 2012-07-15 12:47:50 EDT
Ah so the problem you are trying to solve is that you want to be able to run multiple postgresql instances on the same machine on different ports and we have template units to use just for that =)

Ok so let's create an up2date postgresql service to base our template later on ( the one we are shipping is a bit outdated )...

[Unit]
Description=PostgreSQL database server
Documentation=man:postgres man:initdb(1) man:pg_ctl(1
After=network.target

[Service]
Type=forking
User=postgres
Group=postgres
OOMScoreAdjust=-1000
ExecStartPre=/usr/bin/postgresql-check-db-dir /var/lib/pgsql/data
ExecStart=/usr/bin/pg_ctl start -D /var/lib/pgsql/data -s -o "-p 5432" -w -t 300
ExecStop=/usr/bin/pg_ctl stop -D /var/lib/pgsql/data -s -m fast
ExecReload=/usr/bin/pg_ctl reload -D /var/lib/pgsql/data -s
TimeoutSec=300

[Install]
WantedBy=multi-user.target
 
Now let's create the template unit in /etc/systemd/system directory called "postgresql@.service". 

[Unit]
Description=PostgreSQL database server on port %I
Documentation=man:postgres man:initdb(1) man:pg_ctl(1
After=network.target

[Service]
Type=forking
User=postgres
Group=postgres
OOMScoreAdjust=-1000
ExecStartPre=/usr/bin/postgresql-check-db-dir /var/lib/pgsql/data-%i
ExecStart=/usr/bin/pg_ctl start -D /var/lib/pgsql/data-%i -s -o "-p %i" -w -t 300
ExecStop=/usr/bin/pg_ctl stop -D /var/lib/pgsql/data-%i -s -m fast
ExecReload=/usr/bin/pg_ctl reload -D /var/lib/pgsql/data-%i -s
TimeoutSec=300

( you can ship this one along with postgresql.service they wont conflict )

Restart the systemd daemon

# systemctl daemon-reload

Create 4 new postgresql instances running on port 5433,5434,5435,5436

#for i in 5433 5434 5435 5436; do mkdir /var/lib/pgsql/data-$i; chown postgres: /var/lib/pgsql/data-$i;su -l postgres -c "initdb --pgdata='/var/lib/pgsql/data-$i' --auth='ident'" && ln -s /etc/systemd/system/postgresql@.service /etc/systemd/system/postgresql@$i.service;done

Start all the newly created services including postgresql.service

# systemctl start postgresql.service postgresql@5433.service postgresql@5434.service postgresql@5435.service postgresql@5436.service

Check if all the services are running 

# systemctl status postgresql.service postgresql@5433.service postgresql@5434.service postgresql@5435.service postgresql@5436.service

# netstat -pant | grep postgres

Voila there you have it 5 instances of postgres up and running =)
Comment 4 Jóhann B. Guðmundsson 2012-07-15 13:02:16 EDT
If this is acceptable approach to solve your issue you should note that we are still ironing few issues out with templates ( specifically enabling them which administrators currently have to do manually via symbolic links ) and if you want to implement this you probably want to create an postgresql.target and bind all the templates unit to that.
Comment 5 Tom Lane 2012-07-15 13:09:16 EDT
(In reply to comment #3)
> Voila there you have it 5 instances of postgres up and running =)

Permit me to be unimpressed.  That notation isn't going to scale nicely to anything much more complicated than one integer parameter (in fact, if I read the man page correctly, it doesn't scale to more than one parameter period, and pathnames as parameters are surely going to be a bit problematic).  You're also assuming, quite contrary to fact, that everyone wants to keep their data directory under /var/lib/pgsql/ --- it's fairly popular to want to mount it someplace else.

What I'm looking for is a variable facility.  Not a half-baked kluge.
Comment 6 Jóhann B. Guðmundsson 2012-07-15 13:24:39 EDT
Permit me to be unimpressed how much you want to hand hold administrators hands. 

If an administrator want to run postgresql instance in some other place other than the "stock" place we ship it in they just have to suck it up and modify either an template unit or the standard service units to their liking and it's just as amount of work for them doing without using environment variables as using them. 

Using environments that later get used *only* in the unit itself from my point of view is bad practices and totally unnecessary and simply should not be done et all...
Comment 7 Tom Lane 2012-07-15 13:41:04 EDT
Well, you might as well close this NOTABUG then.  But I think it's symptomatic of everything that's wrong with systemd.  You've built this toy configuration system and you're constantly surprised because it's not up to the job of replacing initscripts.  It is not good enough to tell people that they're doing it wrong because they still want features that existed for years in the initscript world.

As for admins "sucking it up" and modifying unit files, I explained why copy-and-paste is a crappy solution in comment #2: it doesn't work nicely when the package maintainer wants to change the base unit file.  You already took the opportunity to take some pot-shots at the postgresql service file above, but how can I improve it if people are using copies rather than incorporating the current file by reference?
Comment 8 Jóhann B. Guðmundsson 2012-07-15 14:29:46 EDT
"they still want features that existed for years in the initscript world"

Systemd is a completely new init system and should be looked as an such. 

Administrators just have to accept that fact or run an distribution or an OS that still uses the legacy sysv init system if they think it's so far superior to systemd. 

"but how can I"

You should not it's up to the administrator to monitor updates/upgrades and update their *customized* unit files accordingly from my pov...

I do agree with you that there are limitation with using .include which needs to be solved like the lack of the ability to overwrite entries in the parent unit file which I do believe you are trying to solve with the environment entry workaround.
Comment 9 Tomas Mraz 2012-07-16 04:37:07 EDT
(In reply to comment #8)
> I do agree with you that there are limitation with using .include which
> needs to be solved like the lack of the ability to overwrite entries in the
> parent unit file which I do believe you are trying to solve with the
> environment entry workaround.

You believe it wrong.
Comment 10 Jóhann B. Guðmundsson 2012-07-16 07:10:07 EDT
(In reply to comment #9)
> (In reply to comment #8)
> > I do agree with you that there are limitation with using .include which
> > needs to be solved like the lack of the ability to overwrite entries in the
> > parent unit file which I do believe you are trying to solve with the
> > environment entry workaround.
> 
> You believe it wrong.

Care to elaborate how I believe it wrong?

If the administrators could overwrite existing unit entries brought in by the .include they would just add

.include /lib/systemd/system/postgresql.service
[Service]
OWExecStartPre=/usr/bin/postgresql-check-db-dir mypath
OWExecStart=/usr/bin/pg_ctl start -D mypath -s -o "-p myport" -w -t 300
OWExecStop=/usr/bin/pg_ctl stop -D mypath -s -m fast
OWExecReload=/usr/bin/pg_ctl reload -D myport -s

But currently the ability to overwrite does not exist.
Comment 11 Tomas Mraz 2012-07-16 07:23:49 EDT
Because it is not about just simple override but also being able to modify the behavior of the rules specified in the parent unit file with a simple variable assignment.

Why should I have to override all the start/stop/reload/... commands. When it should be just possible to override the "mypath".
Comment 12 Jóhann B. Guðmundsson 2012-07-16 09:40:55 EDT
Why should you as an administrator have to hack either the legacy sysv init script,the unit file or use an environment file when the daemon should be able to parse configuration file(s) that defined all this at startup. 

If you actually are scaling ( think 5+ - 100 instances ) instead of just managing <5 you should be using templates for this ( And work is being to done to improve that for administrators ) and for administrators that are managing less then 5 it's hardly any issue keeping those handful of units up2date manually.
Comment 13 Lennart Poettering 2013-01-14 15:33:08 EST
I think it makes sense to extend PIDFile to do env var expansion.
Comment 14 Fedora End Of Life 2013-01-16 12:43:33 EST
This message is a reminder that Fedora 16 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 16. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as WONTFIX if it remains open with a Fedora 
'version' of '16'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version prior to Fedora 16's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that 
we may not be able to fix it before Fedora 16 is end of life. If you 
would still like to see this bug fixed and are able to reproduce it 
against a later version of Fedora, you are encouraged to click on 
"Clone This Bug" and open it against that version of Fedora.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

The process we are following is described here: 
http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Comment 15 Harald Hoyer 2013-04-10 09:16:20 EDT
Patch sent to ML
http://lists.freedesktop.org/archives/systemd-devel/2013-April/010304.html
Comment 16 Lennart Poettering 2013-04-18 10:06:04 EDT
Hmm, so after thinking about it we figured out adding this probably is the wrong thing to do. For a longer explanation, please see:

http://lists.freedesktop.org/archives/systemd-devel/2013-April/010598.html

Note You need to log in before you can comment on or make changes to this bug.