Skip to content

Commit e620685

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 40022da commit e620685

File tree

9 files changed

+248
-52
lines changed

9 files changed

+248
-52
lines changed

‎cni/network/network_windows.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,7 @@ func (plugin *NetPlugin) isDualNicFeatureSupported(netNs string) bool {
415415
}
416416

417417
func getOverlayGateway(podsubnet *net.IPNet) (net.IP, error) {
418+
log.Printf("WARN: No gateway specified for Overlay NC. CNI will choose one, but connectivity may break.")
418419
ncgw := podsubnet.IP
419420
ncgw[3]++
420421
ncgw = net.ParseIP(ncgw.String())

‎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
@@ -400,8 +400,13 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns.
400400
existingNCInfo, ok := service.getNetworkContainerDetails(req.NetworkContainerid)
401401
if ok {
402402
existingReq := existingNCInfo.CreateNetworkContainerRequest
403-
if !reflect.DeepEqual(existingReq.IPConfiguration, req.IPConfiguration) {
404-
logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s, new CA %s", req.NetworkContainerid, existingReq.PrimaryInterfaceIdentifier, req.PrimaryInterfaceIdentifier)
403+
if !reflect.DeepEqual(existingReq.IPConfiguration.IPSubnet, req.IPConfiguration.IPSubnet) {
404+
logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s/%d, new CA %s/%d",
405+
req.NetworkContainerid,
406+
existingReq.IPConfiguration.IPSubnet.IPAddress,
407+
existingReq.IPConfiguration.IPSubnet.PrefixLength,
408+
req.IPConfiguration.IPSubnet.IPAddress,
409+
req.IPConfiguration.IPSubnet.PrefixLength)
405410
return types.PrimaryCANotSame
406411
}
407412
}

‎cns/restserver/internalapi_test.go‎

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

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

0 commit comments

Comments
 (0)