Merge lp:~themue/juju-core/go-firewaller-added-global-mode into lp:~juju/juju-core/trunk
- go-firewaller-added-global-mode
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Gustavo Niemeyer |
Approved revision: | no longer in the source branch. |
Merged at revision: | 691 |
Proposed branch: | lp:~themue/juju-core/go-firewaller-added-global-mode |
Merge into: | lp:~juju/juju-core/trunk |
Diff against target: |
467 lines (+361/-17) 2 files modified
worker/firewaller/firewaller.go (+127/-17) worker/firewaller/firewaller_test.go (+234/-0) |
To merge this branch: | bzr merge lp:~themue/juju-core/go-firewaller-added-global-mode |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
The Go Language Gophers | Pending | ||
Review via email: mp+130157@code.launchpad.net |
Commit message
Description of the change
firewaller: integrated global mode
Firewaller now recognizes the global mode. It dies a
port usage counting usage counting and opens each
used port only once and closes it after the last using unit
has gone. The handling of ports in case of a restart has
been removed from this CL to a later one.
Gustavo Niemeyer (niemeyer) wrote : | # |
Gustavo Niemeyer (niemeyer) wrote : | # |
Much better thanks.
Still, what about ports previously opened?
https:/
File worker/
https:/
worker/
closes ports global in the environment.
// flushGlobalPorts opens and closes global ports in the environment.
// It keeps a reference count for ports so that only 0-to-1 and 1-to-0
events
// modify the environment.
https:/
worker/
Please drop. Code is shorter and obvious.
https:/
worker/
Same.
https:/
File worker/
https:/
worker/
ports are open in environment.
Please drop comment ("assertEnviron
https:/
worker/
multiple used port on one machine lets it untouched.
// Closing a port opened by a different unit won't touch the
environment.
https:/
worker/
used port is closed.
// Closing a port used just once changes the environment.
https:/
worker/
usage of a port closes it globally.
// Closing the last port also modifies the environment.
Gustavo Niemeyer (niemeyer) wrote : | # |
https:/
File worker/
https:/
worker/
Frank, what happens if you close ports when stuff is running?
Some out-of-the-box thinking is needed. If we have to discuss trivial
stuff repeatedly, there's an inflection point where it's easier to
implement than to review.
Gustavo Niemeyer (niemeyer) wrote : | # |
On Fri, Oct 19, 2012 at 12:24 PM, <email address hidden> wrote:
> If it's trivial and you already have a concept in mind I don't see
> please let me know, I really appreciate it.
(...)
> I removed my initial concept which relied on the retrieved initial open
> ports from the provider and still does a correct port counting later
Your initial concept had the problems I described. Your new concept
has the problems I described. Both are issues I consider obvious
(closing ports that should be open while services are running!?).
What you have is:
1) Ports that are known open in the provider
2) Simple ways to compute what you need open
3) Simple ways to close and open at will
I cannot break down this problem for you further without implementing
the solution myself, which gets back to the point I raised: there's an
inflection point where reviews are counterproductive if compared to
implementing it myself and asking other people to review it.
Please think about the problem. If you cannot solve it, let's talk
next week and see what kind of problem you'd appreciate working on
instead.
Gustavo Niemeyer (niemeyer) wrote : | # |
https:/
File worker/
https:/
worker/
during gathering of the units.
Why? Doesn't that mean we'll run OpenPort on the environment for every
single port that we just confirmed is open?
Gustavo Niemeyer (niemeyer) wrote : | # |
LGTM, assuming that this works with real usage.
https:/
File worker/
https:/
worker/
s/PortsFlag/
s/PortsCount/
Preview Diff
1 | === modified file 'worker/firewaller/firewaller.go' |
2 | --- worker/firewaller/firewaller.go 2012-10-01 12:29:05 +0000 |
3 | +++ worker/firewaller/firewaller.go 2012-10-22 15:47:21 +0000 |
4 | @@ -3,6 +3,7 @@ |
5 | import ( |
6 | "fmt" |
7 | "launchpad.net/juju-core/environs" |
8 | + "launchpad.net/juju-core/environs/config" |
9 | "launchpad.net/juju-core/log" |
10 | "launchpad.net/juju-core/state" |
11 | "launchpad.net/juju-core/state/watcher" |
12 | @@ -13,17 +14,20 @@ |
13 | // Firewaller watches the state for ports opened or closed |
14 | // and reflects those changes onto the backing environment. |
15 | type Firewaller struct { |
16 | - tomb tomb.Tomb |
17 | - st *state.State |
18 | - environ environs.Environ |
19 | - environWatcher *state.EnvironConfigWatcher |
20 | - machinesWatcher *state.MachinesWatcher |
21 | - machineds map[int]*machineData |
22 | - unitsChange chan *unitsChange |
23 | - unitds map[string]*unitData |
24 | - portsChange chan *portsChange |
25 | - serviceds map[string]*serviceData |
26 | - exposedChange chan *exposedChange |
27 | + tomb tomb.Tomb |
28 | + st *state.State |
29 | + environ environs.Environ |
30 | + environWatcher *state.EnvironConfigWatcher |
31 | + machinesWatcher *state.MachinesWatcher |
32 | + machineds map[int]*machineData |
33 | + unitsChange chan *unitsChange |
34 | + unitds map[string]*unitData |
35 | + portsChange chan *portsChange |
36 | + serviceds map[string]*serviceData |
37 | + exposedChange chan *exposedChange |
38 | + globalMode bool |
39 | + globalPortsFlag map[state.Port]bool |
40 | + globalPortsCount map[state.Port]int |
41 | } |
42 | |
43 | // NewFirewaller returns a new Firewaller. |
44 | @@ -54,6 +58,11 @@ |
45 | if err != nil { |
46 | return err |
47 | } |
48 | + if fw.environ.Config().FirewallMode() == config.FwGlobal { |
49 | + if err := fw.initGlobalMode(); err != nil { |
50 | + return err |
51 | + } |
52 | + } |
53 | for { |
54 | select { |
55 | case <-fw.tomb.Dying(): |
56 | @@ -129,6 +138,61 @@ |
57 | panic("not reached") |
58 | } |
59 | |
60 | +// initGlobalMode retrieves the ports that need to be open globally, |
61 | +// opens them and also closes not needed open ports left from an |
62 | +// earlier run. |
63 | +func (fw *Firewaller) initGlobalMode() error { |
64 | + fw.globalMode = true |
65 | + fw.globalPortsFlag = make(map[state.Port]bool) |
66 | + fw.globalPortsCount = make(map[state.Port]int) |
67 | + initialPorts, err := fw.environ.Ports() |
68 | + if err != nil { |
69 | + return err |
70 | + } |
71 | + // Retrieve ports to be open from state. |
72 | + services, err := fw.st.AllServices() |
73 | + if err != nil { |
74 | + return err |
75 | + } |
76 | + for _, service := range services { |
77 | + if !service.IsExposed() { |
78 | + continue |
79 | + } |
80 | + units, err := service.AllUnits() |
81 | + if err != nil { |
82 | + return err |
83 | + } |
84 | + for _, unit := range units { |
85 | + ports := unit.OpenedPorts() |
86 | + for _, port := range ports { |
87 | + fw.globalPortsFlag[port] = true |
88 | + } |
89 | + } |
90 | + } |
91 | + openedPorts := []state.Port{} |
92 | + for port := range fw.globalPortsFlag { |
93 | + openedPorts = append(openedPorts, port) |
94 | + } |
95 | + // Check which ports to open or to close. |
96 | + toOpen := diff(openedPorts, initialPorts) |
97 | + toClose := diff(initialPorts, openedPorts) |
98 | + if len(toOpen) > 0 { |
99 | + if err := fw.environ.OpenPorts(toOpen); err != nil { |
100 | + return err |
101 | + } |
102 | + state.SortPorts(toOpen) |
103 | + log.Printf("firewaller: initially opened ports %v in environment", toOpen) |
104 | + } |
105 | + if len(toClose) > 0 { |
106 | + if err := fw.environ.ClosePorts(toClose); err != nil { |
107 | + return err |
108 | + } |
109 | + state.SortPorts(toClose) |
110 | + log.Printf("firewaller: initially closed ports %v in environment", toClose) |
111 | + } |
112 | + return nil |
113 | +} |
114 | + |
115 | // flushUnits opens and closes ports for the passed unit data. |
116 | func (fw *Firewaller) flushUnits(unitds []*unitData) error { |
117 | machineds := map[int]*machineData{} |
118 | @@ -161,7 +225,55 @@ |
119 | toOpen := diff(want, machined.ports) |
120 | toClose := diff(machined.ports, want) |
121 | machined.ports = want |
122 | - |
123 | + if fw.globalMode { |
124 | + return fw.flushGlobalPorts(toOpen, toClose) |
125 | + } |
126 | + return fw.flushInstancePorts(machined, toOpen, toClose) |
127 | +} |
128 | + |
129 | +// flushGlobalPorts opens and closes global ports in the environment. |
130 | +// It keeps a reference count for ports so that only 0-to-1 and 1-to-0 events |
131 | +// modify the environment. |
132 | +func (fw *Firewaller) flushGlobalPorts(rawOpen, rawClose []state.Port) error { |
133 | + // Filter which ports are really to open or close. |
134 | + var toOpen, toClose []state.Port |
135 | + for _, port := range rawOpen { |
136 | + if !fw.globalPortsFlag[port] { |
137 | + toOpen = append(toOpen, port) |
138 | + fw.globalPortsFlag[port] = true |
139 | + } |
140 | + fw.globalPortsCount[port]++ |
141 | + } |
142 | + for _, port := range rawClose { |
143 | + fw.globalPortsCount[port]-- |
144 | + if fw.globalPortsCount[port] == 0 { |
145 | + toClose = append(toClose, port) |
146 | + delete(fw.globalPortsFlag, port) |
147 | + delete(fw.globalPortsCount, port) |
148 | + } |
149 | + } |
150 | + // Open and close the ports. |
151 | + if len(toOpen) > 0 { |
152 | + if err := fw.environ.OpenPorts(toOpen); err != nil { |
153 | + // TODO(mue) Add local retry logic. |
154 | + return err |
155 | + } |
156 | + state.SortPorts(toOpen) |
157 | + log.Printf("firewaller: opened ports %v in environment", toOpen) |
158 | + } |
159 | + if len(toClose) > 0 { |
160 | + if err := fw.environ.ClosePorts(toClose); err != nil { |
161 | + // TODO(mue) Add local retry logic. |
162 | + return err |
163 | + } |
164 | + state.SortPorts(toClose) |
165 | + log.Printf("firewaller: closed ports %v in environment", toClose) |
166 | + } |
167 | + return nil |
168 | +} |
169 | + |
170 | +// flushGlobalPorts opens and closes ports global on the machine. |
171 | +func (fw *Firewaller) flushInstancePorts(machined *machineData, toOpen, toClose []state.Port) error { |
172 | // If there's nothing to do, do nothing. |
173 | // This is important because when a machine is first created, |
174 | // it will have no instance id but also no open ports - |
175 | @@ -169,7 +281,6 @@ |
176 | if len(toOpen) == 0 && len(toClose) == 0 { |
177 | return nil |
178 | } |
179 | - // Open and close the ports. |
180 | m, err := machined.machine() |
181 | if state.IsNotFound(err) { |
182 | return nil |
183 | @@ -185,9 +296,9 @@ |
184 | if err != nil { |
185 | return err |
186 | } |
187 | + // Open and close the ports. |
188 | if len(toOpen) > 0 { |
189 | - err = instances[0].OpenPorts(machined.id, toOpen) |
190 | - if err != nil { |
191 | + if err := instances[0].OpenPorts(machined.id, toOpen); err != nil { |
192 | // TODO(mue) Add local retry logic. |
193 | return err |
194 | } |
195 | @@ -195,8 +306,7 @@ |
196 | log.Printf("firewaller: opened ports %v on machine %d", toOpen, machined.id) |
197 | } |
198 | if len(toClose) > 0 { |
199 | - err = instances[0].ClosePorts(machined.id, toClose) |
200 | - if err != nil { |
201 | + if err := instances[0].ClosePorts(machined.id, toClose); err != nil { |
202 | // TODO(mue) Add local retry logic. |
203 | return err |
204 | } |
205 | |
206 | === modified file 'worker/firewaller/firewaller_test.go' |
207 | --- worker/firewaller/firewaller_test.go 2012-10-09 08:18:40 +0000 |
208 | +++ worker/firewaller/firewaller_test.go 2012-10-22 15:47:21 +0000 |
209 | @@ -3,6 +3,7 @@ |
210 | import ( |
211 | . "launchpad.net/gocheck" |
212 | "launchpad.net/juju-core/environs" |
213 | + "launchpad.net/juju-core/environs/config" |
214 | "launchpad.net/juju-core/environs/dummy" |
215 | "launchpad.net/juju-core/juju/testing" |
216 | "launchpad.net/juju-core/state" |
217 | @@ -49,6 +50,32 @@ |
218 | panic("unreachable") |
219 | } |
220 | |
221 | +// assertEnvironPorts retrieves the open ports of environment and compares them |
222 | +// to the expected. |
223 | +func (s *FirewallerSuite) assertEnvironPorts(c *C, expected []state.Port) { |
224 | + s.State.StartSync() |
225 | + start := time.Now() |
226 | + for { |
227 | + got, err := s.Conn.Environ.Ports() |
228 | + if err != nil { |
229 | + c.Fatal(err) |
230 | + return |
231 | + } |
232 | + state.SortPorts(got) |
233 | + state.SortPorts(expected) |
234 | + if reflect.DeepEqual(got, expected) { |
235 | + c.Succeed() |
236 | + return |
237 | + } |
238 | + if time.Since(start) > 5*time.Second { |
239 | + c.Fatalf("timed out: expected %q; got %q", expected, got) |
240 | + return |
241 | + } |
242 | + time.Sleep(50 * time.Millisecond) |
243 | + } |
244 | + panic("unreachable") |
245 | +} |
246 | + |
247 | var _ = Suite(&FirewallerSuite{}) |
248 | |
249 | func (s *FirewallerSuite) SetUpTest(c *C) { |
250 | @@ -72,6 +99,32 @@ |
251 | return u, m |
252 | } |
253 | |
254 | +func (s *FirewallerSuite) setGlobalMode(c *C) func(*C) { |
255 | + oldConfig := s.Conn.Environ.Config() |
256 | + restore := func(rc *C) { |
257 | + attrs := oldConfig.AllAttrs() |
258 | + attrs["admin-secret"] = "" |
259 | + oldConfig, err := oldConfig.Apply(attrs) |
260 | + rc.Assert(err, IsNil) |
261 | + err = s.Conn.Environ.SetConfig(oldConfig) |
262 | + rc.Assert(err, IsNil) |
263 | + err = s.State.SetEnvironConfig(oldConfig) |
264 | + rc.Assert(err, IsNil) |
265 | + } |
266 | + |
267 | + attrs := s.Conn.Environ.Config().AllAttrs() |
268 | + attrs["firewall-mode"] = config.FwGlobal |
269 | + attrs["admin-secret"] = "" |
270 | + newConfig, err := s.Conn.Environ.Config().Apply(attrs) |
271 | + c.Assert(err, IsNil) |
272 | + err = s.Conn.Environ.SetConfig(newConfig) |
273 | + c.Assert(err, IsNil) |
274 | + err = s.State.SetEnvironConfig(newConfig) |
275 | + c.Assert(err, IsNil) |
276 | + |
277 | + return restore |
278 | +} |
279 | + |
280 | // startInstance starts a new instance for the given machine. |
281 | func (s *FirewallerSuite) startInstance(c *C, m *state.Machine) environs.Instance { |
282 | inst, err := s.Conn.Environ.StartInstance(m.Id(), testing.InvalidStateInfo(m.Id()), nil) |
283 | @@ -482,3 +535,184 @@ |
284 | err = s.State.RemoveMachine(m.Id()) |
285 | c.Assert(err, IsNil) |
286 | } |
287 | + |
288 | +func (s *FirewallerSuite) TestGlobalMode(c *C) { |
289 | + // Change configuration. |
290 | + restore := s.setGlobalMode(c) |
291 | + defer restore(c) |
292 | + |
293 | + // Start firewall and open ports. |
294 | + fw := firewaller.NewFirewaller(s.State) |
295 | + defer func() { c.Assert(fw.Stop(), IsNil) }() |
296 | + |
297 | + svc1, err := s.State.AddService("wordpress", s.charm) |
298 | + c.Assert(err, IsNil) |
299 | + err = svc1.SetExposed() |
300 | + c.Assert(err, IsNil) |
301 | + |
302 | + u1, m1 := s.addUnit(c, svc1) |
303 | + s.startInstance(c, m1) |
304 | + err = u1.OpenPort("tcp", 80) |
305 | + c.Assert(err, IsNil) |
306 | + err = u1.OpenPort("tcp", 8080) |
307 | + c.Assert(err, IsNil) |
308 | + |
309 | + svc2, err := s.State.AddService("moinmoin", s.charm) |
310 | + c.Assert(err, IsNil) |
311 | + err = svc2.SetExposed() |
312 | + c.Assert(err, IsNil) |
313 | + |
314 | + u2, m2 := s.addUnit(c, svc2) |
315 | + s.startInstance(c, m2) |
316 | + err = u2.OpenPort("tcp", 80) |
317 | + c.Assert(err, IsNil) |
318 | + |
319 | + s.assertEnvironPorts(c, []state.Port{{"tcp", 80}, {"tcp", 8080}}) |
320 | + |
321 | + // Closing a port opened by a different unit won't touch the environment. |
322 | + err = u1.ClosePort("tcp", 80) |
323 | + c.Assert(err, IsNil) |
324 | + s.assertEnvironPorts(c, []state.Port{{"tcp", 80}, {"tcp", 8080}}) |
325 | + |
326 | + // Closing a port used just once changes the environment. |
327 | + err = u1.ClosePort("tcp", 8080) |
328 | + c.Assert(err, IsNil) |
329 | + s.assertEnvironPorts(c, []state.Port{{"tcp", 80}}) |
330 | + |
331 | + // Closing the last port also modifies the environment. |
332 | + err = u2.ClosePort("tcp", 80) |
333 | + c.Assert(err, IsNil) |
334 | + s.assertEnvironPorts(c, nil) |
335 | +} |
336 | + |
337 | +func (s *FirewallerSuite) TestGlobalModeRestart(c *C) { |
338 | + // Change configuration. |
339 | + restore := s.setGlobalMode(c) |
340 | + defer restore(c) |
341 | + |
342 | + // Start firewall and open ports. |
343 | + fw := firewaller.NewFirewaller(s.State) |
344 | + |
345 | + svc, err := s.State.AddService("wordpress", s.charm) |
346 | + c.Assert(err, IsNil) |
347 | + err = svc.SetExposed() |
348 | + c.Assert(err, IsNil) |
349 | + |
350 | + u, m := s.addUnit(c, svc) |
351 | + s.startInstance(c, m) |
352 | + err = u.OpenPort("tcp", 80) |
353 | + c.Assert(err, IsNil) |
354 | + err = u.OpenPort("tcp", 8080) |
355 | + c.Assert(err, IsNil) |
356 | + |
357 | + s.assertEnvironPorts(c, []state.Port{{"tcp", 80}, {"tcp", 8080}}) |
358 | + |
359 | + // Stop firewall and close one and open a different port. |
360 | + err = fw.Stop() |
361 | + c.Assert(err, IsNil) |
362 | + |
363 | + err = u.ClosePort("tcp", 8080) |
364 | + c.Assert(err, IsNil) |
365 | + err = u.OpenPort("tcp", 8888) |
366 | + c.Assert(err, IsNil) |
367 | + |
368 | + // Start firewall and check port. |
369 | + fw = firewaller.NewFirewaller(s.State) |
370 | + defer func() { c.Assert(fw.Stop(), IsNil) }() |
371 | + |
372 | + s.assertEnvironPorts(c, []state.Port{{"tcp", 80}, {"tcp", 8888}}) |
373 | +} |
374 | + |
375 | +func (s *FirewallerSuite) TestGlobalModeRestartUnexposedService(c *C) { |
376 | + // Change configuration. |
377 | + restore := s.setGlobalMode(c) |
378 | + defer restore(c) |
379 | + |
380 | + // Start firewall and open ports. |
381 | + fw := firewaller.NewFirewaller(s.State) |
382 | + |
383 | + svc, err := s.State.AddService("wordpress", s.charm) |
384 | + c.Assert(err, IsNil) |
385 | + err = svc.SetExposed() |
386 | + c.Assert(err, IsNil) |
387 | + |
388 | + u, m := s.addUnit(c, svc) |
389 | + s.startInstance(c, m) |
390 | + err = u.OpenPort("tcp", 80) |
391 | + c.Assert(err, IsNil) |
392 | + err = u.OpenPort("tcp", 8080) |
393 | + c.Assert(err, IsNil) |
394 | + |
395 | + s.assertEnvironPorts(c, []state.Port{{"tcp", 80}, {"tcp", 8080}}) |
396 | + |
397 | + // Stop firewall and clear exposed flag on service. |
398 | + err = fw.Stop() |
399 | + c.Assert(err, IsNil) |
400 | + |
401 | + err = svc.ClearExposed() |
402 | + c.Assert(err, IsNil) |
403 | + |
404 | + // Start firewall and check port. |
405 | + fw = firewaller.NewFirewaller(s.State) |
406 | + defer func() { c.Assert(fw.Stop(), IsNil) }() |
407 | + |
408 | + s.assertEnvironPorts(c, nil) |
409 | +} |
410 | + |
411 | +func (s *FirewallerSuite) TestGlobalModeRestartPortCount(c *C) { |
412 | + // Change configuration. |
413 | + restore := s.setGlobalMode(c) |
414 | + defer restore(c) |
415 | + |
416 | + // Start firewall and open ports. |
417 | + fw := firewaller.NewFirewaller(s.State) |
418 | + |
419 | + svc1, err := s.State.AddService("wordpress", s.charm) |
420 | + c.Assert(err, IsNil) |
421 | + err = svc1.SetExposed() |
422 | + c.Assert(err, IsNil) |
423 | + |
424 | + u1, m1 := s.addUnit(c, svc1) |
425 | + s.startInstance(c, m1) |
426 | + err = u1.OpenPort("tcp", 80) |
427 | + c.Assert(err, IsNil) |
428 | + err = u1.OpenPort("tcp", 8080) |
429 | + c.Assert(err, IsNil) |
430 | + |
431 | + s.assertEnvironPorts(c, []state.Port{{"tcp", 80}, {"tcp", 8080}}) |
432 | + |
433 | + // Stop firewall and add another service using the port. |
434 | + err = fw.Stop() |
435 | + c.Assert(err, IsNil) |
436 | + |
437 | + svc2, err := s.State.AddService("moinmoin", s.charm) |
438 | + c.Assert(err, IsNil) |
439 | + err = svc2.SetExposed() |
440 | + c.Assert(err, IsNil) |
441 | + |
442 | + u2, m2 := s.addUnit(c, svc2) |
443 | + s.startInstance(c, m2) |
444 | + err = u2.OpenPort("tcp", 80) |
445 | + c.Assert(err, IsNil) |
446 | + |
447 | + // Start firewall and check port. |
448 | + fw = firewaller.NewFirewaller(s.State) |
449 | + defer func() { c.Assert(fw.Stop(), IsNil) }() |
450 | + |
451 | + s.assertEnvironPorts(c, []state.Port{{"tcp", 80}, {"tcp", 8080}}) |
452 | + |
453 | + // Closing a port opened by a different unit won't touch the environment. |
454 | + err = u1.ClosePort("tcp", 80) |
455 | + c.Assert(err, IsNil) |
456 | + s.assertEnvironPorts(c, []state.Port{{"tcp", 80}, {"tcp", 8080}}) |
457 | + |
458 | + // Closing a port used just once changes the environment. |
459 | + err = u1.ClosePort("tcp", 8080) |
460 | + c.Assert(err, IsNil) |
461 | + s.assertEnvironPorts(c, []state.Port{{"tcp", 80}}) |
462 | + |
463 | + // Closing the last port also modifies the environment. |
464 | + err = u2.ClosePort("tcp", 80) |
465 | + c.Assert(err, IsNil) |
466 | + s.assertEnvironPorts(c, nil) |
467 | +} |
Needs some love.
https:/ /codereview. appspot. com/6713054/ diff/1/ worker/ firewaller/ firewaller. go firewaller/ firewaller. go (right):
File worker/
https:/ /codereview. appspot. com/6713054/ diff/1/ worker/ firewaller/ firewaller. go#newcode195 firewaller/ firewaller. go:195: m, err := machined.machine()
worker/
Why do we need the machine, and the instance? It's feeling somewhat
hackish. If it's in global mode, it should be handed off to a method
that deals with global mode *only* in some sensible point, rather than
interleaving unnecessary logic like that.
https:/ /codereview. appspot. com/6713054/ diff/1/ worker/ firewaller/ firewaller. go#newcode252 firewaller/ firewaller. go:252: if fw.initialPorts [port] {
worker/
It's not clear to me how initialPorts is working. What if there are open
ports that should not be? What if there are initialPorts that were
closed and need to be re-opened?
The overall approach is asking for some tasteful consideration.
https:/ /codereview. appspot. com/6713054/