Add getipaddrs

[samuel-massinon]

getipaddr would only return the first IP address it would find.
getipaddrs will return all the IP addresses it finds.

We ran into an issue when running on a machine with multiple IP addresses. The IP returned by getipaddr would not be the one we needed. This function solved the issue for us.

[ararslan]

Can you also add this to the Sockets section of the documentation alongside getipaddr?

[ararslan]

Looks good to me, but it would be good to have another set of eyes on this from someone who knows more about these things.

EDIT: Needs an entry in NEWS.md.

[omus]

Looks good to me. I wouldn't mind if getipaddr() used getipaddrs() but to do that efficiently we'd probably want to make getipaddrs return an generator.

[vtjnash]

dropping these seems odd to me. seems like these should probably be returned (with each entry being a namedtuple), or a perhaps configurable by function argument

[StefanKarpinski]

The current behavior seems like it is a reasonable default, so we can add an option to list internal interfaces as well as external ones at a future time, although it would not be a bad addition to make in this PR given that this new feature won't land in a release until 1.2 in the spring.

[samuel-massinon]

I added an include_lo argument.

julia> getipaddrs(false)
1-element Array{IPv4,1}:
 ip"10.255.0.183"

julia> getipaddrs()
1-element Array{IPv4,1}:
 ip"10.255.0.183"

julia> getipaddrs(true)
2-element Array{IPv4,1}:
 ip"127.0.0.1"   
 ip"10.255.0.183"

[samuel-massinon]

Does my latest changes resolve this issue?

[vtjnash]

I think I'd like this to return a vector of namedtuples with all of the various properties exposed (https://nodejs.org/api/os.html#os_os_networkinterfaces)

[samuel-massinon]

@vtjnash

Would this be better in a different function instead of this one? I like the idea of getipaddrs just returning a list of values getipaddr should return.

If we want to go down that path, maybe a function named get_network_interfaces could return somthing like you posted.

[StefanKarpinski]

If we want to go down that path, maybe a function named get_network_interfaces could return somthing like you posted.

Please yes, it would be really unexpected if getipaddr returns one IP address and getipaddrs doesn't return a list of the same. If we want a Sockets.getifaces function that returns a bunch of structs or named tuples, that would be fine, but that's a different feature entirely.

[samuel-massinon]

Can I get this merged/re-reviewed?

[StefanKarpinski]

There's still ongoing discussion about whether to include internal IPs by default. Once that's decided, we can go ahead and merge this. @vtjnash, what are your thoughts on that issue?

[vtjnash]

OK, I'm convinced. I just had a naming question: should we go with getallipaddr, to parallel with getalladdrinfo?

[StefanKarpinski]

No, I think getipaddrs is better—I kind of wish that getalladdrinfo had been called getaddrinfo.

[samuel-massinon]

Is this good to merge? Have all the issues been resolved?

[omus]

I think this is good to go. I'll merge this at the end of my day (6 hours from now) unless there are objections.

[ararslan]

Thanks, Sam! Nice work 👍

[staticfloat]

This seems to have broken the buildbots, likely because of some weirdness within the buildbot setup. In particular, the failing test is:

Error in testset Sockets:
Test Failed at /buildworker/worker/tester_linux32/build/share/julia/stdlib/v1.2/Sockets/test/runtests.jl:427
  Expression: getipaddrs(true) >= getipaddrs()
   Evaluated: Sockets.IPv4[ip"127.0.0.1", ip"172.19.0.4", ip"172.18.0.4"] >= Sockets.IPv4[ip"172.19.0.4", ip"172.18.0.4"]

It looks to me like there might be a problem when a machine has multiple IP addresses?

[samuel-massinon]

The problem is how I compared the lists.
I was thinking that getipaddrs(true) >= getipaddrs() would test if getipaddrs() is a subset of getipaddrs(true). But I guess the order of the list matters.

I think we would want to replace the test with

issubset(getipaddrs(), getipaddrs(true))