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-07-24 #27030

Merged
merged 5 commits into from
Jul 26, 2023
Merged

v1.11 Backports 2023-07-24 #27030

merged 5 commits into from
Jul 26, 2023

Conversation

nbusseneau
Copy link
Member

@nbusseneau nbusseneau commented Jul 24, 2023 •
edited by pchaigno

pchaigno and others added 5 commits July 24, 2023 17:30
@pchaigno @nbusseneau
test: Avoid downloading conntrack package at runtime
223c80b 
[ upstream commit a58cb6a ]

[ Backporter's notes: the changes had to be manually backported to the
  appropriate files for v1.11, as they were renamed in
  ffd7e57 since then.

  (main > v1.11)
  test/k8s/datapath_configuration.go > test/k8sT/DatapathConfiguration.go
  test/k8s/manifests/log-gatherer.yaml > test/k8sT/manifests/log-gatherer.yaml
]

The 'Skip conntrack for pod traffic' test currently downloads the
conntrack package at runtime to be able to flush and list Linux's
conntrack entries. This sometimes fail because of connectivity issues to
the package repositories.

Instead, we've now included the conntrack package in the log-gatherer
image. We can use those pods to run conntrack commands instead of using
the Cilium agent pods.

Fixes: 496ce42 ("iptables: add support for NOTRACK rules for pod-to-pod traffic")
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@pchaigno @nbusseneau
test: Print error messages in case of failure
3b10d77 
[ upstream commit 67a3ab3 ]

[ Backporter's notes: the changes had to be manually backported to the
  appropriate files for v1.11, as they were renamed in
  ffd7e57 since then.

  (main > v1.11)
  test/k8s/datapath_configuration.go > test/k8sT/DatapathConfiguration.go
]

If we check res.WasSuccessful() instead of res, then ginkgo won't print
the error message in case the command wasn't successful.

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@pchaigno @nbusseneau
docs/ipsec: Extend troubleshooting section
4024542 
[ upstream commit 3ba76e5 ]

[ Backporter's notes: the changes had to be manually backported to the
  appropriate files for v1.11, as the docs were restructured in
  fbc53d0 and
  69d07f7 since then.
]

Recent bugs with IPsec have highlighted a need to document several
caveats of IPsec operations. This commit documents those caveats as well
as common XFRM errors.

Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@michi-covalent @nbusseneau
docs: Specify Helm chart version in "cilium install" commands
a92e48e 
[ upstream commit 12fc68a ]

[ Backporter's notes: the changes had to be manually backported to the
  appropriate files for v1.11, as the docs were restructured in
  fbc53d0 and
  69d07f7 since then.
]

- For the main branch latest docs, clone the Cilium GitHub repo and use
 "--chart-directory ./install/kubernetes/cilium" flag.
- For stable branches, set "--version" flag to the version in the
  top-level VERSION file.

Fixes: #26931

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@pchaigno @nbusseneau
docs/ipsec: Document RSS limitation
bd43214 
[ upstream commit c9983ef ]

[ Backporter's notes: the changes had to be manually backported to the
  appropriate files for v1.11, as the docs were restructured in
  fbc53d0 and
  69d07f7 since then.
]

All IPsec traffic between two nodes is always send on a single IPsec
flow (defined by outer source and destination IP addresses). As a
consequence, RSS on such traffic is ineffective and throughput will be
limited to the decryption performance of a single core.

Reported-by: Ryan Drew <ryan.drew@isovalent.com>
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
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 Jul 24, 2023
michi-covalent
michi-covalent approved these changes Jul 24, 2023
View reviewed changes
Copy link
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

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

🥰

@nbusseneau
Copy link
Member Author

@michi-covalent Ah! I knew it! Caught red-handed this time, a blind approve without even reading the commit notes. I could have slipped in literally anything. ANYTHING.
image

@michi-covalent @pchaigno Please see backporter's notes for each commit, conflicts arised due to file renames.

@pchaigno
Copy link
Member

No worries for a too-quick review from me: I don't review draft PRs.

@nbusseneau nbusseneau marked this pull request as ready for review July 24, 2023 16:06
@nbusseneau nbusseneau requested a review from a team as a code owner July 24, 2023 16:06
@nbusseneau
Copy link
Member Author

I always wait for at least image builds to pass before removing draft status on backport PRs 😁

@nbusseneau
Copy link
Member Author

nbusseneau commented Jul 24, 2023 •
edited by maintainer-s-little-helper bot

/test-backport-1.11

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

Click to show.

Test Name

K8sDatapathConfig Host firewall With VXLAN

Failure Output

FAIL: Timed out after 240.000s.

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

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-5.4 so I can create one.

Then please upload the Jenkins artifacts to that issue.

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

Click to show.

Test Name

K8sServicesTest Checks E/W loadbalancing (ClusterIP, NodePort from inside cluster, etc) with L4 policy Tests NodePort with L4 Policy

Failure Output

FAIL: Request from k8s1 to service tftp://[fd04::12]:30627/hello failed

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.20-kernel-4.9/105/

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.

Then please upload the Jenkins artifacts to that issue.

pchaigno
pchaigno approved these changes Jul 24, 2023
View reviewed changes
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 changes look good. Thanks Nicolas!

And thanks for the backporting notes!

@nbusseneau
Copy link
Member Author

The backport only targets docs and tests themselves, and the unquarantined test passed, the 2 failures above cannot be introduced by this PR. Marking ready-to-merge.

@nbusseneau nbusseneau added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 25, 2023
@nbusseneau nbusseneau mentioned this pull request Jul 25, 2023
Closed
@youngnick youngnick merged commit af3facf into v1.11 Jul 26, 2023
50 of 52 checks passed
@youngnick youngnick deleted the pr/v1.11-backport-2023-07-24 branch July 26, 2023 01:02
@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

@pchaigno pchaigno pchaigno approved these changes

@youngnick youngnick youngnick approved these changes

@michi-covalent michi-covalent michi-covalent approved these changes

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
4 participants
@nbusseneau @pchaigno @youngnick @michi-covalent