launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10499
[Merge] lp:~rvb/maas/write-dhcp-server-config into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/write-dhcp-server-config into lp:maas with lp:~rvb/maas/nodegroup-dhcpd-key2 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/write-dhcp-server-config/+merge/117660
This branch:
- removes the generate-dhcp-config command
- adds a task to write the DHCP config file
- adds a task to restart the DHCP server
- adds a method on nodegroup to write the DHCP config file and restart the DHCP server
- changes the names of the variables in the template to make them similar to the fields names on the nodegroup object.
= Pre-imp =
Discussed with Julian about this.
= Notes =
The packaging will need to be updated:
- no more running the (removed) generate-dhcp-config command.
- the worker should be able to write the DHCP config file
- the worker should be able to issue the restart command
I'm not entirely sure that "next_server" (in the template) should be "nodegroup.worker_ip". Seems logical because the TFTP server sits next to the worker but also, I see that the master nodegroup is created with a worker_ip = 127.0.0.1 so I think we've got a problem here.
--
https://code.launchpad.net/~rvb/maas/write-dhcp-server-config/+merge/117660
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/write-dhcp-server-config into lp:maas.
=== modified file 'etc/celeryconfig.py'
--- etc/celeryconfig.py 2012-07-30 13:10:39 +0000
+++ etc/celeryconfig.py 2012-08-01 14:05:22 +0000
@@ -36,6 +36,9 @@
# DHCP leases file, as maintained by ISC dhcpd.
DHCP_LEASES_FILE = '/var/lib/dhcp/dhcpd.leases'
+# ISC dhcpd configuration file.
+DHCP_CONFIG_FILE = '/etc/dhcp/dhcpd.conf'
+
try:
import user_maasceleryconfig
=== modified file 'etc/democeleryconfig.py'
--- etc/democeleryconfig.py 2012-07-17 09:51:38 +0000
+++ etc/democeleryconfig.py 2012-08-01 14:05:22 +0000
@@ -30,3 +30,7 @@
DNS_RNDC_PORT = 9154
+
+DHCP_CONFIG_FILE = os.path.join(
+ DEV_ROOT_DIRECTORY, 'run/dhcpd.conf')
+
=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py 2012-08-01 14:05:22 +0000
+++ src/maasserver/models/nodegroup.py 2012-08-01 14:05:22 +0000
@@ -28,6 +28,10 @@
Token,
)
from provisioningserver.omshell import generate_omapi_key
+from provisioningserver.tasks import (
+ restart_dhcp_server,
+ write_dhcp_config,
+ )
worker_user_name = 'maas-nodegroup-worker'
@@ -132,6 +136,19 @@
ip_range_high = IPAddressField(
editable=True, unique=True, blank=True, null=True, default='')
+ def set_up_dhcp(self):
+ """Write the DHCP configuration file and restart the DHCP server."""
+ # Circular imports.
+ from maasserver.dns import get_dns_server_address
+ write_dhcp_config.delay(
+ subnet=self.ip_range_low,
+ next_server=self.worker_ip,
+ dhcp_key=self.dhcp_key, subnet_mask=self.subnet_mask,
+ broadcast_ip=self.broadcast_ip, router_ip=self.router_ip,
+ dns_servers=get_dns_server_address(),
+ ip_range_low=self.ip_range_low, ip_range_high=self.ip_range_high,
+ callback=restart_dhcp_server.subtask())
+
def is_dhcp_enabled(self):
"""Is the DHCP for this nodegroup enabled?"""
return all([
=== modified file 'src/maasserver/tests/test_nodegroup.py'
--- src/maasserver/tests/test_nodegroup.py 2012-08-01 14:05:22 +0000
+++ src/maasserver/tests/test_nodegroup.py 2012-08-01 14:05:22 +0000
@@ -17,8 +17,12 @@
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase
from maasserver.worker_user import get_worker_user
+from maastesting.fakemethod import FakeMethod
+from maastesting.matchers import ContainsAll
+from provisioningserver import tasks
from provisioningserver.omshell import generate_omapi_key
from testtools.matchers import (
+ FileContains,
GreaterThan,
MatchesStructure,
)
@@ -172,3 +176,28 @@
ip_range_high=factory.getRandomIPAddress(),
)
self.assertTrue(nodegroup.is_dhcp_enabled())
+
+ def test_set_up_dhcp_writes_dhcp_config(self):
+ conf_file = self.make_file(contents=factory.getRandomString())
+ self.patch(tasks, 'DHCP_CONFIG_FILE', conf_file)
+ # Silence dhcpd restart.
+ self.patch(tasks, 'check_call', FakeMethod())
+ nodegroup = factory.make_node_group(
+ dhcp_key=factory.getRandomString())
+ nodegroup.set_up_dhcp()
+ dhcp_params = [
+ 'dhcp_key', 'subnet_mask', 'broadcast_ip', 'router_ip',
+ 'ip_range_low', 'ip_range_high']
+ expected = [getattr(nodegroup, param) for param in dhcp_params]
+ self.assertThat(
+ conf_file,
+ FileContains(
+ matcher=ContainsAll(expected)))
+
+ def test_set_up_dhcp_reloads_dhcp_server(self):
+ self.patch(tasks, 'DHCP_CONFIG_FILE', self.make_file())
+ recorder = FakeMethod()
+ self.patch(tasks, 'check_call', recorder)
+ nodegroup = factory.make_node_group()
+ nodegroup.set_up_dhcp()
+ self.assertEqual(1, recorder.call_count)
=== modified file 'src/provisioningserver/__main__.py'
--- src/provisioningserver/__main__.py 2012-07-23 13:24:17 +0000
+++ src/provisioningserver/__main__.py 2012-08-01 14:05:22 +0000
@@ -12,7 +12,6 @@
__metaclass__ = type
-import provisioningserver.dhcp.writer
import provisioningserver.pxe.install_bootloader
import provisioningserver.pxe.install_image
from provisioningserver.utils import MainScript
@@ -20,9 +19,6 @@
main = MainScript(__doc__)
main.register(
- "generate-dhcp-config",
- provisioningserver.dhcp.writer)
-main.register(
"install-pxe-bootloader",
provisioningserver.pxe.install_bootloader)
main.register(
=== modified file 'src/provisioningserver/dhcp/config.py'
--- src/provisioningserver/dhcp/config.py 2012-07-03 05:26:34 +0000
+++ src/provisioningserver/dhcp/config.py 2012-08-01 14:05:22 +0000
@@ -47,10 +47,10 @@
subnet {{subnet}} netmask {{subnet_mask}} {
next-server {{next_server}};
option subnet-mask {{subnet_mask}};
- option broadcast-address {{broadcast_address}};
+ option broadcast-address {{broadcast_ip}};
option domain-name-servers {{dns_servers}};
- option routers {{gateway}};
- range dynamic-bootp {{low_range}} {{high_range}};
+ option routers {{router_ip}};
+ range dynamic-bootp {{ip_range_low}} {{ip_range_high}};
pool {
allow members of "uboot-highbank";
@@ -61,6 +61,10 @@
filename "{{bootloaders['i386', 'generic']}}";
}
}
+ key maasupdate {
+ algorithm hmac-md5;
+ secret "{{dhcp_key}}";
+ };
""")
template = tempita.Template(
=== modified file 'src/provisioningserver/dhcp/tests/test_config.py'
--- src/provisioningserver/dhcp/tests/test_config.py 2012-07-03 08:29:51 +0000
+++ src/provisioningserver/dhcp/tests/test_config.py 2012-08-01 14:05:22 +0000
@@ -24,14 +24,15 @@
# Simple test version of the DHCP template. Contains parameter
# substitutions, but none that aren't also in the real template.
sample_template = dedent("""\
+ {{dhcp_key}}
{{subnet}}
{{subnet_mask}}
{{next_server}}
- {{broadcast_address}}
+ {{broadcast_ip}}
{{dns_servers}}
- {{gateway}}
- {{low_range}}
- {{high_range}}
+ {{router_ip}}
+ {{ip_range_low}}
+ {{ip_range_high}}
""")
@@ -41,14 +42,15 @@
The sample provides all parameters used by the DHCP template.
"""
return dict(
+ dhcp_key="random",
subnet="10.0.0.0",
subnet_mask="255.0.0.0",
next_server="10.0.0.1",
- broadcast_address="10.255.255.255",
+ broadcast_ip="10.255.255.255",
dns_servers="10.1.0.1 10.1.0.2",
- gateway="10.0.0.2",
- low_range="10.0.0.3",
- high_range="10.0.0.254",
+ router_ip="10.0.0.2",
+ ip_range_low="10.0.0.3",
+ ip_range_high="10.0.0.254",
)
=== removed file 'src/provisioningserver/dhcp/tests/test_writer.py'
--- src/provisioningserver/dhcp/tests/test_writer.py 2012-06-14 15:04:12 +0000
+++ src/provisioningserver/dhcp/tests/test_writer.py 1970-01-01 00:00:00 +0000
@@ -1,79 +0,0 @@
-# Copyright 2012 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Tests for `provisioningserver.dhcp.writer`."""
-
-from __future__ import (
- absolute_import,
- print_function,
- unicode_literals,
- )
-
-__metaclass__ = type
-__all__ = []
-
-from argparse import ArgumentParser
-from io import BytesIO
-from os import path
-import sys
-
-from maastesting.matchers import ContainsAll
-from maastesting.testcase import TestCase
-from provisioningserver.dhcp import writer
-from testtools.matchers import MatchesStructure
-
-
-class TestScript(TestCase):
- """Test the DHCP configuration writer."""
-
- test_args = (
- '--subnet', 'subnet',
- '--subnet-mask', 'subnet-mask',
- '--next-server', 'next-server',
- '--broadcast-address', 'broadcast-address',
- '--dns-servers', 'dns-servers',
- '--gateway', 'gateway',
- '--low-range', 'low-range',
- '--high-range', 'high-range',
- )
-
- def test_arg_setup(self):
- parser = ArgumentParser()
- writer.add_arguments(parser)
- args = parser.parse_args(self.test_args)
- self.assertThat(
- args, MatchesStructure.byEquality(
- subnet='subnet',
- subnet_mask='subnet-mask',
- next_server='next-server',
- broadcast_address='broadcast-address',
- dns_servers='dns-servers',
- gateway='gateway',
- low_range='low-range',
- high_range='high-range'))
-
- def test_run(self):
- self.patch(sys, "stdout", BytesIO())
- parser = ArgumentParser()
- writer.add_arguments(parser)
- args = parser.parse_args(self.test_args)
- writer.run(args)
- output = sys.stdout.getvalue()
- contains_all_params = ContainsAll(
- ['subnet', 'subnet-mask', 'next-server', 'broadcast-address',
- 'dns-servers', 'gateway', 'low-range', 'high-range'])
- self.assertThat(output, contains_all_params)
-
- def test_run_save_to_file(self):
- parser = ArgumentParser()
- writer.add_arguments(parser)
- outfile = path.join(self.make_dir(), "outfile.txt")
- args = parser.parse_args(
- self.test_args + ("--outfile", outfile))
- writer.run(args)
- with open(outfile, "rb") as stream:
- output = stream.read()
- contains_all_params = ContainsAll(
- ['subnet', 'subnet-mask', 'next-server', 'broadcast-address',
- 'dns-servers', 'gateway', 'low-range', 'high-range'])
- self.assertThat(output, contains_all_params)
=== removed file 'src/provisioningserver/dhcp/writer.py'
--- src/provisioningserver/dhcp/writer.py 2012-07-24 04:53:21 +0000
+++ src/provisioningserver/dhcp/writer.py 1970-01-01 00:00:00 +0000
@@ -1,70 +0,0 @@
-# Copyright 2012 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Generate a DHCP server configuration."""
-
-from __future__ import (
- absolute_import,
- print_function,
- unicode_literals,
- )
-
-__metaclass__ = type
-__all__ = [
- "add_arguments",
- "run",
- ]
-
-import sys
-
-from provisioningserver.dhcp import config
-
-
-def add_arguments(parser):
- """Initialise options for generating DHCP configuration.
-
- :param parser: An instance of :class:`ArgumentParser`.
- """
- parser.add_argument(
- "--subnet", action="store", required=True, help=(
- "Base subnet declaration, e.g. 192.168.1.0"))
- parser.add_argument(
- "--subnet-mask", action="store", required=True, help=(
- "The mask for the subnet, e.g. 255.255.255.0"))
- parser.add_argument(
- "--next-server", action="store", required=True, help=(
- "The address of the TFTP server"))
- parser.add_argument(
- "--broadcast-address", action="store", required=True, help=(
- "The broadcast IP address for the subnet, e.g. 192.168.1.255"))
- parser.add_argument(
- "--dns-servers", action="store", required=True, help=(
- "One or more IP addresses of the DNS server for the subnet "
- "separated by spaces."))
- parser.add_argument(
- "--gateway", action="store", required=True, help=(
- "The router/gateway IP address for the subnet"))
- parser.add_argument(
- "--low-range", action="store", required=True, help=(
- "The first IP address in the range of IP addresses to "
- "allocate"))
- parser.add_argument(
- "--high-range", action="store", required=True, help=(
- "The last IP address in the range of IP addresses to "
- "allocate"))
- parser.add_argument(
- "-o", "--outfile", action="store", required=False, help=(
- "A file to save to. When left unspecified the configuration "
- "will be written to stdout. This option is useful when "
- "running outside of a shell."))
-
-
-def run(args):
- """Generate a DHCP server configuration, and write it to stdout."""
- params = vars(args)
- output = config.get_config(**params).encode("ascii")
- if args.outfile is None:
- sys.stdout.write(output)
- else:
- with open(args.outfile, "wb") as out:
- out.write(output)
=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py 2012-07-26 00:52:00 +0000
+++ src/provisioningserver/tasks.py 2012-08-01 14:05:22 +0000
@@ -20,9 +20,14 @@
'write_full_dns_config',
]
-from subprocess import CalledProcessError
+from subprocess import (
+ CalledProcessError,
+ check_call,
+ )
from celery.task import task
+from celeryconfig import DHCP_CONFIG_FILE
+from provisioningserver.dhcp import config
from provisioningserver.dns.config import (
DNSConfig,
execute_rndc_command,
@@ -33,7 +38,7 @@
PowerAction,
PowerActionFail,
)
-
+from provisioningserver.utils import atomic_write
# =====================================================================
# Power-related tasks
@@ -107,8 +112,8 @@
zone.write_config()
zone.write_reverse_config()
# Write main config file.
- config = DNSConfig(zones=zones)
- config.write_config(**kwargs)
+ dns_config = DNSConfig(zones=zones)
+ dns_config.write_config(**kwargs)
if callback is not None:
callback.delay()
@@ -124,8 +129,8 @@
:type callback: callable
:param **kwargs: Keyword args passed to DNSConfig.write_config()
"""
- config = DNSConfig(zones=zones)
- config.write_config(**kwargs)
+ dns_config = DNSConfig(zones=zones)
+ dns_config.write_config(**kwargs)
if callback is not None:
callback.delay()
@@ -203,3 +208,21 @@
# Re-raise, so the job is marked as failed. Only currently
# useful for tests.
raise
+
+
+@task
+def write_dhcp_config(callback=None, **kwargs):
+ """Write out the DHCP configuration file and restart the DHCP server.
+
+ :param **kwargs: Keyword args passed to dhcp.config.get_config()
+ """
+ output = config.get_config(**kwargs).encode("ascii")
+ atomic_write(output, DHCP_CONFIG_FILE)
+ if callback is not None:
+ callback.delay()
+
+
+@task
+def restart_dhcp_server():
+ """Restart the DHCP server."""
+ check_call(['sudo', 'service', 'isc-dhcp-server', 'restart'])
=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py 2012-07-27 03:41:47 +0000
+++ src/provisioningserver/tests/test_tasks.py 2012-08-01 14:05:22 +0000
@@ -34,12 +34,14 @@
from provisioningserver.power.poweraction import PowerActionFail
from provisioningserver.tasks import (
add_new_dhcp_host_map,
+ Omshell,
power_off,
power_on,
remove_dhcp_host_map,
+ restart_dhcp_server,
rndc_command,
- Omshell,
setup_rndc_configuration,
+ write_dhcp_config,
write_dns_config,
write_dns_zone_config,
write_full_dns_config,
@@ -48,6 +50,7 @@
from testresources import FixtureResource
from testtools.matchers import (
Equals,
+ FileContains,
FileExists,
MatchesListwise,
)
@@ -145,6 +148,32 @@
CalledProcessError, remove_dhcp_host_map.delay,
ip, server_address, key)
+ def test_write_dhcp_config_writes_config(self):
+ conf_file = self.make_file(contents=factory.getRandomString())
+ self.patch(tasks, 'DHCP_CONFIG_FILE', conf_file)
+ param_names = [
+ 'dhcp_key', 'subnet', 'subnet_mask', 'next_server',
+ 'broadcast_ip', 'dns_servers', 'router_ip', 'ip_range_low',
+ 'ip_range_high']
+ params = {param: factory.getRandomString() for param in param_names}
+ write_dhcp_config(**params)
+ self.assertThat(
+ conf_file,
+ FileContains(
+ matcher=ContainsAll(
+ [
+ 'class "pxe"',
+ "option subnet-mask",
+ ])))
+
+ def test_restart_dhcp_server_sends_command(self):
+ recorder = FakeMethod()
+ self.patch(tasks, 'check_call', recorder)
+ restart_dhcp_server()
+ self.assertEqual(
+ (1, (['sudo', 'service', 'isc-dhcp-server', 'restart'],)),
+ (recorder.call_count, recorder.extract_args()[0]))
+
class TestDNSTasks(TestCase):
Follow ups
