Browse Source

Refactor DHCP discover command

DHCP discover check should be performed without regarding of tagged
interfaces (as they are not present on bootstrapped nodes), thus now by
default vlans are not considered. Special flag was added
to corresponding command that toggles such extended check.

Also all network interfaces (except admin) on bootstrapped nodes are
down at the moment the check is performed and thus must be UP-ed for the
time it takes.

Helper class utils.IfaceState modified:
- add possibility to work multiple interfaces now;
- 'ip set link' is now used as the command to UP/DOWN the ifaces;
- retry logic has been changed to sleeping (by default for 30 sec.) after the
  command's execution in order to make sure the ifase is UP;
- raising of exception in case iface is still in DOWN state has been changed to
  writing appropriate message to stderr stream so that __exit__'s code
  downs previously successfully UP-ed ifaces.

Several commands of dhcpchecker were renamed to reflect its actual
purpose

Change-Id: I07c3e7acb00e65554983f07e17acbf345c7508d7
Related-Bug: #1569325
Artem Roma 2 years ago
parent
commit
b217027624

+ 27
- 9
dhcp_checker/api.py View File

@@ -107,24 +107,42 @@ def make_listeners(ifaces):
107 107
 
108 108
 
109 109
 @utils.filter_duplicated_results
110
-def check_dhcp_with_vlans(config, timeout=5, repeat=2):
110
+def check_dhcp_with_vlans(config, timeout=5, repeat=2, w_vlans=True):
111 111
     """Provide config of {iface: [vlans..]} pairs
112 112
 
113 113
     @config - {'eth0': (100, 101), 'eth1': (100, 102)}
114
+
115
+    'w_vlans' flag toggles dhcp discover check for tagged interfaces;
116
+    if it is False - the request will be sent only for real interfaces
117
+
118
+    Before the request is sent interfaces are set (if they haven't been before)
119
+    and after - down-ed (only those up-ed for the purpose of this checking)
120
+
121
+    If the flag 'w_vlans' holds True but list of vlans for iface is [0],
122
+    the check will also be performed only for real interfaces (see logic of
123
+    utils.VlansContext context manager)
114 124
     """
115 125
     # vifaces - list of pairs ('eth0', ['eth0.100', 'eth0.101'])
116 126
     with utils.VlansContext(config) as vifaces:
117 127
         ifaces, vlans = zip(*vifaces)
118
-        listeners = make_listeners(ifaces)
119 128
 
120
-        for _ in xrange(repeat):
121
-            for i in utils.filtered_ifaces(itertools.chain(ifaces, *vlans)):
122
-                send_dhcp_discover(i)
123
-            time.sleep(timeout)
129
+        # up interfaces before making the check
130
+        with utils.IfaceState(ifaces) as rdy_ifaces:
131
+            listeners = make_listeners(rdy_ifaces)
132
+
133
+            for _ in xrange(repeat):
134
+                ifaces_to_check = (
135
+                    itertools.chain(rdy_ifaces, *vlans)
136
+                    if w_vlans else rdy_ifaces
137
+                )
138
+
139
+                for i in utils.filtered_ifaces(ifaces_to_check):
140
+                    send_dhcp_discover(i)
141
+                time.sleep(timeout)
124 142
 
125
-        for l in listeners:
126
-            for pkt in l.readpkts():
127
-                yield utils.format_answer(scapy.Ether(pkt[1]), l.name)
143
+            for l in listeners:
144
+                for pkt in l.readpkts():
145
+                    yield utils.format_answer(scapy.Ether(pkt[1]), l.name)
128 146
 
129 147
 
130 148
 @utils.single_format

+ 6
- 3
dhcp_checker/commands.py View File

@@ -92,22 +92,25 @@ class ListDhcpAssignment(lister.Lister, BaseCommand):
92 92
         return columns, [first.values()] + [item.values() for item in res]
93 93
 
94 94
 
95
-class DhcpWithVlansCheck(lister.Lister, BaseCommand):
95
+class DhcpCheckDiscover(lister.Lister, BaseCommand):
96 96
     """Provide iface with list of vlans to check
97 97
 
98 98
     If no vlans created - they will be. After creation they won't be deleted.
99 99
     """
100 100
 
101 101
     def get_parser(self, prog_name):
102
-        parser = super(DhcpWithVlansCheck, self).get_parser(prog_name)
102
+        parser = super(DhcpCheckDiscover, self).get_parser(prog_name)
103 103
         parser.add_argument('config',
104 104
                             help='Ethernet interface name')
105
+        parser.add_argument('--with-vlans', action='store_true',
106
+                            help='Enable the check for tagged ifaces')
105 107
         return parser
106 108
 
107 109
     def take_action(self, parsed_args):
108 110
         res = list(api.check_dhcp_with_vlans(json.loads(parsed_args.config),
109 111
                                              timeout=parsed_args.timeout,
110
-                                             repeat=parsed_args.repeat))
112
+                                             repeat=parsed_args.repeat,
113
+                                             w_vlans=parsed_args.with_vlans))
111 114
         if not res:
112 115
             res = [{}]
113 116
         return (utils.DHCP_OFFER_COLUMNS,

+ 9
- 9
dhcp_checker/tests/system/tests.py View File

@@ -70,9 +70,9 @@ class TestDhcpWithNetworkDown(unittest.TestCase):
70 70
 
71 71
     def test_dhcp_server_on_eth2_down(self):
72 72
         """iface should be ifuped in case it's down and rolledback after"""
73
-        manager = utils.IfaceState(self.iface_down)
74
-        with manager as iface:
75
-            response = api.check_dhcp_on_eth(iface, 2)
73
+        manager = utils.IfaceState([self.iface_down])
74
+        with manager as ifaces:
75
+            response = api.check_dhcp_on_eth(ifaces[0], 2)
76 76
 
77 77
         self.assertEqual(len(response), 1)
78 78
         self.assertTrue(response[0]['server_ip'])
@@ -82,9 +82,9 @@ class TestDhcpWithNetworkDown(unittest.TestCase):
82 82
 
83 83
     def test_dhcp_server_on_eth0_up(self):
84 84
         """Test verifies that if iface is up, it won't be touched"""
85
-        manager = utils.IfaceState(self.iface_up)
86
-        with manager as iface:
87
-            response = api.check_dhcp_on_eth(iface, 2)
85
+        manager = utils.IfaceState([self.iface_up])
86
+        with manager as ifaces:
87
+            response = api.check_dhcp_on_eth(ifaces[0], 2)
88 88
 
89 89
         self.assertEqual(len(response), 1)
90 90
         self.assertTrue(response[0]['server_ip'])
@@ -95,9 +95,9 @@ class TestDhcpWithNetworkDown(unittest.TestCase):
95 95
     def test_dhcp_server_on_nonexistent_iface(self):
96 96
 
97 97
         def test_check():
98
-            manager = utils.IfaceState('eth10')
99
-            with manager as iface:
100
-                api.check_dhcp_on_eth(iface, 2)
98
+            manager = utils.IfaceState(['eth10'])
99
+            with manager as ifaces:
100
+                api.check_dhcp_on_eth(ifaces[0], 2)
101 101
         self.assertRaises(EnvironmentError, test_check)
102 102
 
103 103
     def tearDown(self):

+ 53
- 13
dhcp_checker/tests/unit/test_api.py View File

@@ -45,6 +45,23 @@ expected_response = {
45 45
 }
46 46
 
47 47
 
48
+class IfaceStateMock(object):
49
+
50
+    def __init__(self, ifaces):
51
+        self.ifaces = ifaces
52
+
53
+    def __enter__(self):
54
+        return self.ifaces
55
+
56
+    def __exit__(self, exc_type, exc_val, ex_tb):
57
+        pass
58
+
59
+
60
+def filtered_ifaces_mock(ifaces):
61
+    return ifaces
62
+
63
+
64
+@patch('dhcp_checker.utils.IfaceState', new=IfaceStateMock)
48 65
 class TestDhcpApi(unittest.TestCase):
49 66
 
50 67
     def setUp(self):
@@ -53,9 +70,14 @@ class TestDhcpApi(unittest.TestCase):
53 70
                                                          'dhcp.pcap')))
54 71
         self.dhcp_response = self.scapy_data[1:]
55 72
 
73
+        self.config_sample = {
74
+            'eth0': (100, 101),
75
+            'eth1': (100, 102)
76
+        }
77
+
56 78
     @patch('dhcp_checker.api.scapy.srp')
57 79
     @patch('dhcp_checker.api.scapy.get_if_raw_hwaddr')
58
-    def test_check_dhcp_on_eth(self, raw_hwaddr, srp_mock):
80
+    def test_check_dhcp_on_eth(self, raw_hwaddr, srp_mock, *_):
59 81
         raw_hwaddr.return_value = ('111', '222')
60 82
         srp_mock.return_value = ([self.dhcp_response], [])
61 83
         response = api.check_dhcp_on_eth('eth1', timeout=5)
@@ -63,36 +85,53 @@ class TestDhcpApi(unittest.TestCase):
63 85
 
64 86
     @patch('dhcp_checker.api.scapy.srp')
65 87
     @patch('dhcp_checker.api.scapy.get_if_raw_hwaddr')
66
-    def test_check_dhcp_on_eth_empty_response(self, raw_hwaddr, srp_mock):
88
+    def test_check_dhcp_on_eth_empty_response(self, raw_hwaddr, srp_mock, *_):
67 89
         raw_hwaddr.return_value = ('111', '222')
68 90
         srp_mock.return_value = ([], [])
69 91
         response = api.check_dhcp_on_eth('eth1', timeout=5)
70 92
         self.assertEqual([], response)
71 93
 
94
+    @patch('dhcp_checker.utils.filtered_ifaces')
72 95
     @patch('dhcp_checker.api.send_dhcp_discover')
73 96
     @patch('dhcp_checker.api.make_listeners')
74 97
     def test_check_dhcp_with_multiple_ifaces(
75
-            self, make_listeners, send_discover):
76
-        api.check_dhcp(['eth1', 'eth2'], repeat=1)
98
+            self, make_listeners, send_discover, filtered_ifaces, *_):
99
+        repeat = 1
100
+        ifaces = ['eth1', 'eth2']
101
+
102
+        filtered_ifaces.return_value = ifaces
103
+
104
+        api.check_dhcp(ifaces, repeat=repeat)
105
+
77 106
         make_listeners.assert_called_once_with(('eth2', 'eth1'))
107
+        self.assertEqual(filtered_ifaces.call_count, repeat)
78 108
         self.assertEqual(send_discover.call_count, 2)
79 109
 
110
+    @patch('dhcp_checker.utils.filtered_ifaces', new=filtered_ifaces_mock)
80 111
     @patch('dhcp_checker.api.send_dhcp_discover')
81 112
     @patch('dhcp_checker.api.make_listeners')
82
-    def test_check_dhcp_with_vlans(self, make_listeners, send_discover):
83
-        config_sample = {
84
-            'eth0': (100, 101),
85
-            'eth1': (100, 102)
86
-        }
87
-        api.check_dhcp_with_vlans(config_sample, timeout=1)
113
+    def test_check_dhcp_with_vlans(self, make_listeners, send_discover, *_):
114
+
115
+        api.check_dhcp_with_vlans(self.config_sample, timeout=1, repeat=1,
116
+                                  w_vlans=True)
88 117
         make_listeners.assert_called_once_with(('eth1', 'eth0'))
89 118
         self.assertEqual(send_discover.call_count, 6)
90 119
 
120
+    @patch('dhcp_checker.utils.filtered_ifaces', new=filtered_ifaces_mock)
121
+    @patch('dhcp_checker.api.send_dhcp_discover')
122
+    @patch('dhcp_checker.api.make_listeners')
123
+    def test_check_dhcp_wo_vlans(self, make_listeners, send_discover, *_):
124
+        api.check_dhcp_with_vlans(self.config_sample, timeout=1, repeat=1,
125
+                                  w_vlans=False)
126
+        make_listeners.assert_called_once_with(('eth1', 'eth0'))
127
+        self.assertEqual(send_discover.call_count, 2)
128
+
129
+    @patch('dhcp_checker.utils.filtered_ifaces', new=filtered_ifaces_mock)
91 130
     @patch('dhcp_checker.api.time.sleep')
92 131
     @patch('dhcp_checker.api.send_dhcp_discover')
93 132
     @patch('dhcp_checker.api.make_listeners')
94 133
     def test_check_dhcp_with_vlans_repeat_2(self, make_listeners,
95
-                                            send_discover, sleep_mock):
134
+                                            send_discover, sleep_mock, *_):
96 135
         config_sample = {
97 136
             'eth0': (),
98 137
         }
@@ -101,12 +140,13 @@ class TestDhcpApi(unittest.TestCase):
101 140
         make_listeners.assert_called_once_with(('eth0',))
102 141
         self.assertEqual(send_discover.call_count, 3)
103 142
 
104
-    @patch('dhcp_checker.api.utils.filtered_ifaces')
143
+    @patch('dhcp_checker.utils.filtered_ifaces')
105 144
     @patch('dhcp_checker.api.get_ifaces_exclude_lo')
106 145
     @patch('dhcp_checker.api.send_dhcp_discover')
107 146
     @patch('dhcp_checker.api.make_listeners')
108 147
     def test_check_dhcp_with_no_ifaces(
109
-            self, make_listeners, send_discover, interfaces, filtered_ifaces):
148
+            self, make_listeners, send_discover, interfaces,
149
+            filtered_ifaces, *_):
110 150
         interfaces.return_value = ['eth1']
111 151
         filtered_ifaces.return_value = ['eth1']
112 152
         api.check_dhcp(None, timeout=1, repeat=2)

+ 4
- 3
dhcp_checker/tests/unit/test_commands.py View File

@@ -37,7 +37,7 @@ class TestCommandsInterface(unittest.TestCase):
37 37
 
38 38
     def test_list_dhcp_servers(self, api):
39 39
         api.check_dhcp.return_value = iter([expected_response])
40
-        command = cli.main(['discover', '--ifaces', 'eth0', 'eth1',
40
+        command = cli.main(['listservers', '--ifaces', 'eth0', 'eth1',
41 41
                             '--format', 'json'])
42 42
         self.assertEqual(command, 0)
43 43
         api.check_dhcp.assert_called_once_with(['eth0', 'eth1'],
@@ -57,7 +57,8 @@ class TestCommandsInterface(unittest.TestCase):
57 57
         config_sample = {'eth1': ['100', '101'],
58 58
                          'eth2': range(103, 110)}
59 59
         api.check_dhcp_with_vlans.return_value = iter([expected_response])
60
-        command = cli.main(['vlans', json.dumps(config_sample)])
60
+        command = cli.main(['discover', json.dumps(config_sample),
61
+                            '--with-vlans'])
61 62
         self.assertEqual(command, 0)
62 63
         api.check_dhcp_with_vlans.assert_called_once_with(
63
-            config_sample, repeat=2, timeout=5)
64
+            config_sample, repeat=2, timeout=5, w_vlans=True)

+ 20
- 15
dhcp_checker/tests/unit/test_utils.py View File

@@ -14,9 +14,12 @@
14 14
 #    with this program; if not, write to the Free Software Foundation, Inc.,
15 15
 #    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
16 16
 
17
+import os
18
+import StringIO
19
+
17 20
 from mock import call
18 21
 from mock import patch
19
-import os
22
+
20 23
 import unittest
21 24
 
22 25
 from scapy import all as scapy
@@ -154,30 +157,32 @@ class TestMultiprocMap(unittest.TestCase):
154 157
 class TestIfaceStateHelper(unittest.TestCase):
155 158
 
156 159
     def test_iface_is_up(self, command, iface_state):
157
-        iface_value = iter(('UP',) * 3)
160
+        iface_value = iter(('UP',) * 2)
158 161
         iface_state.side_effect = lambda *args, **kwargs: next(iface_value)
159
-        with utils.IfaceState('eth1') as iface:
160
-            self.assertEqual(iface, 'eth1')
162
+        with utils.IfaceState(['eth1'], wait_up=0) as ifaces:
163
+            self.assertEqual(ifaces[0], 'eth1')
161 164
         self.assertEqual(iface_state.call_count, 2)
162 165
         self.assertEqual(command.call_count, 0)
163 166
 
164 167
     def test_iface_is_down(self, command, iface_state):
165
-        iface_value = iter(('DOWN', 'UP', 'DOWN'))
168
+        iface_value = iter(('DOWN', 'DOWN', 'UP', 'UP'))
166 169
         iface_state.side_effect = lambda *args, **kwargs: next(iface_value)
167
-        with utils.IfaceState('eth1') as iface:
168
-            self.assertEqual(iface, 'eth1')
169
-        self.assertEqual(iface_state.call_count, 3)
170
+        with utils.IfaceState(['eth1'], wait_up=10) as ifaces:
171
+            self.assertEqual(ifaces[0], 'eth1')
172
+        self.assertEqual(iface_state.call_count, 4)
170 173
         self.assertEqual(command.call_count, 2)
171 174
         self.assertEqual(command.call_args_list,
172
-                         [call('ifconfig', 'eth1', 'up'),
173
-                          call('ifconfig', 'eth1', 'down')])
175
+                         [call('ip', 'link', 'set', 'dev', 'eth1', 'up'),
176
+                          call('ip', 'link', 'set', 'dev', 'eth1', 'down')])
174 177
 
175 178
     def test_iface_cant_ifup(self, command, iface_state):
176 179
         iface_value = iter(('DOWN',) * 10)
177 180
         iface_state.side_effect = lambda *args, **kwargs: next(iface_value)
178 181
 
179
-        def test_raises():
180
-            with utils.IfaceState('eth1', retry=4) as iface:
181
-                self.assertEqual(iface, 'eth1')
182
-        self.assertRaises(EnvironmentError, test_raises)
183
-        self.assertEqual(command.call_count, 4)
182
+        with patch('sys.stderr', new=StringIO.StringIO()) as stderr_mock:
183
+            with utils.IfaceState(['eth1'], wait_up=5) as ifaces:
184
+                self.assertEqual(ifaces[0], 'eth1')
185
+
186
+        self.assertEquals(
187
+            stderr_mock.getvalue(), 'Tried my best to ifup iface eth1.')
188
+        self.assertEquals(command.call_count, 1)

+ 32
- 21
dhcp_checker/utils.py View File

@@ -18,6 +18,7 @@ import functools
18 18
 import re
19 19
 import subprocess
20 20
 import sys
21
+import time
21 22
 
22 23
 from netifaces import interfaces
23 24
 from scapy import all as scapy
@@ -207,33 +208,43 @@ class VlansContext(object):
207 208
 
208 209
 
209 210
 class IfaceState(object):
210
-    """Context manager to control state of iface while dhcp checker runs"""
211
+    """Context manager to control state of ifaces while dhcp checker runs"""
211 212
 
212
-    def __init__(self, iface, rollback=True, retry=3):
213
+    def __init__(self, ifaces, rollback=True, wait_up=30):
213 214
         self.rollback = rollback
214
-        self.retry = retry
215
-        self.iface = iface
216
-        self.pre_iface_state = _iface_state(iface)
217
-        self.iface_state = self.pre_iface_state
218
-        self.post_iface_state = ''
219
-
220
-    def iface_up(self):
221
-        while self.retry and self.iface_state != 'UP':
222
-            command_util('ifconfig', self.iface, 'up')
223
-            self.iface_state = _iface_state(self.iface)
224
-            self.retry -= 1
225
-        if self.iface_state != 'UP':
226
-            raise EnvironmentError(
227
-                'Tried my best to ifup iface {0}.'.format(self.iface))
215
+        self.wait_up = wait_up
216
+        self.ifaces = ifaces
217
+        self.pre_ifaces_state = self.get_ifaces_state()
218
+
219
+    def get_ifaces_state(self):
220
+        state = {}
221
+        for iface in self.ifaces:
222
+            state[iface] = _iface_state(iface)
223
+        return state
224
+
225
+    def iface_up(self, iface):
226
+        if _iface_state(iface) != 'UP':
227
+            command_util('ip', 'link', 'set', 'dev', iface, 'up')
228
+
229
+            deadline = time.time() + self.wait_up
230
+            while time.time() < deadline:
231
+                if _iface_state(iface) == 'UP':
232
+                    break
233
+                time.sleep(1)
234
+            else:
235
+                sys.stderr.write(
236
+                    'Tried my best to ifup iface {0}.'.format(iface))
228 237
 
229 238
     def __enter__(self):
230
-        self.iface_up()
231
-        return self.iface
239
+        for iface in self.ifaces:
240
+            self.iface_up(iface)
241
+        return self.ifaces
232 242
 
233 243
     def __exit__(self, exc_type, exc_val, exc_tb):
234
-        if self.pre_iface_state != 'UP' and self.rollback:
235
-            command_util('ifconfig', self.iface, 'down')
236
-        self.post_iface_state = _iface_state(self.iface)
244
+        for iface in self.ifaces:
245
+            if self.pre_ifaces_state[iface] != 'UP' \
246
+                    and _iface_state(iface) == 'UP' and self.rollback:
247
+                command_util('ip', 'link', 'set', 'dev', iface, 'down')
237 248
 
238 249
 
239 250
 def create_mac_filter(iface):

+ 2
- 2
setup.py View File

@@ -39,9 +39,9 @@ setuptools.setup(
39 39
             'urlaccesscheck = url_access_checker.cli:main',
40 40
         ],
41 41
         'dhcp.check': [
42
-            'discover = dhcp_checker.commands:ListDhcpServers',
42
+            'listservers = dhcp_checker.commands:ListDhcpServers',
43 43
             'request = dhcp_checker.commands:ListDhcpAssignment',
44
-            'vlans = dhcp_checker.commands:DhcpWithVlansCheck'
44
+            'discover = dhcp_checker.commands:DhcpCheckDiscover'
45 45
         ],
46 46
         'network_checker': [
47 47
             'multicast = network_checker.multicast.api:MulticastChecker',

Loading…
Cancel
Save