Skip to content

Commit 4a86b29

Browse files
thatmattlongthatmattlong
authored
fix: reserve 0th IP as gateway for overlay on Windows (#1968)
* fix: reserve 0th IP as gateway for overlay on Windows * fix: allow gateway to be updated
1 parent f48b62a commit 4a86b29

File tree

9 files changed

+269
-52
lines changed

9 files changed

+269
-52
lines changed

‎cni/network/network_windows.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,3 +400,25 @@ func platformInit(cniConfig *cni.NetworkConfig) {
400400
network.EnableHnsV2Timeout(cniConfig.WindowsSettings.HnsTimeoutDurationInSeconds)
401401
}
402402
}
403+
404+
// isDualNicFeatureSupported returns if the dual nic feature is supported. Currently it's only supported for windows hnsv2 path
405+
func (plugin *NetPlugin) isDualNicFeatureSupported(netNs string) bool {
406+
useHnsV2, err := network.UseHnsV2(netNs)
407+
if useHnsV2 && err == nil {
408+
return true
409+
}
410+
log.Errorf("DualNicFeature is not supported")
411+
return false
412+
}
413+
414+
func getOverlayGateway(podsubnet *net.IPNet) (net.IP, error) {
415+
log.Printf("WARN: No gateway specified for Overlay NC. CNI will choose one, but connectivity may break.")
416+
ncgw := podsubnet.IP
417+
ncgw[3]++
418+
ncgw = net.ParseIP(ncgw.String())
419+
if ncgw == nil || !podsubnet.Contains(ncgw) {
420+
return nil, errors.Wrap(errInvalidArgs, "%w: Failed to retrieve overlay gateway from podsubnet"+podsubnet.IP.String())
421+
}
422+
423+
return ncgw, nil
424+
}

‎cns/kubecontroller/nodenetworkconfig/conversion.go‎

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ var (
2121
)
2222

2323
// CreateNCRequestFromDynamicNC generates a CreateNetworkContainerRequest from a dynamic NetworkContainer.
24+
//
2425
//nolint:gocritic //ignore hugeparam
2526
func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetworkContainerRequest, error) {
2627
primaryIP := nc.PrimaryIP
@@ -69,6 +70,7 @@ func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwo
6970
}
7071

7172
// CreateNCRequestFromStaticNC generates a CreateNetworkContainerRequest from a static NetworkContainer.
73+
//
7274
//nolint:gocritic //ignore hugeparam
7375
func CreateNCRequestFromStaticNC(nc v1alpha.NetworkContainer) (*cns.CreateNetworkContainerRequest, error) {
7476
primaryPrefix, err := netip.ParsePrefix(nc.PrimaryIP)
@@ -85,24 +87,6 @@ func CreateNCRequestFromStaticNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwor
8587
PrefixLength: uint8(subnetPrefix.Bits()),
8688
}
8789

88-
secondaryIPConfigs := map[string]cns.SecondaryIPConfig{}
89-
90-
// iterate through all IP addresses in the subnet described by primaryPrefix and
91-
// add them to the request as secondary IPConfigs.
92-
for addr := primaryPrefix.Masked().Addr(); primaryPrefix.Contains(addr); addr = addr.Next() {
93-
secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{
94-
IPAddress: addr.String(),
95-
NCVersion: int(nc.Version),
96-
}
97-
}
98-
return &cns.CreateNetworkContainerRequest{
99-
SecondaryIPConfigs: secondaryIPConfigs,
100-
NetworkContainerid: nc.ID,
101-
NetworkContainerType: cns.Docker,
102-
Version: strconv.FormatInt(nc.Version, 10), //nolint:gomnd // it's decimal
103-
IPConfiguration: cns.IPConfiguration{
104-
IPSubnet: subnet,
105-
GatewayIPAddress: nc.DefaultGateway,
106-
},
107-
}, nil
90+
req := createNCRequestFromStaticNCHelper(nc, primaryPrefix, subnet)
91+
return req, nil
10892
}

‎cns/kubecontroller/nodenetworkconfig/conversion_linux.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package nodenetworkconfig
2+
3+
import (
4+
"net/netip"
5+
"strconv"
6+
7+
"github.com/Azure/azure-container-networking/cns"
8+
"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
9+
)
10+
11+
// createNCRequestFromStaticNCHelper generates a CreateNetworkContainerRequest from a static NetworkContainer
12+
// by adding all IPs in the the block to the secondary IP configs list. It does not skip any IPs.
13+
//
14+
//nolint:gocritic //ignore hugeparam
15+
func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet) *cns.CreateNetworkContainerRequest {
16+
secondaryIPConfigs := map[string]cns.SecondaryIPConfig{}
17+
18+
// iterate through all IP addresses in the subnet described by primaryPrefix and
19+
// add them to the request as secondary IPConfigs.
20+
for addr := primaryIPPrefix.Masked().Addr(); primaryIPPrefix.Contains(addr); addr = addr.Next() {
21+
secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{
22+
IPAddress: addr.String(),
23+
NCVersion: int(nc.Version),
24+
}
25+
}
26+
return &cns.CreateNetworkContainerRequest{
27+
SecondaryIPConfigs: secondaryIPConfigs,
28+
NetworkContainerid: nc.ID,
29+
NetworkContainerType: cns.Docker,
30+
Version: strconv.FormatInt(nc.Version, 10), //nolint:gomnd // it's decimal
31+
IPConfiguration: cns.IPConfiguration{
32+
IPSubnet: subnet,
33+
GatewayIPAddress: nc.DefaultGateway,
34+
},
35+
}
36+
}

‎cns/kubecontroller/nodenetworkconfig/conversion_linux_test.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package nodenetworkconfig
2+
3+
import (
4+
"strconv"
5+
6+
"github.com/Azure/azure-container-networking/cns"
7+
)
8+
9+
var validOverlayRequest = &cns.CreateNetworkContainerRequest{
10+
Version: strconv.FormatInt(version, 10),
11+
IPConfiguration: cns.IPConfiguration{
12+
IPSubnet: cns.IPSubnet{
13+
PrefixLength: uint8(subnetPrefixLen),
14+
IPAddress: primaryIP,
15+
},
16+
},
17+
NetworkContainerid: ncID,
18+
NetworkContainerType: cns.Docker,
19+
SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{
20+
"10.0.0.0": {
21+
IPAddress: "10.0.0.0",
22+
NCVersion: version,
23+
},
24+
"10.0.0.1": {
25+
IPAddress: "10.0.0.1",
26+
NCVersion: version,
27+
},
28+
"10.0.0.2": {
29+
IPAddress: "10.0.0.2",
30+
NCVersion: version,
31+
},
32+
"10.0.0.3": {
33+
IPAddress: "10.0.0.3",
34+
NCVersion: version,
35+
},
36+
},
37+
}

‎cns/kubecontroller/nodenetworkconfig/conversion_test.go‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -87,36 +87,6 @@ var validOverlayNC = v1alpha.NetworkContainer{
8787
Version: version,
8888
}
8989

90-
var validOverlayRequest = &cns.CreateNetworkContainerRequest{
91-
Version: strconv.FormatInt(version, 10),
92-
IPConfiguration: cns.IPConfiguration{
93-
IPSubnet: cns.IPSubnet{
94-
PrefixLength: uint8(subnetPrefixLen),
95-
IPAddress: primaryIP,
96-
},
97-
},
98-
NetworkContainerid: ncID,
99-
NetworkContainerType: cns.Docker,
100-
SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{
101-
"10.0.0.0": {
102-
IPAddress: "10.0.0.0",
103-
NCVersion: version,
104-
},
105-
"10.0.0.1": {
106-
IPAddress: "10.0.0.1",
107-
NCVersion: version,
108-
},
109-
"10.0.0.2": {
110-
IPAddress: "10.0.0.2",
111-
NCVersion: version,
112-
},
113-
"10.0.0.3": {
114-
IPAddress: "10.0.0.3",
115-
NCVersion: version,
116-
},
117-
},
118-
}
119-
12090
func TestCreateNCRequestFromDynamicNC(t *testing.T) {
12191
tests := []struct {
12292
name string

‎cns/kubecontroller/nodenetworkconfig/conversion_windows.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package nodenetworkconfig
2+
3+
import (
4+
"net/netip"
5+
"strconv"
6+
7+
"github.com/Azure/azure-container-networking/cns"
8+
"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
9+
)
10+
11+
// createNCRequestFromStaticNCHelper generates a CreateNetworkContainerRequest from a static NetworkContainer.
12+
// If the NC's DefaultGateway is empty, it will set the 0th IP as the gateway IP and all remaining IPs as
13+
// secondary IPs. If the gateway is not empty, it will not reserve the 0th IP and add it as a secondary IP.
14+
//
15+
//nolint:gocritic //ignore hugeparam
16+
func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet) *cns.CreateNetworkContainerRequest {
17+
secondaryIPConfigs := map[string]cns.SecondaryIPConfig{}
18+
19+
// if NC DefaultGateway is empty, set the 0th IP to the gateway and add the rest of the IPs
20+
// as secondary IPs
21+
startingAddr := primaryIPPrefix.Masked().Addr() // the masked address is the 0th IP in the subnet
22+
if nc.DefaultGateway == "" {
23+
nc.DefaultGateway = startingAddr.String()
24+
startingAddr = startingAddr.Next()
25+
}
26+
27+
// iterate through all IP addresses in the subnet described by primaryPrefix and
28+
// add them to the request as secondary IPConfigs.
29+
for addr := startingAddr; primaryIPPrefix.Contains(addr); addr = addr.Next() {
30+
secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{
31+
IPAddress: addr.String(),
32+
NCVersion: int(nc.Version),
33+
}
34+
}
35+
return &cns.CreateNetworkContainerRequest{
36+
SecondaryIPConfigs: secondaryIPConfigs,
37+
NetworkContainerid: nc.ID,
38+
NetworkContainerType: cns.Docker,
39+
Version: strconv.FormatInt(nc.Version, 10), //nolint:gomnd // it's decimal
40+
IPConfiguration: cns.IPConfiguration{
41+
IPSubnet: subnet,
42+
GatewayIPAddress: nc.DefaultGateway,
43+
},
44+
}
45+
}

‎cns/kubecontroller/nodenetworkconfig/conversion_windows_test.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package nodenetworkconfig
2+
3+
import (
4+
"strconv"
5+
6+
"github.com/Azure/azure-container-networking/cns"
7+
)
8+
9+
var validOverlayRequest = &cns.CreateNetworkContainerRequest{
10+
Version: strconv.FormatInt(version, 10),
11+
IPConfiguration: cns.IPConfiguration{
12+
IPSubnet: cns.IPSubnet{
13+
PrefixLength: uint8(subnetPrefixLen),
14+
IPAddress: primaryIP,
15+
},
16+
GatewayIPAddress: "10.0.0.0",
17+
},
18+
NetworkContainerid: ncID,
19+
NetworkContainerType: cns.Docker,
20+
SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{
21+
"10.0.0.1": {
22+
IPAddress: "10.0.0.1",
23+
NCVersion: version,
24+
},
25+
"10.0.0.2": {
26+
IPAddress: "10.0.0.2",
27+
NCVersion: version,
28+
},
29+
"10.0.0.3": {
30+
IPAddress: "10.0.0.3",
31+
NCVersion: version,
32+
},
33+
},
34+
}

‎cns/restserver/internalapi.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,8 +396,13 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns.
396396
existingNCInfo, ok := service.getNetworkContainerDetails(req.NetworkContainerid)
397397
if ok {
398398
existingReq := existingNCInfo.CreateNetworkContainerRequest
399-
if !reflect.DeepEqual(existingReq.IPConfiguration, req.IPConfiguration) {
400-
logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s, new CA %s", req.NetworkContainerid, existingReq.PrimaryInterfaceIdentifier, req.PrimaryInterfaceIdentifier)
399+
if !reflect.DeepEqual(existingReq.IPConfiguration.IPSubnet, req.IPConfiguration.IPSubnet) {
400+
logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s/%d, new CA %s/%d",
401+
req.NetworkContainerid,
402+
existingReq.IPConfiguration.IPSubnet.IPAddress,
403+
existingReq.IPConfiguration.IPSubnet.PrefixLength,
404+
req.IPConfiguration.IPSubnet.IPAddress,
405+
req.IPConfiguration.IPSubnet.PrefixLength)
401406
return types.PrimaryCANotSame
402407
}
403408
}

‎cns/restserver/internalapi_test.go‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,90 @@ func TestCreateOrUpdateNetworkContainerInternal(t *testing.T) {
4545
validateCreateOrUpdateNCInternal(t, 2, "-1")
4646
}
4747

48+
// TestReconcileNCStatePrimaryIPChangeShouldFail tests that reconciling NC state with
49+
// a NC whose IP has changed should fail
50+
func TestReconcileNCStatePrimaryIPChangeShouldFail(t *testing.T) {
51+
restartService()
52+
setEnv(t)
53+
setOrchestratorTypeInternal(cns.KubernetesCRD)
54+
svc.state.ContainerStatus = make(map[string]containerstatus)
55+
56+
// start with a NC in state
57+
ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151"
58+
svc.state.ContainerStatus[ncID] = containerstatus{
59+
ID: ncID,
60+
VMVersion: "0",
61+
HostVersion: "0",
62+
CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{
63+
NetworkContainerid: ncID,
64+
IPConfiguration: cns.IPConfiguration{
65+
IPSubnet: cns.IPSubnet{
66+
IPAddress: "10.0.1.0",
67+
PrefixLength: 24,
68+
},
69+
},
70+
},
71+
}
72+
73+
// now try to reconcile the state where the NC primary IP has changed
74+
resp := svc.ReconcileNCState(&cns.CreateNetworkContainerRequest{
75+
NetworkContainerid: ncID,
76+
IPConfiguration: cns.IPConfiguration{
77+
IPSubnet: cns.IPSubnet{
78+
IPAddress: "10.0.2.0", // note this IP has changed
79+
PrefixLength: 24,
80+
},
81+
},
82+
}, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{})
83+
84+
assert.Equal(t, types.PrimaryCANotSame, resp)
85+
}
86+
87+
// TestReconcileNCStateGatewayChange tests that NC state gets updated when reconciled
88+
// if the NC's gateway IP has changed
89+
func TestReconcileNCStateGatewayChange(t *testing.T) {
90+
restartService()
91+
setEnv(t)
92+
setOrchestratorTypeInternal(cns.KubernetesCRD)
93+
svc.state.ContainerStatus = make(map[string]containerstatus)
94+
95+
// start with a NC in state
96+
ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151"
97+
oldGW := "10.0.0.0"
98+
newGW := "10.0.1.0"
99+
ncPrimaryIP := cns.IPSubnet{
100+
IPAddress: "10.0.1.1",
101+
PrefixLength: 24,
102+
}
103+
svc.state.ContainerStatus[ncID] = containerstatus{
104+
ID: ncID,
105+
VMVersion: "0",
106+
HostVersion: "0",
107+
CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{
108+
NetworkContainerid: ncID,
109+
NetworkContainerType: cns.Kubernetes,
110+
IPConfiguration: cns.IPConfiguration{
111+
IPSubnet: ncPrimaryIP,
112+
GatewayIPAddress: oldGW,
113+
},
114+
},
115+
}
116+
117+
// now try to reconcile the state where the NC gateway has changed
118+
resp := svc.ReconcileNCState(&cns.CreateNetworkContainerRequest{
119+
NetworkContainerid: ncID,
120+
NetworkContainerType: cns.Kubernetes,
121+
IPConfiguration: cns.IPConfiguration{
122+
IPSubnet: ncPrimaryIP,
123+
GatewayIPAddress: newGW, // note this IP has changed
124+
},
125+
}, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{})
126+
127+
assert.Equal(t, types.Success, resp)
128+
// assert the new state reflects the gateway update
129+
assert.Equal(t, newGW, svc.state.ContainerStatus[ncID].CreateNetworkContainerRequest.IPConfiguration.GatewayIPAddress)
130+
}
131+
48132
func TestCreateOrUpdateNCWithLargerVersionComparedToNMAgent(t *testing.T) {
49133
restartService()
50134

0 commit comments

Comments
 (0)