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-09-21 #28251

Merged
merged 18 commits into from Sep 29, 2023
Merged

v1.13 Backports 2023-09-21 #28251

merged 18 commits into from Sep 29, 2023

Conversation

@doniacld doniacld 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 Sep 22, 2023
Copy link
Member

@margamanterola margamanterola left a comment

Choose a reason for hiding this comment

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

Approved for my change

Copy link
Member

@jschwinger233 jschwinger233 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 for my PR.

@doniacld doniacld marked this pull request as ready for review September 25, 2023 09:54
@doniacld doniacld requested review from a team as code owners September 25, 2023 09:54
@doniacld
Copy link
Contributor Author

doniacld commented Sep 25, 2023

I need to fix C code, I did not resolve the conflict correctly. https://github.com/cilium/cilium/actions/runs/6271820562/job/17032148237?pr=28251

Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Thanks Donia! Please find some comments on the docs backports - Let me know if this makes too many conflicts to handle

Documentation/Makefile Outdated Show resolved Hide resolved
Documentation/Makefile Outdated Show resolved Hide resolved
Documentation/Makefile Outdated Show resolved Hide resolved
Documentation/Makefile Outdated Show resolved Hide resolved
.github/workflows/lint-bpf-checks.yaml Outdated Show resolved Hide resolved
.github/workflows/lint-bpf-checks.yaml Outdated Show resolved Hide resolved
Documentation/requirements.txt Outdated Show resolved Hide resolved
@doniacld doniacld force-pushed the pr/v1.13-backport-2023-09-21 branch 4 times, most recently from fc332d4 to 29422c7 Compare September 25, 2023 16:43
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.

Thanks and looks good for my commit ✔️

@giorio94
Copy link
Member

The coccicheck failures are being addressed in #28279

@giorio94
Copy link
Member

Additionally backported 4d699d8, which was previously missing. /cc @qmonnet

@qmonnet
Copy link
Member

qmonnet commented Sep 27, 2023

Additionally backported 4d699d8, which was previously missing. /cc @qmonnet

Not sure what happened, I'm pretty sure this commit was present when I did my first review for this PR. Anyway. Related backport looks good 👍.

oxxenix and others added 18 commits September 27, 2023 11:53
[ upstream commit e10f925 ]

fixes #22937

Signed-off-by: Oksana Baranova <oksana.baranova@intel.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 85481c4 ]

When executing `cilium encrypt status`, cilium-agent lists xfrm states
and counts number of different AEAD keys. However, cilium-agent panics
if there is any xfrm state using non-AEAD algorithm. These unexpected
xfrm states could be installed by other applications.

To reproduce the panic, we can manually install one by running command:

```
ip x s a src 1.1.1.1 dst 1.1.1.2 proto esp spi 0x3 reqid 1 mode tunnel enc aes 0xf0e1d2c3b4a5f60708090a0b0c0d0e0f
```

Then `cilium encrypt status` crashes.

This patch fixes it.

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 3ea5e8c ]

[ backporter's notes: resolved minor conflict due to the different signature
  of the calculateInterval() method. ]

When ToFQDN policies are in use (CIDR) identities are created for each IP
in a DNS response that matches a ToFQDN policy. These identities are garbage
collected when

  1) all endpoints with ToFQDN policies are removed
  2) the maximum number of IPs per host is reached (tofqdn-max-ips-per-host)
  3) when the identity has been unused and not refreshed by conntrack map GC

Problems arise when he conntrack GC interval becomes very long. If it's backed
by a LRU BPF map, the maximum is set to 12 hours (defaults.ConntrackGCMaxLRUInterval),
meaning it will take 12 hours before unused identities are marked dead and collected
(unless tofqdn-max-ips-per-host limit is reached for the FQDN entry).

To allow user to have more control over this add the conntrackGCMaxInterval option
that will allow limiting the maximum interval to something less than the 12 hours.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit ce51167 ]

Previously, the CiliumExecContext() function would retry the cilium command up to 5 times (limitTimes) without verifying the success of previous executions. This pull request introduces a logic enhancement that validates the return code of each executed command and exits upon success.

Additionally, this change optimizes test execution time by reducing unnecessary retries. With a 200ms delay between retries, the overall test execution time is expected to improve by at least 200ms multiplied by the number of cilium commands and the number of cilium pods involved.

Local Ginkgo tests, specifically those focused on 'K8sDatapathBGPTests' 'K8sDatapathCustomCalls' and 'K8sDatapathLRPTests' have shown significant improvements:

Before:
`Ran 2 of 106 Specs in 233.348 seconds`

After:
`Ran 2 of 106 Specs in 168.563 seconds`

Signed-off-by: Boris Petrovic <carnerito.b@gmail.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 071c2af ]

In the past, we have merged automated updates to the coccicheck image
version (for example, [0]) without realising they may break the
workflow, because the workflow wouldn't run on the PR, which contained
no BPF-related changes.

Let's make the workflow run when its definition file is updated, to make
sure we can catch similar issues in the future.

[0] #27947

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 02c17d5 ]

CB_SRC_LABEL is cleared just a few lines above. So to get the source's
actual security identity, we need to use our local variable.

Fixes: e2829a0 ("bpf: lxc: support Pod->Service->Pod hairpinning with endpoint routes")

Fixed conflict by replacing `src_sec_identity` by `src_identity`
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit d987c37 ]

The Makefile for the documentation makes sure that the docs-builder
image is always present and up-to-date before building the docs. To do
so, it simply rebuilds the image, every time.

This is efficient to make sure the image is up-to-date, but not that
much in terms of build duration. Let's make it possible to skip that
step if the user is confident their image is present and up-to-date.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit e58d673 ]

Allow users to set the INCREMENTAL environment variable to avoid passing
the "-E" option to Sphinx when skipping the linters. This way, we can
have Sphinx do incremental builds instead of re-reading and re-writing
all files. This can be used for quick iterations on a local setup.

There is no need to make "-E" conditional for the spell checker run,
given that the spell checker seems to always re-read all files anyway.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 38d5669 ]

The documentation build targets, including html, have a dependency on
update-helm-values. There is no clear reason for that: this seems to be
for historical reasons.

- Initially, the dependency was on $(HELM_VALUES), because the file with
  Helm values had not been committed to the Git repo at the time, so we
  needed to ensure that it was generated first.

- When reorganising checks for Helm values in 6167f8a ("docs: check
  updates for the Helm reference"), we replaced it with
  "update-helm-values" which looked cleaner; but we don't need to run
  the _check_ at this stage, only to make sure the _file_ itself is
  available.

Now that the file is part of the repository, there's no need to check
for its existence, and we can simply remove the target dependency (to
align it with other files that receive automatic updates, such as
codeowners.rst or crdlist.rst).

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit cf3dc21 ]

[ backporter's notes: skipped a few hunks modifying targets not present
  in the v1.13 branch. ]

Let's make sure all Makefile targets are properly documented, and nicely
displayed when running "make help".

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 4d699d8 ]

[ backporter's notes: skipped a few hunks modifying targets not present
  in the v1.13 branch. ]

Move around targets inside of Documentation/Makefile to have them
displayed in a more sensible order on "make help".

    $ make help

    Usage:
      make <target>

    Targets (default: "html")

    Development Images
      base-image                    Build the docs-base image for updating the requirements.txt file.
      builder-image                 Build the docs-builder image for rendering and checking the documentation.

    Auto-generated Contents Updates and Validation
      api-flaggen                   Update the table of API flags restrictions.
      update-cmdref                 Update the command reference documents (agent, bugtool, operators, etc.).
      update-codeowners             Update the description of the code owner teams.
      update-crdlist                Update the list of CRDs.
      update-helm-values            Update the Helm reference documentation.
      check                         Validate command and Helm references, policy examples, and others.

    Build
      html                          Check documentation and render it under the specified format.
      epub                          Check documentation and render it under the specified format.
      latex                         Check documentation and render it under the specified format.
      live-preview                  Build and serve the documentation locally.

    Development
      update-requirements           Regenerate the requirements.txt file from requirements-min/requirements.txt.
      clean                         Clean up all artefacts from documentation.
      help                          Display help for the Makefile.

Also add target update-cmdref to .PHONY.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit 5b77a54 ]

Since commit 944dddf ("docs: Symlink (instead of copying) the API
reference"), we symlink the _api/ directory (from ../api) instead of
copying it. So there's nothing to clean on that side on "make clean",
otherwise we lose the link.

Fixes: 944dddf ("docs: Symlink (instead of copying) the API reference")
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 2b4f6df ]

Rather than having this information hidden in a parenthesis in the
troubleshooting section, let's have a full paragraph in the key rotation
section that explains what needs to be done when using Cluster Mesh.

Signed-off-by: Marga Manterola <marga@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit c36548c ]

Function is_html5_writer_available() is deprecated, its invocation is
unnecessary (the writer _is_ available in the versions of Sphinx we're
using), and it has been removed in more recent versions of Sphinx. Let's
get rid of it.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 94da061 ]

[ backporter's notes: picked Documentation/requirements.txt verbatim
  from the main branch to keep it consistent across versions, as we
  nonetheless use the same image in CI. ]

Now that we have rebased our theme onto the upstream version, we can use
newer versions of docutils, which allows us to update Sphinx and a bunch
of dependencies. We're still limited to docutils 0.18 because of
sphinx-tabs, though.

One immediate improvement is that we get working links for the value
types in the gRPC reference.

There are newer versions of Sphinx, websupport, or some second-level
dependencies available (currently 7.2.6 and 1.2.6, respectively), but
Netlify doesn't know about them yet and fails to build the previews.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 30c4df9 ]

In the past, we disabled Sphinx's output in check-build.sh. This was
because of the way we handle warnings: in order to have them printed at
the end of the logs, we print them to a file, we filter them, and print
the outstanding ones at the end of the output. Keeping the standard
output meant that warnings or errors wouldn't appear in the middle of
this output, and warnings that we filtered out wouldn't appear at all,
which could be confusing.

With the recent Sphinx update, we are able to filter the warnings from
MyST more efficiently, directly from conf.py, and to get rid of the
filters.

Let's take advantage of this to re-enable the output from Sphinx. While
it's not necessary to build the docs, it's occasionally helpful to debug
the setup when things don't go as expected.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 2f7fbfe ]

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 58366eb ]

Originally, this option is added for old minikube version, which
requires privilege permission to mount ebpf dir, however, we have
migrated to kind + metallb in Ingress and Gateway related workflows.

Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Donia <donia.cld@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94
Copy link
Member

Rebased onto the tip of v1.13 to pick the coccicheck fixes.

@giorio94
Copy link
Member

/test-backport-1.13

@giorio94
Copy link
Member

giorio94 commented Sep 27, 2023

/test-1.26-net-next

Something went wrong during the VM initialization.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Sep 29, 2023
@aanm aanm merged commit 79d080e into v1.13 Sep 29, 2023
120 checks passed
@aanm aanm deleted the pr/v1.13-backport-2023-09-21 branch September 29, 2023 12:21
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. 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