Fix for HNS local endpoint was being deleted instead of the remote endpoint.

This commit is contained in:
Prince Pereira 2025-04-01 04:39:47 -07:00
parent 5dc8b8dd26
commit 950bb3baf5
3 changed files with 110 additions and 34 deletions

View File

@ -133,31 +133,32 @@ func (hns hns) getAllEndpointsByNetwork(networkName string) (map[string]*(endpoi
continue continue
} }
// Add to map with key endpoint ID or IP address for index, ipConfig := range ep.IpConfigurations {
// Storing this is expensive in terms of memory, however there is a bug in Windows Server 2019 that can cause two endpoints to be created with the same IP address.
// TODO: Store by IP only and remove any lookups by endpoint ID.
endpointInfos[ep.Id] = &endpointInfo{
ip: ep.IpConfigurations[0].IpAddress,
isLocal: uint32(ep.Flags&hcn.EndpointFlagsRemoteEndpoint) == 0,
macAddress: ep.MacAddress,
hnsID: ep.Id,
hns: hns,
// only ready and not terminating endpoints were added to HNS
ready: true,
serving: true,
terminating: false,
}
endpointInfos[ep.IpConfigurations[0].IpAddress] = endpointInfos[ep.Id]
if len(ep.IpConfigurations) == 1 { if index > 1 {
// Expecting only ipv4 and ipv6 ipaddresses
// This is highly unlikely to happen, but if it does, we should log a warning
// and break out of the loop
klog.Warning("Endpoint ipconfiguration holds more than 2 IP addresses.", "hnsID", ep.Id, "IP", ipConfig.IpAddress, "ipConfigCount", len(ep.IpConfigurations))
break
}
isLocal := uint32(ep.Flags&hcn.EndpointFlagsRemoteEndpoint) == 0
if existingEp, ok := endpointInfos[ipConfig.IpAddress]; ok && isLocal {
// If the endpoint is already part of the queried endpoints map and is local,
// then we should not add it again to the map
// This is to avoid overwriting the remote endpoint info with a local endpoint.
klog.V(3).InfoS("Endpoint already exists in queried endpoints map; skipping.", "newLocalEndpoint", ep, "ipConfig", ipConfig, "existingEndpoint", existingEp)
continue continue
} }
// If ipFamilyPolicy is RequireDualStack or PreferDualStack, then there will be 2 IPS (iPV4 and IPV6) // Add to map with key endpoint ID or IP address
// in the endpoint list // Storing this is expensive in terms of memory, however there is a bug in Windows Server 2019 and 2022 that can cause two endpoints (local and remote) to be created with the same IP address.
endpointDualstack := &endpointInfo{ // TODO: Store by IP only and remove any lookups by endpoint ID.
ip: ep.IpConfigurations[1].IpAddress, epInfo := &endpointInfo{
isLocal: uint32(ep.Flags&hcn.EndpointFlagsRemoteEndpoint) == 0, ip: ipConfig.IpAddress,
isLocal: isLocal,
macAddress: ep.MacAddress, macAddress: ep.MacAddress,
hnsID: ep.Id, hnsID: ep.Id,
hns: hns, hns: hns,
@ -166,7 +167,9 @@ func (hns hns) getAllEndpointsByNetwork(networkName string) (map[string]*(endpoi
serving: true, serving: true,
terminating: false, terminating: false,
} }
endpointInfos[ep.IpConfigurations[1].IpAddress] = endpointDualstack endpointInfos[ep.Id] = epInfo
endpointInfos[ipConfig.IpAddress] = epInfo
}
} }
klog.V(3).InfoS("Queried endpoints from network", "network", networkName, "count", len(endpointInfos)) klog.V(3).InfoS("Queried endpoints from network", "network", networkName, "count", len(endpointInfos))
klog.V(5).InfoS("Queried endpoints details", "network", networkName, "endpointInfos", endpointInfos) klog.V(5).InfoS("Queried endpoints details", "network", networkName, "endpointInfos", endpointInfos)

View File

@ -112,6 +112,70 @@ func TestGetAllEndpointsByNetwork(t *testing.T) {
} }
} }
func TestGetAllEndpointsByNetworkWithDupEP(t *testing.T) {
hcnMock := getHcnMock("L2Bridge")
hns := hns{hcn: hcnMock}
ipv4Config := &hcn.IpConfig{
IpAddress: epIpAddress,
}
ipv6Config := &hcn.IpConfig{
IpAddress: epIpv6Address,
}
remoteEndpoint := &hcn.HostComputeEndpoint{
IpConfigurations: []hcn.IpConfig{*ipv4Config, *ipv6Config},
MacAddress: epMacAddress,
SchemaVersion: hcn.SchemaVersion{
Major: 2,
Minor: 0,
},
Flags: hcn.EndpointFlagsRemoteEndpoint,
}
Network, _ := hcnMock.GetNetworkByName(testNetwork)
remoteEndpoint, err := hns.hcn.CreateEndpoint(Network, remoteEndpoint)
if err != nil {
t.Error(err)
}
// Create a duplicate local endpoint with the same IP address
dupLocalEndpoint := &hcn.HostComputeEndpoint{
IpConfigurations: []hcn.IpConfig{*ipv4Config, *ipv6Config},
MacAddress: epMacAddress,
SchemaVersion: hcn.SchemaVersion{
Major: 2,
Minor: 0,
},
}
dupLocalEndpoint, err = hns.hcn.CreateEndpoint(Network, dupLocalEndpoint)
if err != nil {
t.Error(err)
}
mapEndpointsInfo, err := hns.getAllEndpointsByNetwork(Network.Name)
if err != nil {
t.Error(err)
}
endpointIpv4, ipv4EpPresent := mapEndpointsInfo[ipv4Config.IpAddress]
assert.True(t, ipv4EpPresent, "IPV4 endpoint is missing in Dualstack mode")
assert.Equal(t, endpointIpv4.ip, epIpAddress, "IPV4 IP is missing in Dualstack mode")
assert.Equal(t, endpointIpv4.hnsID, remoteEndpoint.Id, "HNS ID is not matching with remote endpoint")
endpointIpv6, ipv6EpPresent := mapEndpointsInfo[ipv6Config.IpAddress]
assert.True(t, ipv6EpPresent, "IPV6 endpoint is missing in Dualstack mode")
assert.Equal(t, endpointIpv6.ip, epIpv6Address, "IPV6 IP is missing in Dualstack mode")
assert.Equal(t, endpointIpv6.hnsID, remoteEndpoint.Id, "HNS ID is not matching with remote endpoint")
err = hns.hcn.DeleteEndpoint(remoteEndpoint)
if err != nil {
t.Error(err)
}
err = hns.hcn.DeleteEndpoint(dupLocalEndpoint)
if err != nil {
t.Error(err)
}
}
func TestGetEndpointByID(t *testing.T) { func TestGetEndpointByID(t *testing.T) {
// TODO: remove skip once the test has been fixed. // TODO: remove skip once the test has been fixed.
t.Skip("Skipping failing test on Windows.") t.Skip("Skipping failing test on Windows.")

View File

@ -535,6 +535,11 @@ func (ep *endpointInfo) DecrementRefCount() {
if !ep.IsLocal() && ep.refCount != nil && *ep.refCount > 0 { if !ep.IsLocal() && ep.refCount != nil && *ep.refCount > 0 {
*ep.refCount-- *ep.refCount--
} }
refCount := 0
if ep.refCount != nil {
refCount = int(*ep.refCount)
}
klog.V(5).InfoS("Endpoint RefCount after decrement.", "endpointInfo", ep, "refCount", refCount)
} }
func (ep *endpointInfo) Cleanup() { func (ep *endpointInfo) Cleanup() {
@ -1770,10 +1775,14 @@ func (proxier *Proxier) syncProxyRules() {
// remove stale endpoint refcount entries // remove stale endpoint refcount entries
for epIP := range proxier.terminatedEndpoints { for epIP := range proxier.terminatedEndpoints {
if epToDelete := queriedEndpoints[epIP]; epToDelete != nil && epToDelete.hnsID != "" { klog.V(5).InfoS("Terminated endpoints ready for deletion", "epIP", epIP)
if epToDelete := queriedEndpoints[epIP]; epToDelete != nil && epToDelete.hnsID != "" && !epToDelete.IsLocal() {
if refCount := proxier.endPointsRefCount.getRefCount(epToDelete.hnsID); refCount == nil || *refCount == 0 { if refCount := proxier.endPointsRefCount.getRefCount(epToDelete.hnsID); refCount == nil || *refCount == 0 {
klog.V(3).InfoS("Deleting unreferenced remote endpoint", "hnsID", epToDelete.hnsID) klog.V(3).InfoS("Deleting unreferenced remote endpoint", "hnsID", epToDelete.hnsID, "IP", epToDelete.ip)
proxier.hns.deleteEndpoint(epToDelete.hnsID) err := proxier.hns.deleteEndpoint(epToDelete.hnsID)
if err != nil {
klog.ErrorS(err, "Deleting unreferenced remote endpoint failed", "hnsID", epToDelete.hnsID)
}
} }
} }
} }