systemd / systemd

The systemd System and Service Manager
https://systemd.io
GNU General Public License v2.0
13.02k stars 3.73k forks source link

systemctl stop doesn't fail with some non-existent units #25708

Open calestyo opened 1 year ago

calestyo commented 1 year ago

systemd version the issue has been seen with

252.3

Used distribution

Debian sid

Linux kernel version used

6.0.0-5-amd64

CPU architectures issue was seen on

x86_64

Component

systemctl

Expected behaviour you didn't see

Not sure whether the following is actually intended and just unexpected (at least for me) or a bug.

At least I couldn't find anything in the documentation that would explain it, OTOH there seems to be only very little information on the circumstances how/when e.g. systemctl stop fails or not.
E.g. AFAICS, it's nowhere really stated, whether or not stopping non-existent unit shall fail or not.

Anyway, back to the possible issue:

# systemctl stop does-not-exist.service; echo $?
Failed to stop does-not-exist.service: Unit does-not-exist.service not loaded.
5

is kind of expected and makes sense.

The same works with a generated unit (getty@tty1.service does exist and is active):

# systemctl stop gety@tty1.service ; echo $?
Failed to stop gety@tty1.service: Unit gety@tty1.service not loaded.
5

However, when changing anything behind the @ making it thereby non-existent (well is it considered as that?), it no longer works:

# systemctl stop getty@tt1.service ; echo $?
0

The above one, I could somehow "understand" (though it's still a bit unexpected - my naive non-systemd-expert thinking would make me expect that this should fail just as above, as there is no actually existing instance of that generated unit), as it still follows the syntax.
But for:

# systemctl stop getty@tty1.servce ; echo $?
0

it should be clear that this can never be an existing unit, generated by the generator.
Yet it still succeeds.

If this isn't a bug, but expected, it would still be nice if this could somehow be documented/explained (and if it already is and just hadn't found it,... well then sorry for the noise).

Thanks, Chris.

Unexpected behaviour you saw

No response

Steps to reproduce the problem

No response

Additional program output to the terminal or log subsystem illustrating the issue

No response

Werkov commented 1 year ago

There are several ways how unit templates can be instantiated: symlinking instance name to template unit file, adding dependency via instance name or (as it seems here) lazy loading upon DBus call (not the instance argument is a free parameter, PID 1 can't simply check if it has semantics valid wrt e.g. getty-generator). So the unit which wasn't instantiated previously, would be lazily instantiated due to the stop operation, of course, it's in the inactive state, so the stop job is nop and succeeds.

The piece worth documenting may be the DBus-based instantiation.

calestyo commented 1 year ago

Well but what you write... shouldn't that at most apply to the part where I said I could still somehow understand why it doesn't complain?

Even with lazily instantiated units, it should never be possible to get at foo@bar.servie (missing c in the type)?

Werkov commented 1 year ago

Thanks for highlighting it to my blind eyes. Yes, that looks like a bug :-) as it should be caught as an invalid unit name.

calestyo commented 1 year ago

Is there, for units with an @ in the name even a concept of knowing/telling whether the unit "exists"?

I mean I can do e.g.:

# systemctl --property=LoadState,ActiveState show nonexistent.service
LoadState=not-found
ActiveState=inactive

Which tells me it's not existing (and thus not active).

Same again for:

# systemctl --property=LoadState,ActiveState show nonexistent@foobar.service
LoadState=not-found
ActiveState=inactive

But as soon as the portion before the @ exists I can do whatever I want after it, and it will tell me:

# systemctl --property=LoadState,ActiveState show getty@foo.service
LoadState=loaded
ActiveState=inactive
# systemctl --property=LoadState,ActiveState show getty@tty1.bar
LoadState=loaded
ActiveState=inactive
calestyo commented 1 year ago

Thanks for highlighting it to my blind eyes. Yes, that looks like a bug :-) as it should be caught as an invalid unit name.

Ah :-)

Okay, but even then: How do I find out whether a unit "exists" at some point in time at least, when I want to check e.g. the above getty@foo.service?

YHNdnzj commented 1 year ago

Even with lazily instantiated units, it should never be possible to get at foo@bar.servie (missing c in the type)?

If you status on the unit, you will find that it is recognized as foo@bar.servie.service, with the unit file loaded as foo@.service and bar.servie as instance name. Actually, I don't think there would be a reliable way to determine whether a unit like this exists since people are free to use any random string as their instance name :/

Werkov commented 1 year ago

Your explanation is on the spot, @YHNdnzj. I think we can't do anything else than a possible warning. (Cherry-pick/rebase commit if it makes sense to you.)

YHNdnzj commented 1 year ago

I think we can't do anything else than a possible warning. (Cherry-pick/rebase commit if it makes sense to you.)

IMHO this might be too verbose (i.e. warns on every expansion). Maybe only do it when instance name contains . or so? And it should be suppressible by --no-warn.

That said, I'm not a member of systemd, so maybe you can submit this as a PR and others can discuss on the details there :)