-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Generalize sockets library and add test coverage for serviceLB. #37600
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
Generalize sockets library and add test coverage for serviceLB. #37600
Conversation
Commit cb34a1d does not match "(?m)^Signed-off-by:". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
/test |
1ca65cb
to
e7d05a8
Compare
Commit e8628a0 does not match "(?m)^Signed-off-by:". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
e7d05a8
to
93501e9
Compare
Commit e8628a0 does not match "(?m)^Signed-off-by:". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
93501e9
to
2756851
Compare
e9f363f
to
d4f3fc5
Compare
d4f3fc5
to
725cac7
Compare
/test |
0e5cf58
to
030ffd1
Compare
/test |
030ffd1
to
c555be9
Compare
/test |
c555be9
to
ad080c0
Compare
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.
Solid step in generalizing the sockets termination logic. 🚀
It'll be good to fix the inconsistencies in the unit tests.
ad080c0
to
66bc5c3
Compare
/test |
This will be used in subsequent commits for building tests for service UDP socket termination functionality. Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
This provides an easy iterator type for iterating netns in the default pinned directory. This being a simple function that returns the iterator will allow for easy test mocking in subsequent commands where we want to create real network-namespaces, but don't necessarily need to pin them to the filesystem. Errors are aggregated in a buffered channel so they can be accessed during iteration. Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
This abstracts concern related to iterating netns and uses the new netns.All() function to replace the current default functionality parity. This will allow for providing an iterator that doesn't rely on pinned netns (i.e. just ones we create at runtime for the test process) which will make it easier to test. As well, switch global use of *option.Config to a field now provided via hive. This will avoiding global options mutation for tests. Finally, we now return errors from the terminate function, although we do not handle it as errors are already logged. The purpose is to return errors while testing in following commits. Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
This provides integration testing to test service manager TerminateUDPConnectionsToBackend functionality together with sockets.Destroy implementation. Note: This is committed prior to further changes to pkg/datapath/sockets code to validate that tests succeed before and after the change. Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
Instead of having a custom function for iterating UDP connections rename and change this function to: iterateNetlinkSockets. iterateNetlinkSockets will be able iterate any type of socket via the netlink api. In subsequent commits, this will be used to make the datapath/sockets library a general purpose sockets library that allows for separately iterating and destroying sockets via netlink. Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
This is used to validate that subsequent changes do not introduce regressions to functionality relying on sockets.Destroy (i.e. svc-LB UDP sock destroy). Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
Note: Up to this change was benchmarks to validate no memory usage regressions are introduced following previous optimizations in previous commits: ------------- New: ------------- goos: linux goarch: arm64 pkg: github.com/cilium/cilium/pkg/datapath/sockets BenchmarkSocketReqSerialize-8 69173708 174.3 ns/op 128 B/op 2 allocs/op BenchmarkSocketDeserialize-8 159207418 75.43 ns/op 64 B/op 4 allocs/op PASS ok github.com/cilium/cilium/pkg/datapath/sockets 31.865s ------------- Old: ------------- goos: linux goarch: arm64 pkg: github.com/cilium/cilium/pkg/datapath/sockets BenchmarkSocketReqSerialize-8 69530899 172.2 ns/op 128 B/op 2 allocs/op BenchmarkSocketDeserialize-8 163702172 73.25 ns/op 64 B/op 4 allocs/op PASS ok github.com/cilium/cilium/pkg/datapath/sockets 31.551s Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
The only consumer of this code is (currently) service manager so we should reflect that in reviews. Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
66bc5c3
to
9ca58db
Compare
/test |
Rebasing/re-testing: #38566 fixes test failure |
The
datapath/sockets
code evolved from the use-case of serviceLB socket termination. This makes the code very specific to that code. Instead this generalizes the underlying code to expose more general functions for iterating and closing sockets via netlink.