inkblot / puppet-bind

18 stars 82 forks source link

Stub zones #26

Closed smithtrevor closed 9 years ago

smithtrevor commented 9 years ago

Fix the zone template to prevent notify being added for stub zones as this creates an invalid zone. Fix the zone defined type to prevent the zone file from being created in the cache dir for stub zones. With a stub zone you provide the master servers in the zone and bind will retrieve the NS records for that zone and create the zone file. Creating the empty file causes the zone to be broken after the puppet run. This is probably the case with a slave zone as well but don't have anything set up to test that right now.

inkblot commented 9 years ago

The change to the template looks good, but in the manifest shouldn't a stub zone be handled the same as a slave zone?

smithtrevor commented 9 years ago

as in changing the logic on line 49 to also prevent creation of the empty db for a slave zone? I think that it should I just didn't have time to test it for a slave zone. I can add that to it if you want though.

inkblot commented 9 years ago

Actually, I was wondering why the exclusion had to occur at all, but I think I didn't understand what behavior you were seeing. It makes sense now, I think you've found a bug in the existing zone handling for slave zones. Do you mind if I take your branch and do some additional work on it before I merge it? I can test a slave zone and I do think stubs ought to be handled the same.

smithtrevor commented 9 years ago

That works for me. No hurry either. I'm working on our DNS profile module right now but using my fork of your module to do it. Just trying to submit small PR as I find stuff.

inkblot commented 9 years ago

I've played a bit with a slave zone. The fact that puppet initializes the zone file with db.empty isn't really an issue in itself, but the content could use an adjustment. Specifically, db.empty specifies a normal refresh value. Considering its status as placeholder data, the value should probably be much much shorter, so that the name server fetches the correct zone data from the master within a reasonable time frame. The db.empty's default serial of 1 means that it should correctly accept the updated zone.

I still need to see what happens with a stub zone.

inkblot commented 9 years ago

Stub works too. I pushed my changes to the stub_zones branch in my repo. If could please merge into the PR and we'll give it a shot in your environment?

smithtrevor commented 9 years ago

I merged your branch and did some testing on both cent6 and cent7. In both cases the empty db file was replaced with times ranging from 3 to 8 minutes after the puppet run. I did test with the refresh in the SOA record set as low as 10 and got similar result, seeming to trend towards the lower end.

personally I'd prefer to exclude the creation of the file and ensure the zone works after the service refresh. However 3-8 minutes isn't the end of the world.

I'm good with merging this if you are. thanks

inkblot commented 9 years ago

So Travis is being weird. I'll take it anyway.

inkblot commented 9 years ago

PR released on the forge in version 4.0.2

inkblot commented 9 years ago

For what it's worth, I got similar convergence times between Debian 7 servers.