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

dnsdist: Separate the check-config and client modes #8456

Merged
merged 3 commits into from Jan 20, 2020

Conversation

rgacogne
Copy link
Member

@rgacogne rgacogne commented Oct 23, 2019

Short description

In client (and single command) mode we do not care too much about parsing the syntax of commands that do not relate to the console itself, and we do not want to create sockets and touch files on the local filesystem while parsing the configuration.
In check-config mode, however, we want to parse as much configuration directives as possible, and we to generate some files that might be required to validate the rest of the configuration (DNSCrypt certificates and keys, OCSP responses..).

This PR separates the two modes and tries to skip only the following parts when in check-config mode, compared to 'normal' configuration parsing:

  • we still parse and create objects when processing 'addLocal()' and 'setLocal()' directives, since the actual binding is delayed anyway ;
  • we do not connect the outgoing sockets toward our backends ;
  • we do not create Protocol Buffer or DNSTap outgoing connections ;
  • we do not start the console, SNMP or webserver threads.

Should fix #7185

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@rgacogne rgacogne added this to the dnsdist-1.5.0 milestone Oct 23, 2019
@rgacogne
Copy link
Member Author

@rgacogne rgacogne commented Oct 23, 2019

One regression test fails with:

  File "/home/travis/build/rgacogne/pdns/regression-tests.dnsdist/dnsdisttests.py", line 96, in startDNSDist
    raise AssertionError('dnsdist --check-config failed: %s' % output)
AssertionError: dnsdist --check-config failed: Error creating new server: downstream weight value must be greater than 0.
Error creating new server: downstream weight value must be between 1 and 2147483647
Configuration 'configs/dnsdist_TestRoutingBadWeightWRandom.conf' OK!

which is actually an improvement but the test must be fixed.

@rgacogne
Copy link
Member Author

@rgacogne rgacogne commented Oct 26, 2019

but the test must be fixed.

Done.

@rgacogne
Copy link
Member Author

@rgacogne rgacogne commented Nov 7, 2019

The rec failure is unrelated (a rebase would fix it).

rgacogne added 3 commits Nov 12, 2019
In client (and single command) mode we do not care too much about
parsing the syntax of commands that do not relate to the console
itself, and we do not want to create sockets and touch files on the
local filesystem while parsing the configuration.
In check-config mode, however, we want to parse as much configuration
directives as possible, and we to generate some files that might be
required to validate the rest of the configuration (DNSCrypt
certificates and keys, OCSP responses..).

This PR separates the two modes and tries to skip only the following
parts when in check-config mode, compared to 'normal' configuration
parsing:
- we still parse and create objects when processing 'add*Local()'
  and 'set*Local()' directives, since the actual binding is delayed
  anyway ;
- we do not connect the outgoing sockets toward our backends ;
- we do not create Protocol Buffer or DNSTap outgoing connections ;
- we do not start the console, SNMP or webserver threads.
@rgacogne rgacogne force-pushed the ddist-config-check-test branch from b11a0c6 to 11c2231 Compare Nov 12, 2019
@rgacogne
Copy link
Member Author

@rgacogne rgacogne commented Nov 12, 2019

Rebased.

@rgacogne rgacogne merged commit 3aba21c into PowerDNS:master Jan 20, 2020
27 checks passed
@rgacogne rgacogne deleted the ddist-config-check-test branch Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant