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

please downgrade logging about masked units that cannot be added to a transaction #5179

Closed
1 of 2 tasks
wavexx opened this issue Jan 28, 2017 · 14 comments
Closed
1 of 2 tasks
Labels
already-implemented pid1 RFE 🎁 Request for Enhancement, i.e. a feature request systemctl

Comments

@wavexx
Copy link

wavexx commented Jan 28, 2017

Submission type

  • Bug report
  • Request for enhancement (RFE)

systemd version the issue has been seen with

systemd 232

Used distribution

debian unstable

Debian ships with a system-wide user socket for the gnupg-agent in /usr/lib/systemd/user/gpg-agent.socket. This is all fine and dandy, but on server systems I want to mask these without deleting the package's files.

First, there's no way to mask these from systemctl mask. You can create links manually in /etc/systemd/user, but then systemd emits a warning for each new user session:

systemd[1604]: gpg-agent-ssh.socket: Cannot add dependency job, ignoring: Unit gpg-agent-ssh.socket is masked.

Since I disabled this intentionally, I consider the warning inappropriate.

@poettering
Copy link
Member

So, is this simply about the log level?

@poettering
Copy link
Member

And are you saying "systemctl mask --user gpg-agent.socket" doesn't work for you?

@poettering poettering added needs-reporter-feedback ❓ There's an unanswered question, the reporter needs to answer pid1 RFE 🎁 Request for Enhancement, i.e. a feature request systemctl labels Feb 1, 2017
@wavexx
Copy link
Author

wavexx commented Feb 1, 2017 via email

@poettering
Copy link
Member

doesn't "systemctl mask --global gpg-agent.socket" work for you?

@wavexx
Copy link
Author

wavexx commented Feb 1, 2017 via email

@poettering
Copy link
Member

So, I am not entirely sure I agree with downgrading this to nothing. masking after all is an exceptional operation, it always is kind of a "hack", not the default... Due to this we currently log at LOG_WARNING about it... I'd be open to downgrading this to LOG_NOTICE, or even LOG_INFO, but LOG_DEBUG appears too low...

@poettering poettering removed the needs-reporter-feedback ❓ There's an unanswered question, the reporter needs to answer label Feb 1, 2017
@poettering poettering changed the title Allow to disable distribution's /usr/lib/systemd/user units (and make it silent) please downgrade logging about masked units that cannot be added to a transaction Feb 1, 2017
@wavexx
Copy link
Author

wavexx commented Feb 1, 2017 via email

@poettering
Copy link
Member

I cannot fully agree here. I mask several units for multiple reasons. Packages may ship unit files that make no sense in your setup. This is no different than customization. masking is a first class operation for me.

Well, for that level of configuration we have "systemctl enable" and "systemctl disable".

Enabling/disabling is the regular operation. "systemctl mask" however, is the heavy hammer you normally don't need. And if you do, then you are already outside of the intended use...

@wavexx
Copy link
Author

wavexx commented Feb 1, 2017 via email

@poettering
Copy link
Member

well, they shouldn't package it like that, but instead ship this with an [install] section and enable it at package install time with "systemctl preset --global gpg-agent.socket"... that you way can "systemctl disable --global gpg-agent.socket" whenever you like.

@keszybz
Copy link
Member

keszybz commented Feb 1, 2017

Yeah, that's clearly a bug in packaging.

Nevertheless, I agree we should downgrade the warning. After all, we're warning as a result of an explicit configuration choice made by the admin, which isn't very useful: if they decided to mask some unit, they probably had a reason.

@poettering
Copy link
Member

@keszybz i can accept that.

@keszybz
Copy link
Member

keszybz commented Feb 2, 2017

Actually, the original report was wrong: we already downgraded the warning to info a while back. There was some mismatch in error codes, I'll submit a PR to clean that up, but in the few cases I checked, we always use log_info. Nothing to do here.

@keszybz keszybz closed this as completed Feb 2, 2017
keszybz added a commit to keszybz/systemd that referenced this issue Feb 2, 2017
…ollowed units

The warning "Cannot add dependency job, ignoring" was downgraded to info in one
place, but not in the other.

C.f. systemd#5179.
@keszybz
Copy link
Member

keszybz commented Feb 2, 2017

See #5204.

keszybz added a commit to keszybz/systemd that referenced this issue Feb 2, 2017
…ollowed units

The warning "Cannot add dependency job, ignoring" was downgraded to info in one
place, but not in the other.

C.f. systemd#5179.
fbuihuu added a commit to fbuihuu/systemd that referenced this issue Feb 2, 2017
This patch allows masking of .wants symlinks in /usr/lib by /dev/null symlinks
in /etc. IOW it gives the possibility to mask a dependency symlink (in .wants
or .requires), assuming that the later being located in a directory having a
'lower' precedence.

As a consequence a dependency symlink mask in /etc/ will mask all equivalent
dependency symlinks located in directories such as /run, /usr/lib. And a
dependency symlink mask in /usr/lib will has no effect on a dropin symlink
located in /run or /etc.

Consider the following example:

 /etc/systemd/system/foo.service.wants/bar.service -> /dev/null
 /usr/lib/systemd/system/foo.service.wants/bar.service -> ../bar.service

service 'foo' was installed during a package installation with a dependency on
'bar' and later sysadmin decided to disable this dependency by adding a mask in
/etc/. This results in service 'bar' not being pulled in by 'bar' anymore.

The primary use case is to allow a clean separation between service
activation/deactivation done during package installations/updates and those
done by sysadmin.

Assuming that distro dependency symlinks happens in /usr/lib only, the sysadmin
is now able to disable persistently a service by creating a mask in /etc/. This
mask will take precedence over any distro policies (usually defined by presets)
defined in /usr/lib.

The distro policies (presets) are now free to be changed without interfering
with the configuration done by the sysadmin (if any). They will still be
preserved and will still be taken into account.

The assumption on distros creating symlinks in /usr/lib only is currently not
met since this happens in /etc/. However the dropin symlink mask is the first
step to achieve this clean separation. Later changes will ensure that dropin
symlinks created during package installations/updates will happen in /usr/lib
only. See issue systemd#4830.

In order to implement the masking of dependency symlinks properly, this patch
defers the handling of those dependencies until all involved units are fully
loaded. This way all units and especially their aliases are known and a mask
can be effective on a specific unit including all of its aliases.

This is due to the way we currently handle the unit aliases (how a unit and its
aliases are "lazily" merged) and also due to the fact that we can't remove
dependencies added to a unit (until a full daemon-reload happens).

As an example:

 /etc/.../a.service.wants/b1.service -> /dev/null
 /usr/.../a.service.wants/b.service -> ../b.service

and 'b1' is an alias of 'b'. In this example, dropin symlink dependencies
pulling 'b' or any of its aliases (including 'b1') will be masked.

This patch also changes a rather odd behavior that nobody sane should rely on:
it's possible to define an alias for unit 'c' via a dependency symlink:

 /etc/.../a.service.wants/alias-for-c.service -> ../c.service

In this case 'alias-for-c' is an alias of 'c' assuming that 'alias-for-c' unit
doesn't exist.

I'm not sure why symlinks were used to define dependencies in .wants/.requires
in the first place whereas a simple empty file would have been sufficient and
unambiguous. With this patch applied, no more aliases are defined this way.

Fixes: systemd#1169
Fixes: systemd#5179
Fixes: systemd#4830
fbuihuu added a commit to fbuihuu/systemd that referenced this issue Feb 3, 2017
This patch allows masking of .wants symlinks in /usr/lib by /dev/null symlinks
in /etc. IOW it gives the possibility to mask a dependency symlink (in .wants
or .requires), assuming that the later being located in a directory having a
'lower' precedence.

As a consequence a dependency symlink mask in /etc/ will mask all equivalent
dependency symlinks located in directories such as /run, /usr/lib. And a
dependency symlink mask in /usr/lib will has no effect on a dropin symlink
located in /run or /etc.

Consider the following example:

 /etc/systemd/system/foo.service.wants/bar.service -> /dev/null
 /usr/lib/systemd/system/foo.service.wants/bar.service -> ../bar.service

service 'foo' was installed during a package installation with a dependency on
'bar' and later sysadmin decided to disable this dependency by adding a mask in
/etc/. This results in service 'bar' not being pulled in by 'bar' anymore.

The primary use case is to allow a clean separation between service
activation/deactivation done during package installations/updates and those
done by sysadmin.

Assuming that distro dependency symlinks happens in /usr/lib only, the sysadmin
is now able to disable persistently a service by creating a mask in /etc/. This
mask will take precedence over any distro policies (usually defined by presets)
defined in /usr/lib.

The distro policies (presets) are now free to be changed without interfering
with the configuration done by the sysadmin (if any). They will still be
preserved and will still be taken into account.

The assumption on distros creating symlinks in /usr/lib only is currently not
met since this happens in /etc/. However the dropin symlink mask is the first
step to achieve this clean separation. Later changes will ensure that dropin
symlinks created during package installations/updates will happen in /usr/lib
only. See issue systemd#4830.

In order to implement the masking of dependency symlinks properly, this patch
defers the handling of those dependencies until all involved units are fully
loaded. This way all units and especially their aliases are known and a mask
can be effective on a specific unit including all of its aliases.

This is due to the way we currently handle the unit aliases (how a unit and its
aliases are "lazily" merged) and also due to the fact that we can't remove
dependencies added to a unit (until a full daemon-reload happens).

As an example:

 /etc/.../a.service.wants/b1.service -> /dev/null
 /usr/.../a.service.wants/b.service -> ../b.service

and 'b1' is an alias of 'b'. In this example, dropin symlink dependencies
pulling 'b' or any of its aliases (including 'b1') will be masked.

This patch also changes a rather odd behavior that nobody sane should rely on:
it's possible to define an alias for unit 'c' via a dependency symlink:

 /etc/.../a.service.wants/alias-for-c.service -> ../c.service

In this case 'alias-for-c' is an alias of 'c' assuming that 'alias-for-c' unit
doesn't exist.

I'm not sure why symlinks were used to define dependencies in .wants/.requires
in the first place whereas a simple empty file would have been sufficient and
unambiguous. With this patch applied, no more aliases are defined this way.

Fixes: systemd#1169
Fixes: systemd#5179
Fixes: systemd#4830
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
already-implemented pid1 RFE 🎁 Request for Enhancement, i.e. a feature request systemctl
Development

No branches or pull requests

3 participants