8000 Adjust DNS settings by nodeg · Pull Request #2712 · cobbler/cobbler · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Adjust DNS settings #2712

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jul 26, 2021
Merged

Adjust DNS settings #2712

merged 6 commits into from
Jul 26, 2021

Conversation

nodeg
Copy link
Member
@nodeg nodeg commented Jul 15, 2021 •
edited
Loading

This PR fixes some issues regarding DNS:

  • Fixes Cobbler sync: DNS does not work #2710.
  • @@bind_zoneinfo@@ got not replaced during build time
  • Some links in the comments in settings.yaml
  • I left some comments in the named templates regarding the PATH
    Furthermore I rearranged some key/values in settings.yaml to have those belonging to each other closer together.

For now I leave this as a WIP since I am not completely finished testing DNS.

8000

@nodeg nodeg added the main Not a release but referring to the Git main branch label Jul 15, 2021
@nodeg nodeg added this to the v3.3.0 milestone Jul 15, 2021
@nodeg nodeg marked this pull request as draft July 15, 2021 14:57
@SchoolGuy SchoolGuy requested review from SchoolGuy and a team July 15, 2021 14:58
@nodeg
Copy link
Member Author
nodeg commented Jul 16, 2021

Syncing DNS works but we could still improve the named.template in Cobbler.

The hardcoded path bothers me. Maybe we can make this adjustable in our settings.

cobbler/templates/etc/named.template

Lines 1 to 9 in ded3e6e

options {
listen-on port 53 { 127.0.0.1; };
directory "/var/named";
dump-file "/var/named/data/cache_dump.db";
statistics-file "/var/named/data/named_stats.txt";
memstatistics-file "/var/named/data/named_mem_stats.txt";
allow-query { localhost; };
recursion yes;
};

Furthermore the logging as implemented in the config does not work under openSUSE. In Fedora 34 and Debian 10 this works.

cobbler/templates/etc/named.template

Lines 11 to 16 in ded3e6e

logging {
channel default_debug {
file "data/named.run";
severity dynamic;
};
}; 

$ journalctl-u named

Jul 15 10:52:36 localhost.localdomain systemd[1]: Starting Berkeley Internet Name Domain (DNS)...    
Jul 15 10:52:36 localhost.localdomain named[31986]: starting BIND 9.16.18 (Stable Release) <id:1c027c4>                  
Jul 15 10:52:36 localhost.localdomain named[31986]: running on Linux x86_64 5.13.0-1-default #1 SMP Fri Jul 2 05:54:32 UTC 2021 (aa40472)     
(...)
Jul 16 15:18:54 localhost.localdomain named[19937]: command channel listening on ::1#953
Jul 16 15:18:54 localhost.localdomain named[19937]: isc_stdio_open 'data/named.run' failed: file not found
Jul 16 15:18:54 localhost.localdomain named[19937]: configuring logging: file not found
Jul 16 15:18:54 localhost.localdomain named[19937]: loading configuration: file not found
Jul 16 15:18:54 localhost.localdomain named[19937]: exiting (due to fatal error)
Jul 16 15:18:54 localhost.localdomain systemd[1]: named.service: Control process exited, code=exited, status=1/FAILURE
Jul 16 15:18:54 localhost.localdomain systemd[1]: named.service: Failed with result 'exit-code'.
Jul 16 15:18:54 localhost.localdomain systemd[1]: Failed to start Berkeley Internet Name Domain (DNS).

What are your thoughts @SchoolGuy, @watologo1?

@SchoolGuy
Copy link
Member

So my opinion is the following:

  1. If we give that template some love, let's convert it to Jinja2. WIth the magic header we can use both Jinja2 for this and Cheetah a 8000 s the default for all others at the same time.
  2. We are requiring the distro module for Cobbler, thus we can use it in this template. This means we can have an include in Jinja2 which depending on the OS then lazy loads the appropriate file for both sections which are different.
  3. The approach found in the second point I made will be required for more then only the named.template, I would like to formalize that process, so we can repeat it in other templates.
  4. I personally don't see this as a blocker for the release. A template can be easily touched by an admin and they should do it, so what we deliver is a best effort not a "works-out-of-the-box". The scenarios in which Cobbler is used are too broad to do that. So once this PR is merged, I would open a second one for fixing up the template.

@codecov
Copy link
codecov bot commented Jul 22, 2021 •
edited
Loading

Codecov Report

Merging #2712 (48729ee) into master (cdfb082) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2712   +/-   ##
=======================================
  Coverage   41.48%   41.48%           
=======================================
  Files          86       86           
  Lines       13453    13453           
=======================================
  Hits         5581     5581           
  Misses       7872     7872
Impacted Files Coverage Δ
cobbler/modules/managers/bind.py 9.19% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cdfb082...48729ee. Read the comment docs.

@nodeg
Copy link
Member Author
nodeg commented Jul 26, 2021

Rebased onto master.

@nodeg
Copy link
Member Author
nodeg commented Jul 26, 2021

I was not able to find any other issues besides the things @SchoolGuy mentioned. I think the PR is ready so far and everything template related should be done in a seperate PR.

@nodeg nodeg marked this pull request as ready for review July 26, 2021 12:34
@nodeg nodeg changed the title WIP: Adjust DNS settings Adjust DNS settings Jul 26, 2021
nodeg added 4 commits July 26, 2021 14:37
@nodeg
fix: NetworkInterface Object not subscriptable
42f9e97 
Signed-off-by: Dominik Gedon <dgedon@suse.de>
@nodeg
Rearange key/values in settings.yaml
b57bfe2 
Signed-off-by: Dominik Gedon <dgedon@suse.de>
@nodeg
fix: bind_zonefiles not getting replaced
10ff603 
Signed-off-by: Dominik Gedon <dgedon@suse.de>
@nodeg
style: Add comments regarding path in named
db85870 
On SUSE distros the path for named is not /var/named but /var/lib/named

Signed-off-by: Dominik Gedon <dgedon@suse.de>
SchoolGuy
SchoolGuy approved these changes Jul 26, 2021
View reviewed changes
Copy link
Member
@SchoolGuy SchoolGuy left a comment

Choose a reason for hiding this comment

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

LGTM

@SchoolGuy SchoolGuy self-assigned this Jul 26, 2021
@nodeg nodeg requested a review from watologo1 July 26, 2021 12:39
@nodeg
refactor: Remove unnecessary list() method
29672b0 
Signed-off-by: Dominik Gedon <dgedon@suse.de>
@agraul
Copy link
Contributor
agraul commented Jul 26, 2021

So my opinion is the following:

1. If we give that template some love, let's convert it to Jinja2. WIth the magic header we can use both Jinja2 for this and Cheetah as the default for all others at the same time.

I don't fully understand the second part. Do you want to replace Cheetah with Jinja2 for only some of the templates? Or do you want to use Jinja2 for everything if the system running Cobbler supports it and Cheetah otherwise? Or just move to Jinja2 completely? Personally I think that only one templating system should be used for each Cobbler version and changing it should only be done in a major release (without traces of the old templating system.)

Is this what you have in mind yourself @SchoolGuy ?

@nodeg
fixup: Adjust path in bind templates
48729ee 
This will remove the static path for named and replace it with the one
defined in setup.py. Thank you @watologo1 for the suggestion!

Signed-off-by: Dominik Gedon <dgedon@suse.de>
@SchoolGuy
Copy link
Member

@agraul The plan was for 3.4.0 to convert all Cobbler default templates to Jinja2 and let the default at Cheetah, so we don't break templates without a "shebang", the Cobbler version after that would then switch to Jinja2 as a default and with 4.0.0 I would remove the Cheetah Support and only allow Jinja2. I would announce that with the release of Cobbler 3.3.0 to the broader public.

watologo1
watologo1 approved these changes Jul 26, 2021
View reviewed changes
Copy link
Contributor
@watologo1 watologo1 left a comment

Choose a reason for hiding this comment

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

Nice cleanup!

@watologo1 watologo1 merged commit ab95091 into master Jul 26, 2021
@watologo1 watologo1 deleted the fix/sync-dns branch July 26, 2021 15:27
@th-2021 th-2021 mentioned this pull request Nov 11, 2021
Closed
@parkour99 parkour99 mentioned this pull request Dec 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@watologo1 watologo1 watologo1 approved these changes

@SchoolGuy SchoolGuy SchoolGuy approved these changes

Assignees

@SchoolGuy SchoolGuy

Labels
main Not a release but referring to the Git main branch
Projects
None yet
Milestone
v3.3.0
Development

Successfully merging this pull request may close these issues.

Cobbler sync: DNS does not work
4 participants
@nodeg @SchoolGuy @agraul @watologo1
0