v1.15 Backports 2024-03-26 #31601
Merged
v1.15 Backports 2024-03-26 #31601
Conversation
This file contains 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
sayboras
added
kind/backports
sayboras
force-pushed
the
pr/v1.15-backport-2024-03-26-11-11
branch
from
63b73d3
to
30d72e4
Compare
|
/test-backport-1.15 |
mhofstetter
approved these changes
Mar 26, 2024
|
Looks like there's a conflict, @sayboras |
julianwiedmann
added
the
dont-merge/needs-rebase
sayboras
force-pushed
the
pr/v1.15-backport-2024-03-26-11-11
branch
from
30d72e4
to
cec682f
Compare
sayboras
removed
the
dont-merge/needs-rebase
pchaigno
requested changes
Mar 26, 2024
sayboras
force-pushed
the
pr/v1.15-backport-2024-03-26-11-11
branch
from
cec682f
to
e3e6dfe
Compare
|
/test-backport-1.15 |
pchaigno
approved these changes
Mar 26, 2024
|
/test-backport-1.15 |
|
Required reviews are in, CI is green. Marking this ready to merge. |
sayboras
added
the
dont-merge/preview-only
|
@sayboras did you mean to add |
ah don't merge label was added intentionally, I need to check if there is any pending work from the release perspective. |
squeed
approved these changes
Mar 27, 2024
|
@sayboras there's a merge conflict :-( |
Thanks, let me rebase again it 👍 |
sayboras
force-pushed
the
pr/v1.15-backport-2024-03-26-11-11
branch
from
e3e6dfe
to
a95a89d
Compare
|
/test |
sayboras
added
ready-to-merge
[ upstream commit 234e7d0 ] Go's race detector was unhappy with this test due to unserialised concurrent access to the last value of the fake gauge. Use an atomic float value instead, to ensure no weirdness can occur, and placate the race detector. Race detector warning (mildly edited): WARNING: DATA RACE Read at 0x00c000930488 by goroutine 205: github.com/cilium/cilium/pkg/endpointmanager.TestPolicyMapPressure.TestPolicyMapPressure.func1.func2() cilium/pkg/endpointmanager/policymap_pressure_test.go:27 +0x69 github.com/stretchr/testify/assert.Eventually.func1() cilium/vendor/github.com/stretchr/testify/assert/assertions.go:1902 +0x33 Previous write at 0x00c000930488 by goroutine 203: github.com/cilium/cilium/pkg/endpointmanager.(*fakeGague).Set() cilium/pkg/endpointmanager/policymap_pressure_test.go:45 +0x30 github.com/cilium/cilium/pkg/endpointmanager.(*policyMapPressure).update() cilium/pkg/endpointmanager/policymap_pressure.go:82 +0x32a github.com/cilium/cilium/pkg/endpointmanager.newPolicyMapPressure.func1() cilium/pkg/endpointmanager/policymap_pressure.go:57 +0x2e github.com/cilium/cilium/pkg/trigger.(*Trigger).waiter() cilium/pkg/trigger/trigger.go:201 +0x771 github.com/cilium/cilium/pkg/trigger.NewTrigger.gowrap1() cilium/pkg/trigger/trigger.go:122 +0x33 Goroutine 205 (running) created at: github.com/stretchr/testify/assert.Eventually() cilium/vendor/github.com/stretchr/testify/assert/assertions.go:1902 +0x3d5 github.com/stretchr/testify/assert.(*Assertions).Eventually() cilium/vendor/github.com/stretchr/testify/assert/assertion_forward.go:319 +0xc7 github.com/cilium/cilium/pkg/endpointmanager.TestPolicyMapPressure.func1() cilium/pkg/endpointmanager/policymap_pressure_test.go:26 +0x2a8 github.com/cilium/cilium/pkg/endpointmanager.TestPolicyMapPressure() cilium/pkg/endpointmanager/policymap_pressure_test.go:30 +0x205 Goroutine 203 (running) created at: github.com/cilium/cilium/pkg/trigger.NewTrigger() cilium/pkg/trigger/trigger.go:122 +0x36d github.com/cilium/cilium/pkg/endpointmanager.newPolicyMapPressure() cilium/pkg/endpointmanager/policymap_pressure.go:52 +0x287 github.com/cilium/cilium/pkg/endpointmanager.TestPolicyMapPressure() cilium/pkg/endpointmanager/policymap_pressure_test.go:18 +0x84 Fixes: 28ce005 (endpointmanager: fix bpf policy pressure getting stuck.) Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit 0b56a21 ] IDPool contains a mutex, passing copies around is a potential footgun. I don't think we ever used it incorrectly, but I don't see a reason for all the copying either. Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit 59a3100 ] Transform the test from using checkmate to standard Go tests, as it was not using any of the features anyway. Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit 58ad35a ] The endpointmanagers idallocator package was using a package global pool for its identifier allocation. That's fine for running the agent, but causes flakes in testing when multiple tests access the same pool. It's also not idiomatic Go. This patch makes the local endpoint identifier allocator a struct, and the next patch will move it into the endpointmanager package itself, as there is no other consumer. While at it, also ensure that the RemoveAll method is only called from a testing context, by taking a testing.TB as an argument. We cannot simply move the method into the _test.go files, as tests from other packages use it. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
[ upstream commit a1a03cf ] The endpoint manager assumed it was the only consumer of the idallocator pkg anyway. Having a pkg that only has one consumer is pointless, hence move it, including tests. This also allows unexporting everything, and reducing API surface. Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit 0f27591 ] TestLookup could fail with --- FAIL: Test (0.12s) --- FAIL: Test/EndpointManagerSuite (0.12s) --- FAIL: Test/EndpointManagerSuite/TestLookup (0.00s) manager_test.go:438: ... value *endpoint.Endpoint = &endpoint.Endpoint{[...]") ... Test Name: endpoint does not exist In about 0.02% of the runs. Specifically, it would fail iff the endpoint created happened to randomly get ID 1234, out of the pool of 4096 IDs. Fix this by not creating an endpoint unconditionally in the test, so that there is no chance of creating one with the "wrong" id. Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit fce1121 ] For some reason, lots of calls to expose didn't check the error returned. Specifically in the test context, this is not great, as it makes debugging test failures more difficult than necessary. Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit 6045c61 ] Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit e2fbd38 ] Instead of having to import the testing pkg in non-testing code, let's remove the blocker on just having this method in the _test.go files. The endpoint manager in the daemon_test is re-initialized for every test, and since we don't have package global state any more we can just remove the call to RemoveAll, which solves all the problem nicely. Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit bee8535 ] Those helper functions to retrieve the local IPs are all IPsec specific so let's move them to the ipsec.go file. No functional changes in this commit. Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
[ upstream commit 4687325 ] This commit extends getDefaultEncryptionInterface to also handle tunneling mode. That change allows us to start using getDefaultEncryptionInterface everywhere we need to retrieve the default IPsec interface. The unit test for IPsec in subnet encryption mode (ENI and Azure IPAM modes) must be updated. Subnet encryption is only ever possible in native routing mode. If we were doing subnet encryption with tunneling, it would cause undefined behaviors (in the test would fail :)). Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com> Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit 3c6f957 ] The agent supported attaching the IPsec decryption logic to interfaces given via --devices. In that case, this logic was contained in bpf_host instead of bpf_network. This support is partially covered in ginkgo end-to-end tests. That support is however broken, as there doesn't seem to be anything preventing bpf_network from being reloaded in place of bpf_host on the same interfaces. This commit fixes it by implementing proper support for --devices in IPsec. If no devices flag is given then we fallback to using the encrypt-interface flag. That should allow us to deprecate encrypt-interface at a latter time. Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
[ upstream commit f153f42 ] Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
[ upstream commit d8082f9 ] Currently, while translating K8s Ingress or Gateway API resources into Envoy resources, the filterchain is in random order. This leads to situations (especially in combination with Shared Ingress) where the order of the filterchains isn't guaranteed - resulting in unnecessary reconciliations. Therefore, this commit orders the filterchains within a Envoy Listener by the namespace and name of the TLS secret. This makes the translation deterministic. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com> Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit 305ea74 ] Currently, while translating K8s Ingress or Gateway API resources into Envoy resources, the filterchain for TLS listeners is in random order. This leads to situations (especially in combination with Shared Ingress) where the order of the filterchains isn't guaranteed - resulting in unnecessary reconciliations. Therefore, this commit orders the filterchains within a Envoy Listener by the name of the backends. This makes the translation deterministic. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com> Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras
force-pushed
the
pr/v1.15-backport-2024-03-26-11-11
branch
from
a95a89d
to
cde6274
Compare
|
/test |
sayboras
removed
the
ready-to-merge
|
/test-backport-1.15 |
maintainer-s-little-helper
bot
added
the
ready-to-merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
--devices#31345 (@pchaigno)Once this PR is merged, a GitHub action will update the labels of these PRs: