Merge ~alfonsosanchezbeato/snappy-hwe-snaps/+git/wifi-ap:fix-wifi-ap-restart into ~snappy-hwe-team/snappy-hwe-snaps/+git/wifi-ap:master

Proposed by Alfonso Sanchez-Beato
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)
Reviewer Review Type Date Requested Status
System Enablement Bot continuous-integration Approve
Oliver Grawert Approve
Review via email: mp+355136@code.launchpad.net

Description of the change

Some clean-ups and fix for lp: #1792923

To post a comment you must log in.
Revision history for this message
Oliver Grawert (ogra) wrote :

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)

review: Approve
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :

PASSED: Successfully build documentation, rev: b619722a038c20a74191c3071a57cfd236b3b256

Generated documentation is available at https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1117/

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :

PASSED: Successfully build documentation, rev: b619722a038c20a74191c3071a57cfd236b3b256

Generated documentation is available at https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1118/

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :

PASSED: Successfully build documentation, rev: b619722a038c20a74191c3071a57cfd236b3b256

Generated documentation is available at https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1119/

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :

PASSED: Successfully build documentation, rev: 2d5eda248ff006740f11d3ae612b787389ba7a92

Generated documentation is available at https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1120/

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :

PASSED: Successfully build documentation, rev: 684c97d1238dbde1071f6c99d03c2bdc03039faa

Generated documentation is available at https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1121/

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/bin/ap.sh b/bin/ap.sh
2index 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
256diff --git a/bin/automatic-setup.sh b/bin/automatic-setup.sh
257index 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
280diff --git a/bin/config-internal.sh b/bin/config-internal.sh
281index 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
302diff --git a/bin/helper.sh b/bin/helper.sh
303index 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 }
341diff --git a/snapcraft.yaml b/snapcraft.yaml
342index 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
383diff --git a/tests/lib/prepare.sh b/tests/lib/prepare.sh
384index 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

Subscribers

People subscribed via source and target branches