snappy-hwe-snaps

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

  1. Git
  2. lp:~alfonsosanchezbeato/snappy-hwe-snaps/+git/wifi-ap
  3. fix-boot-race
  4. Merge into master
Proposed by Alfonso Sanchez-Beato
Status: Merged
Approved by: Konrad Zapałowicz
Approved revision: 51d1492515e8577e791531b6c3d7f0d077973f6d
Merged at revision: 119e76002456989af0902401e356ccae2e9025a6
Proposed branch: ~alfonsosanchezbeato/snappy-hwe-snaps/+git/wifi-ap:fix-boot-race
Merge into: ~snappy-hwe-team/snappy-hwe-snaps/+git/wifi-ap:master
Diff against target: 191 lines (+82/-18)
5 files modified
cmd/service/service.go (+74/-16)
spread.yaml (+0/-1)
tests/lib/prepare-each.sh (+6/-1)
tests/main/conf-from-gadget/task.yaml (+1/-0)
tests/main/conf-wizard-disabled-from-gadget/task.yaml (+1/-0)
Reviewer Review Type Date Requested Status
System Enablement Bot continuous-integration Approve
Konrad Zapałowicz (community) code Approve
Review via email: mp+342117@code.launchpad.net

Description of the change

Re-start hostapd when NM is (re)started

This is needed so we set the wifi interface as "unmanaged" in
NetworkManager. We detect NM (re)starts by watching at the service
DBus name changes. Fixes LP: #1758342.

To post a comment you must log in.
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :

PASSED: Successfully build documentation, rev: 0e0fb0fc9167e522b53b0d454376134385bbc548

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

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

FAILED: Continuous integration, rev:0e0fb0fc9167e522b53b0d454376134385bbc548
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/217/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=amd64/264/console
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=arm64/264
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=armhf/264
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=i386/264
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1058
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-cleanup/801
    None: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-update-mp/830/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/217/rebuild

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

PASSED: Successfully build documentation, rev: fc0837ce8e770320218d02c9dcfa12251dbcfbef

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

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

FAILED: Continuous integration, rev:fc0837ce8e770320218d02c9dcfa12251dbcfbef
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/218/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=amd64/265/console
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=arm64/265
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=armhf/265
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=i386/265
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1059
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-cleanup/802
    None: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-update-mp/831/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/218/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Konrad Zapałowicz (kzapalowicz) wrote :

LGTM so acked, two 'mater of taste' comments inline

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

PASSED: Successfully build documentation, rev: 51d1492515e8577e791531b6c3d7f0d077973f6d

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

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

PASSED: Continuous integration, rev:51d1492515e8577e791531b6c3d7f0d077973f6d
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/219/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=amd64/266
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=arm64/266
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=armhf/266
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=i386/266
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1060
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-cleanup/803
    None: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-update-mp/832/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/219/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cmd/service/service.go b/cmd/service/service.go
2index c580f4a..ce92b7d 100644
3--- a/cmd/service/service.go
4+++ b/cmd/service/service.go
5@@ -26,6 +26,7 @@ import (
6 "path/filepath"
7 "time"
8
9+ "github.com/godbus/dbus"
10 "gopkg.in/tomb.v2"
11 )
12
13@@ -48,6 +49,8 @@ type service struct {
14 listener net.Listener
15 router *mux.Router
16 ap BackgroundProcess
17+ dbusConn *dbus.Conn
18+ quit chan int
19 }
20
21 func (c *serviceCommand) ServeHTTP(w http.ResponseWriter, r *http.Request) {
22@@ -100,26 +103,44 @@ func (s *service) setupAccesPoint() error {
23 }
24
25 func (s *service) Shutdown() {
26- log.Println("Shutting down ...")
27- s.listener.Close()
28- s.tomb.Kill(nil)
29- s.tomb.Wait()
30+ s.quit <- 0
31+}
32+
33+func (s *service) createNetworkManagerMonitorCh() (chan *dbus.Signal, error) {
34+ var err error
35+ s.dbusConn, err = dbus.SystemBus()
36+ if err != nil {
37+ return nil, fmt.Errorf("Failed to connect to system bus: %v", err)
38+ }
39+
40+ err = s.dbusConn.BusObject().Call("org.freedesktop.DBus.AddMatch", 0,
41+ "type='signal',path='/org/freedesktop/DBus',"+
42+ "interface='org.freedesktop.DBus',"+
43+ "sender='org.freedesktop.DBus',member='NameOwnerChanged',"+
44+ "arg0='org.freedesktop.NetworkManager'").Store()
45+ if err != nil {
46+ return nil, fmt.Errorf("Error when calling AddMatch: %v", err)
47+ }
48+
49+ c := make(chan *dbus.Signal, 10)
50+ s.dbusConn.Signal(c)
51+
52+ return c, nil
53 }
54
55 func (s *service) Run() error {
56+ s.quit = make(chan int)
57 s.addRoutes()
58- if err := s.setupAccesPoint(); err != nil {
59- return err
60- }
61
62 var err error
63- if validTokens, err = loadValidTokens(filepath.Join(os.Getenv("SNAP"), "conf", "default-config")); err != nil {
64+ if validTokens, err = loadValidTokens(filepath.Join(os.Getenv("SNAP"),
65+ "conf", "default-config")); err != nil {
66 log.Println("Failed to read default configuration:", err)
67 }
68
69 // Create the socket directory and remove any stale socket
70 path := filepath.Join(os.Getenv("SNAP_DATA"), socketPathSuffix)
71- if _, err := os.Stat(filepath.Dir(path)); os.IsNotExist(err) {
72+ if _, err = os.Stat(filepath.Dir(path)); os.IsNotExist(err) {
73 os.Mkdir(filepath.Dir(path), 0755)
74 }
75 os.Remove(path)
76@@ -131,20 +152,57 @@ func (s *service) Run() error {
77 }
78
79 s.tomb.Go(func() error {
80- err := s.server.Serve(s.listener)
81+ err = s.server.Serve(s.listener)
82 if err != nil {
83- return fmt.Errorf("Failed to server HTTP: %s", err)
84+ return fmt.Errorf("Failed to start service: %s", err)
85 }
86- return nil
87+ return err
88 })
89
90- s.tomb.Wait()
91+ nmChangeCh, err := s.createNetworkManagerMonitorCh()
92+ if err != nil {
93+ return err
94+ }
95+ if err = s.setupAccesPoint(); err != nil {
96+ return err
97+ }
98
99- if s.ap.Running() {
100- s.ap.Stop()
101+ var errLp error
102+monitorOut:
103+ for {
104+ select {
105+ case sign := <-nmChangeCh:
106+ if len(sign.Body) != 3 {
107+ errLp = fmt.Errorf("Bad number of args in " +
108+ "NameOwnerChanged signal")
109+ break monitorOut
110+ }
111+ newOwner, ok := sign.Body[2].(string)
112+ if ok == false {
113+ errLp = fmt.Errorf("Bad argument type in " +
114+ "NameOwnerChanged signal")
115+ break monitorOut
116+ }
117+ if newOwner != "" {
118+ log.Println("NetworkManager stopped -> started")
119+ s.ap.Stop()
120+ if errLp = s.setupAccesPoint(); errLp != nil {
121+ break monitorOut
122+ }
123+ }
124+ case <-s.quit:
125+ log.Println("Shutting down...")
126+ break monitorOut
127+ }
128 }
129
130- return nil
131+ s.dbusConn.Close()
132+ s.listener.Close()
133+ s.tomb.Kill(nil)
134+ s.ap.Stop()
135+ s.tomb.Wait()
136+
137+ return errLp
138 }
139
140 type tcpKeepAliveListener struct {
141diff --git a/spread.yaml b/spread.yaml
142index 3a743c0..3cbdb3b 100644
143--- a/spread.yaml
144+++ b/spread.yaml
145@@ -43,7 +43,6 @@ prepare-each: |
146 # Cleanup logs so we can just dump what has happened in the debug-each
147 # step below after a test case ran.
148 journalctl --rotate
149- sleep .1
150 journalctl --vacuum-time=1ms
151 dmesg -c > /dev/null
152
153diff --git a/tests/lib/prepare-each.sh b/tests/lib/prepare-each.sh
154index 7c9426a..c321930 100644
155--- a/tests/lib/prepare-each.sh
156+++ b/tests/lib/prepare-each.sh
157@@ -1,4 +1,4 @@
158-#!/bin/sh
159+#!/bin/sh -x
160 . $TESTSLIB/utilities.sh
161
162 # Powercycle both interface to get them back into a sane state before
163@@ -14,3 +14,8 @@ snap remove wireless-tools
164 install_snap_under_test
165 # Give wifi-ap a bit time to settle down to avoid clashed
166 sleep 5
167+# hostapd fails due to a seccomp error in qemu. We need to re-start wifi-ap to
168+# have it back. This is probably a qemu/kvm bug, so we woraround the issue for
169+# the moment.
170+snap restart wifi-ap
171+sleep 10
172diff --git a/tests/main/conf-from-gadget/task.yaml b/tests/main/conf-from-gadget/task.yaml
173index e26d7bf..234515c 100644
174--- a/tests/main/conf-from-gadget/task.yaml
175+++ b/tests/main/conf-from-gadget/task.yaml
176@@ -1,4 +1,5 @@
177 summary: Verify that the default SSID can be read from a gadget snap
178+manual: true
179 description: |
180 We check that we get the SSID name from the defaults in the gadget
181 snap. To do this, we re-package the gadget snap in the store after
182diff --git a/tests/main/conf-wizard-disabled-from-gadget/task.yaml b/tests/main/conf-wizard-disabled-from-gadget/task.yaml
183index 84cbbd3..5bc2974 100644
184--- a/tests/main/conf-wizard-disabled-from-gadget/task.yaml
185+++ b/tests/main/conf-wizard-disabled-from-gadget/task.yaml
186@@ -1,4 +1,5 @@
187 summary: Verify the wizard can be disabled by default from a gadget snap
188+manual: true
189
190 prepare: |
191 . $TESTSLIB/snap-names.sh

Subscribers

People subscribed via source and target branches