https://codereview.appspot.com/6374069/diff/8001/worker/firewaller/firewaller.go File worker/firewaller/firewaller.go (right):
https://codereview.appspot.com/6374069/diff/8001/worker/firewaller/firewaller.go#newcode63 worker/firewaller/firewaller.go:63: panic("can't stop machine tracker") As I mentioned on IRC, I think your approach of logging is actually right. What's misguiding here is the comment: the machine tracker *is* stopped if we got here. The message should look like:
"machine tracker returned error when stopping: %v"
https://codereview.appspot.com/6374069/diff/12001/worker/firewaller/firewaller.go File worker/firewaller/firewaller.go (right):
https://codereview.appspot.com/6374069/diff/12001/worker/firewaller/firewaller.go#newcode63 worker/firewaller/firewaller.go:63: delete(fw.machines, removedMachine.Id()) This should also be moved above the if block.
https://codereview.appspot.com/6374069/
« Back to merge proposal
https:/ /codereview. appspot. com/6374069/ diff/8001/ worker/ firewaller/ firewaller. go firewaller/ firewaller. go (right):
File worker/
https:/ /codereview. appspot. com/6374069/ diff/8001/ worker/ firewaller/ firewaller. go#newcode63 firewaller/ firewaller. go:63: panic("can't stop machine tracker")
worker/
As I mentioned on IRC, I think your approach of logging is actually
right. What's misguiding here is the comment: the machine tracker *is*
stopped if we got here. The message should look like:
"machine tracker returned error when stopping: %v"
https:/ /codereview. appspot. com/6374069/ diff/12001/ worker/ firewaller/ firewaller. go firewaller/ firewaller. go (right):
File worker/
https:/ /codereview. appspot. com/6374069/ diff/12001/ worker/ firewaller/ firewaller. go#newcode63 firewaller/ firewaller. go:63: delete(fw.machines, Id())
worker/
removedMachine.
This should also be moved above the if block.
https:/ /codereview. appspot. com/6374069/