-
Notifications
You must be signed in to change notification settings - Fork 260
feat: create a dummy NC to store secondary IPs in nodesubnet deployments with Cilium #3057
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 27 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
ed9efaa
WIP
santhoshmprabhu fb25300
chore: first set of files for nodesubnet nc
santhoshmprabhu 4604d04
chore: add ipam reconciler interface
santhoshmprabhu 75e963b
feat: add ability to save fake nodesubnet nc
santhoshmprabhu 6dd09d4
Merge branch 'sanprabhu/cilium-node-subnet-nc' of github.com:Azure/az…
santhoshmprabhu 6d39383
fix: make linter happy, cleanup
santhoshmprabhu c885c73
Merge branch 'master' into sanprabhu/cilium-node-subnet-nc
santhoshmprabhu 2f39935
chore: cleanup
santhoshmprabhu 2983336
Merge branch 'sanprabhu/cilium-node-subnet-nc' of github.com:Azure/az…
santhoshmprabhu 61a1fd8
fix: make linter happy
santhoshmprabhu 3d0ba54
fix: make linter happy
santhoshmprabhu f829e43
fix: fix failing test
santhoshmprabhu 2d80ab1
Merge branch 'master' into sanprabhu/cilium-node-subnet-nc
santhoshmprabhu 0258be2
refactor: remove public ipam reconciler interface
santhoshmprabhu f591ed0
Merge branch 'master' into sanprabhu/cilium-node-subnet-nc
santhoshmprabhu 4247539
fix: fix compile after unexporting interface
santhoshmprabhu fbb4b9a
Merge branch 'sanprabhu/cilium-node-subnet-nc' of github.com:Azure/az…
santhoshmprabhu 734c505
Merge branch 'master' into sanprabhu/cilium-node-subnet-nc
santhoshmprabhu e3a7876
Merge branch 'master' into sanprabhu/cilium-node-subnet-nc
santhoshmprabhu 7b04a36
refactor: break down IPAM reconciliation to address Evan's comment
santhoshmprabhu f19428c
chore: fix comment
santhoshmprabhu ec29b0d
Merge branch 'master' into sanprabhu/cilium-node-subnet-nc
santhoshmprabhu b19d622
Merge branch 'master' into sanprabhu/cilium-node-subnet-nc
santhoshmprabhu d14d768
fix:make linter happy
santhoshmprabhu 7c7ae05
Merge branch 'master' into sanprabhu/cilium-node-subnet-nc
santhoshmprabhu 1a0fdb1
fix: Address comments, add todo for Evan's feedback
santhoshmprabhu e245f9d
Merge branch 'sanprabhu/cilium-node-subnet-nc' of github.com:Azure/az…
santhoshmprabhu 632b0da
Address comments
santhoshmprabhu 2a39f9e
fix: fix tests
santhoshmprabhu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package nodesubnet | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "github.com/Azure/azure-container-networking/cns" | ||
| "github.com/Azure/azure-container-networking/cns/logger" | ||
| cnstypes "github.com/Azure/azure-container-networking/cns/types" | ||
| "github.com/pkg/errors" | ||
| "golang.org/x/exp/maps" | ||
| ) | ||
|
|
||
| type ipamReconciler interface { | ||
| ReconcileIPAMStateForNodeSubnet(ncRequests []*cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.PodInfo) cnstypes.ResponseCode | ||
| } | ||
|
|
||
| func ReconcileInitialCNSState(_ context.Context, ipamReconciler ipamReconciler, podInfoByIPProvider cns.PodInfoByIPProvider) (int, error) { | ||
| // Get previous PodInfo state from podInfoByIPProvider | ||
| podInfoByIP, err := podInfoByIPProvider.PodInfoByIP() | ||
santhoshmprabhu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if err != nil { | ||
| return 0, errors.Wrap(err, "provider failed to provide PodInfoByIP") | ||
| } | ||
|
|
||
| logger.Printf("Reconciling initial CNS state with %d IPs", len(podInfoByIP)) | ||
|
|
||
| // Create a network container request that holds all the IPs from PodInfoByIP | ||
| secondaryIPs := maps.Keys(podInfoByIP) | ||
| ncRequest := CreateNodeSubnetNCRequest(secondaryIPs) | ||
| responseCode := ipamReconciler.ReconcileIPAMStateForNodeSubnet([]*cns.CreateNetworkContainerRequest{ncRequest}, podInfoByIP) | ||
|
|
||
| if responseCode != cnstypes.Success { | ||
| return 0, errors.Errorf("failed to reconcile initial CNS state: %d", responseCode) | ||
| } | ||
|
|
||
| return len(secondaryIPs), nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| package nodesubnet_test | ||
|
|
||
| import ( | ||
| "context" | ||
| "net" | ||
| "testing" | ||
|
|
||
| "github.com/Azure/azure-container-networking/cns" | ||
| "github.com/Azure/azure-container-networking/cns/cnireconciler" | ||
| "github.com/Azure/azure-container-networking/cns/logger" | ||
| "github.com/Azure/azure-container-networking/cns/nodesubnet" | ||
| "github.com/Azure/azure-container-networking/cns/restserver" | ||
| "github.com/Azure/azure-container-networking/cns/types" | ||
| "github.com/Azure/azure-container-networking/store" | ||
| ) | ||
|
|
||
| func getMockStore() store.KeyValueStore { | ||
| mockStore := store.NewMockStore("") | ||
| endpointState := map[string]*restserver.EndpointInfo{ | ||
| "12e65d89e58cb23c784e97840cf76866bfc9902089bdc8e87e9f64032e312b0b": { | ||
| PodName: "coredns-54b69f46b8-ldmwr", | ||
| PodNamespace: "kube-system", | ||
| IfnameToIPMap: map[string]*restserver.IPInfo{ | ||
| "eth0": { | ||
| IPv4: []net.IPNet{ | ||
| { | ||
| IP: net.IPv4(10, 10, 0, 52), | ||
| Mask: net.CIDRMask(24, 32), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| "1fc5176913a3a1a7facfb823dde3b4ded404041134fef4f4a0c8bba140fc0413": { | ||
| PodName: "load-test-7f7d49687d-wxc9p", | ||
| PodNamespace: "load-test", | ||
| IfnameToIPMap: map[string]*restserver.IPInfo{ | ||
| "eth0": { | ||
| IPv4: []net.IPNet{ | ||
| { | ||
| IP: net.IPv4(10, 10, 0, 63), | ||
| Mask: net.CIDRMask(24, 32), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| err := mockStore.Write(restserver.EndpointStoreKey, endpointState) | ||
| if err != nil { | ||
| return nil | ||
| } | ||
| return mockStore | ||
| } | ||
|
|
||
| type MockIpamStateReconciler struct{} | ||
|
|
||
| func (m *MockIpamStateReconciler) ReconcileIPAMStateForNodeSubnet(ncRequests []*cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.PodInfo) types.ResponseCode { | ||
| if len(ncRequests) == 1 && len(ncRequests[0].SecondaryIPConfigs) == len(podInfoByIP) { | ||
| return types.Success | ||
| } | ||
|
|
||
| return types.UnexpectedError | ||
| } | ||
|
|
||
| func TestNewCNSPodInfoProvider(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| store store.KeyValueStore | ||
| wantErr bool | ||
| reconciler *MockIpamStateReconciler | ||
| exp int | ||
| }{ | ||
| { | ||
| name: "happy_path", | ||
| store: getMockStore(), | ||
| wantErr: false, | ||
| reconciler: &MockIpamStateReconciler{}, | ||
| exp: 2, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| tt := tt | ||
|
|
||
| t.Run(tt.name, func(t *testing.T) { | ||
| ctx, cancel := testContext(t) | ||
| defer cancel() | ||
|
|
||
| podInfoByIPProvider, err := cnireconciler.NewCNSPodInfoProvider(tt.store) | ||
| checkErr(t, err, false) | ||
|
|
||
| got, err := nodesubnet.ReconcileInitialCNSState(ctx, tt.reconciler, podInfoByIPProvider) | ||
| checkErr(t, err, tt.wantErr) | ||
| if got != tt.exp { | ||
| t.Errorf("got %d IPs reconciled, expected %d", got, tt.exp) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // testContext creates a context from the provided testing.T that will be | ||
| // canceled if the test suite is terminated. | ||
| func testContext(t *testing.T) (context.Context, context.CancelFunc) { | ||
| if deadline, ok := t.Deadline(); ok { | ||
| return context.WithDeadline(context.Background(), deadline) | ||
| } | ||
| return context.WithCancel(context.Background()) | ||
| } | ||
|
|
||
| func init() { | ||
| logger.InitLogger("testlogs", 0, 0, "./") | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| package nodesubnet | ||
|
|
||
| import ( | ||
| "strconv" | ||
|
|
||
| "github.com/Azure/azure-container-networking/cns" | ||
| "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" | ||
| ) | ||
|
|
||
| const ( | ||
| // ID for fake NC that we create to store NodeSubnet IPS | ||
| NodeSubnetNCID = "55022629-3854-499b-7133-5e6887959f4ea" // md5sum of "NodeSubnetNC_IPv4" | ||
| NodeSubnetNCVersion = 0 | ||
| NodeSubnetHostVersion = "0" | ||
| NodeSubnetNCStatus = v1alpha.NCUpdateSuccess | ||
| NodeSubnetHostPrimaryIP = "" | ||
| ) | ||
|
|
||
| // CreateNodeSubnetNCRequest generates a CreateNetworkContainerRequest that simply stores the static secondary IPs. | ||
| func CreateNodeSubnetNCRequest(secondaryIPs []string) *cns.CreateNetworkContainerRequest { | ||
| secondaryIPConfigs := map[string]cns.SecondaryIPConfig{} | ||
|
|
||
| for _, secondaryIP := range secondaryIPs { | ||
| // iterate through all secondary IP addresses add them to the request as secondary IPConfigs. | ||
| secondaryIPConfigs[secondaryIP] = cns.SecondaryIPConfig{ | ||
| IPAddress: secondaryIP, | ||
| NCVersion: NodeSubnetNCVersion, | ||
| } | ||
| } | ||
|
|
||
| return &cns.CreateNetworkContainerRequest{ | ||
| HostPrimaryIP: NodeSubnetHostPrimaryIP, | ||
| SecondaryIPConfigs: secondaryIPConfigs, | ||
| NetworkContainerid: NodeSubnetNCID, | ||
| NetworkContainerType: cns.NodeSubnet, | ||
| Version: strconv.FormatInt(NodeSubnetNCVersion, 10), //nolint:gomnd // it's decimal | ||
| IPConfiguration: cns.IPConfiguration{}, | ||
| NCStatus: NodeSubnetNCStatus, | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| package nodesubnet_test | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/Azure/azure-container-networking/cns" | ||
| "github.com/Azure/azure-container-networking/cns/logger" | ||
| "github.com/Azure/azure-container-networking/cns/nodesubnet" | ||
| "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" | ||
| "github.com/google/go-cmp/cmp" | ||
| ) | ||
|
|
||
| func TestCreateNodeSubnetNCRequest_EmptySecondaryIPs(t *testing.T) { | ||
| secondaryIPs := []string{} | ||
| expectedRequest := &cns.CreateNetworkContainerRequest{ | ||
| HostPrimaryIP: nodesubnet.NodeSubnetHostPrimaryIP, | ||
| SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{}, | ||
| NetworkContainerid: nodesubnet.NodeSubnetNCID, | ||
| NetworkContainerType: cns.NodeSubnet, | ||
| Version: "0", | ||
| IPConfiguration: cns.IPConfiguration{}, | ||
| NCStatus: v1alpha.NCUpdateSuccess, | ||
| } | ||
|
|
||
| request := nodesubnet.CreateNodeSubnetNCRequest(secondaryIPs) | ||
| if !cmp.Equal(request, expectedRequest) { | ||
| t.Errorf("Unexepected diff in NodeSubnetNCRequest: %v", cmp.Diff(request, expectedRequest)) | ||
| } | ||
| } | ||
|
|
||
| func TestCreateNodeSubnetNCRequest_NonEmptySecondaryIPs(t *testing.T) { | ||
| secondaryIPs := []string{"10.0.0.1", "10.0.0.2"} | ||
| expectedRequest := &cns.CreateNetworkContainerRequest{ | ||
| HostPrimaryIP: nodesubnet.NodeSubnetHostPrimaryIP, | ||
| SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{ | ||
| "10.0.0.1": {IPAddress: "10.0.0.1", NCVersion: nodesubnet.NodeSubnetNCVersion}, | ||
| "10.0.0.2": {IPAddress: "10.0.0.2", NCVersion: nodesubnet.NodeSubnetNCVersion}, | ||
| }, | ||
| NetworkContainerid: nodesubnet.NodeSubnetNCID, | ||
| NetworkContainerType: cns.NodeSubnet, | ||
| Version: "0", | ||
| IPConfiguration: cns.IPConfiguration{}, | ||
| NCStatus: v1alpha.NCUpdateSuccess, | ||
| } | ||
|
|
||
| request := nodesubnet.CreateNodeSubnetNCRequest(secondaryIPs) | ||
| if !cmp.Equal(request, expectedRequest) { | ||
| t.Errorf("Unexepected diff in NodeSubnetNCRequest: %v", cmp.Diff(request, expectedRequest)) | ||
| } | ||
| } | ||
|
|
||
| func init() { | ||
| logger.InitLogger("testlogs", 0, 0, "./") | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.