Merge ~alfonsosanchezbeato/snappy-hwe-snaps/+git/wifi-ap:fix-wifi-ap-restart into ~snappy-hwe-team/snappy-hwe-snaps/+git/wifi-ap:master
- Git
- lp:~alfonsosanchezbeato/snappy-hwe-snaps/+git/wifi-ap
- fix-wifi-ap-restart
- Merge into master
Status: | Merged |
---|---|
Approved by: | Alfonso Sanchez-Beato |
Approved revision: | b619722a038c20a74191c3071a57cfd236b3b256 |
Merged at revision: | 06b22bcbb7cf0aa96fb80a26b4c3fa18cedd91de |
Proposed branch: | ~alfonsosanchezbeato/snappy-hwe-snaps/+git/wifi-ap:fix-wifi-ap-restart |
Merge into: | ~snappy-hwe-team/snappy-hwe-snaps/+git/wifi-ap:master |
Diff against target: |
396 lines (+87/-76) 6 files modified
bin/ap.sh (+57/-52) bin/automatic-setup.sh (+3/-3) bin/config-internal.sh (+3/-3) bin/helper.sh (+5/-5) snapcraft.yaml (+19/-10) tests/lib/prepare.sh (+0/-3) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
System Enablement Bot | continuous-integration | Approve | |
Oliver Grawert | Approve | ||
Review via email: mp+355136@code.launchpad.net |
Commit message
Description of the change
Some clean-ups and fix for lp: #1792923
System Enablement Bot (system-enablement-ci-bot) wrote : | # |
PASSED: Successfully build documentation, rev: b619722a038c20a
Generated documentation is available at https:/
System Enablement Bot (system-enablement-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:b619722a038
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
None: https:/
Click here to trigger a rebuild:
https:/
System Enablement Bot (system-enablement-ci-bot) wrote : | # |
PASSED: Successfully build documentation, rev: b619722a038c20a
Generated documentation is available at https:/
System Enablement Bot (system-enablement-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:b619722a038
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
None: https:/
Click here to trigger a rebuild:
https:/
System Enablement Bot (system-enablement-ci-bot) wrote : | # |
PASSED: Successfully build documentation, rev: b619722a038c20a
Generated documentation is available at https:/
System Enablement Bot (system-enablement-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:b619722a038
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
None: https:/
Click here to trigger a rebuild:
https:/
System Enablement Bot (system-enablement-ci-bot) wrote : | # |
PASSED: Successfully build documentation, rev: 2d5eda248ff0067
Generated documentation is available at https:/
System Enablement Bot (system-enablement-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2d5eda248ff
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
None: https:/
Click here to trigger a rebuild:
https:/
System Enablement Bot (system-enablement-ci-bot) wrote : | # |
PASSED: Successfully build documentation, rev: 684c97d1238dbde
Generated documentation is available at https:/
System Enablement Bot (system-enablement-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:684c97d1238
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
None: https:/
Click here to trigger a rebuild:
https:/
Preview Diff
1 | diff --git a/bin/ap.sh b/bin/ap.sh |
2 | index be1f440..add2212 100755 |
3 | --- a/bin/ap.sh |
4 | +++ b/bin/ap.sh |
5 | @@ -14,22 +14,22 @@ |
6 | # You should have received a copy of the GNU General Public License |
7 | # along with this program. If not, see <http://www.gnu.org/licenses/>. |
8 | |
9 | -if [ $(id -u) -ne 0 ] ; then |
10 | +if [ "$(id -u)" -ne 0 ] ; then |
11 | echo "ERROR: $0 needs to be executed as root!" |
12 | exit 1 |
13 | fi |
14 | |
15 | -. $SNAP/bin/config-internal.sh |
16 | +. "$SNAP"/bin/config-internal.sh |
17 | |
18 | -if [ $DEBUG = "true" ]; then |
19 | +if [ "$DEBUG" = "true" ]; then |
20 | set -x |
21 | fi |
22 | |
23 | # Now after we have enabled debugging or not we can safely load |
24 | # all others necessary bits. |
25 | -. $SNAP/bin/helper.sh |
26 | +. "$SNAP"/bin/helper.sh |
27 | |
28 | -if [ $DISABLED = "true" ] ; then |
29 | +if [ "$DISABLED" = "true" ] ; then |
30 | echo "Not starting as WiFi AP is disabled" |
31 | exit 0 |
32 | fi |
33 | @@ -38,23 +38,24 @@ DEFAULT_ACCESS_POINT_INTERFACE="ap0" |
34 | |
35 | # Make sure the configured WiFi interface is really available before |
36 | # doing anything. |
37 | -if ! ifconfig $WIFI_INTERFACE ; then |
38 | +if ! ifconfig "$WIFI_INTERFACE" ; then |
39 | echo "ERROR: WiFi interface $WIFI_INTERFACE is not available!" |
40 | exit 1 |
41 | fi |
42 | |
43 | -cleanup_on_exit() { |
44 | - read HOSTAPD_PID <$SNAP_DATA/hostapd.pid |
45 | - if [ -n "$HOSTAPD_PID" ] ; then |
46 | - kill -TERM $HOSTAPD_PID || true |
47 | - wait $HOSTAPD_PID |
48 | +do_cleanup() { |
49 | + if [ -f "$SNAP_DATA"/hostapd.pid ] ; then |
50 | + read -r HOSTAPD_PID <"$SNAP_DATA"/hostapd.pid |
51 | + kill -TERM "$HOSTAPD_PID" || true |
52 | + wait "$HOSTAPD_PID" |
53 | + rm "$SNAP_DATA"/hostapd.pid |
54 | fi |
55 | |
56 | - read DNSMASQ_PID <$SNAP_DATA/dnsmasq.pid |
57 | - if [ -n "$DNSMASQ_PID" ] ; then |
58 | + if [ -f "$SNAP_DATA"/dnsmasq.pid ] ; then |
59 | + read -r DNSMASQ_PID <"$SNAP_DATA"/dnsmasq.pid |
60 | # If dnsmasq is already gone don't error out here |
61 | - kill -TERM $DNSMASQ_PID || true |
62 | - wait $DNSMASQ_PID |
63 | + kill -TERM "$DNSMASQ_PID" || true |
64 | + wait "$DNSMASQ_PID" |
65 | fi |
66 | |
67 | iface=$WIFI_INTERFACE |
68 | @@ -62,28 +63,30 @@ cleanup_on_exit() { |
69 | iface=$DEFAULT_ACCESS_POINT_INTERFACE |
70 | fi |
71 | |
72 | - if [ $SHARE_DISABLED = "false" ] ; then |
73 | + if [ "$SHARE_DISABLED" = "false" ] ; then |
74 | # flush forwarding rules out |
75 | - iptables --table nat --delete POSTROUTING --out-interface $SHARE_NETWORK_INTERFACE -j MASQUERADE |
76 | + iptables --table nat --delete POSTROUTING --out-interface "$SHARE_NETWORK_INTERFACE" -j MASQUERADE |
77 | iptables --delete FORWARD --in-interface $iface -j ACCEPT |
78 | sysctl -w net.ipv4.ip_forward=0 |
79 | fi |
80 | |
81 | - if is_nm_running ; then |
82 | + if [ "$WIFI_INTERFACE_MODE" = "virtual" ] ; then |
83 | + "$SNAP"/bin/iw dev $iface del |
84 | + elif is_nm_running ; then |
85 | # Hand interface back to network-manager. This will also trigger the |
86 | # auto connection process inside network-manager to get connected |
87 | # with the previous network. |
88 | - $SNAP/bin/nmcli d set $iface managed yes |
89 | - fi |
90 | - |
91 | - if [ "$WIFI_INTERFACE_MODE" = "virtual" ] ; then |
92 | - $SNAP/bin/iw dev $iface del |
93 | + "$SNAP"/bin/nmcli d set $iface managed yes |
94 | fi |
95 | } |
96 | |
97 | # We need to install this right before we do anything to |
98 | # ensure that we cleanup everything again when we termiante. |
99 | -trap cleanup_on_exit TERM |
100 | +trap do_cleanup TERM |
101 | + |
102 | +# Make initial clean-up for the case of the script being killed without |
103 | +# a chance of finishing cleanly. |
104 | +do_cleanup |
105 | |
106 | iface=$WIFI_INTERFACE |
107 | if [ "$WIFI_INTERFACE_MODE" = "virtual" ] ; then |
108 | @@ -92,7 +95,7 @@ if [ "$WIFI_INTERFACE_MODE" = "virtual" ] ; then |
109 | # Make sure if the real wifi interface is connected we use |
110 | # the same channel for our AP as otherwise the created AP |
111 | # will not work. |
112 | - channel_in_use=$($SNAP/bin/iw dev $WIFI_INTERFACE info |awk '/channel/{print$2}') |
113 | + channel_in_use=$("$SNAP"/bin/iw dev "$WIFI_INTERFACE" info |awk '/channel/{print$2}') |
114 | if [ -z "$channel_in_use" ]; then |
115 | echo "WARNING: WiFi is currently not connected so we can't determine" |
116 | echo " which channel we can use for the AP. This will most" |
117 | @@ -109,10 +112,9 @@ fi |
118 | # Create our AP interface if required |
119 | if [ "$WIFI_INTERFACE_MODE" = "virtual" ] ; then |
120 | iface=$DEFAULT_ACCESS_POINT_INTERFACE |
121 | - $SNAP/bin/iw dev $WIFI_INTERFACE interface add $iface type __ap |
122 | - if [ $? -ne 0 ] ; then |
123 | + if ! "$SNAP"/bin/iw dev "$WIFI_INTERFACE" interface add $iface type __ap ; then |
124 | echo "ERROR: Failed to create virtual WiFi network interface" |
125 | - cleanup_on_exit |
126 | + do_cleanup |
127 | fi |
128 | wait_until_interface_is_available $iface |
129 | fi |
130 | @@ -126,53 +128,52 @@ if is_nm_running ; then |
131 | # Prevent network-manager from touching the interface we want to use. If |
132 | # network-manager was configured to use the interface its nothing we want |
133 | # to prevent here as this is how the user configured the system. |
134 | - $SNAP/bin/nmcli d set $iface managed no |
135 | + "$SNAP"/bin/nmcli d set $iface managed no |
136 | fi |
137 | |
138 | # Initial wifi interface configuration |
139 | -ifconfig $iface up |
140 | -if [ $? -ne 0 ] ; then |
141 | +if ! ifconfig "$iface" up ; then |
142 | echo "ERROR: Failed to enable WiFi network interface '$iface'" |
143 | |
144 | # Remove virtual interface again if we created one |
145 | if [ "$WIFI_INTERFACE_MODE" = "virtual" ] ; then |
146 | - $SNAP/bin/iw dev $iface del |
147 | + "$SNAP"/bin/iw dev $iface del |
148 | fi |
149 | |
150 | if is_nm_running ; then |
151 | # Hand interface back to network-manager. This will also trigger the |
152 | # auto connection process inside network-manager to get connected |
153 | # with the previous network. |
154 | - $SNAP/bin/nmcli d set $iface managed yes |
155 | + "$SNAP"/bin/nmcli d set $iface managed yes |
156 | fi |
157 | |
158 | exit 1 |
159 | fi |
160 | |
161 | # Configure interface and give it a moment to settle |
162 | -ifconfig $iface $WIFI_ADDRESS netmask $WIFI_NETMASK |
163 | +ifconfig "$iface" "$WIFI_ADDRESS" netmask "$WIFI_NETMASK" |
164 | sleep 2 |
165 | |
166 | -if [ $SHARE_DISABLED = "false" ] ; then |
167 | +if [ "$SHARE_DISABLED" = "false" ] ; then |
168 | # Enable NAT to forward our network connection |
169 | - iptables --table nat --append POSTROUTING --out-interface $SHARE_NETWORK_INTERFACE -j MASQUERADE |
170 | + iptables --table nat --append POSTROUTING --out-interface "$SHARE_NETWORK_INTERFACE" -j MASQUERADE |
171 | iptables --append FORWARD --in-interface $iface -j ACCEPT |
172 | sysctl -w net.ipv4.ip_forward=1 |
173 | fi |
174 | |
175 | -generate_dnsmasq_config $SNAP_DATA/dnsmasq.conf |
176 | -$SNAP/bin/dnsmasq \ |
177 | +generate_dnsmasq_config "$SNAP_DATA"/dnsmasq.conf |
178 | +"$SNAP"/bin/dnsmasq \ |
179 | -k \ |
180 | - -C $SNAP_DATA/dnsmasq.conf \ |
181 | - -l $SNAP_DATA/dnsmasq.leases \ |
182 | - -x $SNAP_DATA/dnsmasq.pid \ |
183 | + -C "$SNAP_DATA"/dnsmasq.conf \ |
184 | + -l "$SNAP_DATA"/dnsmasq.leases \ |
185 | + -x "$SNAP_DATA"/dnsmasq.pid \ |
186 | -u root -g root \ |
187 | & |
188 | |
189 | driver=$WIFI_HOSTAPD_DRIVER |
190 | |
191 | # Generate our hostapd configuration file |
192 | -cat <<EOF > $SNAP_DATA/hostapd.conf |
193 | +cat <<EOF > "$SNAP_DATA"/hostapd.conf |
194 | interface=$iface |
195 | driver=$driver |
196 | channel=$WIFI_CHANNEL |
197 | @@ -214,7 +215,7 @@ wmm_ac_vo_acm=0 |
198 | EOF |
199 | |
200 | if [ -n "$WIFI_COUNTRY_CODE" ] ; then |
201 | - cat <<-EOF >> $SNAP_DATA/hostapd.conf |
202 | + cat <<-EOF >> "$SNAP_DATA"/hostapd.conf |
203 | # Regulatory domain options |
204 | country_code=$WIFI_COUNTRY_CODE |
205 | # Send country code in beacon frames |
206 | @@ -226,7 +227,7 @@ if [ -n "$WIFI_COUNTRY_CODE" ] ; then |
207 | # End reg domain options |
208 | EOF |
209 | else |
210 | - cat <<-EOF >> $SNAP_DATA/hostapd.conf |
211 | + cat <<-EOF >> "$SNAP_DATA"/hostapd.conf |
212 | # Regulatory domain options |
213 | # Country code set to global |
214 | country_code=XX |
215 | @@ -236,11 +237,11 @@ fi |
216 | |
217 | case "$WIFI_SECURITY" in |
218 | open) |
219 | - cat <<-EOF >> $SNAP_DATA/hostapd.conf |
220 | + cat <<-EOF >> "$SNAP_DATA"/hostapd.conf |
221 | EOF |
222 | ;; |
223 | wpa2) |
224 | - cat <<-EOF >> $SNAP_DATA/hostapd.conf |
225 | + cat <<-EOF >> "$SNAP_DATA"/hostapd.conf |
226 | wpa=2 |
227 | wpa_key_mgmt=WPA-PSK |
228 | wpa_passphrase=$WIFI_SECURITY_PASSPHRASE |
229 | @@ -255,16 +256,20 @@ esac |
230 | |
231 | EXTRA_ARGS= |
232 | if [ "$DEBUG" = "true" ] ; then |
233 | - EXTRA_ARGS="$EXTRA_ARGS -ddd -t" |
234 | + EXTRA_ARGS="-ddd -t" |
235 | fi |
236 | |
237 | -hostapd=$SNAP/bin/hostapd |
238 | +hostapd="$SNAP"/bin/hostapd |
239 | |
240 | # Startup hostapd with the configuration we've put in place |
241 | -$hostapd $EXTRA_ARGS $SNAP_DATA/hostapd.conf & |
242 | +# We need to tolerate no double quotes for EXTRA_ARGS, as it can be |
243 | +# empty - this is safe as the definition of the variable is internal. |
244 | +# shellcheck disable=SC2086 |
245 | +$hostapd $EXTRA_ARGS "$SNAP_DATA"/hostapd.conf & |
246 | hostapd_pid=$! |
247 | -echo $hostapd_pid > $SNAP_DATA/hostapd.pid |
248 | -wait $hostapd_pid |
249 | +echo $hostapd_pid > "$SNAP_DATA"/hostapd.pid |
250 | +# When TERM is received, wait will return and immediately after that |
251 | +# do_cleanup will be called. |
252 | +wait |
253 | |
254 | -cleanup_on_exit |
255 | exit 0 |
256 | diff --git a/bin/automatic-setup.sh b/bin/automatic-setup.sh |
257 | index 871429b..1b8f79f 100755 |
258 | --- a/bin/automatic-setup.sh |
259 | +++ b/bin/automatic-setup.sh |
260 | @@ -19,7 +19,7 @@ set -x |
261 | # Wait for the snap to be successfully setup. This will only be true |
262 | # when the snap is started the first time and the configure hook was |
263 | # never called before. |
264 | -while [ ! -e $SNAP_COMMON/.setup_done ]; do |
265 | +while [ ! -e "$SNAP_COMMON"/.setup_done ]; do |
266 | sleep 0.5 |
267 | done |
268 | |
269 | @@ -27,8 +27,8 @@ done |
270 | |
271 | [ -f "$SNAP_DATA/config" ] && exit 0 |
272 | |
273 | -while ! $SNAP/bin/client status; do |
274 | +while ! "$SNAP"/bin/client status; do |
275 | sleep .5 |
276 | done |
277 | |
278 | -exec $SNAP/bin/client wizard --auto |
279 | +exec "$SNAP"/bin/client wizard --auto |
280 | diff --git a/bin/config-internal.sh b/bin/config-internal.sh |
281 | index aa48897..c2768ac 100755 |
282 | --- a/bin/config-internal.sh |
283 | +++ b/bin/config-internal.sh |
284 | @@ -14,14 +14,14 @@ |
285 | # You should have received a copy of the GNU General Public License |
286 | # along with this program. If not, see <http://www.gnu.org/licenses/>. |
287 | |
288 | -. $SNAP/conf/default-config |
289 | +. "$SNAP"/conf/default-config |
290 | |
291 | # We allow the user to place two configuration files. One which |
292 | # he can provide on its own in $SNAP_USER_DATA/config and one |
293 | # which only our scripts will modify in $SNAP_DATA/config |
294 | if [ -e "$SNAP_DATA/config" ] ; then |
295 | - . $SNAP_DATA/config |
296 | + . "$SNAP_DATA"/config |
297 | fi |
298 | if [ -e "$SNAP_USER_DATA/config" ] ; then |
299 | - . $SNAP_USER_DATA/config |
300 | + . "$SNAP_USER_DATA"/config |
301 | fi |
302 | diff --git a/bin/helper.sh b/bin/helper.sh |
303 | index 147fe89..f3426a3 100644 |
304 | --- a/bin/helper.sh |
305 | +++ b/bin/helper.sh |
306 | @@ -15,18 +15,18 @@ |
307 | # along with this program. If not, see <http://www.gnu.org/licenses/>. |
308 | |
309 | does_interface_exist() { |
310 | - [ -d /sys/class/net/$1 ] |
311 | + [ -d /sys/class/net/"$1" ] |
312 | } |
313 | |
314 | wait_until_interface_is_available() { |
315 | - while ! does_interface_exist $1; do |
316 | + while ! does_interface_exist "$1"; do |
317 | # Wait for 200ms |
318 | sleep 0.2 |
319 | done |
320 | } |
321 | |
322 | assert_not_managed_by_ifupdown() { |
323 | - if [ -e /etc/network/interfaces.d/$1 ]; then |
324 | + if [ -e /etc/network/interfaces.d/"$1" ]; then |
325 | echo "ERROR: Interface $1 is managed by ifupdown and can't be used" |
326 | exit 1 |
327 | fi |
328 | @@ -49,10 +49,10 @@ generate_dnsmasq_config() { |
329 | dhcp-range=$DHCP_RANGE_START,$DHCP_RANGE_STOP,$DHCP_LEASE_TIME |
330 | dhcp-option=6, $WIFI_ADDRESS |
331 | EOF |
332 | - } > $1 |
333 | + } > "$1" |
334 | } |
335 | |
336 | is_nm_running() { |
337 | - nm_status=`$SNAP/bin/nmcli -t -f RUNNING general` |
338 | + nm_status=$("$SNAP"/bin/nmcli -t -f RUNNING general) |
339 | [ "$nm_status" = "running" ] |
340 | } |
341 | diff --git a/snapcraft.yaml b/snapcraft.yaml |
342 | index fbbdb2d..a1d3190 100644 |
343 | --- a/snapcraft.yaml |
344 | +++ b/snapcraft.yaml |
345 | @@ -70,18 +70,27 @@ parts: |
346 | prime: |
347 | - $binaries |
348 | |
349 | + go: |
350 | + source-tag: go1.9.2 |
351 | + |
352 | service: |
353 | - plugin: go |
354 | - source: . |
355 | - go-importpath: launchpad.net/wifi-ap |
356 | - prime: |
357 | - - bin |
358 | - install: | |
359 | - export GOPATH=$PWD/../go |
360 | + after: [go] |
361 | + plugin: make |
362 | + build: | |
363 | + set -ex |
364 | + export GOPATH=$(mktemp -d) |
365 | + (src_path="$GOPATH"/src/launchpad.net/wifi-ap |
366 | + mkdir -p "$src_path" |
367 | + cp -a cmd "$src_path" |
368 | + cd "$src_path" |
369 | + go get -t -d ... |
370 | + mkdir -p $SNAPCRAFT_PART_INSTALL/bin/ |
371 | for d in client service ; do |
372 | - cd $GOPATH/src/launchpad.net/wifi-ap/cmd/$d |
373 | - go test -v |
374 | - done |
375 | + cd $GOPATH/src/launchpad.net/wifi-ap/cmd/"$d" |
376 | + go build |
377 | + cp -f "$d" $SNAPCRAFT_PART_INSTALL/bin/ |
378 | + done) |
379 | + rm -rf "$GOPATH" |
380 | |
381 | dnsmasq: |
382 | plugin: make |
383 | diff --git a/tests/lib/prepare.sh b/tests/lib/prepare.sh |
384 | index 7e27528..be54593 100644 |
385 | --- a/tests/lib/prepare.sh |
386 | +++ b/tests/lib/prepare.sh |
387 | @@ -20,9 +20,6 @@ for name in $gadget_name $kernel_name $core_name; do |
388 | fi |
389 | done |
390 | |
391 | -echo "Kernel has a store revision" |
392 | -snap list | grep ^${kernel_name} | grep -E " [0-9]+\s+canonical" |
393 | - |
394 | # Snapshot of the current snapd state for a later restore |
395 | if [ ! -f $SPREAD_PATH/snapd-state.tar.gz ] ; then |
396 | systemctl stop snapd.service snapd.socket |
looks good (unrelated, i'm not sure if core18 will still ship the ifconfig binary, it might be good to move the ifconfig calls to become "ip" calls in a future commit)