-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
v1.15 Backports 2024-03-19 #31490
v1.15 Backports 2024-03-19 #31490
Commits on Mar 20, 2024
-
controlplane: fix panic: send on closed channel
[ upstream commit 7df437a ] Rarely, the control plane test panics, due to a send on a closed channel. This can occur in a narrow race window in the filteringWatcher: 1. Stop is called on the child watcher 2. Child watcher calls stop on parent watcher 3. Concurrently, an event is dequeued from the parent result chan, and we enter the filtering logic. 4. The parent result chan is closed, and we close the child event channel 5. The filter is matched, and we attempt to write on the closed channel, which causes the panic. Instead of closing the channel in the Stop method, close the channel from the writing goroutine (as is commonly considered best practice in Go.) Fixes: fa89802 (controlplane: Implement filtering of objects with field selectors) Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 0db73f1 - Browse repository at this point
Copy the full SHA 0db73f1View commit details -
controlplane: add mechanism to wait for watchers
[ upstream commit ba99d74 ] [ backporter notes: Whitespace conflict at the new `establishedWatchers` field in test/controlplane/suite/testcase.go ] We've recently learned that the fake k8s client set's object tracker do not respect the semantics of the real api-server when it comes to 'Watch': since the object tracker does not care for ResourceVersions, it cannot respect the version from which it ought to replay events. As a result, the default informer (more precisely, its reflector) is racy: it uses a ListAndWatch approach, which relies on this resource version to avoid a race window between the end of list and the beginning of watch. Therefore, all informers used in cilium have a low chance of hitting this race when used with a k8s fake object tracker. This is somewhat known in the k8s community, see for example [1]. However, the upstream response is that one simply shouldn't be using the fake infrastructure to test real informers. Unfortunately, this pattern is used somewhat pervasively inside the cilium tests, specifically so in the controlplane tests. This patch introduces a mechanism which reduces the likelihood of hitting the flake, under the assumption that we do not (often) establish multiple watchers for the same resource. In the following patch, we'll use the new infrastructure to reduce the flakiness of tests. [1]: kubernetes/kubernetes#95372 Co-authored-by: Fabian Fischer <fabian.fischer@isovalent.com> Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for dc76693 - Browse repository at this point
Copy the full SHA dc76693View commit details -
controlplane: wait for watcher establishment
[ upstream commit 955049a ] [ backporter notes: Also updated status_updates_gc.go ] Use the infrastructure introduced in the previous commit to deflake control plane tests which update k8s state after starting the agent. Co-authored-by: Fabian Fischer <fabian.fischer@isovalent.com> Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for a5fc7a6 - Browse repository at this point
Copy the full SHA a5fc7a6View commit details -
job: avoid a race condition in ExitOnCloseFnCtx
[ upstream commit 6b2d186 ] The test attempted to avoid closing a channel multiple times by setting 'started' to nil. However, since the outer scope will wait on 'started', if started is set to nil before the outer scope waits, it will wait indefinitely - resulting in a time out of the test. Fixes: daa85a0 (jobs,test: Fix TestTimer_ExitOnCloseFnCtx channel close panic) Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 1ef26a1 - Browse repository at this point
Copy the full SHA 1ef26a1View commit details -
controlplane: fix mechanism for ensuring watchers
[ upstream commit fe71a4a ] [ backporter notes: Minor conflicts in the fields of `ControlPlaneTest` (fakeDatapath package has been renamed) ] I realized that the fix for controlplane tests isn't complete. There is still a (small) race window: The current watch reaction records a watcher as established without "handling" the watch itself, i.e. it lets the default watch reaction actually call 'Watch' on the tracker. This is racy, as things can happen in the window between recordng and actually watching. To fix this, add the recording unconditionally in the existing tracker augmentation. Fixes: ba99d74 (controlplane: add mechanism to wait for watchers) Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 3a71d40 - Browse repository at this point
Copy the full SHA 3a71d40View commit details -
Handle InvalidParameterValue as well for PD fallback
[ upstream commit 5a487b5 ] #30536 prematurely concluded that AWS now uses InsufficientCidrBlocks to indicate the subnet is out of prefixes. Looks like AWS still uses InvalidParameterValue and "There aren't sufficient free Ipv4 addresses or prefixes" to indicate subnet is at capacity. In addition to this InsufficientCidrBlocks is returned when subnet is at capacity potentially due to fragmentation. In either case, it's worth trying to fallback since /32 IPs might still be available compared to /28. See PR for details from AWS support ticket. Signed-off-by: Hemanth Malla <hemanth.malla@datadoghq.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 42ef0c1 - Browse repository at this point
Copy the full SHA 42ef0c1View commit details -
Adding unit test for PD fallback
[ upstream commit 0007e35 ] [ backporter notes: Minor import conflict in pkg/aws/ec2/mock/mock.go ] Signed-off-by: Hemanth Malla <hemanth.malla@datadoghq.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 7fef7f5 - Browse repository at this point
Copy the full SHA 7fef7f5View commit details -
slices: don't modify missed input slice in test
[ upstream commit 08d898d ] The commit below correctly identified that modifying the test input makes the test flaky due to being dependent on the non-deterministic ordering. However, it missed adding the work-around for the first test, TestUnique. Fixes: 32543a4 (slices: don't modify input slices in test) Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 171b64a - Browse repository at this point
Copy the full SHA 171b64aView commit details -
bgpv1: avoid object tracker vs informer race
[ upstream commit cfd1790 ] The fake k8s testing infrastructure has an unfortunate interaction between real informers and the object tracker used in fake clientsets: the informer uses ListAndWatch to subscribe to the api server, but ListAndWatch relies on the ResourceVersion field to bridge the gap between the List and the Watch. The simple object tracker does not provide the resource versioning, nor does its Watch implementation attempt to replay existing state. The race window, hence, allows for creation of objects _after_ the initial list, but _before_ the establishment of Watch. These objects are not observed by the informer and thus tests are likely to fail. As a workaround, we ensure that the object tracker has registered a watcher before we start the test in earnest. It is only important that we do not create/update/delete objects in the window between starting the hive (and hence running of the informer) and having ensured that the watcher is in place. Creation prior to starting the hive is okay, as well as after ensuring the watcher exists. Signed-off-by: David Bimmler <david.bimmler@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 30dbb6b - Browse repository at this point
Copy the full SHA 30dbb6bView commit details -
doc: Clarified GwAPI KPR prerequisites
[ upstream commit 1321e03 ] Before the GwAPI doc listed KPR mode as a prerequisite. However, it's actually only required to enable BPF nodePort support. Signed-off-by: Philip Schmid <philip.schmid@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 08b7d91 - Browse repository at this point
Copy the full SHA 08b7d91View commit details -
helm: Add pod affinity for cilium-envoy
[ upstream commit 44aeb53 ] [ backporter notes: Minor conflict in values.yaml due to different intendation of podAntiAffinity in v1.15 ] This commit is to avoid cilium-envoy running on the node without cilium agent. Two main changes are: - nodeAffinity to make sure that cilium-envoy will not be scheduled on node without cilium agent - podAffinity with requiredDuringSchedulingIgnoredDuringExecution to cilium agent Relates: #25081, #30034 Fixes: #31149 Signed-off-by: Tam Mach <tam.mach@cilium.io> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 7733839 - Browse repository at this point
Copy the full SHA 7733839View commit details -
bgpv1: fix Test_PodIPPoolAdvert flakiness
[ upstream commit 696a4fd ] Since the test updates multiple k8s resources in each test step, and each step depends on the previous one, we should be careful about how many k8s resources are we actually changing in each step, to not trigger multiple reconciliations with different results (advertisements) in a single step. This change ensures we change only one value that affects the advertisement in each test step. Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 5f194fa - Browse repository at this point
Copy the full SHA 5f194faView commit details -
bpf, maps: Don't propagate nodeID to bpf map when allocation fails.
[ upstream commit 4f4b0a7 ] When we run out of IDs to allocate for nodes, we were propagating zero ID to bpf map. Now we just simply return error and not modify bpf map instead. Also clean up incorrectly mapped nodeids on startup in case that happened. Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for a1e7b6b - Browse repository at this point
Copy the full SHA a1e7b6bView commit details -
policy: Fix missing labels from SelectorCache selectors
[ upstream commit 65bbf3f ] During the refactor of the below commit, it seems the labels were left out inadvertently, breaking the `cilium policy selectors` command that displays the labels/name of the policy from which the selectors originate from. Fixes: 501944c ("policy/selectorcache: invert identitySelector interface") Signed-off-by: Chris Tarazi <chris@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 46d3c06 - Browse repository at this point
Copy the full SHA 46d3c06View commit details -
ci: Bump lvh-kind ssh-startup-wait-retries
[ upstream commit a5eafe0 ] Recently, we frequently see the CI failure with lvh-kind startup failure with exit code 41. This indicates the timeout of the task waiting for the SSH startup. Bump the timeout (retry) to 600 (10min) as a workaround. Fixes: #31336 Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for b606421 - Browse repository at this point
Copy the full SHA b606421View commit details -
datapath: Remove unnecessary IPsec code
[ upstream commit 4ba7e6a ] Commit 891fa78 ("bpf: Delete obsolete do_netdev_encrypt_pools()") removed the special code we had to rewrite the IPsec outer header. The code removed in the present commit is therefore not required anymore. Fixes: 891fa78 ("bpf: Delete obsolete do_netdev_encrypt_pools()") Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for d7d315e - Browse repository at this point
Copy the full SHA d7d315eView commit details -
operator: fix errors/warnings metric.
[ upstream commit f61651f ] This was broken during transition of pkg/metrics to integrate with Hive where relevant operator metrics where never initialized. This adds a init func specific for operator and cleans up the "flush" logic used as a work around for errors/warnings emitted prior to agent starting (in the case of the operator). Addresses: #29525 Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 349478e - Browse repository at this point
Copy the full SHA 349478eView commit details -
loader: add message if error is ENOTSUP
[ upstream commit 6436447 ] I was trying to install cilium in a machine without a vxlan module available. Add a helpful message for the next time this happens. Tested by running: > PRIVILEGED_TESTS=1 go test -test.v -test.run TestSetupTunnelDevice on said machine, and got the following output: ... ``` Error: Received unexpected error: setting up vxlan device: creating vxlan device: creating device cilium_vxlan: operation not supported, maybe kernel module for vxlan is not available? Test: TestSetupTunnelDevice/Vxlan ``` Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 3fa18da - Browse repository at this point
Copy the full SHA 3fa18daView commit details -
ipam: fix azure ipam test panics due to shared pointers.
[ upstream commit 1ca6141 ] pkg/ipam/types.(*InstanceMap).DeepCopy(...) will iterate for all instances/interfaces in order to copy the data. However, unlike what the name suggests, underlying instance pkg/ipam/types.Interface pointers are copied and shared in the returned instance map. In some cases, this case result in memory corruption issues resulting in confusing panics while running tests such as: ``` panic: runtime error: makeslice: cap out of range goroutine 1366 [running]: strconv.appendQuotedWith({0xc000576208, 0x0, 0x3f?}, {0x1000000, 0x100000001000000}, 0x22, 0x0, 0x0) /opt/hostedtoolcache/go/1.22.0/x64/src/strconv/quote.go:35 +0x85 strconv.AppendQuote(...) ... ``` Capturing such an event in a debugger you would see a AzureInterface struct such as this with the IP string memory being corrupt (likely due to an interleaved read/write) being passed to logrus causing a crash. ``` github.com/cilium/cilium/pkg/azure/types.AzureAddress { IP: "\x10\x01\x00\x00\x00\x00\x00\x00\x10\x01\x00\x00\x00\x00\x00\x007d�_\x02\b\b\x19\x00\x00\x00\x00\x00\x00\x00\x00�\x1f�\x03\x00\x00\x00\x00W�\b\x00\x00\x00\x00\x00\x00p \x03\x00\x00\x00\x00�qi\x03\x00\x00\x00\x00...+51559946186908214 more", Subnet: "subsys", State: "instanceID",} ``` This ensures that the revision interface is correctly deepcopied such that the underlying resource is also safely copied. Fixed: #31059 Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 9edeb88 - Browse repository at this point
Copy the full SHA 9edeb88View commit details -
gateway-api: Retrieve LB service from same namespace
[ upstream commit e8bed8d ] [ backporter notes: conflicts in unit tests. Had to change introduce the `NoError` check instead of expecting a specific error ] This commit is to add the same namespace while listing generated LB service, the main reason is to avoid wrong status update for gateways having the same name, but belonged to different namespace. Testing was done locally as per below: Before the fix: ``` $ kg gateway -A NAMESPACE NAME CLASS ADDRESS PROGRAMMED AGE another my-gateway cilium 10.110.222.237 True 4s default my-gateway cilium 10.110.222.237 True 56s ``` After the fix: ``` $ kg gateway -A NAMESPACE NAME CLASS ADDRESS PROGRAMMED AGE another my-gateway cilium 10.102.170.180 True 14m default my-gateway cilium 10.110.222.237 True 14m $ kg services -A NAMESPACE NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE another cilium-gateway-my-gateway LoadBalancer 10.102.170.180 10.102.170.180 80:31424/TCP 15m default cilium-gateway-my-gateway LoadBalancer 10.110.222.237 10.110.222.237 80:31889/TCP 15m ... ``` Fixes: #31270 Signed-off-by: Tam Mach <tam.mach@cilium.io> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for a95ca7c - Browse repository at this point
Copy the full SHA a95ca7cView commit details -
ci-e2e: Add matrix for bpf.tproxy
[ upstream commit 5884af1 ] This is to make sure that we have the coverage for bpf.tproxy enabled. The first step is to test it with Ingress Controller enabled, we can add more settings if required later. Relates: #30331, #30404 Suggested-by: Marco Hofstetter <marco.hofstetter@isovalent.com> Signed-off-by: Tam Mach <tam.mach@cilium.io> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 6f827e2 - Browse repository at this point
Copy the full SHA 6f827e2View commit details -
gha: disable fail-fast on integration tests
[ upstream commit 0fb203e ] So that the failure of one matrix entry (e.g., caused by a flake) doesn't cancel the other ongoing tests, if any. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for b2f8e8d - Browse repository at this point
Copy the full SHA b2f8e8dView commit details -
hive/cell/health: don't warn when reporting on stopped reporter.
[ upstream commit 44e9005 ] Currently in cases where reporters emit status on stopped reporters, a warning is logged. This causes failures in CI related to endpoints, as those close their reporters after endpoint is deleted. This scenario is probably not a valuable log for users and generally such occurrences should be harmless. This moves the log to debug level. Fixes: #31147 Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 5738e81 - Browse repository at this point
Copy the full SHA 5738e81View commit details -
docs: Warn on key rotations during upgrades
[ upstream commit b639eab ] In general, it is not recommended to carry several admin. operations on the cluster at the same time, as it can make troubleshooting in case of issues a lot more complicated. Mixing operations is also less likely to be covered in CI so more likely to hit corner cases. Performing IPsec key rotations during Cilium up/downgrades is one such case. Let's document it explicitly to discourage users from doing that. Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Configuration menu - View commit details
-
Copy full SHA for 0ef35be - Browse repository at this point
Copy the full SHA 0ef35beView commit details