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

fix: ensure systemd-resolved is set by default #344

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tulilirockz
Copy link
Collaborator

Fixes: #246

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Feb 26, 2025
@tulilirockz tulilirockz requested a review from bsherman February 26, 2025 20:32
Comment on lines 27 to 28
# Resolved by default as DNS resolver
ln -sf /run/systemd/resolve/resolv.conf /etc/resolv.conf
Copy link
Member

@p5 p5 Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it best to symlink this here, or via tmpfiles?
Is /etc/resolv.conf something the user may want to change?

This may be perfectly fine - I'm just not sure of any implications of symlinking via the container build if the user wants to update the config file


I guess it should be fine - the symlink will just mean the changes on a running system are accessible in /run/systemd/..?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably here, if there is any /etc/resolv.conf file there by default it doesnt get replaced by networkmanager or tailscale. We can iterate on this later if for whatever reason this breaks

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it should be here (or some similarly appropriate location in the Container build). It should not be in tmpfiles since tmpfiles may override a user's change, but doing it here we default to the desired behavior (using systemd-resolved's stub config) but anyone can modify this.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably should change this symlink however...

ln -sf ../run/systemd/resolve/stub-resolv.conf /etc/resolv.conf is how this is configured on Fedora

@tulilirockz tulilirockz linked an issue Feb 26, 2025 that may be closed by this pull request
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 26, 2025
@tulilirockz
Copy link
Collaborator Author

image
It will need to be done via systemd-tmpfiles as the container build doesnt let us that.

@tulilirockz tulilirockz marked this pull request as draft February 26, 2025 20:41
@bsherman
Copy link

image It will need to be done via systemd-tmpfiles as the container build doesnt let us that.

why?

@tulilirockz
Copy link
Collaborator Author

@bsherman When we build stuff, /etc/resolv.conf is automatically bound from the host so that we have a working network connection inside of the container. We cant override that file as it is a read-only mountpoint from podman

@bsherman
Copy link

@bsherman When we build stuff, /etc/resolv.conf is automatically bound from the host so that we have a working network connection inside of the container. We cant override that file as it is a read-only mountpoint from podman

I thought it would be something like this.

I'm quite strongly opposed to the use of tmpfiles.d for this symlink. So perhaps we need to change that flow.

It may work to follow the lead of systemd-resolved itself...
We could:

  1. use a podman bind mount of the host's /etc/resolv.conf to /run/systemd/resolve/resolv.conf
  2. first thing in Containerfile, create the symlink ln -sf ../run/systemd/resolve/stub-resolv.conf /etc/resolv.conf

That should work for DNS lookups from within container, and create the symlink pointing to the desired location.
MAYBE we should not use that specific path and instead could bind mount to /tmp/host-resolv.conf but then we'd have to do a dance of setting this up once and fixing it up at the end of the build.

@tulilirockz tulilirockz marked this pull request as ready for review February 27, 2025 05:08
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Feb 27, 2025
# Necessary so that we get DNS resolution when podman build --dns none is specified
# FIXME: Maybe figure out some better solution for this
cp -f /etc/resolv-host.conf /etc/resolv.conf
cat /etc/resolv.conf

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tulilirockz I think you've confirmed this approach is working, correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works locally, but github actions hates it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this would work locally unless you aren't using systemd-resolved on your local build host.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tailscale Tailscale does not work due to systemd-resolved not running
3 participants