Support IPv6 for SubnetPort #1439
Conversation
yanjunz97
force-pushed
the
subnetport-ipv6
branch
2 times, most recently
from
af22382 to
ced00a0
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1439 +/- ##
==========================================
- Coverage 77.03% 73.81% -3.22%
==========================================
Files 156 169 +13
Lines 22221 23689 +1468
==========================================
+ Hits 17118 17487 +369
- Misses 3891 4030 +139
- Partials 1212 2172 +960
🚀 New features to boost your workflow:
|
yanjunz97
force-pushed
the
subnetport-ipv6
branch
4 times, most recently
from
43882e8 to
abfbb5a
Compare
yanjunz97
changed the title
| // is only updated after the updating succeeds. | ||
| updatedSubnet := *vpcSubnets[i] // #nosec G602 | ||
| updatedSubnet.Tags = newTags | ||
| updatedSubnet.IpAddressType = String(controllercommon.ConvertCRIPAddressTypeToNSX(subnetsetCR.Spec.IPAddressType)) |
There was a problem hiding this comment.
IpAddressType is overwritten unconditionally on every UpdateSubnetSet call. If spec.ipAddressType is not set (empty string), ConvertCRIPAddressTypeToNSX defaults to "IPV4", silently changing the address type of existing IPv6 or dual-stack NSX subnets ?
There was a problem hiding this comment.
I think this shall be fine as here we only update Subnet created by NSX Operator.
And before we introduce ipAddressType to Subnet CRD, NSX Operator only creates IPv4 Subnet, so there is no existing IPv6 or dual-stack NSX subnets will be updated to IPv4 in this case?
| case model.VpcSubnet_IP_ADDRESS_TYPE_IPV4_IPV6: | ||
| info.dirtyCount += 1 | ||
| info.dirtyCountIPv6 += 1 | ||
| log.Trace("Allocate Subnetport to dual-stack Subnet", "Subnet", *subnet.Path, "dirtyPortCount", info.dirtyCount, "dirtyPortCountIPv6", info.dirtyCountIPv6) |
There was a problem hiding this comment.
When using dual-stack allocation, both dirtyCount and dirtyCountIPv6 will be incremented simultaneously. However, it seems that ReleasePortInSubnet only decrements dirtyCount?
There was a problem hiding this comment.
Good caught. Yes, ReleasePortInSubnet needs to consume ipAddressType and decrement dirtyCount accordingly.
| if macAddress == "" && binding.Binding.MacAddress != nil { | ||
| macAddress = strings.Trim(*binding.Binding.MacAddress, "\"") | ||
| } | ||
| subnetPort.Status.NetworkInterfaceConfig.IPAddresses[i].IPAddress = *binding.Binding.IpAddress |
There was a problem hiding this comment.
Writes values by index here, Could it possibly panic or use the wrong address family?
There was a problem hiding this comment.
By design we should not get out of index error, but it makes sense to defense here. I changed it to append when it is out of index and record in log.
Signed-off-by: Yanjun Zhou <yanjun.zhou@broadcom.com>
Signed-off-by: Yanjun Zhou <yanjun.zhou@broadcom.com>
yanjunz97
force-pushed
the
subnetport-ipv6
branch
from
ce167d4 to
8a00b8c
Compare
Signed-off-by: Yanjun Zhou <yanjun.zhou@broadcom.com>
yanjunz97
force-pushed
the
subnetport-ipv6
branch
from
8a00b8c to
8eab5f5
Compare
|
/e2e |
2 similar comments
|
/e2e |
|
/e2e |
5 participants
This PR focuses on happy path for IPv6/Dual Stack in SubnetPort
It contains the following changes
Testing done:
Tested regression case for Pod/VM with IPv4