Skip to content

Commit f91ab26

Browse files
santhoshmprabhusanthoshmprabhu
authored
feat: create a dummy NC to store secondary IPs in nodesubnet deployments with Cilium (#3057)
* WIP * chore: first set of files for nodesubnet nc * chore: add ipam reconciler interface * feat: add ability to save fake nodesubnet nc * fix: make linter happy, cleanup * chore: cleanup * fix: make linter happy * fix: make linter happy * fix: fix failing test * refactor: remove public ipam reconciler interface * fix: fix compile after unexporting interface * refactor: break down IPAM reconciliation to address Evan's comment * chore: fix comment * fix:make linter happy * fix: Address comments, add todo for Evan's feedback * Address comments * fix: fix tests
1 parent 5f4c36c commit f91ab26

File tree

9 files changed

+344
-39
lines changed

9 files changed

+344
-39
lines changed

‎cns/NetworkContainerContract.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,13 @@ const (
9999
Managed = "Managed"
100100
CRD = "CRD"
101101
MultiTenantCRD = "MultiTenantCRD"
102+
AzureHost = "AzureHost"
102103
)
103104

104-
var ErrInvalidNCID = errors.New("invalid NetworkContainerID")
105-
var ErrInvalidIP = errors.New("invalid IP")
105+
var (
106+
ErrInvalidNCID = errors.New("invalid NetworkContainerID")
107+
ErrInvalidIP = errors.New("invalid IP")
108+
)
106109

107110
// CreateNetworkContainerRequest specifies request to create a network container or network isolation boundary.
108111
type CreateNetworkContainerRequest struct {

‎cns/nodesubnet/initialization.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package nodesubnet
2+
3+
import (
4+
"context"
5+
6+
"github.com/Azure/azure-container-networking/cns"
7+
"github.com/Azure/azure-container-networking/cns/logger"
8+
cnstypes "github.com/Azure/azure-container-networking/cns/types"
9+
"github.com/pkg/errors"
10+
"golang.org/x/exp/maps"
11+
)
12+
13+
type ipamReconciler interface {
14+
ReconcileIPAMStateForNodeSubnet(ncRequests []*cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.PodInfo) cnstypes.ResponseCode
15+
}
16+
17+
func ReconcileInitialCNSState(_ context.Context, ipamReconciler ipamReconciler, podInfoByIPProvider cns.PodInfoByIPProvider) (int, error) {
18+
// Get previous PodInfo state from podInfoByIPProvider
19+
podInfoByIP, err := podInfoByIPProvider.PodInfoByIP()
20+
if err != nil {
21+
return 0, errors.Wrap(err, "provider failed to provide PodInfoByIP")
22+
}
23+
24+
logger.Printf("Reconciling initial CNS state with %d IPs", len(podInfoByIP))
25+
26+
// Create a network container request that holds all the IPs from PodInfoByIP
27+
secondaryIPs := maps.Keys(podInfoByIP)
28+
ncRequest := CreateNodeSubnetNCRequest(secondaryIPs)
29+
responseCode := ipamReconciler.ReconcileIPAMStateForNodeSubnet([]*cns.CreateNetworkContainerRequest{ncRequest}, podInfoByIP)
30+
31+
if responseCode != cnstypes.Success {
32+
return 0, errors.Errorf("failed to reconcile initial CNS state: %d", responseCode)
33+
}
34+
35+
return len(secondaryIPs), nil
36+
}

‎cns/nodesubnet/initialization_test.go‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
package nodesubnet_test
2+
3+
import (
4+
"context"
5+
"net"
6+
"testing"
7+
8+
"github.com/Azure/azure-container-networking/cns"
9+
"github.com/Azure/azure-container-networking/cns/cnireconciler"
10+
"github.com/Azure/azure-container-networking/cns/logger"
11+
"github.com/Azure/azure-container-networking/cns/nodesubnet"
12+
"github.com/Azure/azure-container-networking/cns/restserver"
13+
"github.com/Azure/azure-container-networking/cns/types"
14+
"github.com/Azure/azure-container-networking/store"
15+
)
16+
17+
func getMockStore() store.KeyValueStore {
18+
mockStore := store.NewMockStore("")
19+
endpointState := map[string]*restserver.EndpointInfo{
20+
"12e65d89e58cb23c784e97840cf76866bfc9902089bdc8e87e9f64032e312b0b": {
21+
PodName: "coredns-54b69f46b8-ldmwr",
22+
PodNamespace: "kube-system",
23+
IfnameToIPMap: map[string]*restserver.IPInfo{
24+
"eth0": {
25+
IPv4: []net.IPNet{
26+
{
27+
IP: net.IPv4(10, 10, 0, 52),
28+
Mask: net.CIDRMask(24, 32),
29+
},
30+
},
31+
},
32+
},
33+
},
34+
"1fc5176913a3a1a7facfb823dde3b4ded404041134fef4f4a0c8bba140fc0413": {
35+
PodName: "load-test-7f7d49687d-wxc9p",
36+
PodNamespace: "load-test",
37+
IfnameToIPMap: map[string]*restserver.IPInfo{
38+
"eth0": {
39+
IPv4: []net.IPNet{
40+
{
41+
IP: net.IPv4(10, 10, 0, 63),
42+
Mask: net.CIDRMask(24, 32),
43+
},
44+
},
45+
},
46+
},
47+
},
48+
}
49+
50+
err := mockStore.Write(restserver.EndpointStoreKey, endpointState)
51+
if err != nil {
52+
return nil
53+
}
54+
return mockStore
55+
}
56+
57+
type MockIpamStateReconciler struct{}
58+
59+
func (m *MockIpamStateReconciler) ReconcileIPAMStateForNodeSubnet(ncRequests []*cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.PodInfo) types.ResponseCode {
60+
if len(ncRequests) == 1 && len(ncRequests[0].SecondaryIPConfigs) == len(podInfoByIP) {
61+
return types.Success
62+
}
63+
64+
return types.UnexpectedError
65+
}
66+
67+
func TestNewCNSPodInfoProvider(t *testing.T) {
68+
tests := []struct {
69+
name string
70+
store store.KeyValueStore
71+
wantErr bool
72+
reconciler *MockIpamStateReconciler
73+
exp int
74+
}{
75+
{
76+
name: "happy_path",
77+
store: getMockStore(),
78+
wantErr: false,
79+
reconciler: &MockIpamStateReconciler{},
80+
exp: 2,
81+
},
82+
}
83+
84+
for _, tt := range tests {
85+
tt := tt
86+
87+
t.Run(tt.name, func(t *testing.T) {
88+
ctx, cancel := testContext(t)
89+
defer cancel()
90+
91+
podInfoByIPProvider, err := cnireconciler.NewCNSPodInfoProvider(tt.store)
92+
checkErr(t, err, false)
93+
94+
got, err := nodesubnet.ReconcileInitialCNSState(ctx, tt.reconciler, podInfoByIPProvider)
95+
checkErr(t, err, tt.wantErr)
96+
if got != tt.exp {
97+
t.Errorf("got %d IPs reconciled, expected %d", got, tt.exp)
98+
}
99+
})
100+
}
101+
}
102+
103+
// testContext creates a context from the provided testing.T that will be
104+
// canceled if the test suite is terminated.
105+
func testContext(t *testing.T) (context.Context, context.CancelFunc) {
106+
if deadline, ok := t.Deadline(); ok {
107+
return context.WithDeadline(context.Background(), deadline)
108+
}
109+
return context.WithCancel(context.Background())
110+
}
111+
112+
func init() {
113+
logger.InitLogger("testlogs", 0, 0, "./")
114+
}

‎cns/nodesubnet/nodesubnet_nc.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package nodesubnet
2+
3+
import (
4+
"strconv"
5+
6+
"github.com/Azure/azure-container-networking/cns"
7+
"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
8+
)
9+
10+
const (
11+
// ID for fake NC that we create to store NodeSubnet IPS
12+
NodeSubnetNCID = "55022629-3854-499b-7133-5e6887959f4ea" // md5sum of "NodeSubnetNC_IPv4"
13+
NodeSubnetNCVersion = 0
14+
NodeSubnetHostVersion = "0"
15+
NodeSubnetNCStatus = v1alpha.NCUpdateSuccess
16+
NodeSubnetHostPrimaryIP = ""
17+
)
18+
19+
// CreateNodeSubnetNCRequest generates a CreateNetworkContainerRequest that simply stores the static secondary IPs.
20+
func CreateNodeSubnetNCRequest(secondaryIPs []string) *cns.CreateNetworkContainerRequest {
21+
secondaryIPConfigs := map[string]cns.SecondaryIPConfig{}
22+
23+
for _, secondaryIP := range secondaryIPs {
24+
// iterate through all secondary IP addresses add them to the request as secondary IPConfigs.
25+
secondaryIPConfigs[secondaryIP] = cns.SecondaryIPConfig{
26+
IPAddress: secondaryIP,
27+
NCVersion: NodeSubnetNCVersion,
28+
}
29+
}
30+
31+
return &cns.CreateNetworkContainerRequest{
32+
HostPrimaryIP: NodeSubnetHostPrimaryIP,
33+
SecondaryIPConfigs: secondaryIPConfigs,
34+
NetworkContainerid: NodeSubnetNCID,
35+
NetworkContainerType: cns.Docker, // Using docker as the NC type for NodeSubnet to match Swift. (The NC is not real)
36+
Version: strconv.FormatInt(NodeSubnetNCVersion, 10), //nolint:gomnd // it's decimal
37+
IPConfiguration: cns.IPConfiguration{},
38+
NCStatus: NodeSubnetNCStatus,
39+
}
40+
}

‎cns/nodesubnet/nodesubnet_nc_test.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package nodesubnet_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/Azure/azure-container-networking/cns"
7+
"github.com/Azure/azure-container-networking/cns/logger"
8+
"github.com/Azure/azure-container-networking/cns/nodesubnet"
9+
"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
10+
"github.com/google/go-cmp/cmp"
11+
)
12+
13+
func TestCreateNodeSubnetNCRequest_EmptySecondaryIPs(t *testing.T) {
14+
secondaryIPs := []string{}
15+
expectedRequest := &cns.CreateNetworkContainerRequest{
16+
HostPrimaryIP: nodesubnet.NodeSubnetHostPrimaryIP,
17+
SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{},
18+
NetworkContainerid: nodesubnet.NodeSubnetNCID,
19+
NetworkContainerType: cns.Docker,
20+
Version: "0",
21+
IPConfiguration: cns.IPConfiguration{},
22+
NCStatus: v1alpha.NCUpdateSuccess,
23+
}
24+
25+
request := nodesubnet.CreateNodeSubnetNCRequest(secondaryIPs)
26+
if !cmp.Equal(request, expectedRequest) {
27+
t.Errorf("Unexepected diff in NodeSubnetNCRequest: %v", cmp.Diff(request, expectedRequest))
28+
}
29+
}
30+
31+
func TestCreateNodeSubnetNCRequest_NonEmptySecondaryIPs(t *testing.T) {
32+
secondaryIPs := []string{"10.0.0.1", "10.0.0.2"}
33+
expectedRequest := &cns.CreateNetworkContainerRequest{
34+
HostPrimaryIP: nodesubnet.NodeSubnetHostPrimaryIP,
35+
SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{
36+
"10.0.0.1": {IPAddress: "10.0.0.1", NCVersion: nodesubnet.NodeSubnetNCVersion},
37+
"10.0.0.2": {IPAddress: "10.0.0.2", NCVersion: nodesubnet.NodeSubnetNCVersion},
38+
},
39+
NetworkContainerid: nodesubnet.NodeSubnetNCID,
40+
NetworkContainerType: cns.Docker,
41+
Version: "0",
42+
IPConfiguration: cns.IPConfiguration{},
43+
NCStatus: v1alpha.NCUpdateSuccess,
44+
}
45+
46+
request := nodesubnet.CreateNodeSubnetNCRequest(secondaryIPs)
47+
if !cmp.Equal(request, expectedRequest) {
48+
t.Errorf("Unexepected diff in NodeSubnetNCRequest: %v", cmp.Diff(request, expectedRequest))
49+
}
50+
}
51+
52+
func init() {
53+
logger.InitLogger("testlogs", 0, 0, "./")
54+
}

‎cns/restserver/internalapi.go‎

Lines changed: 74 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818

1919
"github.com/Azure/azure-container-networking/cns"
2020
"github.com/Azure/azure-container-networking/cns/logger"
21+
"github.com/Azure/azure-container-networking/cns/nodesubnet"
2122
"github.com/Azure/azure-container-networking/cns/types"
2223
"github.com/Azure/azure-container-networking/common"
2324
"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
@@ -275,22 +276,7 @@ func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMo
275276
return len(programmedNCs), nil
276277
}
277278

278-
func (service *HTTPRestService) ReconcileIPAMState(ncReqs []*cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.PodInfo, nnc *v1alpha.NodeNetworkConfig) types.ResponseCode {
279-
logger.Printf("Reconciling CNS IPAM state with nc requests: [%+v], PodInfo [%+v], NNC: [%+v]", ncReqs, podInfoByIP, nnc)
280-
// if no nc reqs, there is no CRD state yet
281-
if len(ncReqs) == 0 {
282-
logger.Printf("CNS starting with no NC state, podInfoMap count %d", len(podInfoByIP))
283-
return types.Success
284-
}
285-
286-
// first step in reconciliation is to create all the NCs in CNS, no IP assignment yet.
287-
for _, ncReq := range ncReqs {
288-
returnCode := service.CreateOrUpdateNetworkContainerInternal(ncReq)
289-
if returnCode != types.Success {
290-
return returnCode
291-
}
292-
}
293-
279+
func (service *HTTPRestService) ReconcileIPAssignment(podInfoByIP map[string]cns.PodInfo, ncReqs []*cns.CreateNetworkContainerRequest) types.ResponseCode {
294280
// index all the secondary IP configs for all the nc reqs, for easier lookup later on.
295281
allSecIPsIdx := make(map[string]*cns.CreateNetworkContainerRequest)
296282
for i := range ncReqs {
@@ -321,6 +307,7 @@ func (service *HTTPRestService) ReconcileIPAMState(ncReqs []*cns.CreateNetworkCo
321307
// }
322308
//
323309
// such that we can iterate over pod interfaces, and assign all IPs for it at once.
310+
324311
podKeyToPodIPs, err := newPodKeyToPodIPsMap(podInfoByIP)
325312
if err != nil {
326313
logger.Errorf("could not transform pods indexed by IP address to pod IPs indexed by interface: %v", err)
@@ -378,12 +365,69 @@ func (service *HTTPRestService) ReconcileIPAMState(ncReqs []*cns.CreateNetworkCo
378365
}
379366
}
380367

368+
return types.Success
369+
}
370+
371+
func (service *HTTPRestService) CreateNCs(ncReqs []*cns.CreateNetworkContainerRequest) types.ResponseCode {
372+
for _, ncReq := range ncReqs {
373+
returnCode := service.CreateOrUpdateNetworkContainerInternal(ncReq)
374+
if returnCode != types.Success {
375+
return returnCode
376+
}
377+
}
378+
379+
return types.Success
380+
}
381+
382+
func (service *HTTPRestService) ReconcileIPAMStateForSwift(ncReqs []*cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.PodInfo, nnc *v1alpha.NodeNetworkConfig) types.ResponseCode {
383+
logger.Printf("Reconciling CNS IPAM state with nc requests: [%+v], PodInfo [%+v], NNC: [%+v]", ncReqs, podInfoByIP, nnc)
384+
// if no nc reqs, there is no CRD state yet
385+
if len(ncReqs) == 0 {
386+
logger.Printf("CNS starting with no NC state, podInfoMap count %d", len(podInfoByIP))
387+
return types.Success
388+
}
389+
390+
// first step in reconciliation is to create all the NCs in CNS, no IP assignment yet.
391+
if returnCode := service.CreateNCs(ncReqs); returnCode != types.Success {
392+
return returnCode
393+
}
394+
395+
logger.Debugf("ncReqs created successfully, now save IPs")
396+
// now reconcile IPAM state.
397+
if returnCode := service.ReconcileIPAssignment(podInfoByIP, ncReqs); returnCode != types.Success {
398+
return returnCode
399+
}
400+
381401
if err := service.MarkExistingIPsAsPendingRelease(nnc.Spec.IPsNotInUse); err != nil {
382402
logger.Errorf("[Azure CNS] Error. Failed to mark IPs as pending %v", nnc.Spec.IPsNotInUse)
383403
return types.UnexpectedError
384404
}
385405

386-
return 0
406+
return types.Success
407+
}
408+
409+
// todo: there is some redundancy between this funcation and ReconcileIPAMStateForNodeSubnet. The difference is that this one
410+
// doesn't include the NNC parameter. We may want to unify the common parts.
411+
func (service *HTTPRestService) ReconcileIPAMStateForNodeSubnet(ncReqs []*cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.PodInfo) types.ResponseCode {
412+
logger.Printf("Reconciling CNS IPAM state with nc requests: [%+v], PodInfo [%+v]", ncReqs, podInfoByIP)
413+
414+
if len(ncReqs) != 1 {
415+
logger.Errorf("Nodesubnet should always have 1 NC to hold secondary IPs")
416+
return types.NetworkContainerNotSpecified
417+
}
418+
419+
// first step in reconciliation is to create all the NCs in CNS, no IP assignment yet.
420+
if returnCode := service.CreateNCs(ncReqs); returnCode != types.Success {
421+
return returnCode
422+
}
423+
424+
logger.Debugf("ncReqs created successfully, now save IPs")
425+
// now reconcile IPAM state.
426+
if returnCode := service.ReconcileIPAssignment(podInfoByIP, ncReqs); returnCode != types.Success {
427+
return returnCode
428+
}
429+
430+
return types.Success
387431
}
388432

389433
var (
@@ -526,11 +570,19 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns.
526570
return types.UnsupportedOrchestratorType
527571
}
528572

529-
// Validate PrimaryCA must never be empty
530-
err := validateIPSubnet(req.IPConfiguration.IPSubnet)
531-
if err != nil {
532-
logger.Errorf("[Azure CNS] Error. PrimaryCA is invalid, NC Req: %v", req)
533-
return types.InvalidPrimaryIPConfig
573+
if req.NetworkContainerid == nodesubnet.NodeSubnetNCID {
574+
// For NodeSubnet scenarios, Validate PrimaryCA must be empty
575+
if req.IPConfiguration.IPSubnet.IPAddress != "" {
576+
logger.Errorf("[Azure CNS] Error. PrimaryCA is invalid, NC Req: %v", req)
577+
return types.InvalidPrimaryIPConfig
578+
}
579+
} else {
580+
// For Swift scenarios, Validate PrimaryCA must never be empty
581+
err := validateIPSubnet(req.IPConfiguration.IPSubnet)
582+
if err != nil {
583+
logger.Errorf("[Azure CNS] Error. PrimaryCA is invalid, NC Req: %v", req)
584+
return types.InvalidPrimaryIPConfig
585+
}
534586
}
535587

536588
// Validate SecondaryIPConfig

0 commit comments

Comments
 (0)