[fix] Fixed subnet /32 & /128 pie chart error #129

[Aryamanz29]

• Fixed subnet /32 pie chart.
  Fixes [bug] Subnet /32 gives error in pie chart #129

See comment #129 (comment)

[coveralls]

Coverage increased (+0.001%) to 99.854% when pulling dda339c on Aryamanz29:issue-129/subnet-32-fix-chart into 4f15237 on openwisp:master.

[Aryamanz29]

Coverage is decreasing, I need to work on this 😅

[Aryamanz29]

Coverage is decreasing, I need to work on this sweat_smile

Done 👍

[pandafy]

@Aryamanz29 there's no new test for testing this fix explicitly. Can you add a test that fails without this fix?

[Aryamanz29]

@pandafy Done requested changes 👍

[nemesifier]

@Aryamanz29 the solution work and the code is good. I haven't tried the test yet but I will.

I have one request: I am not sure why I didn't test the same for IPv6 too but it seems to me that the problem happens there too with /128 subnets, can you fix that as well please? The solution and test used can be equivalent.

[Aryamanz29]

@Aryamanz29 the solution work and the code is good. I haven't tried the test yet but I will.

I have one request: I am not sure why I didn't test the same for IPv6 too but it seems to me that the problem happens there too with /128 subnets, can you fix that as well please? The solution and test used can be equivalent.

Sure, I'll check with /128 subnets and update 👍

[Aryamanz29]

@Aryamanz29 the solution work and the code is good. I haven't tried the test yet but I will.
I have one request: I am not sure why I didn't test the same for IPv6 too but it seems to me that the problem happens there too with /128 subnets, can you fix that as well please? The solution and test used can be equivalent.

Sure, I'll check with /128 subnets and update +1

Update :
Yes it's not showing pie chart with subnet/128, Also instead of raising ValueError at this endpoint http://127.0.0.1:8000/api/v1/ipam/subnet/64d5dfc7-7845-42c1-8856-d3436dddc750/hosts/, It is returning empty host list.

,

[Aryamanz29]

PR now ready for testing 👍

[nemesifier]

Good progress @Aryamanz29, we're almost there, I noticed an issue with the visualization though, it happens both on IPv4 and IPv6:

In this case I created the IP address for this subnet to ensure the pie chart shows correctly.
However, the "subnet visual display" below is showing an address that doesn't belong to the subnet,

It should be showing 192.168.0.250 but it's instead showing the next address, it happens also on IPv6 can you fix that please?

[Aryamanz29]

@nemesisdesign
This problem occurs due to this line 👇

openwisp-ipam/openwisp_ipam/api/views.py

Line 132 in 4f15237

host = self.subnet.subnet._address_class(self.network + 1 + i + self.start)

because of extra +1 in host (But don't know why we're adding +1 here 🤔 ), It always shows the next address. After removing, It looks good 👇

[Aryamanz29]

Update : Only in case of single hosts (ie . in case of subnet /32 and /128), We must return first address otherwise It can starts from next address.

import ipaddress

subnet128 = ipaddress.ip_network('2001:db00::/128')
subnet32 = ipaddress.ip_network('198.168.0.250/32')
subnet8 = ipaddress.ip_network('10.0.0.0/8')

print(list(subnet128.hosts()))
print(list(subnet32.hosts()))
# Contains 16777214 hosts
print(list(subnet8.hosts())[:5])

(env) ➜  ipam-test py test.py
[IPv6Address('2001:db00::')]
[IPv4Address('198.168.0.250')]
[IPv4Address('10.0.0.1'), IPv4Address('10.0.0.2'), IPv4Address('10.0.0.3'), IPv4Address('10.0.0.4'), IPv4Address('10.0.0.5')]

[nemesifier]

Good work @Aryamanz29, there's one outstanding issue which I highlighted below.

[nemesifier]

Thanks 👍