Merge lp:~jtv/maas/independence-for-dnsconfig into lp:~maas-committers/maas/trunk

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 1836
Proposed branch: lp:~jtv/maas/independence-for-dnsconfig
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 204 lines (+38/-61)
3 files modified
src/maasserver/management/commands/get_named_conf.py (+2/-2)
src/provisioningserver/dns/config.py (+26/-35)
src/provisioningserver/dns/tests/test_config.py (+10/-24)
To merge this branch: bzr merge lp:~jtv/maas/independence-for-dnsconfig
Reviewer Review Type Date Requested Status
Raphaël Badin (community) Approve
Review via email: mp+202459@code.launchpad.net

Commit message

Take DNSConfig out of the DNSConfigBase class hierarchy.

Description of the change

This will allow me to simplify the data paths within the remaining hierarchy later. It also makes it easier to tailor the write_config implementations for DNS config files and zone files. For example, it doesn't seem as if the one for zone files actually uses its kwargs. I also isolated get_include_snippet a bit more by making it a class method. Tightening up the data flows creates elbow room for the changes that these branches ultimately support: multiple managed interfaces per cluster.

After this, I can start consolidating the remaining DNSConfigBase hierarchy: DNSConfigBase ← DNSZoneConfig ← {DNSForwardZoneConfig, DNSReverseZoneConfig}.

You may notice how DNSConfig comes out as more of a function than a class. Actual use in the MAAS codebase instantiates the class, immediately calls the object's write_config method, and then forgets about the object. I kept the class because these classes were always intended mostly as a kind of sub-module. It's a friendly way of grouping together the writing of the DNS config and generation of the matching DNS include snippet.

Jeroen

To post a comment you must log in.
Revision history for this message
Raphaël Badin (rvb) wrote :

I must say this is starting to come together pretty nicely!

review: Approve
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Thanks. Lots more to do though. :)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/management/commands/get_named_conf.py'
2--- src/maasserver/management/commands/get_named_conf.py 2013-10-18 16:57:37 +0000
3+++ src/maasserver/management/commands/get_named_conf.py 2014-01-21 13:43:39 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2012 Canonical Ltd. This software is licensed under the
6+# Copyright 2012-2014 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 """Django command: get the named configuration snippet used to hook
10@@ -54,7 +54,7 @@
11 def handle(self, *args, **options):
12 edit = options.get('edit')
13 config_path = options.get('config_path')
14- include_snippet = DNSConfig().get_include_snippet()
15+ include_snippet = DNSConfig.get_include_snippet()
16
17 if edit is True:
18 with open(config_path, "ab") as conf_file:
19
20=== modified file 'src/provisioningserver/dns/config.py'
21--- src/provisioningserver/dns/config.py 2014-01-21 12:29:35 +0000
22+++ src/provisioningserver/dns/config.py 2014-01-21 13:43:39 +0000
23@@ -260,22 +260,8 @@
24 parameters used when rendering the template."""
25 return {}
26
27- def write_config(self, overwrite=True, **kwargs):
28- """Write out this DNS config file.
29-
30- This raises DNSConfigDirectoryMissing if any
31- "No such file or directory" error is raised because that would mean
32- that the directory containing the write to be written does not exist.
33- """
34- content = render_dns_template(
35- self.template_path, kwargs, self.get_context())
36- with report_missing_config_dir():
37- atomic_write(
38- content, self.target_path, overwrite=overwrite,
39- mode=self.access_permissions)
40-
41-
42-class DNSConfig(DNSConfigBase):
43+
44+class DNSConfig:
45 """A DNS configuration file.
46
47 Encapsulation of DNS config templates and parameter substitution.
48@@ -284,29 +270,35 @@
49 template_file_name = 'named.conf.template'
50 target_file_name = MAAS_NAMED_CONF_NAME
51
52- def __init__(self, zones=()):
53+ def __init__(self, zones=None):
54+ if zones is None:
55+ zones = ()
56 self.zones = zones
57- return super(DNSConfig, self).__init__()
58-
59- @property
60- def template_path(self):
61- return os.path.join(self.template_dir, self.template_file_name)
62-
63- @property
64- def target_path(self):
65- return os.path.join(self.target_dir, self.target_file_name)
66-
67- def get_context(self):
68- return {
69+
70+ def write_config(self, overwrite=True, **kwargs):
71+ """Write out this DNS config file.
72+
73+ :raises DNSConfigDirectoryMissing: if the DNS configuration directory
74+ does not exist.
75+ """
76+ template_path = locate_config(TEMPLATES_DIR, self.template_file_name)
77+ context = {
78 'zones': self.zones,
79 'DNS_CONFIG_DIR': conf.DNS_CONFIG_DIR,
80 'named_rndc_conf_path': get_named_rndc_conf_path(),
81 'modified': unicode(datetime.today()),
82 }
83+ content = render_dns_template(template_path, kwargs, context)
84+ target_path = compose_config_path(self.target_file_name)
85+ with report_missing_config_dir():
86+ atomic_write(content, target_path, overwrite=overwrite, mode=0644)
87
88- def get_include_snippet(self):
89- assert '"' not in self.target_path, self.target_path
90- return 'include "%s";\n' % self.target_path
91+ @classmethod
92+ def get_include_snippet(cls):
93+ target_path = compose_config_path(cls.target_file_name)
94+ assert '"' not in target_path, (
95+ "DNS config path contains quote: %s." % target_path)
96+ return 'include "%s";\n' % target_path
97
98
99 def shortened_reversed_ip(ip, byte_num):
100@@ -359,7 +351,7 @@
101 return os.path.join(
102 self.target_dir, 'zone.%s' % self.zone_name)
103
104- def write_config(self, **kwargs):
105+ def write_config(self):
106 """Write out this DNS zone file.
107
108 There is a subtlety with zone files: their filesystem timestamp must
109@@ -367,8 +359,7 @@
110 support a resolution of one second, and so this method may set an
111 unexpected modification time in order to maintain that property.
112 """
113- content = render_dns_template(
114- self.template_path, kwargs, self.get_context())
115+ content = render_dns_template(self.template_path, self.get_context())
116 with report_missing_config_dir():
117 incremental_write(
118 content, self.target_path, mode=self.access_permissions)
119
120=== modified file 'src/provisioningserver/dns/tests/test_config.py'
121--- src/provisioningserver/dns/tests/test_config.py 2014-01-21 12:29:35 +0000
122+++ src/provisioningserver/dns/tests/test_config.py 2014-01-21 13:43:39 +0000
123@@ -296,19 +296,9 @@
124 class TestDNSConfig(MAASTestCase):
125 """Tests for DNSConfig."""
126
127- def test_DNSConfig_defaults(self):
128- dnsconfig = DNSConfig()
129- self.assertEqual(
130- (
131- locate_config(TEMPLATES_DIR, 'named.conf.template'),
132- os.path.join(conf.DNS_CONFIG_DIR, MAAS_NAMED_CONF_NAME)
133- ),
134- (dnsconfig.template_path, dnsconfig.target_path))
135-
136 def test_write_config_DNSConfigDirectoryMissing_if_dir_missing(self):
137 dnsconfig = DNSConfig()
138- dir_name = self.make_dir()
139- os.rmdir(dir_name)
140+ dir_name = factory.make_name('nonesuch')
141 self.patch(DNSConfig, 'target_dir', dir_name)
142 self.assertRaises(DNSConfigDirectoryMissing, dnsconfig.write_config)
143
144@@ -321,8 +311,7 @@
145 def test_write_config_skips_writing_if_overwrite_false(self):
146 # If DNSConfig is created with overwrite=False, it won't
147 # overwrite an existing config file.
148- target_dir = self.make_dir()
149- self.patch(DNSConfig, 'target_dir', target_dir)
150+ target_dir = patch_dns_config_path(self)
151 random_content = factory.getRandomString()
152 factory.make_file(
153 location=target_dir, name=MAAS_NAMED_CONF_NAME,
154@@ -336,8 +325,7 @@
155 def test_write_config_writes_config_if_no_existing_file(self):
156 # If DNSConfig is created with overwrite=False, the config file
157 # will be written if no config file exists.
158- target_dir = self.make_dir()
159- self.patch(DNSConfig, 'target_dir', target_dir)
160+ target_dir = patch_dns_config_path(self)
161 dnsconfig = DNSConfig()
162 dnsconfig.write_config(overwrite=False)
163 self.assertThat(
164@@ -345,8 +333,7 @@
165 FileExists())
166
167 def test_write_config_writes_config(self):
168- target_dir = self.make_dir()
169- self.patch(DNSConfig, 'target_dir', target_dir)
170+ target_dir = patch_dns_config_path(self)
171 domain = factory.getRandomString()
172 network = IPNetwork('192.168.0.3/24')
173 ip = factory.getRandomIPInNetwork(network)
174@@ -369,24 +356,23 @@
175 ])))
176
177 def test_write_config_makes_config_world_readable(self):
178- target_dir = self.make_dir()
179- self.patch(DNSConfig, 'target_dir', target_dir)
180+ target_dir = patch_dns_config_path(self)
181 DNSConfig().write_config()
182 config_file = FilePath(os.path.join(target_dir, MAAS_NAMED_CONF_NAME))
183 self.assertTrue(config_file.getPermissions().other.read)
184
185 def test_get_include_snippet_returns_snippet(self):
186- target_dir = self.make_dir()
187- self.patch(DNSConfig, 'target_dir', target_dir)
188- dnsconfig = DNSConfig()
189- snippet = dnsconfig.get_include_snippet()
190+ target_dir = patch_dns_config_path(self)
191+ snippet = DNSConfig.get_include_snippet()
192 self.assertThat(
193 snippet,
194 MatchesAll(
195 Not(StartsWith('\n')),
196 EndsWith('\n'),
197 Contains(target_dir),
198- Contains('include "%s"' % dnsconfig.target_path)))
199+ Contains(
200+ 'include "%s/%s"'
201+ % (conf.DNS_CONFIG_DIR, DNSConfig.target_file_name))))
202
203
204 class TestIPUtilities(MAASTestCase):