Here's the rest of my review. Overall it looks really good! I approve as long as the duplication in 1 & 2 get cleaned up. I think some more near duplication could be cleaned up with some creativity but I wouldn't block on it.
+def find_mac_via_ip(ip):
+ """Find the MAC address for `ip` by reading the output of arp -n.
+
+ Returns `None` if the IP is not found.
+
+ We do this because we aren't necessarily the only DHCP server on the
+ network, so we can't check our own leases file and be guaranteed to find an
+ IP that matches.
+
+ :param ip: The ip address, e.g. '192.168.1.1'.
+ """
+
+ output = call_capture_and_check(['arp', '-n']).split('\n')
+
+ for line in sorted(output):
+ columns = line.split()
+ if len(columns) == 5 and columns[0] == ip:
+ return columns[2]
+ return None
Here's the rest of my review. Overall it looks really good! I approve as long as the duplication in 1 & 2 get cleaned up. I think some more near duplication could be cleaned up with some creativity but I wouldn't block on it.
[9]
- Architecture( name="ppc64el/ generic" , description= "ppc64el" ), generic" , description= "ppc64el" , options= ['rootdelay= 60']),
+ Architecture(
+ name="ppc64el/
+ kernel_
A comment on why the rootdelay is needed and why it's 60 would be nice.
[10]
=== modified file 'src/provisioni ngserver/ import_ images/ download_ resources. py' gserver/ import_ images/ download_ resources. py 2014-05-23 09:48:49 +0000 gserver/ import_ images/ download_ resources. py 2014-05-28 21:38:08 +0000
--- src/provisionin
+++ src/provisionin
@@ -19,6 +19,8 @@
from datetime import datetime
from gzip import GzipFile
import os.path
+import tempfile
+from urlparse import urlsplit
from provisioningser ver.import_ images. helpers import ( signing_ policy,
get_
Why are these new imports added when there are no other changes to this file?
=== modified file 'src/provisioni ngserver/ utils/_ _init__ .py' gserver/ utils/_ _init__ .py 2014-05-05 23:55:46 +0000 gserver/ utils/_ _init__ .py 2014-05-28 21:38:08 +0000
--- src/provisionin
+++ src/provisionin
@@ -829,3 +829,24 @@
if len(columns) == 5 and columns[2] == mac:
return columns[0]
return None
[11]
+def find_mac_ via_ip( ip): and_check( ['arp', '-n']).split('\n')
+ """Find the MAC address for `ip` by reading the output of arp -n.
+
+ Returns `None` if the IP is not found.
+
+ We do this because we aren't necessarily the only DHCP server on the
+ network, so we can't check our own leases file and be guaranteed to find an
+ IP that matches.
+
+ :param ip: The ip address, e.g. '192.168.1.1'.
+ """
+
+ output = call_capture_
+
+ for line in sorted(output):
+ columns = line.split()
+ if len(columns) == 5 and columns[0] == ip:
+ return columns[2]
+ return None
I don't see a unit test for this method.