Merge ~kissiel/plainbox-provider-checkbox:iperf-fixes into plainbox-provider-checkbox:master

Proposed by Maciej Kisielewski
Status: Merged
Approved by: Maciej Kisielewski
Approved revision: d904ac30495d0e1acacfbc7ba6eec279aab71083
Merged at revision: 77ccfaf23a429fb9a8dc6292079ddb13b9daca47
Proposed branch: ~kissiel/plainbox-provider-checkbox:iperf-fixes
Merge into: plainbox-provider-checkbox:master
Diff against target: 91 lines (+31/-19)
2 files modified
bin/network.py (+25/-19)
units/ethernet/test-plan.pxu (+6/-0)
Reviewer Review Type Date Requested Status
Jonathan Cave (community) Approve
Review via email: mp+403199@code.launchpad.net

Description of the change

This MR brings fixes iperf testing for commercial projects.
See individual commits for details.

To post a comment you must log in.
Revision history for this message
Jonathan Cave (jocave) wrote :

I think previously we had discussed a resource manager type design that would try to catch crashes/errors and restore the initial state. Was that idea rejected for some reason?

Revision history for this message
Maciej Kisielewski (kissiel) wrote :

> I think previously we had discussed a resource manager type design that would
> try to catch crashes/errors and restore the initial state. Was that idea
> rejected for some reason?

Not rejected, but ... scheduled for a later date.
With all the things happening, I wanted to fix the most annoying problem first.

The 'architecture' of network.py makes any major refactor a significant undertaking. I don't know if it's of the highest priority.

Revision history for this message
Jonathan Cave (jocave) wrote :

Thanks, I'm ok with that approach. Code looks good to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/bin/network.py b/bin/network.py
2index 60f7b0a..3ed4770 100755
3--- a/bin/network.py
4+++ b/bin/network.py
5@@ -631,18 +631,19 @@ def interface_test(args):
6
7 error_number = 0
8 # Stop all other interfaces
9- extra_interfaces = \
10- [iface for iface in os.listdir("/sys/class/net")
11- if iface != "lo" and iface != args.interface]
12+ if not args.dont_toggle_ifaces:
13+ extra_interfaces = \
14+ [iface for iface in os.listdir("/sys/class/net")
15+ if iface != "lo" and iface != args.interface]
16
17- for iface in extra_interfaces:
18- logging.debug("Shutting down interface:%s", iface)
19- try:
20- check_call(["ip", "link", "set", "dev", iface, "down"])
21- except CalledProcessError as interface_failure:
22- logging.error("Failed to shut down %s:%s",
23- iface, interface_failure)
24- error_number = 3
25+ for iface in extra_interfaces:
26+ logging.debug("Shutting down interface:%s", iface)
27+ try:
28+ check_call(["ip", "link", "set", "dev", iface, "down"])
29+ except CalledProcessError as interface_failure:
30+ logging.error("Failed to shut down %s:%s",
31+ iface, interface_failure)
32+ error_number = 3
33
34 if error_number == 0:
35 start_time = datetime.datetime.now()
36@@ -664,14 +665,16 @@ def interface_test(args):
37 time.sleep(30)
38 first_loop = False
39
40- for iface in extra_interfaces:
41- logging.debug("Restoring interface:%s", iface)
42- try:
43- check_call(["ip", "link", "set", "dev", iface, "up"])
44- wait_for_iface_up(iface, args.iface_timeout)
45- except CalledProcessError as interface_failure:
46- logging.error("Failed to restore %s:%s", iface, interface_failure)
47- error_number = 3
48+ if not args.dont_toggle_ifaces:
49+ for iface in extra_interfaces:
50+ logging.debug("Restoring interface:%s", iface)
51+ try:
52+ check_call(["ip", "link", "set", "dev", iface, "up"])
53+ wait_for_iface_up(iface, args.iface_timeout)
54+ except CalledProcessError as interface_failure:
55+ logging.error(
56+ "Failed to restore %s:%s", iface, interface_failure)
57+ error_number = 3
58
59 # Restore routing table to original state
60 temp.seek(0)
61@@ -833,6 +836,9 @@ TEST_TARGET_IPERF = iperf-server.example.com
62 test_parser.add_argument(
63 '--reverse', default=False, action="store_true",
64 help="Run in reverse mode (server sends, client receives)")
65+ test_parser.add_argument(
66+ '--dont-toggle-ifaces', default=False, action="store_true",
67+ help="Do not turn of other interfaces while testing.")
68
69 # Sub info options
70 info_parser.add_argument(
71diff --git a/units/ethernet/test-plan.pxu b/units/ethernet/test-plan.pxu
72index 5e003ae..cfc23bd 100644
73--- a/units/ethernet/test-plan.pxu
74+++ b/units/ethernet/test-plan.pxu
75@@ -68,6 +68,9 @@ include:
76 ethernet/ethtool_info
77 ethernet/ethertool_check_.*
78 ethernet/multi_iperf3_nic_device.*
79+bootstrap_include:
80+ device
81+ executable
82
83 id: server-ethernet-underspeed
84 unit: test plan
85@@ -79,3 +82,6 @@ include:
86 ethernet/ethtool_info
87 ethernet/ethertool_check_.*
88 ethernet/multi_iperf3_nic_underspeed_device.*
89+bootstrap_include:
90+ device
91+ executable

Subscribers

People subscribed via source and target branches