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

Add nss-lookup to the systemd Wants targets #792

Closed
wants to merge 2 commits into from

Conversation

johnmaguire
Copy link
Collaborator

@johnmaguire johnmaguire commented Dec 12, 2022

This builds on #791 but hits two more .service files in the repository. Thanks @Fale! edit: #791 updated.

I did notice that there are some other differences in these files:

  • example and ansible scripts do not reference network-online.target, unlike Arch and Fedora.
  • Arch and ansible do not have a Before=sshd.target, Fedora and example do.

@Fale
Copy link
Contributor

Fale commented Dec 14, 2022

On the network-online.target, I had the doubt that is pointless, since I would expect nss-lokup.target to be always after network-online.target. Thought, in the documentation I've not found anything that clearly states this, so I left it with the idea that worst-case is a check that always return true, since a more stricter test is ANDed to it

I don't know enough arch to say if Before=sshd.target is safe there or not, but I would assume it is.

@johnmaguire
Copy link
Collaborator Author

Good point on network-online.target. We haven't heard reports either so I'm not sweating it.

@coderobe Apologies for the random ping, but it appears you last packaged nebula for Arch. Any thoughts on the PR?

@Fale
Copy link
Contributor

Fale commented Dec 17, 2022

I've bumped #791 since I've added additional fixes.

I've gone in the rabbit hole, and I can confirm that network-online.target can happen after nss-lookup.target, and therefore in our case we need them both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants