-
Notifications
You must be signed in to change notification settings - Fork 260
feat: CNS RequestIPAddress branching for MT/V2 #2300
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 all commits
Commits
Show all changes
148 commits
Select commit
Hold shift + click to select a range
ace96ff
fix overlay IPAM not reporting version
nddq f0b4f19
Merge branch 'master' into master
nddq f1b5dcb
revert file and var naming, add correct path to makefile
nddq eebfbc8
Merge branch 'Azure:master' into master
nddq 84abe01
Merge branch 'Azure:master' into master
nddq 994dc04
Merge branch 'Azure:master' into master
nddq 8475126
Merge branch 'Azure:master' into master
nddq 29c8206
Merge branch 'Azure:master' into master
nddq 62e6df8
Merge branch 'Azure:master' into master
nddq 20988bd
proposal design for multitenant IPAM flow
nddq 2e6842f
change podipinfo + linter issue
nddq 227fd89
pointer issues for printf
nddq d5b00db
update IPAM branching
nddq e53f1e7
remove comments
nddq f99525c
pod client placeholder
nddq c7f1bca
address lint issue for httpservicefake
nddq f2359ac
getting pod info in validator
nddq fa2cac5
linter issue
nddq 79aef2e
Merge branch 'master' into swift2-cns-ipam-branching
nddq fd2803f
Merge branch 'master' into swift2-cns-ipam-branching
nddq dc08bfe
Merge branch 'master' into swift2-cns-ipam-branching
nddq 7792ac4
Merge branch 'master' into swift2-cns-ipam-branching
nddq dfd8764
Merge branch 'master' into swift2-cns-ipam-branching
nddq a60f9e1
Merge branch 'master' into swift2-cns-ipam-branching
nddq c58fef7
Merge branch 'master' into swift2-cns-ipam-branching
nddq 50deef4
Merge branch 'master' into swift2-cns-ipam-branching
nddq cc91071
update network container contract
nddq 0674e06
renaming
nddq fd03f7c
mtpnc changes
nddq baeb65b
Merge branch 'Azure:master' into master
nddq 1e7c99d
rebase
nddq 6e71052
revert file and var naming, add correct path to makefile
nddq 9225521
resolved merge conflicts
nddq ad1bf07
add default route
nddq 78bc86f
Merge branch 'master' into swift2-cns-ipam-branching
nddq 31ebd90
Merge branch 'master' into swift2-cns-ipam-branching
nddq e26f7c3
add unit tests
nddq 9f97340
Merge branch 'master' into swift2-cns-ipam-branching
nddq 1d4a4bf
Merge branch 'master' into swift2-cns-ipam-branching
nddq 0860321
update unit tests for ipam
nddq 1286167
Merge branch 'master' into swift2-cns-ipam-branching
nddq 5a89c51
go get to fix linter
nddq e193b82
go mod tidy
nddq d1db1ab
update routes
nddq c3e470a
update routes
nddq 7458cf1
Merge branch 'master' into swift2-cns-ipam-branching
nddq c091ec3
Merge branch 'master' into swift2-cns-ipam-branching
nddq dd45bed
remove stale comments + remove redundant method
nddq 745f2cf
Merge branch 'master' into swift2-cns-ipam-branching
nddq bf483aa
add contexts + change address type
nddq 47ce73f
Merge branch 'master' into swift2-cns-ipam-branching
nddq 01310d2
Merge branch 'master' into swift2-cns-ipam-branching
nddq 16e7c50
Merge branch 'master' into swift2-cns-ipam-branching
nddq 99d1e8d
Merge branch 'master' into swift2-cns-ipam-branching
nddq fcdaae0
Merge branch 'master' into swift2-cns-ipam-branching
nddq 843a64e
addressed review
nddq 04ee54c
embedded client to mock + enum for address type
nddq 08b4a78
fix error
nddq da54a6e
Merge branch 'master' into swift2-cns-ipam-branching
nddq 01cc008
Merge branch 'master' into swift2-cns-ipam-branching
nddq 6bb2b7d
change addressType to NICType
nddq 4ee5937
change isDefaultRoute to SkipDefaultRoutes
nddq 7c49876
address comments
nddq aa80c2b
Merge branch 'master' into swift2-cns-ipam-branching
nddq c9baa54
Merge branch 'master' into swift2-cns-ipam-branching
nddq cb8481f
Merge branch 'master' into swift2-cns-ipam-branching
nddq d36f4e7
Merge branch 'master' into swift2-cns-ipam-branching
nddq 33c25f6
refractor: make changes according to cni/cns contract
nddq 2d5c6bd
Merge branch 'master' into swift2-cns-ipam-branching
nddq c0e2ae7
Merge branch 'master' into swift2-cns-ipam-branching
nddq 0972ce4
refractor: make adding route its own func + move swift v2 ipam branch…
nddq 3fc14bd
refractor: change vars naming
nddq 28ba7b2
refractor: more var naming
nddq b067200
test: add test for podv6cidr
nddq 4ffe8d6
refractor: make the returning podIpInfo init cleaner in swiftv2.go
nddq 975e9b4
Merge branch 'master' into swift2-cns-ipam-branching
nddq 0a25e65
refractor + tests: add contexts to ipconfigs req validators + set rou…
nddq d87e4dc
Merge branch 'master' into swift2-cns-ipam-branching
nddq 7153f9b
refractor: change labels for swift v2 pods
nddq d294df3
fix: fix swift v2 UT
nddq e4c60cf
refractor: add v4/v6 distinction for service cidr
nddq 0437f9b
Merge branch 'Azure:master' into swift2-cns-ipam-branching
nddq 5113094
rebase
nddq aee4f2e
revert file and var naming, add correct path to makefile
nddq 68e65e7
rebase
nddq fdebcf4
revert file and var naming, add correct path to makefile
nddq f4802eb
change podipinfo + linter issue
nddq ece68a6
update IPAM branching
nddq ddf472a
pod client placeholder
nddq 691f733
getting pod info in validator
nddq e11247b
linter issue
nddq b6d6c26
rebase
nddq 48a911d
revert file and var naming, add correct path to makefile
nddq 29b6f67
refractor: fix conflicts
nddq c79a0e6
refractor: revert podwatcher code changes
nddq ff291a2
docs: change comment
nddq 17702e9
refractor: change CIDR to CDIRs
nddq 62080d6
Merge branch 'master' into swift2-cns-ipam-branching
nddq 40964f9
Merge branch 'master' into swift2-cns-ipam-branching
nddq 43d7d40
Merge branch 'master' into swift2-cns-ipam-branching
nddq 0e88376
refractor: parse CIDRs as semicolons separated string from env in Set…
nddq b99f5fa
Merge branch 'master' into swift2-cns-ipam-branching
nddq 308d799
Merge branch 'master' into swift2-cns-ipam-branching
nddq e59220e
docs: add minor comment
nddq edcdb2f
Merge branch 'master' into swift2-cns-ipam-branching
nddq 8f7e3c0
Merge branch 'master' into swift2-cns-ipam-branching
nddq 7e086db
Merge branch 'master' into swift2-cns-ipam-branching
nddq 979c417
Merge branch 'master' into swift2-cns-ipam-branching
nddq 48f3895
Merge branch 'master' into swift2-cns-ipam-branching
nddq 627b1a0
refractor: change separator for parsing CIDRs
nddq 6085da3
feat: add rbac roles
nddq c96cc66
fix: gofumpt
nddq f4f2f8e
fix: update clusterrole
nddq 0b55822
fix: add namespace to clusterrolebinding
nddq 4b85934
Merge branch 'master' into featSWIFTv2-IPAM-branching
nddq b7639ed
fix: UT
nddq 75a2110
Merge branch 'master' into featSWIFTv2-IPAM-branching
nddq 768a9de
fix: add labels toswift v2 clusterrole
nddq 610446c
Merge branch 'master' into featSWIFTv2-IPAM-branching
nddq 65977ed
Merge branch 'master' into featSWIFTv2-IPAM-branching
nddq 7e00999
fix: release default ipconfig early if getting swiftv2 ipconfig failed
nddq 7d791f3
test: add more UT
nddq 7b3987a
fix: parsing MTPNC as CIDR instead
nddq 28b0bfa
fix: toggle skipDefaultRoutes for infraNic to true
nddq b307254
fix: add route for node cidr in ipv4 podipconfig
nddq 38fc471
feat: add node cidrs route
nddq d1522cd
fix: linter
nddq de1c82d
address comments
nddq 9f8f7d4
Merge branch 'master' into featSWIFTv2-IPAM-branching
nddq 432a79d
Merge branch 'master' into featSWIFTv2-IPAM-branching
nddq 5ef9623
fix: minor logs formatting
nddq adb1813
feat: move cns yaml for swiftv2 scenario to a diff file + more loggin…
nddq 5660542
fix: log debugf to printf
nddq dc005cd
fix: add testmain to avoid nil pointer error for loggers
nddq 94a8e2d
Update azure-cns.yaml
nddq 382708d
Merge branch 'master' into featSWIFTv2-IPAM-branching
nddq ae5d28d
Merge branch 'master' into featSWIFTv2-IPAM-branching
nddq 2d305e1
fix: move parseCIDRs to a common package, use net/netip instead of net
nddq 54aa8be
fix: exhaustive all switch case for nic type
nddq 86b9e7d
fix: exhaustive all switch case for nic type
nddq 9417618
Merge branch 'master' into featSWIFTv2-IPAM-branching
nddq 8fed7b9
refractor: change fmt.Errorf to errors.Wrapf
nddq d57e6b3
fix: add mtpnc status check in validator + use netip package
nddq 04f08fe
fix: minor
nddq 790e787
Merge branch 'master' into featSWIFTv2-IPAM-branching
nddq 0688baf
Merge branch 'master' into featSWIFTv2-IPAM-branching
nddq 82016cd
Merge branch 'master' into featSWIFTv2-IPAM-branching
nddq 113021d
revert: azure-cns.yaml
nddq 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
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
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 |
|---|---|---|
|
|
@@ -11,13 +11,27 @@ const ( | |
| EnvNodeName = "NODENAME" | ||
| // EnvNodeIP is the IP of the node running this CNS binary | ||
| EnvNodeIP = "NODE_IP" | ||
| // LabelSwiftV2 is the Node label for Swift V2 | ||
| LabelSwiftV2 = "kubernetes.azure.com/podnetwork-multi-tenancy" | ||
| // LabelNodeSwiftV2 is the Node label for Swift V2 | ||
| LabelNodeSwiftV2 = "kubernetes.azure.com/podnetwork-multi-tenancy-enabled" | ||
| // LabelPodSwiftV2 is the Pod label for Swift V2 | ||
| LabelPodSwiftV2 = "kubernetes.azure.com/pod-network" | ||
| EnvPodCIDRs = "POD_CIDRs" | ||
| EnvServiceCIDRs = "SERVICE_CIDRs" | ||
| EnvNodeCIDRs = "NODE_CIDRs" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will change to infraVnet CIDR in future PR |
||
| ) | ||
|
|
||
| // ErrNodeNameUnset indicates the the $EnvNodeName variable is unset in the environment. | ||
| var ErrNodeNameUnset = errors.Errorf("must declare %s environment variable", EnvNodeName) | ||
|
|
||
| // ErrPodCIDRsUnset indicates the the $EnvPodCIDRs variable is unset in the environment. | ||
| var ErrPodCIDRsUnset = errors.Errorf("must declare %s environment variable", EnvPodCIDRs) | ||
|
|
||
| // ErrServiceCIDRsUnset indicates the the $EnvServiceCIDRs variable is unset in the environment. | ||
| var ErrServiceCIDRsUnset = errors.Errorf("must declare %s environment variable", EnvServiceCIDRs) | ||
|
|
||
| // ErrNodeCIDRsUnset indicates the the $EnvNodeCIDRs variable is unset in the environment. | ||
| var ErrNodeCIDRsUnset = errors.Errorf("must declare %s environment variable", EnvNodeCIDRs) | ||
|
|
||
| // NodeName checks the environment variables for the NODENAME and returns it or an error if unset. | ||
| func NodeName() (string, error) { | ||
| nodeName := os.Getenv(EnvNodeName) | ||
|
|
@@ -31,3 +45,27 @@ func NodeName() (string, error) { | |
| func NodeIP() string { | ||
| return os.Getenv(EnvNodeIP) | ||
| } | ||
|
|
||
| func PodCIDRs() (string, error) { | ||
| podCIDRs := os.Getenv(EnvPodCIDRs) | ||
| if podCIDRs == "" { | ||
| return "", ErrPodCIDRsUnset | ||
| } | ||
| return podCIDRs, nil | ||
| } | ||
|
|
||
| func ServiceCIDRs() (string, error) { | ||
| serviceCIDRs := os.Getenv(EnvServiceCIDRs) | ||
| if serviceCIDRs == "" { | ||
| return "", ErrServiceCIDRsUnset | ||
| } | ||
| return serviceCIDRs, nil | ||
| } | ||
|
|
||
| func NodeCIDRs() (string, error) { | ||
| nodeCIDRs := os.Getenv(EnvNodeCIDRs) | ||
| if nodeCIDRs == "" { | ||
| return "", ErrNodeCIDRsUnset | ||
| } | ||
| return nodeCIDRs, 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
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,76 @@ | ||
| package mock | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "github.com/Azure/azure-container-networking/cns/configuration" | ||
| "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" | ||
| "github.com/pkg/errors" | ||
| v1 "k8s.io/api/core/v1" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| ) | ||
|
|
||
| var ( | ||
| ErrPodNotFound = errors.New("pod not found") | ||
| ErrMTPNCNotFound = errors.New("mtpnc not found") | ||
| ) | ||
|
|
||
| // Client implements the client.Client interface for testing. We only care about Get, the rest is nil ops. | ||
| type Client struct { | ||
| client.Client | ||
| mtPodCache map[string]*v1.Pod | ||
| mtpncCache map[string]*v1alpha1.MultitenantPodNetworkConfig | ||
| } | ||
|
|
||
| // NewClient returns a new MockClient. | ||
| func NewClient() *Client { | ||
| const podNetwork = "azure" | ||
|
|
||
| testPod1 := v1.Pod{} | ||
| testPod1.Labels = make(map[string]string) | ||
| testPod1.Labels[configuration.LabelPodSwiftV2] = podNetwork | ||
|
|
||
| testPod3 := v1.Pod{} | ||
| testPod3.Labels = make(map[string]string) | ||
| testPod3.Labels[configuration.LabelPodSwiftV2] = podNetwork | ||
|
|
||
| testPod4 := v1.Pod{} | ||
| testPod4.Labels = make(map[string]string) | ||
| testPod4.Labels[configuration.LabelPodSwiftV2] = podNetwork | ||
|
|
||
| testMTPNC1 := v1alpha1.MultitenantPodNetworkConfig{} | ||
|
|
||
| testMTPNC1.Status.PrimaryIP = "192.168.0.1/32" | ||
| testMTPNC1.Status.MacAddress = "00:00:00:00:00:00" | ||
| testMTPNC1.Status.GatewayIP = "10.0.0.1" | ||
| testMTPNC1.Status.NCID = "testncid" | ||
|
|
||
| testMTPNC4 := v1alpha1.MultitenantPodNetworkConfig{} | ||
|
|
||
| return &Client{ | ||
| mtPodCache: map[string]*v1.Pod{"testpod1namespace/testpod1": &testPod1, "testpod3namespace/testpod3": &testPod3, "testpod4namespace/testpod4": &testPod4}, | ||
| mtpncCache: map[string]*v1alpha1.MultitenantPodNetworkConfig{ | ||
| "testpod1namespace/testpod1": &testMTPNC1, | ||
| "testpod4namespace/testpod4": &testMTPNC4, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| // Get implements client.Client.Get. | ||
| func (c *Client) Get(_ context.Context, key client.ObjectKey, obj client.Object, _ ...client.GetOption) error { | ||
| switch o := obj.(type) { | ||
| case *v1.Pod: | ||
| if pod, ok := c.mtPodCache[key.String()]; ok { | ||
| *o = *pod | ||
| } else { | ||
| return ErrPodNotFound | ||
| } | ||
| case *v1alpha1.MultitenantPodNetworkConfig: | ||
| if mtpnc, ok := c.mtpncCache[key.String()]; ok { | ||
| *o = *mtpnc | ||
| } else { | ||
| return ErrMTPNCNotFound | ||
| } | ||
| } | ||
| return nil | ||
| } |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this func need to return
(types.ResponseCode, string)? If it is just a validator, returning err or nil should be plenty?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see from the impl that this is not actually a pure function, as
-Validatorimplies.It also isn't really middleware/decorator - that would require the input to the middleware func to the next func to call in the chain...this is just a dependent object.
Since it mutates the input
*IPConfigsRequest, the field in the "Middleware" object should be renamed to something likeSetHasSecondaryInterface. I don't have a good idea for what the interface def should be renamed to, but validator it is not.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi @miguelgoms, we had a discussion regarding this offline. Currently the way I implement the logic behind SWIFTv2Middleware cherry-picked middleware behaviors, but it is not "true middleware". It is a design review so it is not blocking, we can still go ahead with this since everything is working now, but I can go back to this after we got everything merged to revise the design.