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

Building a recursor from source and major cleanup #31

Merged
merged 42 commits into from Jan 4, 2016

Conversation

Projects
None yet
3 participants
@therobot
Copy link
Contributor

commented Jan 4, 2016

This PR attempts to accomodate the valuable bits of #27 in the new attribute-based way of deploying PowerDNS servers.

In the way, a lot has been done, including a big code refactor that eliminates several recipes, prunes some attributes, removes templates and so on. Some additional functionality has been improved such us the ability to deploy pipe based backends. Documentation has alsob been improved.

A 2.0.0 release should be desirable since there is a few breaking changes:

  • Resolver no longer uses a separated template for configuration and it uses the same
    attribute (flavor) to decide the functionality, so it is not possible to install a resolver and an authoritative on the same machine anymore.
  • Only authoritative servers install or compile backends now.

This is ready for review and a release will be done soon

skemper and others added some commits Jul 6, 2015

Moving configuration recursor parts here
Adapting the existing template to be in use for
both flavors
This is no longer in use
Since relevant has been merged into _source recipe
This recipe now works for every flavor of pdns server
Additionally configuration bits have been moved over
it's designated recipe.
Removing backends coming from several locations
Since backends are exclusive from authoritative
servers, they will be exclusively configured 
there. Particularly in the attributes file of 
authoritative.
Adding bind which does require additional package
Taking into account that backends go only in
authoritative flavor
Deleting the bind backend config file
Since it makes pdns refuse to start when using
another backend. This file is installed by default
by the ubuntu package. See:

https://groups.google.com/forum/#!topic/linux.debian.bugs.dist/o0ucSTZI6AM
This is the only recipe needed
Settings are controled by attributes
Fixing how the recipe is being called
Accordingly with the new attribute based structure
Moving the backend to /var/tmp
And setting root permissions for it since pdns
user is not created yet in the fixture

### Cleanup

* Mayor code refactor

This comment has been minimized.

Copy link
@martinisoft

martinisoft Jan 4, 2016

Contributor

s/Mayor/Major/


### Enhancements

* Adds the capability of install a recursor from source

This comment has been minimized.

Copy link
@martinisoft

martinisoft Jan 4, 2016

Contributor

s/install/installing/

README.md Outdated

The different combinations of install method and functionality are controlled by the default attributes 'build_method' and 'flavor'
The different combinations of install method and functionality are handled by the attributes 'build_method' and 'flavor' located in the default attributes file. You can set this attributes accordingly to your needs.

This comment has been minimized.

Copy link
@martinisoft

martinisoft Jan 4, 2016

Contributor

s/this/these/ s/accordingly/according/

README.md Outdated

To set up an slave server, add `recipe[pdns::default]` to you run list and set the attribute `node['pdns']['flavor']` to 'slave'. Choose between 'package', 'source' with the `node['pdns']['install_method']` attribute. Tune your server specific configuration with `node['pdns']['slave']['config']`.
There is several combinations of backends or flavors available, currently a few of them have been tested, more or less the ones that represented in `.kitchen.yml` file, specifically:

This comment has been minimized.

Copy link
@martinisoft

martinisoft Jan 4, 2016

Contributor

s/is/are/ s/or/and/

@@ -31,3 +31,8 @@
default['pdns']['authoritative']['config']['guardian'] = true
default['pdns']['authoritative']['config']['default_ttl'] = '3600'

# This attribute is only required in authoritative package installs
case node['pdns']['build_method']

This comment has been minimized.

Copy link
@martinisoft

martinisoft Jan 4, 2016

Contributor

I would change this to a simple if statement versus a case since there is only one branch in this code. Also, is there is alternate way to set this because node is a fully rendered node object and prevents wrapping.

default['pdns']['source']['path'] = '/opt'
default['pdns']['source']['backends'] = %w( bind )

case node['pdns']['flavor']

This comment has been minimized.

Copy link
@martinisoft

martinisoft Jan 4, 2016

Contributor

I would change this from a case to an if/else and as mentioned earlier, want to avoid use of the node object in attributes as it prevents wrapper cookbooks since it gets evaluated too soon.

@@ -44,13 +44,19 @@ def pdns_package_module_requirements
required_packages << 'pdns-backend-pgsql'
when 'gmysql'
required_packages << 'pdns-backend-mysql'
when 'random'
# Random isn't available on Ubuntu

This comment has been minimized.

Copy link
@martinisoft

martinisoft Jan 4, 2016

Contributor

You can put a next in there under the comment to jump to the next loop iteration.

end

it 'can resolve example.com' do
expect(command('dig @localhost example.com').stdout).to \

This comment has been minimized.

Copy link
@martinisoft

martinisoft Jan 4, 2016

Contributor

For this to be a realistic test you'll want to also use the resolve cookbook to swap the default stub resolver with 127.0.0.1

This comment has been minimized.

Copy link
@therobot

therobot Jan 4, 2016

Author Contributor

Updating the default stub resolver makes the vagrant instance unable to resolve names.

This comment has been minimized.

Copy link
@martinisoft

martinisoft Jan 4, 2016

Contributor

@therobot Really? That is interesting because it should still work. That or add 8.8.8.8

@martinisoft

This comment has been minimized.

Copy link
Contributor

commented Jan 4, 2016

Thank you for addressing my concerns. Will give this my 👍

therobot added a commit that referenced this pull request Jan 4, 2016

Merge pull request #31 from aetrion/recursor_from_source
Building a recursor from source and major cleanup

@therobot therobot merged commit c9cce02 into master Jan 4, 2016

@therobot therobot deleted the recursor_from_source branch May 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.