jackpal / gateway

A golang library for discovering the address of a LAN gateway.
BSD 3-Clause "New" or "Revised" License
227 stars 69 forks source link

tests now hang in limited debian chroot #8

Closed aviau closed 8 years ago

aviau commented 8 years ago

Hello!

I don't know if this is related to the newely merged commit, but the tests hang on my machine.

Here is the output:

    go test -v github.com/jackpal/gateway
=== RUN   TestGateway

//cc @calmh

jackpal commented 8 years ago

What sort of machine do you have?

jackpal commented 8 years ago

(By that I mean, what OS? Also, what version of golang are you using.)

fwiw the tests pass for me on OSX 10.11 and go version go1.5.2 darwin/amd64

aviau commented 8 years ago

go version go1.6.1 linux/amd64

I am on Debian.

I have just noticed that the tests pass when I run them directly but not in a chroot.

Do you have an idea of something gateway could be relying on that would not be available in my limited environment?

Note that the tests used to pass not long ago.

jackpal commented 8 years ago

Also checked that version go1.6.2 darwin/amd64 works

Can you tell me what this command line prints in your limited chroot?

ip route show
jackpal commented 8 years ago

Also, does this work in you limited chroot?

/usr/bin/ip route show

How about this?

/sbin/routel

Or this?

/usr/bin/route -n

jackpal commented 8 years ago

I pushed version 1.0.2 that uses full paths and also closes stdout before waiting for the command to exit.

Please let me know if that fixes the problem.

calmh commented 8 years ago

I'd suggest using CombinedOutput on those as well, as it's simpler and correctly managed by the stdlib. I don't think there is any case in which Wait() needs you to close the commands Stdout to return.

calmh commented 8 years ago

I have a branch that breaks out all the parsing stuff into common and tests it, and uses CombinedOutput for the execution. Is that something you would accept as a PR, or too radical a change?

jackpal commented 8 years ago

calmh -- yeah, I suspect CombinedOutput() is the right approach for this situation. The danger with CombinedOutput() is that some command goes nuts and spews gigabytes of data. But we don't expect that for these commands.

I would be interested in seeing your fork. It sounds like you've thought about this.

jackpal commented 8 years ago

Or would you like to be a co-maintainer of palevich/gateway? Happy to add you.

jackpal commented 8 years ago

avisu, could you pull and try v1.0.4 ?

aviau commented 8 years ago

The following things break:

Given that I fix the paths and run the commands you asked me, I get the following output:

echo "running /bin/ip route show"
running /bin/ip route show
/bin/ip route show
echo "running /usr/bin/routel"
running /usr/bin/routel
/usr/bin/routel
         target            gateway          source    proto    scope    dev tbl
      127.0.0.0          broadcast       127.0.0.1   kernel     link     lo local
      127.0.0.0 8            local       127.0.0.1   kernel     host     lo local
      127.0.0.1              local       127.0.0.1   kernel     host     lo local
127.255.255.255          broadcast       127.0.0.1   kernel     link     lo local
        default        unreachable                   kernel              lo unspec
            ::1              local                     none              lo local
        default        unreachable                   kernel              lo unspec
echo "running /sbin/route -n"
running /sbin/route -n
/sbin/route -n
Kernel IP routing table
Destination     Gateway         Genmask         Flags Metric Ref    Use Iface

I have noticed that /sbin/route was missing in the chroot I was using. This didn't prevent the tests from passing in the past, but that might be it now? Given that you fix the bin paths, it might be fixed. It would be a wierd behaviour for a missing binary.

I have just pulled 1.0.4 and the tests pass on my machine, about to test in the chroot.

aviau commented 8 years ago

as of 1.0.4, the tests pass!

I'll upload it to Debian in 2 minutes ;)