fix: don't panic when IPv6 is not supported

[gabrielmougard]

closes #159

• This fix enable the detection for a cluster member for IPv6 support to set the appropriate flag for hashicorp/mdns to avoid panicking if IPv6 is not supported,
• This also contains a fix for avoiding an infinite timeout issue in the lookupPeers function.
• However, the mDNS query (without support for IPv6) being received by the other cluster member mDNS servers, whose the responses are well written back to the UDPv4 connection opened on the server, are not received by the client and the reason still remains a mystery...

[markylaing]

Just a couple of small changes. Thanks.

[masnax]

Looks good, just a couple questions/changes, thanks!

[tomponline]

• However, the mDNS query (without support for IPv6) being received by the other cluster member mDNS servers, whose the responses are well written back to the UDPv4 connection opened on the server, are not received by the client and the reason still remains a mystery...

Could you explain this one for me? Are you saying that if you have IPv6 disabled, this prevents the panic, but auto discovery doesn't work still?

Yes, exactly. I'm also trying to see if it could not be related to some settings in /proc/sys/net/ipv4/conf/{iface} like forwarding, mc_forwarding or rp_filter, but so far they seem to be ok so I think the problem might be elsewhere...

[gabrielmougard]

@tomponline As this unexpected behaviour seems to be quite different (discovery issue vs panicking) from the original issue title, should I create a new related ticket ?

[tomponline]

I would suggest that you keep the changes in this PR - ultimately if we cant get it working without ipv6 enabled then we need to detect that any show an error, rather than trying to proceed and then failing silently not being able to detect other members.

[tomponline]

@gabrielmougard also could you quote and reply to my questions rather than editing them in the future. Thanks

[gabrielmougard]

@masnax do you have an idea on what could be going wrong here? I must admit that I'm running out of ideas...

[masnax]

@gabrielmougard I'm not able to replicate the discovery issue with this branch myself.

I've tried setting disable_ipv6 to 1 for every combination of all, default, and enp5s0, and I've also manually cleared every ipv6 address with ip a del. At least on my end, the systems are all still discoverable with ipv4.

Can you provide some reproducer steps for the discoverability issue?

Also, I just remembered that we will have to add permissions to the snap to allow accessing those /proc/sys/net paths as otherwise it's going to result in an apparmor denial.

[gabrielmougard]

@masnax should I open a new issue with a reproducer with this ipv4 discoverability issue I experience as this seems out of scope for this ticket (it is likely that my microcloud cluster is misconfigured somehow) so that we can close this one (avoiding the panicking issue) ?

[masnax]

@masnax should I open a new issue with a reproducer with this ipv4 discoverability issue I experience as this seems out of scope for this ticket (it is likely that my microcloud cluster is misconfigured somehow) so that we can close this one (avoiding the panicking issue) ?

It would be great if you could file an issue with reproducer steps. I think we ought to take a look at the issue further before we go ahead and merge this. If it ends up being unrelated then we can go ahead but we should investigate first.

[gabrielmougard]

@masnax I made it work! It's been a while since I experienced the last IPv4 issue (so I don't really remember the setup steps I took), so now after following the official microcloud doc and manually disabling IPv6, I was able to get mDNS to work on IPv4 :). I updated the Lookup() function though, just to return an error early if IPv6 AND IPv4 are both not supported.

[markylaing]

Looks good, just a few minor tweaks :)

[gabrielmougard]

@markylaing @tomponline updated

[masnax]

Also, I just remembered that we will have to add permissions to the snap to allow accessing those /proc/sys/net paths as otherwise it's going to result in an apparmor denial.

Looks like this is going to be an issue, judging from the tests:

Error: Failed to check IP status: open /proc/sys/net/ipv6/conf/all/disable_ipv6: permission denied

I'm not sure how feasible it would be to add read permissions for this path to MicroCloud. LXD probably already has it so I wonder if we could add something to lxc info --resources that records this information? @tomponline

[tomponline]

Can you infer this from ipv6 link local addresses on the interfaces instead?

[masnax]

Can you infer this from ipv6 link local addresses on the interfaces instead?

I tried testing it locally and so far checking for the link local address seems sufficient.

Given just 1 interface enp5s0, the panic seems to occur in these 3 cases:

• echo 1 > /proc/sys/net/ipv6/conf/all/disable_ipv6
• echo 1 > /proc/sys/net/ipv6/conf/enp5s0/disable_ipv6
• ip a del fe80... dev enp5s0

It doesn't happen when setting /proc/sys/net/ipv6/conf/default/disable_ipv6 because that seems to only apply to newly created interfaces.

And in all 3 cases, the link local address is indeed missing from ip a show, so I think it would be sufficient to check for that.

[gabrielmougard]

@masnax @simondeziel tests should be good now :)

[tomponline]

@simondeziel is the DCO check no longer occurring or happening elsewhere and needs to be removed from Required checks?

[simondeziel]

@tomponline I'll check with IS what's up with the DCO check. For this PR, I confirmed the 4 existing commits have the signed-off-by tag.

[gabrielmougard]

tests working now (except the usual timeouts in some interactive tests)

[masnax]

There is 1 change here that needs to be made, particularly checking if FAN would be supported on all systems and not just the local system before proceeding.

But I can add that in the refactor PR, so this one can be merged as-is.

Changes addressed