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

Use multiple upstreams for bootstrapDns. #760

Closed
ftpd opened this issue Nov 21, 2022 · 5 comments · Fixed by #782
Closed

Use multiple upstreams for bootstrapDns. #760

ftpd opened this issue Nov 21, 2022 · 5 comments · Fixed by #782
Assignees
Labels
🔨 enhancement New feature or request
Milestone

Comments

@ftpd
Copy link

ftpd commented Nov 21, 2022

Correct me if I'm wrong, but it seems now I can define only one upstream for bootstrapDns. In worst case scenario when my upstream is down, blocky won't download filters when started.

Can we have bootstrapDns.upstream as a list, maybe?

@0xERR0R
Copy link
Owner

0xERR0R commented Nov 24, 2022

Yes, you're right, you can define only one bootstrap DNS server.

It would make sense to allow multiple bootstrap DNS server. Since we allow also the definition of IP addresses (for DoH and DoT):

bootstrapDns:
      upstream: tcp-tls:dns.example.com
      ips:
      - 123.123.123.123

we can't just change upstream to a list structure (or comma separated string). I think, we need more complex structure and we should also support current config (to avoid breaking changes).

@0xERR0R 0xERR0R added the 🔨 enhancement New feature or request label Nov 24, 2022
@0xERR0R 0xERR0R added this to the future milestone Nov 24, 2022
@ThinkChaos
Copy link
Collaborator

Started looking into this.

For the config, we should be able to transparently support the old format with something like:

bootstrapDns:
  - upstream: tcp-tls:dns1.example.com
    ips:
      - 123.123.123.123
  - upstream: tcp-tls:dns2.example.com
    ips:
      - 456.456.456.456

For the implementation, there's a little refactoring needed, but I don't foresee anything major. The awkward part is that with multiple bootstrap upstreams, we want to race them using ParallelBestResolver, but that resolver uses the Bootstrap to test the upstreams it uses. So it's a dependency loop.
There's code to bypass those checks, only used by tests at the moment, so my plan is to also use that for construction as part of the Bootstrap.
Hopefully I'll make a PR soon!

@ftpd
Copy link
Author

ftpd commented Jan 18, 2023

Thanks for this, @0xERR0R and @ThinkChaos. Any idea, when it will be included in images on dockerhub/GCR?

@ThinkChaos
Copy link
Collaborator

The development tag has this already (CI had failed before, but I retried it), so if you want to try it out before a full release you can play with that.

@ftpd
Copy link
Author

ftpd commented Jan 18, 2023

Nice, thanks again.

@0xERR0R 0xERR0R modified the milestones: future, v0.21 Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants