Handle IP to int conversion for inet + inet6 #1477
Conversation
- IPv4 can have IPv6 next hops, so we could use the wrong `.to_u*` method - Lets try both before throwing on int conversions for sorting purposes Addresses more for #1474 Signed-off-by: Cooper Lees <me@cooperlees.com>
Signed-off-by: Cooper Lees <me@cooperlees.com>
|
Can you add a unit test for this @cooperlees |
lib/ohai/plugins/network.rb
Outdated
| ipaddress.to_u32 | ||
| rescue NoMethodError | ||
| ipaddress.to_u128 | ||
| end |
There was a problem hiding this comment.
Cathing NoMethodError leads to all sorts of fun bugs. Instead try:
ipaddress.respond_to?(:to_u32) ? ipaddress.to_u32 : ipaddress.to_u128
There was a problem hiding this comment.
Actually, why are we doing this at all? .to_i method does the right thing in all cases:
irb(main):003:0> x = IPAddress('10.1.1.1')
=> #<IPAddress::IPv4:0x0000000002aa1930 @address="10.1.1.1", @prefix=32, @octets=[10, 1, 1, 1], @u32=167837953>
irb(main):004:0> x.ipv4?
=> true
irb(main):005:0> x.to_u32
=> 167837953
irb(main):006:0> x.to_i
=> 167837953
irb(main):007:0> x = IPAddress('fe80::96c6:91ff:fea0:7a03')
=> #<IPAddress::IPv6:0x00000000024ad678 @groups=[65152, 0, 0, 0, 38598, 37375, 65184, 31235], @address="fe80:0000:0000:0000:96c6:91ff:fea0:7a03", @compressed="fe80::96c6:91ff:fea0:7a03", @prefix=128>
irb(main):008:0> x.to_i
=> 338288524927261089664883428521100212739
irb(main):009:0> x.to_u128
=> 338288524927261089664883428521100212739
lib/ohai/plugins/network.rb
Outdated
| @@ -25,6 +25,13 @@ | |||
|
|
|||
| depends "network/interfaces" | |||
|
|
|||
| # Try to u32 an IPv4 and fallback to u128 if it fails before throwing | |||
| def int_an_ip(ipaddress) | |||
lib/ohai/plugins/network.rb
Outdated
| ipaddress.to_u32 | ||
| rescue NoMethodError | ||
| ipaddress.to_u128 | ||
| end |
There was a problem hiding this comment.
Actually, why are we doing this at all? .to_i method does the right thing in all cases:
irb(main):003:0> x = IPAddress('10.1.1.1')
=> #<IPAddress::IPv4:0x0000000002aa1930 @address="10.1.1.1", @prefix=32, @octets=[10, 1, 1, 1], @u32=167837953>
irb(main):004:0> x.ipv4?
=> true
irb(main):005:0> x.to_u32
=> 167837953
irb(main):006:0> x.to_i
=> 167837953
irb(main):007:0> x = IPAddress('fe80::96c6:91ff:fea0:7a03')
=> #<IPAddress::IPv6:0x00000000024ad678 @groups=[65152, 0, 0, 0, 38598, 37375, 65184, 31235], @address="fe80:0000:0000:0000:96c6:91ff:fea0:7a03", @compressed="fe80::96c6:91ff:fea0:7a03", @prefix=128>
irb(main):008:0> x.to_i
=> 338288524927261089664883428521100212739
irb(main):009:0> x.to_u128
=> 338288524927261089664883428521100212739
|
I wondered the same thing, but am more focused on removing IPv4 Link Local from FB's infra and this is holding me up :) Can I get support to do the renames, smarter exception handling + add a unit test and we file an issue to follow up and look into this and evaluate its importance? |
Signed-off-by: Cooper Lees <me@cooperlees.com>
|
So I was able to clean up the code back to a one liner, but I don't see a direct test today for cooper-mbp1:ohai cooper$ grep -R sorted_ips lib/ohai/plugins/*
lib/ohai/plugins/network.rb: def sorted_ips(family = "inet")
lib/ohai/plugins/network.rb: ips = sorted_ips(family) |
Signed-off-by: Cooper Lees <me@cooperlees.com>
lib/ohai/plugins/network.rb
Outdated
| @@ -54,7 +54,7 @@ def sorted_ips(family = "inet") | |||
| ipaddresses.sort_by do |v| | |||
| [ ( scope_prio.index(v[:scope]) || 999999 ), | |||
| 128 - v[:ipaddress].prefix.to_i, | |||
| ( family == "inet" ? v[:ipaddress].to_u32 : v[:ipaddress].to_u128 ), | |||
| v[:ipaddress].respond_to?(:to_u32) ? v[:ipaddress].to_u32 : v[:ipaddress].to_u128, | |||
Signed-off-by: Cooper Lees <me@cooperlees.com>
|
FWIW - this has been backported and is in production @ Facebook. (note, there are not many boxes with IPv4 inet6 routes, but there will be soon). |
|
I forgot about this one. Sorry for the delay |
.to_u*methodAddresses more for #1474
Signed-off-by: Cooper Lees me@cooperlees.com