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.13 backports 2023-07-31 #27154

Merged
merged 3 commits into from Aug 3, 2023
Merged

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented Jul 31, 2023

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

$ for pr in 26880 27015 25324; do contrib/backporting/set-labels.py $pr done 1.13; done

@sayboras sayboras requested a review from a team as a code owner July 31, 2023 11:17
@sayboras sayboras added kind/backports This PR provides functionality previously merged into master. backport/1.13 This PR represents a backport for Cilium 1.13.x of a PR that was merged to main. labels Jul 31, 2023
@sayboras
Copy link
Member Author

sayboras commented Jul 31, 2023

/test-backport-1.13

Job 'Cilium-PR-K8s-1.24-kernel-4.19' failed:

Click to show.

Test Name

K8sAgentPolicyTest Multi-node policy test with L7 policy using connectivity-check to check datapath

Failure Output

FAIL: connectivity-check pods are not ready after timeout

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.24-kernel-4.19/102/

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

Then please upload the Jenkins artifacts to that issue.

Job 'Cilium-PR-K8s-1.21-kernel-4.19' failed:

Click to show.

Test Name

K8sAgentPolicyTest Multi-node policy test with L7 policy using connectivity-check to check datapath

Failure Output

FAIL: cannot install connectivity-check

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.21-kernel-4.19/102/

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

Then please upload the Jenkins artifacts to that issue.

Copy link
Contributor

@learnitall learnitall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks!

@sayboras
Copy link
Member Author

sayboras commented Aug 2, 2023

@julianwiedmann there is bpf unit test failure due to the below error, seems like v1.13 didn't have the new attribute local_ivp4 in bpf.h (added as part of #25438). Any suggestion on how to move forward ?

https://github.com/cilium/cilium/actions/runs/5714021888/job/15480514433?pr=27154

l4lb_ipip_health_check_host.c:70:12: error: no member named 'local_ipv4' in 'struct bpf_tunnel_key'
        if (from->local_ipv4 != 0)

@julianwiedmann
Copy link
Member

@julianwiedmann there is bpf unit test failure due to the below error, seems like v1.13 didn't have the new attribute local_ivp4 in bpf.h (added as part of #25438). Any suggestion on how to move forward ?

Oh of course 🤦.

It's fine to just drop that specific condition check from the test - thanks for digging it up @sayboras ! Should be straight-forward enough, but let me know if you want me to provide a custom backport.

[ upstream commit d140758 ]

Signed-off-by: Vipul Singh <singhvipul@microsof.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
@sayboras sayboras force-pushed the pr/v1.13-backport-2023-07-31 branch 2 times, most recently from e590b79 to aca0171 Compare August 2, 2023 12:14
julianwiedmann and others added 2 commits August 2, 2023 22:18
[ upstream commit ebdfb37 ]

If handle_nat_fwd() expands into a tail-call (eg. because both ENABLE_IPV4
and ENABLE_IPV6 are set), control doesn't return to the main to-netdev
program. Thus the packet path never passes through the L4LB health-check
processing.

This is a regression caused by 54a8631 - previously, we would skip
the NAT check in a full-DSR config (and the L4LB IPIP mode is always
full-DSR).

Fix it by pulling the health-check section up, so that it gets applied
before walking down the NAT path. Also add a minimal integration test to
cover the expected encapsulation behaviour.

Fixes: 54a8631 ("bpf: nodeport: handle revDNAT for local backends at to-netdev/to-overlay")
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit 901b749 ]

This commit adjusts the logic that the operator uses to control endpoint
garbage collection and syncing in order to account for the case when
CiliumEndpoint CRDs are disabled in kvstore mode. The operator will now
only start the garbage collector if CiliumEndpoint CRD mode is enabled,
and if it isn't enabled, the operator will check to ensure that the
CiliumEndpoint CRD is installed in the cluster before starting a one-off
gc sync. This will allow users to use kvstore mode without having to
worry about setting the endpoint gc interval to zero, and it covers the
case where a user transitions from CRD mode to kvstore mode.

Fixes: cilium#24440

Signed-off-by: Ryan Drew <ryan.drew@isovalent.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
@sayboras sayboras force-pushed the pr/v1.13-backport-2023-07-31 branch from aca0171 to 44a1a43 Compare August 2, 2023 12:18
@sayboras
Copy link
Member Author

sayboras commented Aug 2, 2023

Seems like the below patch helps

diff --git a/bpf/tests/l4lb_ipip_health_check_host.c b/bpf/tests/l4lb_ipip_health_check_host.c
index ce9bd98fcd..e2f55e9dc7 100644
--- a/bpf/tests/l4lb_ipip_health_check_host.c
+++ b/bpf/tests/l4lb_ipip_health_check_host.c
@@ -67,10 +67,8 @@ int mock_skb_set_tunnel_key(__maybe_unused struct __sk_buff *skb,
 {
 	if (from->tunnel_id != 0)
 		return -1;
-	if (from->local_ipv4 != 0)
-		return -2;
 	if (from->remote_ipv4 != bpf_ntohl(BACKEND_IP))
-		return -3;
+		return -2;
 	return 0;
 }

@sayboras
Copy link
Member Author

sayboras commented Aug 2, 2023

/test-backport-1.13

Job 'Cilium-PR-K8s-1.23-kernel-4.19' failed:

Click to show.

Test Name

K8sAgentPolicyTest Multi-node policy test with L7 policy using connectivity-check to check datapath

Failure Output

FAIL: cannot install connectivity-check

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.23-kernel-4.19/159/

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

Then please upload the Jenkins artifacts to that issue.

Job 'Cilium-PR-K8s-1.25-kernel-4.19' failed:

Click to show.

Test Name

K8sDatapathServicesTest Checks E/W loadbalancing (ClusterIP, NodePort from inside cluster, etc) Tests NodePort inside cluster (kube-proxy) with IPSec and externalTrafficPolicy=Local

Failure Output

FAIL: Request from k8s1 to service tftp://[fd04::11]:31289/hello failed

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.25-kernel-4.19/812/

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

Then please upload the Jenkins artifacts to that issue.

@sayboras
Copy link
Member Author

sayboras commented Aug 2, 2023

/test-1.23-4.19

@sayboras
Copy link
Member Author

sayboras commented Aug 2, 2023

/test-1.25-4.19

@sayboras sayboras added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 2, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 3, 2023
@aanm aanm merged commit f7a29d3 into cilium:v1.13 Aug 3, 2023
59 checks passed
@sayboras sayboras deleted the pr/v1.13-backport-2023-07-31 branch August 3, 2023 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.13 This PR represents a backport for Cilium 1.13.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants