Skip to content
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.10 backports 2022-11-22 #22310

Merged
merged 5 commits into from
Nov 25, 2022

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Nov 22, 2022

Skipped:

Once this PR is merged, you can update the PR labels via:

$ for pr in 22213 22069 22127; do contrib/backporting/set-labels.py $pr done 1.10; done

@tklauser tklauser requested a review from a team as a code owner November 22, 2022 13:02
@tklauser tklauser added backport/1.10 kind/backports This PR provides functionality previously merged into master. labels Nov 22, 2022
@tklauser
Copy link
Member Author

tklauser commented Nov 22, 2022

/test-backport-1.10

Job 'Cilium-PR-K8s-1.19-kernel-4.9' failed:

Click to show.

Test Name

K8sConformance Portmap Chaining Check connectivity-check compliance with portmap chaining

Failure Output

FAIL: connectivity-check pods are not ready after timeout

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.19-kernel-4.9 so I can create one.

@tklauser
Copy link
Member Author

tklauser commented Nov 23, 2022

Waiting for #22330 to be merged to fix the GitHub action failures. Rebased

@tklauser
Copy link
Member Author

/test-backport-1.10

@jrajahalme jrajahalme added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Nov 24, 2022
@jrajahalme
Copy link
Member

@tklauser Some CI fixes have merged into v1.10, set the needs-rebase label.

aanm and others added 2 commits November 24, 2022 14:26
[ upstream commit 6c98f15 ]

When CEP was converted to an internal CEP structure, the UID
field was not copied, causing the delete requests of CEPs to have their
UID precondition set as empty. When kube-apiserver received this delete
request it didn't delete the CEP because an empty CEP UID didn't match
an existent UID.

Fixes: 6f7bf6c ("Prevent CiliumEndpoint removal by non-owning agent")

Reported-by: Bruno Custódio <bruno@isovalent.com>
Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Tobias Klauser <tobias@cilium.io>
[ upstream commit 3a650c3 ]

When we know the encryption interface, we can jump directly from
bpf_host to that interface using bpf_redirect. For that to work, we
however need to rewrite the MAC addresses. This is currently done in
bpf_host with a FIB lookup to retrieve the MAC addresses.

The performance gain we get from that redirect is however expected to be
negligible because we already traversed the stack several times for
IPsec and we also spent a fair amount of cycles just encrypting the
payloads.

This commit therefore removes the redirect and related FIB lookup. This
change makes the logic for IPsec a little simpler (less error cases
without the FIB lookup). It also makes the logic more consistent across
setups (the FIB lookup was currently only possible on AKS & GKE).
Finally, a later change to IPsec will break the FIB lookup on AKS
anyway.

Signed-off-by: Paul Chaignon <paul@cilium.io>
@tklauser
Copy link
Member Author

@jrajahalme thanks, rebased.

@tklauser
Copy link
Member Author

tklauser commented Nov 24, 2022

/test-backport-1.10

Job 'Cilium-PR-K8s-GKE' failed:

Click to show.

Test Name

K8sChaosTest Connectivity demo application Endpoint can still connect while Cilium is not running

Failure Output

FAIL: Timed out after 242.013s.

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-GKE so I can create one.

@tklauser tklauser removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Nov 24, 2022
[ upstream commit 0696874 ]

When there is an annotation in the k8s node object, the annotation
`io.cilium.network.ipv4-cilium-host` is used as the CiliumInternal IP
address of the CiliumNode object in [1]. Whenever Cilium is updating any
state into the CiliumNode it retrieves all IP address from k8s node,
including the ones from annotations, and appends the local node's IP
addresses, including the newly correct internal / router IP
address, in [2]. Since this is a list, the annotation's IP address is
always used first and all other Cilium agents will wrongly use it for
any operation.

[1] https://github.com/cilium/cilium/blob/927bd8c26904ff92e42c61cec6d00ea8ac062c05/pkg/nodediscovery/nodediscovery.go#L453-L459
[2] https://github.com/cilium/cilium/blob/927bd8c26904ff92e42c61cec6d00ea8ac062c05/pkg/nodediscovery/nodediscovery.go#L474-L489

Fixes: 73d6cae ("install: default AnnotateK8sNode to false")
Signed-off-by: André Martins <andre@cilium.io>
[ upstream commit 1e947e9 ]

When using CiliumNode, the agent's source of truth should be the agent
itself and not k8s node annotations. Thus we will not use the
annotations for the CiliumInternalIP address when generating a
CiliumNode from the k8s Node resource.

Signed-off-by: André Martins <andre@cilium.io>
[ upstream commit ee4ea1a ]

We try to restore the router IP both from the filesystem (first) and
from Kubernetes objects (as a fallback). If the two IP addresses don't
match, we emit a warning.

There is no good reason for this to happen in CI so we should fail the
test if that warning ever shows up. Doing so would have prevented the
flake fixed by the previous commit.

Signed-off-by: Paul Chaignon <paul@cilium.io>
@jrajahalme
Copy link
Member

/test-backport-1.10

@tklauser
Copy link
Member Author

Conformance test on GKE passed but cleanup of the test namespace failed: https://github.com/cilium/cilium/actions/runs/3547069263

All other tests passed and reviews are in, marking as ready to merge.

@tklauser tklauser added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 25, 2022
@jrajahalme
Copy link
Member

ci-gke-1.10 timed out, but has been canceled, so it can not be retried.

@jrajahalme jrajahalme added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. and removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Nov 25, 2022
@jrajahalme jrajahalme merged commit bb03d95 into cilium:v1.10 Nov 25, 2022
@tklauser tklauser deleted the pr/v1.10-backport-2022-11-22 branch November 25, 2022 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants