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

net/renderer/sysconfig: also use dns information from interface config #5401

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

ani-sinha
Copy link
Contributor

@ani-sinha ani-sinha commented Jun 12, 2024

sysconfig renderer currently only uses global dns and search domain
configuration in order to populate /etc/resolv.conf. This means it ignores
interface specific dns configuration completely. This means, when global dns
information is absent and only interface specific dns configuration is present,
/etc/resolv.conf will not have complete dns information. Fix this so that
per interface dns information is also taken into account along with global dns
configuration in order to populate /etc/resolv.conf.

Fixes: GH-5400

tox results looks good

  py3: OK (76.88=setup[0.03]+cmd[76.85] seconds)
  black: OK (0.51=setup[0.00]+cmd[0.51] seconds)
  ruff: OK (0.03=setup[0.00]+cmd[0.02] seconds)
  isort: OK (0.79=setup[0.00]+cmd[0.79] seconds)
  mypy: OK (9.27=setup[0.00]+cmd[9.27] seconds)
  pylint: OK (14.00=setup[0.01]+cmd[13.99] seconds)
  congratulations :) (101.54 seconds)

@ani-sinha ani-sinha force-pushed the fix-sysconfig-dns branch 2 times, most recently from ece2bf6 to 94ad377 Compare June 13, 2024 08:06
@catmsred
Copy link
Collaborator

Thanks for finding and quickly fixing this issue! I'm wondering if we should check the other renderers to see if this problem persists across them. Is it worth adding a unit test to capture this edge case?

@ani-sinha
Copy link
Contributor Author

Thanks for finding and quickly fixing this issue! I'm wondering if we should check the other renderers to see if this problem persists across them. Is it worth adding a unit test to capture this edge case?

yes I got lazy and did not add the unit test changes. I did now. It should check most combinations if not all.
As for other renderers, I do not know much about them. Red Hat only cares about sysconfig and network manager.

sysconfig renderer currently only uses global dns and search domain
configuration in order to populate /etc/resolv.conf. This means it ignores
interface specific dns configuration completely. This means, when global dns
information is absent and only interface specific dns configuration is present,
/etc/resolv.conf will not have complete dns information. Fix this so that
per interface dns information is also taken into account along with global dns
configuration in order to populate /etc/resolv.conf.

Fixes: canonicalGH-5400
Signed-off-by: Ani Sinha <anisinha@redhat.com>
@TheRealFalcon
Copy link
Member

Thanks for addressing this!

Network ifcfg files support DNS and search, correct? For DNS defined at the interface level, shouldn't we store DNS there instead?

E.g., if I use network v2:

network:
  version: 2
  ethernets:
    eth0:
      nameservers:
        search: [lab, home]
        addresses: [8.8.8.8, FEDC::1]
      addresses:
        - 192.168.1.20/16
        - 2001:bc8:1210:232:dc00:ff:fe20:185/64

I get a config file like:

# Created by cloud-init automatically, do not edit.
#
BOOTPROTO=none
DEVICE=eth0
DNS1=8.8.8.8
DNS2=FEDC::1
DOMAIN="lab home"
IPADDR=192.168.1.20
IPV6ADDR=2001:bc8:1210:232:dc00:ff:fe20:185/64
IPV6INIT=yes
IPV6_AUTOCONF=no
IPV6_FORCE_ACCEPT_RA=no
NETMASK=255.255.0.0
ONBOOT=yes
TYPE=Ethernet
USERCTL=no

and nothing in resolv.conf.

The code to do it is here, though it looks like it's only limited static addresses. If that restriction was removed, I believe it should also work for v1.

Also, do you know how much usage the sysconfig renderer gets these days? I was under the assumption that the network-manager renderer is used on RHEL and RHEL-derived systems these days and that the sysconfig renderer is more-or-less legacy. Is that not true?

@TheRealFalcon TheRealFalcon self-assigned this Jun 20, 2024
@ani-sinha
Copy link
Contributor Author

ani-sinha commented Jun 20, 2024

Thanks for addressing this!

Network ifcfg files support DNS and search, correct? For DNS defined at the interface level, shouldn't we store DNS there instead?

Yes we are doing that already.

E.g., if I use network v2:

network:
  version: 2
  ethernets:
    eth0:
      nameservers:
        search: [lab, home]
        addresses: [8.8.8.8, FEDC::1]
      addresses:
        - 192.168.1.20/16
        - 2001:bc8:1210:232:dc00:ff:fe20:185/64

I get a config file like:

# Created by cloud-init automatically, do not edit.
#
BOOTPROTO=none
DEVICE=eth0
DNS1=8.8.8.8
DNS2=FEDC::1
DOMAIN="lab home"

Yes so here the search domain is added.

IPADDR=192.168.1.20
IPV6ADDR=2001:bc8:1210:232:dc00:ff:fe20:185/64
IPV6INIT=yes
IPV6_AUTOCONF=no
IPV6_FORCE_ACCEPT_RA=no
NETMASK=255.255.0.0
ONBOOT=yes
TYPE=Ethernet
USERCTL=no

and nothing in resolv.conf.

Indeed and this is where the problem lies. It should be added to resolv.conf as well. At least that is what the spec also says. See my description in #5400 .

The code to do it is here, though it looks like it's only limited static addresses. If that restriction was removed, I believe it should also work for v1.

Also, do you know how much usage the sysconfig renderer gets these days?

sysconfig renderer in RHEL9 is the default and is widely used. network manager renderer in RHEL 9 is "opt in".

I was under the assumption that the network-manager renderer is used on RHEL and RHEL-derived systems these days and that the sysconfig renderer is more-or-less legacy. Is that not true?

No. For RHEL9, sysconfig is default and network manager is opt in. For RHEL10, we plan to make network manager the default. sysconfig renderer probably won't work since support for ifcfg files with use of network manager will be dropped.
So for widely used RHEL9 releases, we do need to support sysconfig for the forseeable future.

Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

At least that is what the spec also says.

Just to be clear, this is documentation and not a spec. In this case, the documentation is incorrect because sysconfig is the only renderer that modifies resolv.conf at all, and so it really isn't valid for any other renderers.

While I still think using interface-specific ifcfg options is preferable, this does solve the problem and will improve things when using sysconfig, so +1 from me.

@TheRealFalcon TheRealFalcon merged commit 1b8030e into canonical:main Jun 20, 2024
23 checks passed
holmanb pushed a commit that referenced this pull request Jun 28, 2024
sysconfig renderer currently only uses global dns and search domain
configuration in order to populate /etc/resolv.conf. This means it ignores
interface specific dns configuration completely. This means, when global dns
information is absent and only interface specific dns configuration is present,
/etc/resolv.conf will not have complete dns information. Fix this so that
per interface dns information is also taken into account along with global dns
configuration in order to populate /etc/resolv.conf.

Fixes: GH-5400

Signed-off-by: Ani Sinha <anisinha@redhat.com>
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.

search domains missing in /etc/resolv.conf
3 participants