Merge lp:~jtv/maas-test/tolerate-addressless-interface into lp:maas-test

[Jeroen T. Vermeulen]

Continue past DHCP server check if testing network interface has no IP address.

I apologise for the brevity of this merge proposal. It was written in a hurry. See bug for details; the symptom of the problem is an IOError with errno = 99 (EADDRNOTAVAIL). We need to bind to that IP address, or invite problems if a dhclient is running on another interface.

On the bright side, if the interface has no IP address, that probably means that there's no DHCP server running anyway!

Jeroen

[Raphaël Badin (rvb)]

Nice!

review: Approve

=== modified file 'maastest/detect_dhcp.py'
--- maastest/detect_dhcp.py	2013-12-06 07:25:45 +0000
+++ maastest/detect_dhcp.py	2013-12-12 09:57:33 +0000
@@ -21,6 +21,7 @@
 
 
 from contextlib import contextmanager
+from errno import EADDRNOTAVAIL
 import fcntl
 import logging
 from random import randint
@@ -130,7 +131,14 @@
     ifreq = struct.pack(
         b'16sH14s', interface.encode('ascii')[:15],
         socket.AF_INET, b'\x00' * 14)
-    info = fcntl.ioctl(sock, SIOCGIFADDR, ifreq)
+    try:
+        info = fcntl.ioctl(sock, SIOCGIFADDR, ifreq)
+    except IOError as e:
+        if e.errno == EADDRNOTAVAIL:
+            # Interface has no IP address.
+            return None
+        else:
+            raise
     ip = struct.unpack(b'16sH2x4s8x', info)[2]
     return socket.inet_ntoa(ip)
 
@@ -148,11 +156,17 @@
 
 
 def request_dhcp(interface):
-    """Broadcast a DHCP discovery request.  Return DHCP transaction ID."""
+    """Broadcast a DHCP discovery request.
+
+    Returns DHCP transaction ID, or `None` in the special case where the
+    network interface has no IP address.
+    """
     with udp_socket() as sock:
         sock.setsockopt(socket.SOL_SOCKET, socket.SO_BROADCAST, 1)
         mac = get_interface_MAC(sock, interface)
         bind_address = get_interface_IP(sock, interface)
+        if bind_address is None:
+            return None
         discover = DHCPDiscoverPacket(mac)
         sock.bind((bind_address, BOOTP_CLIENT_PORT))
         sock.sendto(discover.packet, ('<broadcast>', BOOTP_SERVER_PORT))
@@ -197,4 +211,9 @@
     # UDP is not reliable at any rate.  If detection is important, we should
     # send out repeated requests.
     transaction_id = request_dhcp(interface)
+    if transaction_id is None:
+        logging.info(
+            "Network interface %s has no IP address.  "
+            "Can't scan for DHCP servers.", interface)
+        return set()
     return receive_offers(transaction_id)

=== modified file 'maastest/tests/test_detect_dhcp.py'
--- maastest/tests/test_detect_dhcp.py	2013-12-06 07:02:08 +0000
+++ maastest/tests/test_detect_dhcp.py	2013-12-12 09:57:33 +0000
@@ -14,6 +14,8 @@
 __metaclass__ = type
 __all__ = []
 
+from errno import EADDRNOTAVAIL
+import fcntl
 import os
 from random import randint
 import socket
@@ -28,6 +30,7 @@
     get_interface_IP,
     get_interface_MAC,
     make_transaction_ID,
+    probe_dhcp,
     receive_offers,
     request_dhcp,
     udp_socket,
@@ -71,7 +74,7 @@
     return os.urandom(length)
 
 
-class MakeTransactionID(TestCase):
+class TestMakeTransactionID(TestCase):
     """Tests for `make_transaction_ID`."""
 
     def test_produces_well_formed_ID(self):
@@ -159,6 +162,12 @@
         sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
         self.assertEqual('127.0.0.1', get_interface_IP(sock, 'lo'))
 
+    def test_returns_None_if_no_address(self):
+        failure = IOError(EADDRNOTAVAIL, "Interface has no address")
+        self.patch(fcntl, 'ioctl', mock.MagicMock(side_effect=failure))
+        sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+        self.assertIsNone(get_interface_IP(sock, make_name('itf')))
+
 
 def patch_socket(testcase):
     """Patch `socket.socket` to return a mock."""
@@ -222,6 +231,15 @@
             mock.MagicMock(return_value=transaction_id))
         return transaction_id
 
+    def test_returns_None_if_no_IP_address(self):
+        self.patch(
+            detect_module, 'get_interface_MAC',
+            mock.MagicMock(return_value=make_MAC()))
+        self.patch(
+            detect_module, 'get_interface_IP',
+            mock.MagicMock(return_value=None))
+        self.assertIsNone(request_dhcp(make_name('itf')))
+
     def test_sends_discover_packet(self):
         sock = patch_socket(self)
         self.patch_interface_MAC()
@@ -266,34 +284,36 @@
             return self.packets.pop(0)
 
 
+def patch_recv(testcase, sock, num_packets=0):
+    """Patch up socket's `recv` to return `num_packets` arbitrary packets.
+
+    After that, further calls to `recv` will raise a timeout.
+    """
+    packets = [make_bytes() for _ in range(num_packets)]
+    receiver = FakePacketReceiver(packets)
+    testcase.patch(sock, 'recv', receiver)
+    return receiver
+
+
+def patch_offer_packet(testcase):
+    """Patch a mock `DHCPOfferPacket`."""
+    transaction_id = make_bytes(4)
+    packet = mock.MagicMock()
+    packet.transaction_ID = transaction_id
+    packet.dhcp_server_ID = make_IP()
+    testcase.patch(
+        detect_module, 'DHCPOfferPacket',
+        mock.MagicMock(return_value=packet))
+    return packet
+
+
 class TestReceiveOffers(TestCase):
     """Tests for `receive_offers`."""
 
-    def patch_recv(self, sock, num_packets=0):
-        """Patch up socket's `recv` to return `num_packets` arbitrary packets.
-
-        After that, further calls to `recv` will raise a timeout.
-        """
-        packets = [make_bytes() for _ in range(num_packets)]
-        receiver = FakePacketReceiver(packets)
-        self.patch(sock, 'recv', receiver)
-        return receiver
-
-    def patch_offer_packet(self):
-        """Patch a mock `DHCPOfferPacket`."""
-        transaction_id = make_bytes(4)
-        packet = mock.MagicMock()
-        packet.transaction_ID = transaction_id
-        packet.dhcp_server_ID = make_IP()
-        self.patch(
-            detect_module, 'DHCPOfferPacket',
-            mock.MagicMock(return_value=packet))
-        return packet
-
     def test_receives_from_socket(self):
         sock = patch_socket(self)
-        receiver = self.patch_recv(sock)
-        transaction_id = self.patch_offer_packet().transaction_ID
+        receiver = patch_recv(self, sock)
+        transaction_id = patch_offer_packet(self).transaction_ID
 
         receive_offers(transaction_id)
 
@@ -307,15 +327,15 @@
 
     def test_returns_empty_if_nothing_received(self):
         sock = patch_socket(self)
-        self.patch_recv(sock)
-        transaction_id = self.patch_offer_packet().transaction_ID
+        patch_recv(self, sock)
+        transaction_id = patch_offer_packet(self).transaction_ID
 
         self.assertEqual(set(), receive_offers(transaction_id))
 
     def test_processes_offer(self):
         sock = patch_socket(self)
-        self.patch_recv(sock, 1)
-        packet = self.patch_offer_packet()
+        patch_recv(self, sock, 1)
+        packet = patch_offer_packet(self)
 
         self.assertEqual(
             {packet.dhcp_server_ID},
@@ -323,8 +343,8 @@
 
     def test_ignores_other_transactions(self):
         sock = patch_socket(self)
-        self.patch_recv(sock, 1)
-        self.patch_offer_packet()
+        patch_recv(self, sock, 1)
+        patch_offer_packet(self)
         other_transaction_id = make_bytes(4)
 
         self.assertEqual(set(), receive_offers(other_transaction_id))
@@ -339,3 +359,30 @@
         self.assertRaises(
             InducedError,
             receive_offers, make_bytes(4))
+
+
+class TestProbeDHCP(TestCase):
+
+    def test_detects_dhcp_servers(self):
+        sock = patch_socket(self)
+        patch_recv(self, sock, 1)
+        packet = patch_offer_packet(self)
+        transaction_id = packet.transaction_ID
+        server = packet.dhcp_server_ID
+        self.patch(
+            detect_module, 'request_dhcp',
+            mock.MagicMock(return_value=transaction_id))
+
+        servers = probe_dhcp(make_name('itf'))
+
+        self.assertEqual({server}, servers)
+
+    def test_tolerates_interface_without_address(self):
+        self.patch(
+            detect_module, 'get_interface_MAC',
+            mock.MagicMock(return_value=make_MAC()))
+        self.patch(
+            detect_module, 'get_interface_IP',
+            mock.MagicMock(return_value=None))
+        servers = probe_dhcp(make_name('itf'))
+        self.assertEqual(set(), servers)