Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFE: allow variable expansion in command in ExecStart #1274

Open
inducer opened this issue Sep 15, 2015 · 5 comments
Open

RFE: allow variable expansion in command in ExecStart #1274

inducer opened this issue Sep 15, 2015 · 5 comments
Labels
pid1 RFE 🎁 Request for Enhancement, i.e. a feature request

Comments

@inducer
Copy link

inducer commented Sep 15, 2015

Guided by the (broken) example:

https://celery.readthedocs.org/en/latest/tutorials/daemonizing.html#usage-systemd

I got this:

[/etc/systemd/system/celery.service:22] Executable path is not absolute, ignoring: $CELERY_BIN multi stopwait $CELERYD_NODES      --pidfile=$CELERYD_PID_FILE

Nothing clued me in (at least for a while) that using a variable as the first argument wasn't acceptable. Could you test for that and provide a more informative message? Think of the man-(/woman-)hours saved! :)

@poettering poettering added RFE 🎁 Request for Enhancement, i.e. a feature request pid1 needs-better-log-message labels Sep 18, 2015
@XANi
Copy link

XANi commented Mar 1, 2017

Note that the check itself is incorrect, it should expand variables before checking if it is absolute or not.

@keszybz
Copy link
Member

keszybz commented May 21, 2019

This now prints:

systemd[1]: /etc/systemd/system/bad-execstart.service:4: Executable "$CELERY_BIN" not found in path "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin".

And the service is not started at all. So I think the "bettter error message" part is handled.

The question remains: should we allow variables to be used in ExecStart's first field?

@keszybz keszybz changed the title Should provide a more obvious error message if Command in ExecStart is a variable RFE: allow variable expansion in command in ExecStart May 21, 2019
@twojstaryzdomu
Copy link

twojstaryzdomu commented May 3, 2022

This now prints:

systemd[1]: /etc/systemd/system/bad-execstart.service:4: Executable "$CELERY_BIN" not found in path "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin".

And the service is not started at all. So I think the "bettter error message" part is handled.

The question remains: should we allow variables to be used in ExecStart's first field?

Since has been closed #23235, I'll give you a few arguments:

  1. It's inconsistent with the variables already allowed in the commandline parameters (argc > 1). Disallowing variable use in command runs contrary to the expectation of most users that use variables elsewhere in an Exec* statement.
  2. The restriction is responsible countless lost hours of user productivity trying to figure out why their variables worked as parameters and not as the command.
  3. It leads to bad quality work-arounds: encapsulating the command in an overarching shell command or plain including a shell script as command.
    This introduces its share of other problems, one being the need to quote them properly ($${VARIABLE}), and many users again lose productivity trying to adjust their parameters due to the use of shell, merely to work around the restriction.
    The alternatives that have been suggested elsewhere like the use of generators seem overkill for the simple task of specifying a command via a variable in an existing service.
  4. It makes some tasks like upgrades more difficult to accomplish, where the absolute path of the command (including the binary name) needs to change. The overrides that involve a drop-in require a user acquaint themselves with the quirky way of overriding an Exec* statement (i.e. first reset its value with an empty statement, then define it). Again lost productivity. Not to mention the redundancy in having to repeat the entire statement with the parameters. Room for error.

Contrast that with the simple definition of a command in an environment file. A user will avoid all of those roadblocks.

Variables are simple enough and answer a legitimate need of many users. If they're already permitted as parameters, it is only natural to permit them for use as command. I understand there may be political arguments at stake why this hasn't yet gone through.

@dtardon
Copy link
Collaborator

dtardon commented May 4, 2022

Since has been closed #23235, I'll give you a few arguments:

  1. It's inconsistent with the variables already allowed in the commandline parameters (argc > 1). Disallowing variable use in command runs contrary to the expectation of most users that use variables elsewhere in an Exec* statement.

That depends on how one looks about it. E.g., my view is that the command line arguments are configuration, therefore it kinda makes sense to allow to pass them using env. vars (and many init scripts used to do this). The command itself is not configuration.

  1. The restriction is responsible countless lost hours of user productivity trying to figure out why their variables worked as parameters and not as the command.

[citation needed]

  1. It leads to bad quality work-arounds: encapsulating the command in an overarching shell command or plain including a shell script as command.
    This introduces its share of other problems, one being the need to quote them properly ($${VARIABLE}), and many users again lose productivity trying to adjust their parameters due to the use of shell, merely to work around the restriction.

Again, [citation needed]. So far you haven't provided any credible real world use case for this.

  1. It makes some tasks like upgrades more difficult to accomplish, where the absolute path of the command (including the binary name) needs to change. The overrides that involve a drop-in require a user acquaint themselves with the quirky way of overriding an Exec* statement (i.e. first reset its value with an empty statement, then define it). Again lost productivity. Not to mention the redundancy in having to repeat the entire statement with the parameters. Room for error.

Why? Either the unit file is a part of the service being upgraded, in which case it should already use the right path; if it's not, it's a bug. Or it's a unit file written in-house for a third-party service that doesn't have systemd support itself, in which case the logical thing is to update the unit before deployment. I don't see any advantage in setting the new path indirectly via an env. file.

I understand there may be political arguments at stake why this hasn't yet gone through.

Or maybe just nobody cares about this. The fact that you are only the second person to complain about it in all those years certainly gives credibility to that view...

@twojstaryzdomu
Copy link

twojstaryzdomu commented May 4, 2022

Since has been closed #23235, I'll give you a few arguments:

  1. It's inconsistent with the variables already allowed in the commandline parameters (argc > 1). Disallowing variable use in command runs contrary to the expectation of most users that use variables elsewhere in an Exec* statement.

That depends on how one looks about it. E.g., my view is that the command line arguments are configuration, therefore it kinda makes sense to allow to pass them using env. vars (and many init scripts used to do this). The command itself is not configuration.

Many users would beg to differ. Especially those that wrap their commands through shell. I doesn't matter what you consider configuration but what users do and expect.

The command name becomes part of configuration the moment it is entered into a service configuration file. Your view it isn't doesn't change that fact. Everything in a service file is configuration.

  1. The restriction is responsible countless lost hours of user productivity trying to figure out why their variables worked as parameters and not as the command.

[citation needed]

No citation needed but see below. You have the ability to look through the support case base at what stuff users arrive at in the systemd services.
But really it takes a bit of imagination and abandoning a best-case scenario, tunnel vision approach where there is in fact a steep learning curve to systemd.
Realistically, you assume for the good of the user quirks and design inconsistencies like this (${0} = fail to resolve vars, ${n} > 0 = resolve vars) take time to find the reason for and work-around.

Besides, that's the logical assumption to be drawn from any example that uses a shell-based work-around. User COULD avoid part of that research effort in many cases, were variable expansion available for the command.

And last but not least, I'm speaking on behalf of several users that voiced their gripes with systemd. This did come up.

  1. It leads to bad quality work-arounds: encapsulating the command in an overarching shell command or plain including a shell script as command.
    This introduces its share of other problems, one being the need to quote them properly ($${VARIABLE}), and many users again lose productivity trying to adjust their parameters due to the use of shell, merely to work around the restriction.

Again, [citation needed]. So far you haven't provided any credible real world use case for this.

No citation needed but see below. You didn't ever see a work-around that uses a shell script or an encapsulated shell one-liner? Search your support case base. See the examples I linked to below.
Is it a good quality work-around or not to use the shell? Care to comment as an employee of a systemd sponsor (according to your github profile) that supports systemd commercially in their product?

  1. It makes some tasks like upgrades more difficult to accomplish, where the absolute path of the command (including the binary name) needs to change. The overrides that involve a drop-in require a user acquaint themselves with the quirky way of overriding an Exec* statement (i.e. first reset its value with an empty statement, then define it). Again lost productivity. Not to mention the redundancy in having to repeat the entire statement with the parameters. Room for error.

Why? Either the unit file is a part of the service being upgraded, in which case it should already use the right path; if it's not, it's a bug. Or it's a unit file written in-house for a third-party service that doesn't have systemd support itself, in which case the logical thing is to update the unit before deployment. I don't see any advantage in setting the new path indirectly via an env. file.

Software that is installed to a versioned directory is one example. The logical thing is to allow a flexible configuration that doesn't discriminate against a variable in the command, for user's convenience. Users receive tons of things with systemd they didn't ask for, but their calls to fix political things like this go unanswered.

Again, your employment at a sponsor of systemd makes this response disingenuous.

I understand there may be political arguments at stake why this hasn't yet gone through.

Or maybe just nobody cares about this. The fact that you are only the second person to complain about it in all those years certainly gives credibility to that view...

Or perhaps they can't be bothered to open a bug? Or they don't know the facilities, they don't know how or where and when they find the solution, they are happy the shell-workaround? The widespread use of those work-arounds vindicates that view.

Only if you were to outright ban the use of the shell work-around and scripts with a future release, would you really know how its users feel about this restriction not to allow variables in place of the command.

Besides, you're not counting right, it's at least a 5th such call for expanding variables in the command position, you need to include this:

  1. Make environment expansion in ExecStart= and friends optional #11654
  2. RFE: support specifier expansion for first word of ExecStart= directives #3061
  3. resolve variables in place of the command in ExecStart/ExecStop statements  #23235 (disclosure, my call opened prior to this discussion).
  4. systemctl start - option to set environment variables for the service #9370
  5. in addition to this issue (RFE: allow variable expansion in command in ExecStart #1274).

See #11654 (comment) & #9370 (comment), answers your calls for a real-world use cases. See the work around in #6035 (comment).

Need more? I'll oblige you with the search for ExecStart=$. I see 657 000 entries.

How many entries with "ExecStart=$" turn up in your company's support case records?

Contrary to your idealised views, this is NOT an imaginary problem!

It's funny though that you choose to comment on it not being a political argument why it's not resolved since 2015.

Are you commenting here as an employee of a company sponsoring and supporting systemd commercially in a product, on company's orders, or privately? I'm flattered either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pid1 RFE 🎁 Request for Enhancement, i.e. a feature request
Development

No branches or pull requests

6 participants