Skip to content

Commit

Permalink
dhcp: small refactor
Browse files Browse the repository at this point in the history 
* Rename switch_port to switch_iface to avoid confusions.
* Rename the context manager from dhcp_push() to config() as it's more
  natural to use: with dhcp.config(my_config): # do something
* Simplify formatting of templates, added ignores to vulture for false
  positives, see also:
  jendrikseipp/vulture#264
* Add constructor documentation to the dataclasses.

Change-Id: I3a1114118150189c267476b022d540df22f26aca
  • Loading branch information
@volans-
volans- committed Sep 7, 2021
1 parent eb083e0 commit 0a806d1
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 36 deletions.
66 changes: 39 additions & 27 deletions spicerack/dhcp.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,32 @@ def filename(self) -> str:

@dataclass
class DHCPConfOpt82(DHCPConfiguration):
"""A configuration generator for host installation DHCP entries."""
"""A configuration generator for host installation DHCP entries.
Arguments:
hostname (str): the hostname to generate the DHCP matching block for.
fqdn (str): the FQDN of the same host.
switch_hostname (str): the hostname of the switch the host is connected to.
switch_iface (str): the name of the switch interface the host is connected to.
vlan (str): the name of the VLAN the host is configured for.
ttys (int): which ttyS to use for this host, accepted values are 0 and 1.
distro (str, optional): the codename of the Debian distribution to use for the PXE installer, if different
from the default distribution configured by Puppet globally.
"""

hostname: str
fqdn: str
switch_hostname: str
switch_port: str
switch_iface: str
vlan: str
ttys: int = 1
distro: Optional[str] = None

_template = """
host {hostname} {{
host-identifier option agent.circuit-id "{switch_hostname}:{switch_port}:{vlan}";
fixed-address {fqdn};
host {s.hostname} {{
host-identifier option agent.circuit-id "{s.switch_hostname}:{s.switch_iface}:{s.vlan}";
fixed-address {s.fqdn};
{pathprefix}
}}
"""
Expand All @@ -69,16 +81,8 @@ def __str__(self) -> str:
pathprefix = ""
if self.distro is not None:
pathprefix = f"""option pxelinux.pathprefix "http://apt.wikimedia.org/tftpboot/{self.distro}-installer/";"""
return textwrap.dedent(
self._template.format(
hostname=self.hostname,
switch_hostname=self.switch_hostname,
switch_port=self.switch_port,
vlan=self.vlan,
fqdn=self.fqdn,
pathprefix=pathprefix,
)
)

return textwrap.dedent(self._template.format(s=self, pathprefix=pathprefix))

@property
def filename(self) -> str:
Expand All @@ -88,20 +92,28 @@ def filename(self) -> str:

@dataclass
class DHCPConfMgmt(DHCPConfiguration):
"""A configuration for management network DHCP entries."""
"""A configuration for management network DHCP entries.
Arguments:
datacenter (str): the name of the Datacenter the host is.
serial (str): the vendor serial of the host.
fqdn (str): the management console FQDN to use for this host.
ip_address (ipaddress.IPv4Address): the IP address to give the management interface.
"""

datacenter: str
serial: str
fqdn: str
ip_address: IPv4Address

_template = """
class "{fqdn}" {{
match if (option host-name = "iDRAC-{serial}")
class "{s.fqdn}" {{
match if (option host-name = "iDRAC-{s.serial}")
}}
pool {{
allow members of "{fqdn}";
range {ip_address} {ip_address};
allow members of "{s.fqdn}";
range {s.ip_address} {s.ip_address};
}}
"""

Expand All @@ -114,7 +126,7 @@ def __post_init__(self) -> None:

def __str__(self) -> str:
"""Return the rendered DHCP configuration snippet."""
return textwrap.dedent(self._template.format(fqdn=self.fqdn, serial=self.serial, ip_address=self.ip_address))
return textwrap.dedent(self._template.format(s=self))

@property
def filename(self) -> str:
Expand All @@ -129,7 +141,7 @@ def __init__(self, hosts: RemoteHosts):
"""Create a DHCP instance.
Arguments:
hosts (`spicerack.remote.RemoteHosts`): The target datacenter's install servers.
hosts (spicerack.remote.RemoteHosts): The target datacenter's install servers.
"""
if len(hosts) < 1:
Expand All @@ -147,8 +159,8 @@ def push_configuration(self, configuration: DHCPConfiguration) -> None:
"""Push a specified file with specified content to DHCP server and call refresh_dhcp.
Arguments:
configuration (`spicerack.dhcp.DHCPConfiguration`): An instance which provides content and filename for a
configuration.
configuration (spicerack.dhcp.DHCPConfiguration): An instance which provides content and filename for a
configuration.
"""
filename = configuration.filename
Expand Down Expand Up @@ -198,11 +210,11 @@ def remove_configuration(self, configuration: DHCPConfiguration, force: bool = F
self.refresh_dhcp()

@contextmanager
def dhcp_push(self, dhcp_config: DHCPConfiguration) -> Iterator[None]:
"""Adapt the DHCP configuration manager to context manager.
def config(self, dhcp_config: DHCPConfiguration) -> Iterator[None]:
"""A context manager to perform actions while the given DHCP config is valid.
Arguments:
dhcp_config (spicerack.dhcp.DHCPConfiguration intance): The configuration instance to operate with.
dhcp_config (spicerack.dhcp.DHCPConfiguration): The DHCP configuration to use.
"""
self.push_configuration(dhcp_config)
Expand Down
15 changes: 6 additions & 9 deletions spicerack/tests/unit/test_dhcp.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def get_mock_config():
"hostname": "testhost0",
"fqdn": "testhost0.eqiad.wmnet",
"switch_hostname": "asw2-d-eqiad",
"switch_port": "ge-0/0/0",
"switch_iface": "ge-0/0/0",
"vlan": 1021,
},
(
Expand All @@ -71,7 +71,7 @@ def get_mock_config():
"hostname": "testhost0",
"fqdn": "testhost0.eqiad.wmnet",
"switch_hostname": "asw2-d-eqiad",
"switch_port": "ge-0/0/0",
"switch_iface": "ge-0/0/0",
"vlan": 1021,
"ttys": 0,
},
Expand All @@ -91,7 +91,7 @@ def get_mock_config():
"hostname": "testhost0",
"fqdn": "testhost0.eqiad.wmnet",
"switch_hostname": "asw2-d-eqiad",
"switch_port": "ge-0/0/0",
"switch_iface": "ge-0/0/0",
"vlan": 1021,
"ttys": 0,
"distro": "stretch",
Expand Down Expand Up @@ -314,9 +314,6 @@ def test_remove_config_rm_fail(self):
self.dhcp.remove_configuration(config)
assert str(exc.value) == "Can't remove {}.".format(config.filename)

# test DHCP.dhcp_push context manager
# - startup works?
# - teardown works?
def test_push_context_manager(self):
"""Test push context manager success."""
config = get_mock_config()
Expand All @@ -329,8 +326,8 @@ def test_push_context_manager(self):
configsha256 = sha256(str(config.__str__.return_value).encode()).hexdigest()
mock_remotehosts.results_to_list.return_value = [[None, "{} {}".format(configsha256, config.filename)]]

with self.dhcp.dhcp_push(config):
...
with self.dhcp.config(config):
pass

hosts.run_sync.assert_has_calls([call_sha256, call_rm, call_refresh])

Expand All @@ -357,7 +354,7 @@ def test_push_context_manager_raise(self):
configsha256 = sha256(str(config.__str__.return_value).encode()).hexdigest()
mock_remotehosts.results_to_list.return_value = [[None, "{} {}".format(configsha256, config.filename)]]
with pytest.raises(Exception):
with self.dhcp.dhcp_push(config):
with self.dhcp.config(config):
raise Exception()

hosts.run_sync.assert_has_calls([call_test, call_write, call_refresh, call_sha256, call_rm, call_refresh])
8 changes: 8 additions & 0 deletions spicerack/tests/vulture_whitelist.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ def __getattr__(self, _):
whitelist_ganeti = Whitelist()
whitelist_ganeti.Ganeti._http_session.auth

# Needed because of https://github.com/jendrikseipp/vulture/issues/264
whitelist_dhcp = Whitelist()
whitelist_dhcp.DHCPConfOpt82.switch_hostname
whitelist_dhcp.DHCPConfOpt82.switch_iface
whitelist_dhcp.DHCPConfOpt82.vlan
whitelist_dhcp.DHCPConfMgmt.serial
whitelist_dhcp.DHCPConfMgmt.ip_address

whitelist_dnsdisc = Whitelist()
whitelist_dnsdisc.pool
whitelist_dnsdisc.depool
Expand Down

0 comments on commit 0a806d1

Please sign in to comment.