Merge ~gavin.lin/plainbox-provider-snappy:wifi-client-np into plainbox-provider-snappy:master
- Git
- lp:~gavin.lin/plainbox-provider-snappy
- wifi-client-np
- Merge into master
Status: | Superseded |
---|---|
Proposed branch: | ~gavin.lin/plainbox-provider-snappy:wifi-client-np |
Merge into: | plainbox-provider-snappy:master |
Diff against target: |
353 lines (+318/-0) 3 files modified
bin/wifi_client_test_netplan (+172/-0) units/wireless/jobs.pxu (+114/-0) units/wireless/test-plan.pxu (+32/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Maciej Kisielewski | Approve | ||
Review via email: mp+341646@code.launchpad.net |
This proposal supersedes a proposal from 2018-03-19.
This proposal has been superseded by a proposal from 2018-04-09.
Commit message
Description of the change
Add automated wifi client test using netplan, this is the updated version.
Maciej Kisielewski (kissiel) wrote : Posted in a previous version of this proposal | # |
Gavin Lin (gavin.lin) wrote : | # |
Updated according to Maciej's comments, thanks a lot.
Separated the copy part when backing up configuration file, so if anything goes wrong and unrecoverable, at least the backup is still there.
Also replaced all file manipulation with shutil module, and try to recover when things not going as expected.
Configuration file template and ping test are also updated.
Maciej Kisielewski (kissiel) wrote : | # |
Personally I would find it much easier to work with if the wifi_client_
Minor typo inline + PEP8 issues. The top-level comment in the script is misfit.
If you've got a minute run flake8 on the script to make it clean. Provisory +1
Gavin Lin (gavin.lin) wrote : | # |
I'll update the script base on comments later, thanks a lot for pointing them out!
Unmerged commits
- 4cc9630... by Gavin Lin
-
Add automated wifi client test using netplan
Preview Diff
1 | diff --git a/bin/wifi_client_test_netplan b/bin/wifi_client_test_netplan |
2 | new file mode 100755 |
3 | index 0000000..86b6f01 |
4 | --- /dev/null |
5 | +++ b/bin/wifi_client_test_netplan |
6 | @@ -0,0 +1,172 @@ |
7 | +#!/usr/bin/env python3 |
8 | +# Copyright 2018 Canonical Ltd. |
9 | +# All rights reserved. |
10 | +# |
11 | +# Written by: |
12 | +# Gavin Lin <gavin.lin@canonical.com> |
13 | + |
14 | +""" |
15 | +This script will test Wi-Fi device with netplan automatically. |
16 | + |
17 | +To see how to use, please run "./wifi_client_test_netplan --help" |
18 | +""" |
19 | + |
20 | +import argparse |
21 | +import os |
22 | +import subprocess |
23 | +import sys |
24 | +import time |
25 | +import shutil |
26 | + |
27 | + |
28 | +# Configuration file path |
29 | +if "SNAP_USER_DATA" in os.environ: |
30 | + TMP_PATH = os.path.join(os.environ["SNAP_USER_DATA"], |
31 | + "WIFI-TEST-NETPLAN-CREATED-BY-CHECKBOX") |
32 | +else: |
33 | + TMP_PATH = os.path.join("/tmp", "WIFI-TEST-NETPLAN-CREATED-BY-CHECKBOX") |
34 | +CFG_PATH = "/etc/netplan" |
35 | +CFG_FILE = "/etc/netplan/99-CREATED-BY-CHECKBOX.yaml" |
36 | + |
37 | +# Read arguments |
38 | +parser = argparse.ArgumentParser( |
39 | + description=("This script will test wireless network with netplan" |
40 | + " in client mode.")) |
41 | +parser.add_argument( |
42 | + "-i", "--interface", type=str, help=( |
43 | + "The interface which will be tested, default is wlan0"), |
44 | + default="wlan0") |
45 | +parser.add_argument( |
46 | + "-s", "--ssid", type=str, help=( |
47 | + "SSID of target network, this is required argument"), |
48 | + required=True) |
49 | +parser.add_argument( |
50 | + "-k", "--psk", type=str, help=( |
51 | + "Pre-shared key of target network, this is optional argument," |
52 | + " only for PSK protected network")) |
53 | +parser.add_argument( |
54 | + "-d", "--dhcp", action='store_true', help=( |
55 | + "Enable DHCP for IPv4"), |
56 | + default=False) |
57 | +parser.add_argument( |
58 | + "-a", "--address", type=str, help=( |
59 | + "Set ip address for the test interface, optional argument," |
60 | + " example: 192.168.1.1/24"), |
61 | + default="") |
62 | +parser.add_argument( |
63 | + "-p", "--ping", action='store_true', help=( |
64 | + "Enable ping test, will only check link status if not enabled"), |
65 | + default=False) |
66 | +parser.add_argument( |
67 | + "-v", "--server", type=str, help=( |
68 | + "Set ip of server which respond to ping, default is default gateway," |
69 | + " example: 192.168.1.2"), |
70 | + default="") |
71 | +args = parser.parse_args() |
72 | + |
73 | +# Check if executed with root |
74 | +if os.geteuid() != 0: |
75 | + raise SystemExit("Error: please run this command as root") |
76 | + |
77 | +# Backup current network configuration file |
78 | +shutil.rmtree(TMP_PATH, ignore_errors=True) |
79 | +try: |
80 | + shutil.copytree(CFG_PATH, TMP_PATH, symlinks=True) |
81 | +except: |
82 | + shutil.rmtree(TMP_PATH, ignore_errors=True) |
83 | + raise SystemExit("Failed to backup network config files, exiting...") |
84 | + |
85 | +# Remove configuration file, since backup created successfully, |
86 | +# ignore exception while deleting |
87 | +for item in os.listdir(CFG_PATH): |
88 | + target_path = os.path.join(CFG_PATH, item) |
89 | + if os.path.isfile(target_path): |
90 | + try: |
91 | + os.remove(target_path) |
92 | + except: |
93 | + pass |
94 | + elif os.path.isdir(target_path): |
95 | + shutil.rmtree(target_path, ignore_errors=True) |
96 | + |
97 | +# If there's any file left in configuration folder, |
98 | +# then there's something not excepted, stop the test |
99 | +if os.listdir(CFG_PATH): |
100 | + print("Failed to manipulate network config file") |
101 | + print("Restoring network config file from backup...") |
102 | + try: |
103 | + for item in os.listdir(TMP_PATH): |
104 | + target_path = os.path.join(TMP_PATH, item) |
105 | + shutil.move(target_path, CFG_PATH) |
106 | + except: |
107 | + raise SystemExit("Failed to restore network config file from " + |
108 | + TMP_PATH + ", please check it manually") |
109 | + shutil.rmtree(TMP_PATH, ignore_errors=True) |
110 | + raise SystemExit("Configutation file restored, exiting...") |
111 | + |
112 | +# Create wireless network test configuration file |
113 | +if args.psk: |
114 | + pass_field = "password: " + args.psk |
115 | +else: |
116 | + pass_field = "" |
117 | + |
118 | +# Template |
119 | +np_cfg = """# This is the network config written by checkbox |
120 | +network: |
121 | + version: 2 |
122 | + wifis: |
123 | + {interface}: |
124 | + access-points: |
125 | + {ssid}: {{{password}}} |
126 | + addresses: [{address}] |
127 | + dhcp4: {dhcp} |
128 | + nameservers: {{}}""" |
129 | + |
130 | +# Fill data and write to file |
131 | +cfg_data = {"interface": args.interface, "ssid": args.ssid, |
132 | + "password": pass_field, "address": args.address, |
133 | + "dhcp": str(args.dhcp)} |
134 | +with open(CFG_FILE, "w", encoding="utf-8") as wifi_cfg_file: |
135 | + wifi_cfg_file.write(np_cfg.format(**cfg_data)) |
136 | + |
137 | +# Bring up the interface |
138 | +subprocess.call(["netplan", "apply"]) |
139 | +time.sleep(10) |
140 | + |
141 | +# Check connection by ping or link status |
142 | +server_ip = "" |
143 | +if args.ping: |
144 | + # Get gateway ip for ping test |
145 | + if args.server == "": |
146 | + route_table = subprocess.check_output(["route", "-n"]).decode() |
147 | + for line in route_table.split("\n"): |
148 | + if args.interface in line and "UG" in line: |
149 | + server_ip = line.split()[1] |
150 | + else: |
151 | + server_ip = args.server |
152 | + # Test connection by ping |
153 | + exit_code = subprocess.call([ |
154 | + "ping", "-c", "5", "-I", args.interface, server_ip]) |
155 | +else: |
156 | + # Test connection by checking if link established |
157 | + test_output = subprocess.check_output([ |
158 | + "iw", args.interface, "link"]).decode() |
159 | + if test_output.find("SSID") != -1: |
160 | + print("Connection test passed") |
161 | + exit_code = 0 |
162 | + else: |
163 | + print("Connection test failed") |
164 | + exit_code = 1 |
165 | + |
166 | +# Restore configuration file and apply |
167 | +os.remove(CFG_FILE) |
168 | +try: |
169 | + for item in os.listdir(TMP_PATH): |
170 | + target_path = os.path.join(TMP_PATH, item) |
171 | + shutil.move(target_path, CFG_PATH) |
172 | +except: |
173 | + raise SystemExit("Failed to restore network config file from " + |
174 | + TMP_PATH + ", please check it manually") |
175 | +shutil.rmtree(TMP_PATH, ignore_errors=True) |
176 | +subprocess.call(["netplan", "apply"]) |
177 | +time.sleep(10) |
178 | +sys.exit(exit_code) |
179 | diff --git a/units/wireless/jobs.pxu b/units/wireless/jobs.pxu |
180 | index dded027..ac8b4da 100644 |
181 | --- a/units/wireless/jobs.pxu |
182 | +++ b/units/wireless/jobs.pxu |
183 | @@ -130,6 +130,120 @@ flags: preserve-locale also-after-suspend |
184 | |
185 | unit: template |
186 | template-resource: device |
187 | +template-filter: device.category == 'WIRELESS' |
188 | +template-engine: jinja2 |
189 | +template-unit: job |
190 | +id: wireless/wireless_connection_open_ac_np_{{ interface }} |
191 | +_summary: |
192 | + Connect to unencrypted 802.11ac Wi-Fi network on {{ interface }} - netplan |
193 | +_purpose: |
194 | + Check system can connect to insecure 802.11ac AP using netplan |
195 | +plugin: shell |
196 | +command: |
197 | + wifi_client_test_netplan -i {{ interface }} -s $OPEN_AC_SSID -dp |
198 | +user: root |
199 | +environ: LD_LIBRARY_PATH OPEN_AC_SSID |
200 | +category_id: wifi |
201 | +estimated_duration: 15 |
202 | +flags: preserve-locale also-after-suspend |
203 | + |
204 | +unit: template |
205 | +template-resource: device |
206 | +template-filter: device.category == 'WIRELESS' |
207 | +template-engine: jinja2 |
208 | +template-unit: job |
209 | +id: wireless/wireless_connection_open_bg_np_{{ interface }} |
210 | +_summary: |
211 | + Connect to unencrypted 802.11b/g Wi-Fi network on {{ interface }} - netplan |
212 | +_purpose: |
213 | + Check system can connect to insecure 802.11b/g AP using netplan |
214 | +plugin: shell |
215 | +command: |
216 | + wifi_client_test_netplan -i {{ interface }} -s $OPEN_BG_SSID -dp |
217 | +user: root |
218 | +environ: LD_LIBRARY_PATH OPEN_BG_SSID |
219 | +category_id: wifi |
220 | +estimated_duration: 15 |
221 | +flags: preserve-locale also-after-suspend |
222 | + |
223 | +unit: template |
224 | +template-resource: device |
225 | +template-filter: device.category == 'WIRELESS' |
226 | +template-engine: jinja2 |
227 | +template-unit: job |
228 | +id: wireless/wireless_connection_open_n_np_{{ interface }} |
229 | +_summary: |
230 | + Connect to unencrypted 802.11n Wi-Fi network on {{ interface }} - netplan |
231 | +_purpose: |
232 | + Check system can connect to insecure 802.11n AP using netplan |
233 | +plugin: shell |
234 | +command: |
235 | + wifi_client_test_netplan -i {{ interface }} -s $OPEN_N_SSID -dp |
236 | +user: root |
237 | +environ: LD_LIBRARY_PATH OPEN_N_SSID |
238 | +category_id: wifi |
239 | +estimated_duration: 15 |
240 | +flags: preserve-locale also-after-suspend |
241 | + |
242 | +unit: template |
243 | +template-resource: device |
244 | +template-filter: device.category == 'WIRELESS' |
245 | +template-engine: jinja2 |
246 | +template-unit: job |
247 | +id: wireless/wireless_connection_wpa_ac_np_{{ interface }} |
248 | +_summary: |
249 | + Connect to WPA-encrypted 802.11ac Wi-Fi network on {{ interface }} - netplan |
250 | +_purpose: |
251 | + Check system can connect to 802.11ac AP with wpa security using netplan |
252 | +plugin: shell |
253 | +command: |
254 | + wifi_client_test_netplan -i {{ interface }} -s $WPA_AC_SSID -k $WPA_AC_PSK -dp |
255 | +user: root |
256 | +environ: LD_LIBRARY_PATH WPA_AC_SSID WPA_AC_PSK |
257 | +category_id: wifi |
258 | +estimated_duration: 15 |
259 | +flags: preserve-locale also-after-suspend |
260 | + |
261 | +unit: template |
262 | +template-resource: device |
263 | +template-filter: device.category == 'WIRELESS' |
264 | +template-engine: jinja2 |
265 | +template-unit: job |
266 | +id: wireless/wireless_connection_wpa_bg_np_{{ interface }} |
267 | +_summary: |
268 | + Connect to WPA-encrypted 802.11b/g Wi-Fi network on {{ interface }} - netplan |
269 | +_purpose: |
270 | + Check system can connect to 802.11b/g AP with wpa security using netplan |
271 | +plugin: shell |
272 | +command: |
273 | + wifi_client_test_netplan -i {{ interface }} -s $WPA_BG_SSID -k $WPA_BG_PSK -dp |
274 | +user: root |
275 | +environ: LD_LIBRARY_PATH WPA_BG_SSID WPA_BG_PSK |
276 | +category_id: wifi |
277 | +estimated_duration: 15 |
278 | +flags: preserve-locale also-after-suspend |
279 | + |
280 | +unit: template |
281 | +template-resource: device |
282 | +template-filter: device.category == 'WIRELESS' |
283 | +template-engine: jinja2 |
284 | +template-unit: job |
285 | +id: wireless/wireless_connection_wpa_n_np_{{ interface }} |
286 | +_summary: |
287 | + Connect to WPA-encrypted 802.11n Wi-Fi network on {{ interface }} - netplan |
288 | +_purpose: |
289 | + Check system can connect to 802.11n AP with wpa security using netplan |
290 | +plugin: shell |
291 | +command: |
292 | + wifi_client_test_netplan -i {{ interface }} -s $WPA_N_SSID -k $WPA_N_PSK -dp |
293 | +user: root |
294 | +environ: LD_LIBRARY_PATH WPA_N_SSID WPA_N_PSK |
295 | +category_id: wifi |
296 | +estimated_duration: 15 |
297 | +flags: preserve-locale also-after-suspend |
298 | + |
299 | +unit: template |
300 | +template-resource: device |
301 | template-filter: device.category == 'WIRELESS' and device.mac != 'UNKNOWN' |
302 | id: wireless/wowlan_S5_{interface}_wakeonlan |
303 | _summary: Wake on Wireless LAN (WoWLAN) test from S5 - {interface} - wakeonlan |
304 | diff --git a/units/wireless/test-plan.pxu b/units/wireless/test-plan.pxu |
305 | index 0650f97..f7817ce 100644 |
306 | --- a/units/wireless/test-plan.pxu |
307 | +++ b/units/wireless/test-plan.pxu |
308 | @@ -40,6 +40,22 @@ include: |
309 | bootstrap_include: |
310 | device |
311 | |
312 | +id: wireless-netplan-automated |
313 | +unit: test plan |
314 | +_name: Automated tests for wireless using netplan |
315 | +_description: |
316 | + Automated connection tests for unencrypted or WPA-encrypted 802.11 bg, n, ac |
317 | + networks using netplan. |
318 | +include: |
319 | + wireless/wireless_connection_open_ac_np_.* |
320 | + wireless/wireless_connection_open_bg_np_.* |
321 | + wireless/wireless_connection_open_n_np_.* |
322 | + wireless/wireless_connection_wpa_ac_np_.* |
323 | + wireless/wireless_connection_wpa_bg_np_.* |
324 | + wireless/wireless_connection_wpa_n_np_.* |
325 | +bootstrap_include: |
326 | + device |
327 | + |
328 | id: wireless-wifi-master-mode |
329 | unit: test plan |
330 | _name: QA tests for wifi master mode |
331 | @@ -124,6 +140,22 @@ include: |
332 | bootstrap_include: |
333 | device |
334 | |
335 | +id: after-suspnend-wireless-netplan-automated |
336 | +unit: test plan |
337 | +_name: Automated tests for wireless using netplan (after suspend) |
338 | +_description: |
339 | + Automated connection tests for unencrypted or WPA-encrypted 802.11 bg, n, ac |
340 | + networks using netplan. |
341 | +include: |
342 | + after-suspend-wireless/wireless_connection_open_ac_np_.* |
343 | + after-suspend-wireless/wireless_connection_open_bg_np_.* |
344 | + after-suspend-wireless/wireless_connection_open_n_np_.* |
345 | + after-suspend-wireless/wireless_connection_wpa_ac_np_.* |
346 | + after-suspend-wireless/wireless_connection_wpa_bg_np_.* |
347 | + after-suspend-wireless/wireless_connection_wpa_n_np_.* |
348 | +bootstrap_include: |
349 | + device |
350 | + |
351 | id: after-suspend-wireless-wifi-master-mode |
352 | unit: test plan |
353 | _name: QA tests for wifi master mode (after suspend) |
One big problem is a potential loss of config due to imperative moves on /etc/netplan.
I recommend using ContextManagers to guarantee proper rollback.
As for things that are not dangerous, but could be improved:
ping and connection tests should be functions returning a boolean indicating success, this way nested if/elses would disappear, as only return test_ping() would suffice.
Instead of using shell utilities I recommend using builtin shutil module. It's cleaner, less error prone and more informative when something goes wrong.
Also see inlines.