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.12 backports 2023-01-24 #23301

Merged
merged 19 commits into from
Jan 26, 2023

Conversation

ldelossa
Copy link
Contributor

@ldelossa ldelossa commented Jan 24, 2023

Dropped PRs:

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

$ for pr in 23075 23077 23095 23135 23140 22864 23134 23058 23158 23195 22695 23225 22921 23254 23235; do contrib/backporting/set-labels.py $pr done 1.12; done

@ldelossa ldelossa requested review from a team as code owners January 24, 2023 17:38
@ldelossa ldelossa added backport/1.12 This PR represents a backport for Cilium 1.12.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. labels Jan 24, 2023
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Jan 24, 2023
@ldelossa
Copy link
Contributor Author

@joestringer
Copy link
Member

@ldelossa I suspect that is related to #22220. Maybe try dropping that backport for now?

@michi-covalent
Copy link
Contributor

michi-covalent commented Jan 24, 2023

/test-backport-1.12

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

Click to show.

Test Name

K8sServicesTest Checks graceful termination of service endpoints Checks client terminates gracefully on service endpoint deletion

Failure Output

FAIL: Found 1 k8s-app=cilium logs matching list of errors that must be investigated:

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

Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

My 3 PRs look good. Thanks!

@ldelossa
Copy link
Contributor Author

/test-1.20-4.9

Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

Apart from below points, the rest looks good to me, thanks a lot 💯

.github/workflows/conformance-gateway-api.yaml Outdated Show resolved Hide resolved
.github/workflows/conformance-ingress-shared.yaml Outdated Show resolved Hide resolved
@michi-covalent michi-covalent added the release-blocker/1.12 This issue will prevent the release of the next version of Cilium. label Jan 25, 2023
Copy link
Member

@julianwiedmann julianwiedmann left a comment

Choose a reason for hiding this comment

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

Some changes needed for conflict resolutions.

bpf/lib/nodeport.h Outdated Show resolved Hide resolved
bpf/lib/lb.h Outdated Show resolved Hide resolved
@ldelossa
Copy link
Contributor Author

@julianwiedmann fixed your issues, please check again.

Copy link
Member

@nbusseneau nbusseneau left a comment

Choose a reason for hiding this comment

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

LGTM after we remove the unneeded changes, as said by @sayboras :)

@michi-covalent
Copy link
Contributor

dropped #23099

@michi-covalent
Copy link
Contributor

julianwiedmann and others added 19 commits January 25, 2023 23:57
[ upstream commit 10738e7 ]

Handle any error returned by ipv6_store_daddr().

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit 9ccaaf8 ]

If XDS listener configuration validation failed, addListener() would not
unlock the XDSServer.mutex, leading to the lock being locked forever.
Fix it by using a standard defer() approach.

CC: Jarno Rajahalme <jarno@isovalent.com>
Fixes: 1042b81 ("envoy: Add xDS resource validation")
Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit a65ffa2 ]

We anticipate fixing this soon, but we will likely not backport the fix
to all branches. For older releases, update the documentation to
highlight the details of the issue and how to track ongoing progress /
which releases will address it.

Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit 71eb25e ]

Several of the IPsec troubleshooting tips are outdated or misleading.
For example, they show an output that can differ a lot between IPAM
providers (e.g., GKE vs. EKS) and may lead users to think that this is
cause for concern.

Reported-by: Liz Rice <liz@lizrice.com>
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit 5f0894a ]

To troubleshoot IPsec issues, we can now rely on the cilium encrypt
status CLI command, which should expose the required information without
needing to dig into Linux XFRM state.

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit 4db75c9 ]

In the IPsec guide, we use tcpdump to check that traffic is indeed
encrypted. tcpdump can however buffer the output which can lead to users
thinking that the traffic is not encrypted when the output is actually
just delayed a bit.

We can avoid that with the -l flag which makes the stdout line buffered.

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit 95e7b0c ]

The interface shown in the output of the tcpdump command doesn't match
the interface passed as an argument. This commit fixes it.

Reported-by: Liz Rice <liz@lizrice.com>
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit 0a9fa2f ]

Signed-off-by: fsl <1171313930@qq.com>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit 58449da ]

These messages are reported to be very noisy in some environments, where hubble
can't keep up with the load (e.g. frequent traffic bursts). There are also
equally noisy "hubble events queue is processing messages again" messages. We
consider the queue "back to normal" after only one event is received, but then
it's full again and another line is logged.

This patch mitigates the issue by:
* simply rate limiting the "queue is full" logs
* reducing the level of "N messages were lost" logs from warning to info

Fixes cilium#19202

Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit cea9e65 ]

Signed-off-by: Bill Mulligan <billmulligan516@gmail.com>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit 21efbd2 ]

This is just to use check box so that individual reviewer can just tick
after review. IMO, this is helpful for a few cases:

- Backport with long list of commits, e.g. cilium#23001. Tophat can quickly
  check which one is pending.
- Backport with commits from external contributor. Tophat can easily and
  quickly focus on these commits and review again if required.

Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit 6bb084a ]

ipSecReplaceStateIn was called with the local IP first and the remote IP
second but its prototype indicates that the first argument is the
remoteIP and the second is the localIP (inverted).

This all worked fine because the function would then set the XFRM IN
state source to the `localIP` (actually the remote IP). That doesn't
make any sense given that the XFRM IN state is for decryption so the
source of the packet is the remote IP.

This commit fixes it such that the state source is set to the `remoteIP`
variable as one would expect.

This commit doesn't have any functional changes.

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit 6345321 ]

This is simply for consistency with ipSecReplaceStateIn

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit 974232c ]

Since the page https://github.com/cilium/cilium-olm/archive/ does not exist anymore, for fetching the cilium manifests a git clone is performed instead of the curl to the non-existing site.

Signed-off-by: Zisis Lianas <zl@consol.de>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit ef73a50 ]

The label used to identify the gateway node isn't the same in the guide
and in the example policy, leading to an unapplied egress policy. This
commit fixes it.

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit bc2ed14 ]

Because the helm chart generates cert manager issuers and attaches them
to certificates, we have to remove validations which fail if we don't
specify certManagerIssuerRef.

Fixes: cilium#22784

Signed-off-by: Shunsuke Tokunaga <tkngsnsk313320@gmail.com>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit c5996e9 ]

Make it easier to find users of the TCP flags, and hide the gory details.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit 9f35b12 ]

For DSR on TCP connections, we only want to insert the DSR information into
the SYN packet. This is an obvious optimization, and allows us to avoid any
risk of exceeding the MTU.

The support for this functionality currently only exists in the IPv4 path.
Also add it to the IPv6 path.

Fixes: cilium#21991
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
[ upstream commit 1d5b321 ]

Add some minimal coverage for the IPv6 path.

Also fix up a simple copy&paste error in an error message, and slightly
improve the log output for testCurlFromOutsideWithLocalPort().

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
@michi-covalent
Copy link
Contributor

michi-covalent commented Jan 26, 2023

/test-backport-1.12

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

Click to show.

Test Name

K8sNode Node labels updates are reflected in CiliumNode objects

Failure Output

FAIL: Unable to scale down DNS pods, command 'kubectl get deploy -n kube-system -l k8s-app=kube-dns -o jsonpath='{.items[*].metadata.name}'': Exitcode: -1 

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

@michi-covalent
Copy link
Contributor

michi-covalent commented Jan 26, 2023

/mlh new-flake Cilium-PR-K8s-1.22-kernel-4.9

👍 created #23368

@michi-covalent
Copy link
Contributor

/test-1.22-4.9

@michi-covalent michi-covalent merged commit b06f547 into cilium:v1.12 Jan 26, 2023
@pchaigno
Copy link
Member

pchaigno commented May 8, 2023

#23254 -- ipsec: Fix packet mark for FWD XFRM policy (@pchaigno)

  • "helpers.RunOnAKS" does not exist and is expected in second commit.

That PR was marked as backported even though it was never backported. Just noticed now due to conflicts. I'll relabel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.12 This PR represents a backport for Cilium 1.12.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. kind/community-contribution This was a contribution made by a community member. release-blocker/1.12 This issue will prevent the release of the next version of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet