Skip to content

Commit 83fca75

Browse files
jaer-tsunJaeryn
jaer-tsun
and
Jaeryn
authored
feat: cni refactor for swift v2 (#2330)
* feat: update contracts to support swift 2 * add comments * rename AddressType to NICType * update contract names and comments * address comments * feat: update invokers to support swift 2 * address comments * address comments * refactor cns invoker per comments * update invokers based on contract change * update test * update with contract changes * fix linter errs * fix naming * fix linter * fix linter * address comments * update tests * add tests * address nit comments * add comments * address comments * fix casing * address comments * feat: update invokers to support swift 2 * address comments * feat: update invokers to support swift 2 * feat: update invokers to support swift 2 * feat: update endpoint clients for swift 2 * address comments * fix lint errs * update endpoint clients based on contract changes * update tests * only skip adding default route * modify AddEndpoints per comments * address comments * update deleteendpoint * enter ns before moving interface back to vm ns * update delete endpoint test * add namespace interface for testing * fix lint * fix lint * add comment * add extra delete endpoint test * update test * feat: update invokers to support swift 2 * address comments * address comments * feat: refactor endpoint create/delete flow for swift 2 * address comments * address comments * address linter * update based on contract changes * update with contract changes * add more tests and address comments * modify AddEndpoints per comments * update test for invoker add and endpoint client add failure * address comments * fix lint * update windows tests * update refactor with namespace interface * fix lint * rebasing fixes * address comments --------- Co-authored-by: Jaeryn <[email protected]>
1 parent 84e82e5 commit 83fca75

33 files changed

+1887
-606
lines changed

‎cni/network/invoker.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,18 @@ type IPAMAddConfig struct {
2727
}
2828

2929
type IPAMAddResult struct {
30-
ipv4Result *cniTypesCurr.Result
31-
ipv6Result *cniTypesCurr.Result
30+
// Splitting defaultInterfaceInfo from secondaryInterfacesInfo so we don't need to loop for default CNI result every time
31+
defaultInterfaceInfo InterfaceInfo
32+
secondaryInterfacesInfo []InterfaceInfo
33+
// ncResponse is used for Swift 1.0 multitenancy
3234
ncResponse *cns.GetNetworkContainerResponse
3335
hostSubnetPrefix net.IPNet
36+
ipv6Enabled bool
37+
}
38+
39+
type InterfaceInfo struct {
40+
ipResult *cniTypesCurr.Result
41+
nicType cns.NICType
42+
macAddress net.HardwareAddr
43+
skipDefaultRoutes bool
3444
}

‎cni/network/invoker_azure.go‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/Azure/azure-container-networking/cni"
1111
"github.com/Azure/azure-container-networking/cni/log"
12+
"github.com/Azure/azure-container-networking/cns"
1213
"github.com/Azure/azure-container-networking/common"
1314
"github.com/Azure/azure-container-networking/ipam"
1415
"github.com/Azure/azure-container-networking/network"
@@ -46,10 +47,7 @@ func NewAzureIpamInvoker(plugin *NetPlugin, nwInfo *network.NetworkInfo) *AzureI
4647
}
4748

4849
func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, error) {
49-
var (
50-
addResult = IPAMAddResult{}
51-
err error
52-
)
50+
addResult := IPAMAddResult{}
5351

5452
if addConfig.nwCfg == nil {
5553
return addResult, invoker.plugin.Errorf("nil nwCfg passed to CNI ADD, stack: %+v", string(debug.Stack()))
@@ -60,23 +58,25 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er
6058
}
6159

6260
// Call into IPAM plugin to allocate an address pool for the network.
63-
addResult.ipv4Result, err = invoker.plugin.DelegateAdd(addConfig.nwCfg.IPAM.Type, addConfig.nwCfg)
64-
61+
result, err := invoker.plugin.DelegateAdd(addConfig.nwCfg.IPAM.Type, addConfig.nwCfg)
6562
if err != nil && strings.Contains(err.Error(), ipam.ErrNoAvailableAddressPools.Error()) {
6663
invoker.deleteIpamState()
6764
logger.Info("Retry pool allocation after deleting IPAM state")
68-
addResult.ipv4Result, err = invoker.plugin.DelegateAdd(addConfig.nwCfg.IPAM.Type, addConfig.nwCfg)
65+
result, err = invoker.plugin.DelegateAdd(addConfig.nwCfg.IPAM.Type, addConfig.nwCfg)
6966
}
7067

7168
if err != nil {
7269
err = invoker.plugin.Errorf("Failed to allocate pool: %v", err)
7370
return addResult, err
7471
}
72+
if len(result.IPs) > 0 {
73+
addResult.hostSubnetPrefix = result.IPs[0].Address
74+
}
7575

7676
defer func() {
7777
if err != nil {
78-
if len(addResult.ipv4Result.IPs) > 0 {
79-
if er := invoker.Delete(&addResult.ipv4Result.IPs[0].Address, addConfig.nwCfg, nil, addConfig.options); er != nil {
78+
if len(addResult.defaultInterfaceInfo.ipResult.IPs) > 0 {
79+
if er := invoker.Delete(&addResult.defaultInterfaceInfo.ipResult.IPs[0].Address, addConfig.nwCfg, nil, addConfig.options); er != nil {
8080
err = invoker.plugin.Errorf("Failed to clean up IP's during Delete with error %v, after Add failed with error %w", er, err)
8181
}
8282
} else {
@@ -95,13 +95,18 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er
9595
nwCfg6.IPAM.Subnet = invoker.nwInfo.Subnets[1].Prefix.String()
9696
}
9797

98-
addResult.ipv6Result, err = invoker.plugin.DelegateAdd(nwCfg6.IPAM.Type, &nwCfg6)
98+
var ipv6Result *cniTypesCurr.Result
99+
ipv6Result, err = invoker.plugin.DelegateAdd(nwCfg6.IPAM.Type, &nwCfg6)
99100
if err != nil {
100101
err = invoker.plugin.Errorf("Failed to allocate v6 pool: %v", err)
102+
} else {
103+
result.IPs = append(result.IPs, ipv6Result.IPs...)
104+
result.Routes = append(result.Routes, ipv6Result.Routes...)
105+
addResult.ipv6Enabled = true
101106
}
102107
}
103108

104-
addResult.hostSubnetPrefix = addResult.ipv4Result.IPs[0].Address
109+
addResult.defaultInterfaceInfo = InterfaceInfo{ipResult: result, nicType: cns.InfraNIC}
105110

106111
return addResult, err
107112
}

‎cni/network/invoker_azure_test.go‎

Lines changed: 49 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,24 @@ import (
1616
"github.com/stretchr/testify/require"
1717
)
1818

19+
const (
20+
ipv4cidr = "10.0.0.1/24"
21+
v4NetCidr = "10.0.0.0/24"
22+
ipv4cidr2 = "10.0.0.4/24"
23+
ipv6cidr = "2001:0db8:abcd:0015::0/64"
24+
v6NetCidr = "2001:db8:abcd:0012::0/64"
25+
)
26+
1927
type mockDelegatePlugin struct {
2028
add
2129
del
2230
}
2331

2432
type add struct {
33+
resultsIPv4 []*cniTypesCurr.Result
34+
resultsIPv6 []*cniTypesCurr.Result
2535
resultsIPv4Index int
26-
resultsIPv4 [](*cniTypesCurr.Result)
2736
resultsIPv6Index int
28-
resultsIPv6 [](*cniTypesCurr.Result)
2937
errv4 error
3038
errv6 error
3139
}
@@ -82,8 +90,8 @@ func getCIDRNotationForAddress(ipaddresswithcidr string) *net.IPNet {
8290
return ipnet
8391
}
8492

85-
func getResult(ip string) []*cniTypesCurr.Result {
86-
res := []*cniTypesCurr.Result{
93+
func getSingleResult(ip string) []*cniTypesCurr.Result {
94+
return []*cniTypesCurr.Result{
8795
{
8896
IPs: []*cniTypesCurr.IPConfig{
8997
{
@@ -92,6 +100,14 @@ func getResult(ip string) []*cniTypesCurr.Result {
92100
},
93101
},
94102
}
103+
}
104+
105+
// getResult will return a slice of IPConfigs
106+
func getResult(ips ...string) *cniTypesCurr.Result {
107+
res := &cniTypesCurr.Result{}
108+
for _, ip := range ips {
109+
res.IPs = append(res.IPs, &cniTypesCurr.IPConfig{Address: *getCIDRNotationForAddress(ip)})
110+
}
95111
return res
96112
}
97113

@@ -127,46 +143,44 @@ func TestAzureIPAMInvoker_Add(t *testing.T) {
127143
fields fields
128144
args args
129145
want *cniTypesCurr.Result
130-
want1 *cniTypesCurr.Result
131146
wantErr bool
132147
}{
133148
{
134149
name: "happy add ipv4",
135150
fields: fields{
136151
plugin: &mockDelegatePlugin{
137152
add: add{
138-
resultsIPv4: getResult("10.0.0.1/24"),
153+
resultsIPv4: getSingleResult(ipv4cidr),
139154
},
140155
del: del{},
141156
},
142-
nwInfo: getNwInfo("10.0.0.0/24", ""),
157+
nwInfo: getNwInfo(v4NetCidr, ""),
143158
},
144159
args: args{
145160
nwCfg: &cni.NetworkConfig{},
146-
subnetPrefix: getCIDRNotationForAddress("10.0.0.0/24"),
161+
subnetPrefix: getCIDRNotationForAddress(v4NetCidr),
147162
},
148-
want: getResult("10.0.0.1/24")[0],
163+
want: getResult(ipv4cidr),
149164
wantErr: false,
150165
},
151166
{
152167
name: "happy add ipv4+ipv6",
153168
fields: fields{
154169
plugin: &mockDelegatePlugin{
155170
add: add{
156-
resultsIPv4: getResult("10.0.0.1/24"),
157-
resultsIPv6: getResult("2001:0db8:abcd:0015::0/64"),
171+
resultsIPv4: getSingleResult(ipv4cidr),
172+
resultsIPv6: getSingleResult(ipv6cidr),
158173
},
159174
},
160-
nwInfo: getNwInfo("10.0.0.0/24", "2001:db8:abcd:0012::0/64"),
175+
nwInfo: getNwInfo(v4NetCidr, v6NetCidr),
161176
},
162177
args: args{
163178
nwCfg: &cni.NetworkConfig{
164179
IPV6Mode: network.IPV6Nat,
165180
},
166-
subnetPrefix: getCIDRNotationForAddress("10.0.0.0/24"),
181+
subnetPrefix: getCIDRNotationForAddress(v4NetCidr),
167182
},
168-
want: getResult("10.0.0.1/24")[0],
169-
want1: getResult("2001:0db8:abcd:0015::0/64")[0],
183+
want: getResult(ipv4cidr, ipv6cidr),
170184
wantErr: false,
171185
},
172186
{
@@ -177,34 +191,32 @@ func TestAzureIPAMInvoker_Add(t *testing.T) {
177191
errv4: errors.New("test error"), //nolint:goerr113
178192
},
179193
},
180-
nwInfo: getNwInfo("10.0.0.0/24", ""),
194+
nwInfo: getNwInfo(v4NetCidr, ""),
181195
},
182196
args: args{
183197
nwCfg: &cni.NetworkConfig{},
184198
},
185199
want: nil,
186-
want1: nil,
187200
wantErr: true,
188201
},
189202
{
190203
name: "error on ipv4+ipv6",
191204
fields: fields{
192205
plugin: &mockDelegatePlugin{
193206
add: add{
194-
resultsIPv4: getResult("10.0.0.1/24"),
207+
resultsIPv4: getSingleResult(ipv4cidr),
195208
errv6: errors.New("test v6 error"), //nolint:goerr113
196209
},
197210
},
198-
nwInfo: getNwInfo("10.0.0.0/24", ""),
211+
nwInfo: getNwInfo(v4NetCidr, ""),
199212
},
200213
args: args{
201214
nwCfg: &cni.NetworkConfig{
202215
IPV6Mode: network.IPV6Nat,
203216
},
204-
subnetPrefix: getCIDRNotationForAddress("10.0.0.0/24"),
217+
subnetPrefix: getCIDRNotationForAddress(v4NetCidr),
205218
},
206-
want: getResult("10.0.0.1/24")[0],
207-
want1: nil,
219+
want: getResult(ipv4cidr),
208220
wantErr: true,
209221
},
210222
}
@@ -226,8 +238,8 @@ func TestAzureIPAMInvoker_Add(t *testing.T) {
226238
require.Nil(err)
227239
}
228240

229-
require.Exactly(tt.want, ipamAddResult.ipv4Result)
230-
require.Exactly(tt.want1, ipamAddResult.ipv6Result)
241+
fmt.Printf("want:%+v\nrest:%+v\n", tt.want, ipamAddResult.defaultInterfaceInfo.ipResult)
242+
require.Exactly(tt.want, ipamAddResult.defaultInterfaceInfo.ipResult)
231243
})
232244
}
233245
}
@@ -256,10 +268,10 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) {
256268
plugin: &mockDelegatePlugin{
257269
del: del{},
258270
},
259-
nwInfo: getNwInfo("10.0.0.0/24", ""),
271+
nwInfo: getNwInfo(v4NetCidr, ""),
260272
},
261273
args: args{
262-
address: getCIDRNotationForAddress("10.0.0.4/24"),
274+
address: getCIDRNotationForAddress(ipv4cidr2),
263275
nwCfg: &cni.NetworkConfig{
264276
IPAM: cni.IPAM{
265277
Address: "10.0.0.4",
@@ -273,7 +285,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) {
273285
plugin: &mockDelegatePlugin{
274286
del: del{},
275287
},
276-
nwInfo: getNwInfo("10.0.0.0/24", "2001:db8:abcd:0012::0/64"),
288+
nwInfo: getNwInfo(v4NetCidr, v6NetCidr),
277289
},
278290
args: args{
279291
address: getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"),
@@ -292,7 +304,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) {
292304
err: errors.New("error when address is nil"), //nolint:goerr113
293305
},
294306
},
295-
nwInfo: getNwInfo("", "2001:db8:abcd:0012::0/64"),
307+
nwInfo: getNwInfo("", v6NetCidr),
296308
},
297309
args: args{
298310
address: nil,
@@ -312,13 +324,13 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) {
312324
err: errors.New("error on v4 delete"), //nolint:goerr113
313325
},
314326
},
315-
nwInfo: getNwInfo("10.0.0.0/24", ""),
327+
nwInfo: getNwInfo(v4NetCidr, ""),
316328
},
317329
args: args{
318-
address: getCIDRNotationForAddress("10.0.0.4/24"),
330+
address: getCIDRNotationForAddress(ipv4cidr2),
319331
nwCfg: &cni.NetworkConfig{
320332
IPAM: cni.IPAM{
321-
Address: "10.0.0.4/24",
333+
Address: ipv4cidr2,
322334
},
323335
},
324336
},
@@ -332,13 +344,13 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) {
332344
err: errors.New("error on v6 delete"), //nolint:goerr113
333345
},
334346
},
335-
nwInfo: getNwInfo("10.0.0.0/24", "2001:db8:abcd:0012::0/64"),
347+
nwInfo: getNwInfo(v4NetCidr, v6NetCidr),
336348
},
337349
args: args{
338350
address: getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"),
339351
nwCfg: &cni.NetworkConfig{
340352
IPAM: cni.IPAM{
341-
Address: "10.0.0.4/24",
353+
Address: ipv4cidr2,
342354
},
343355
},
344356
},
@@ -393,17 +405,17 @@ func TestRemoveIpamState_Add(t *testing.T) {
393405
fields: fields{
394406
plugin: &mockDelegatePlugin{
395407
add: add{
396-
resultsIPv4: getResult("10.0.0.1/24"),
408+
resultsIPv4: getSingleResult(ipv4cidr),
397409
errv4: ipam.ErrNoAvailableAddressPools,
398410
},
399411
},
400-
nwInfo: getNwInfo("10.0.0.0/24", ""),
412+
nwInfo: getNwInfo(v4NetCidr, ""),
401413
},
402414
args: args{
403415
nwCfg: &cni.NetworkConfig{},
404-
subnetPrefix: getCIDRNotationForAddress("10.0.0.0/24"),
416+
subnetPrefix: getCIDRNotationForAddress(v4NetCidr),
405417
},
406-
want: getResult("10.0.0.1/24")[0],
418+
want: getResult(ipv4cidr),
407419
wantErrMsg: ipam.ErrNoAvailableAddressPools.Error(),
408420
wantErr: true,
409421
},

0 commit comments

Comments
 (0)