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.

Sign up for GitHub

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

Jump to bottom

v1.11 Backports 2023-06-22 #26419

Merged
merged 10 commits into from
Jun 29, 2023
Merged

v1.11 Backports 2023-06-22 #26419

merged 10 commits into from
Jun 29, 2023

Conversation

nbusseneau
Copy link
Member

@nbusseneau nbusseneau commented Jun 22, 2023 •
edited by jrajahalme

jrajahalme and others added 10 commits June 22, 2023 18:09
@jrajahalme @nbusseneau
proxy: Do not panic on local error
5667971 
[ upstream commit 525007f ]

[ backporter's notes: conflicts due to proxy_test.go not existing on
  v1.11, these changes were skipped. ]

CreateOrUpdateRedirect called nil revertFunc when any local error was
returned. This was done using the pattern `return 0, err, nil, nil` which
sets the revertFunc return variable as nil, but this was called on a
deferred function to revert any changes on a local error.

Fix this by calling ReverStack.Revert() directly on the deferred
function, and setting the return variable if there was no local error.

This was hit any time a CiliumNetworkPolicy referred to a non-existing
listener.

Add a test case that reproduced the panic and works after the fix.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@jrajahalme @nbusseneau
proxy: Only update redirects with configured proxy ports
86a37fd 
[ upstream commit ca61998 ]

[ backporter's notes: conflicts due to ProxyType not existing on v1.11,
  used parserType as the v1.11 equivalent. ]

Only update an existing redirect if it is configured. This prevents
Cilium agent panic when trying to update redirect with released proxy
port.

This has only been observed to happen with explicit Envoy listener
redirects in CiliumNetworkPolicy when the listener has been removed.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@jrajahalme @nbusseneau
proxy: Increment non-DNS proxy ports on failure
bd67477 
[ upstream commit 894aa4e ]

[ backporter's notes: conflicts due to ProxyType not existing on v1.11,
  used parserType as the v1.11 equivalent. ]

Increment non-DNS proxy ports on failure even if DNS has been configured
with a static port.

Fixes: cilium#20896
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@jrajahalme @nbusseneau
docker: Detect default "desktop-linux" builder
334439d 
[ upstream commit 13f146e ]

New Docker desktop may have a default builder with name "desktop-linux"
that is not buildx capable. Detect that name as well as the old "default"
for the need to create a new buildx builder.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@peterj @nbusseneau
docs: reword incorrect L7 policy description
05de698 
[ upstream commit 68bff35 ]

[ backporter's notes: conflicts due to docs structure change, manually
  applied changes to the corresponding file pre-structure change. ]

Fixing incorrect description of the GET /public policy in
the L7 section.

Signed-off-by: Peter Jausovec <peter.jausovec@solo.io>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@wedaly @nbusseneau
docs: clarify that L3 DNS policies require L7 proxy enabled
fc6de54 
[ upstream commit e0931df ]

[ backporter's notes: conflicts due to docs structure change, manually
  applied changes to the corresponding file pre-structure change. ]

Add a note to the L3 policy documentation clarifying that
L3 DNS policies require the L7 proxy enabled and an L7
policy for DNS traffic so Cilium can intercept DNS responses.

Previously, the documentation linked to other sections describing
the DNS Proxy, but I know at least a few people who were surprised
that a policy under "L3 Examples" would require an L7 proxy.
Hopefully adding a note near the beginning of the section
will make this requirement more obvious.

Signed-off-by: Will Daly <widaly@microsoft.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@brb @nbusseneau
bugtool: Add cilium bpf nodeid list
ea0164a 
[ upstream commit 18f85a0 ]

To help to detect when IPcache is out of sync with locally stored Node
IDs.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@brb @nbusseneau
cli: Print NodeID in hex
d798161 
[ upstream commit e956bb1 ]

[ backporter's notes: conflicts due to string format being different in
  v1.11, applied changes based on v1.11 format. ]

The Node ID is used in SKB mark used by XFRM policies. The latter print
it in hex. So, let's reduce a mental strain by a bit when debugging
IPsec issues.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@jschwinger233 @nbusseneau
ipsec: Split removeStaleXFRMOnce to fix deprioritization issue
884dbb4 
[ upstream commit f4f3656 ]

We expect deprioritizeOldOutPolicy() to be executed for IPv4 and IPv6,
but removeStaleXFRMOnce prevents the second call. If both IPv4 and IPv6
are enabled, v6 xfrm policy won't be deprioritized due to this issue.

This commit fixes it by spliting removeStaleXFRMOnce into
removeStaleIPv4XFRMOnce and removeStaleIPv6XFRMOnce.

Fixes: cilium@688dc9a

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@joestringer @nbusseneau
install: Don't install CNI binaries if cni.install=false
b856330 
[ upstream commit 390b4dc ]

Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@nbusseneau nbusseneau added kind/backports This PR provides functionality previously merged into master. backport/1.11 This PR represents a backport for Cilium 1.11.x of a PR that was merged to main. labels Jun 22, 2023
@nbusseneau
Copy link
Member Author

nbusseneau commented Jun 22, 2023 •
edited

@jrajahalme @brb Conflicts arised during backport of your commits, please check the notes in commits for context and validate that it looks good to you :)

@nbusseneau
Copy link
Member Author

/test-backport-1.11

brb
brb approved these changes Jun 23, 2023
View reviewed changes
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

Thanks!

@nbusseneau
Copy link
Member Author

nbusseneau commented Jun 26, 2023 •
edited

/test-1.16-netnext

EDIT: it hit provisioning issues, re-triggering.

@nbusseneau
Copy link
Member Author

nbusseneau commented Jun 26, 2023 •
edited

/test-1.17-4.9

EDIT: I think it hit #24840, re-running to check.

@nbusseneau nbusseneau removed the request for review from aanm June 27, 2023 13:21
@nbusseneau
Copy link
Member Author

The GKE failures are expected due to 1.11 not supporting K8s 1.24. This is caused by an infrastructure setup conundrum and will be addressed as part of infrastructure rework items this week (not feasible to simply rollback, unfortunately). In the meantime, we should ignore these failures on 1.11.

@nbusseneau nbusseneau marked this pull request as ready for review June 27, 2023 15:44
@nbusseneau nbusseneau requested a review from a team as a code owner June 27, 2023 15:44
jrajahalme
jrajahalme approved these changes Jun 28, 2023
View reviewed changes
Copy link
Member

@jrajahalme jrajahalme 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 to my PRs :)

@nbusseneau nbusseneau added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 28, 2023
@nbusseneau
Copy link
Member Author

nbusseneau commented Jun 28, 2023 •
edited

CI passed (barring note above) and most reviews are in (most importantly those for commits with conflicts), marking ready to merge.

@borkmann borkmann merged commit 7cf6b03 into cilium:v1.11 Jun 29, 2023
51 of 53 checks passed
@gentoo-root gentoo-root mentioned this pull request Jul 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brb brb brb approved these changes

@jschwinger233 jschwinger233 jschwinger233 approved these changes

@jrajahalme jrajahalme jrajahalme approved these changes

@joestringer joestringer Awaiting requested review from joestringer

Assignees
No one assigned
Labels
backport/1.11 This PR represents a backport for Cilium 1.11.x of a PR that was merged to main. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@nbusseneau @brb @jschwinger233 @jrajahalme @borkmann @peterj @wedaly @joestringer