Apply Fan subnet fixes to Address collection in incoming network configuration

[manadart]

Some time ago we did a significant overhaul of the payload delivery for updating machine address configuration. Included were changes to use members of the NetworkConfig.Addresses entries instead of those on the top level NetworkConfig. One of these was the address subnet CIDR.

What we missed at the time was to change NetworkConfigAPI.fixUpFanSubnets so that it applied the Fan updates to the Addresses entries and not the top-level CIDR member.

This meant that Fan addresses were being stored with subnets indicating the /8 overlay instead of the zone-specific /12 segments in AWS, resulting in the observed bug.

Here we fix fixUpFanSubnets so that we get the fixed Fan segment subnets against incoming addresses.

QA steps

• juju bootstrap aws patch-test --debug --no-gui --build-agent
• juju add-space space-1 172.31.0.0/20
• juju add-machine --constraints "spaces=space-1" --series bionic
• juju deploy cs:ubuntu --to lxd:0 --series focal --constraints "spaces=space-1" --bind "space-1"
• Observe that the container is provisioned.

Documentation changes

None.

Bug reference

https://bugs.launchpad.net/juju/+bug/1942950

[manadart]

Fix EC2 container fan networking

[wallyworld]

!!build!!

[wallyworld]

!!build!!

[jameinel]

I think the loop could be tweaked to be easier to understand, but otherwise I think this is ok.

[jameinel]

Is it actually possible for fanNet to be nil? I don't mind the caution, but given you just populate the map a few lines above, I wanted to make sure I understood the algorithm correctly. Is it just that ParsedCIDRNetwork might return nil for the subnet but not give an error?
Or is this just a belt-and-suspenders check to make sure we don't have a panic where it would be stupid to not just skip the item.

Given the nesting, this might be clearer as:

for i, cfg := range networkConfig {
	for j, addr := range cfg.Addresses {
          ip := net.ParseIP(addr.Value)
		for cidr, fanNet := range fanSubnets {
    if fanNet == nil {
      continue
    }
    if fanNet.Contains(ip) {
      // update the original maps rather than the local variables
      networkConfig[i].Addresses[j].CIDR = cidr
      break
    }
...

[jameinel]

(it also led me to notice that you're parsing the Address for every fanSubnet, which can be pulled out of the inner loop)

[wallyworld]

I pushed a small tweak to pull out the address parsing to the top of the loop

[jameinel]

So is this the core testing change? That the NetworkConfig is returning 10.0.0.0/8 but that should not cause us to change the CIDR that we are reporting in line 170/177 ?

[wallyworld]

$$merge$$