Check for ExecFunc failure#1459
Conversation
Signed-off-by: Santhosh Manohar <santhosh@docker.com>
| sb.osSbox.InvokeFunc(sb.resolver.SetupFunc(0)) | ||
| if err := sb.resolver.Start(); err != nil { | ||
| log.Errorf("Resolver Setup/Start failed for container %s, %q", sb.ContainerID(), err) | ||
| if err := sb.osSbox.InvokeFunc(sb.resolver.SetupFunc(0)); err == nil { |
There was a problem hiding this comment.
I don't think invokeFunc does return any error from the execution of the parameter function.
https://github.com/docker/libnetwork/blob/master/osl/namespace_linux.go#L333
never mind...
|
LGTM |
|
@sanimej just to be clear. #1412 already addressed the panic issue & this PR doesn't address the panic issue directly. The primary purpose of this PR is to cherry-pick into release/v0.8. And #1412 is NOT in that branch. Hence i think you should push another PR just fixing the panic issue to release/v0.8. |
|
since this is going in master... its fine I guess. When cherry-picking to 1.12.2, we have to redo the patch to take into account that #1412 is not there. LGTM |
|
#1412 will not fix the panic. Because the error from InvokeFunc is not being returned in ExecFunc. |
4 participants
Related to docker #26356
Panic was in ServeDNS(), in this dereference extConn.LocalAddr().String()
It can happen only if the
execFuncfailed for some reason. #1412 already added a check for the execFunc failure. But the error from InvokeFunc was being ignored. Fixed that and also added error checks for other calls to InvokeFunc()There is a 2nd panic mentioned in the same issue. Its a different decode. We can ask the submitter to open a different issue after collecting the required info to root cause it.
Signed-off-by: Santhosh Manohar santhosh@docker.com