v1.12 Backports 2023-09-26 #28295
Merged
v1.12 Backports 2023-09-26 #28295
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
giorio94
added
kind/backports
giorio94
requested review from
julianwiedmann,
margamanterola,
nbusseneau,
jschwinger233 and
qmonnet
|
/test-backport-1.12 |
qmonnet
approved these changes
Sep 26, 2023
jschwinger233
approved these changes
Sep 27, 2023
|
/test-1.16-4.9 |
|
/test-1.22-4.9 |
|
/test-1.24-net-next |
|
/test-runtime |
|
/test-1.23-4.19 |
|
/test-upstream-k8s |
|
All previous failures occurred because Jenkins had been restarted while the tests were in progress. |
|
The build commits check failed due to timeout. Manually run:
which both completed successfully locally. |
|
For my docs commit, in the 1.13 backport, Donia had actually introduced the whole paragraph: Can we do the same here? Thanks! |
Do we need to bump the timeout then? |
giorio94
force-pushed
the
pr/v1.12-backport-2023-09-26
branch
from
664891b
to
913188a
Compare
Sure, no problem. I've updated the commit to additionally include that paragraph. |
That's what I wonder as well. The most demanding part seems to be building all the datapath configurations, which is skipped if no BPF changes are part of the PR. That's probably the reason why we don't hit this issue consistently in backport PRs (and then the number of commits also plays an important role). |
|
/test-backport-1.12 Job 'Cilium-PR-K8s-1.22-kernel-4.9' failed: Click to show.Test NameFailure OutputJenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.22-kernel-4.9/140/ If it is a flake and a GitHub issue doesn't already exist to track it, comment Then please upload the Jenkins artifacts to that issue. Job 'Cilium-PR-K8s-1.17-kernel-4.9' hit: #23318 (94.58% similarity) |
margamanterola
approved these changes
Sep 27, 2023
[ upstream commit 48023b9 ] This commit makes conn-disrupt-test a github action, so upgrade test and IPsec key rotation test don't have to copy and paste everywhere. The idea is to allow caller workflow to specify the commands to execute, then this action will follow the steps: 1. Run "cilium-cli connectivity test --conn-disrupt-test-setup"; 2. Run whatever caller workflow passes: could be upgrade operation or IPsec key rotation; 3. Run "cilium-cli connectivity test --include-conn-disrupt-test"; Signed-off-by: Zhichuan Liang <gray.liang@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: 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: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 071c2af ] [ backporter's notes: skipped the hunk related to bpf_tests, as not present in the v1.12 branch. ] 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: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 02c17d5 ] [ backporter's notes: adapted the hunk to the different variable name. ] 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") Signed-off-by: Julian Wiedmann <jwi@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: 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: 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: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit cf3dc21 ] [ backporter's notes: skipped a few hunks modifying targets not present in the v1.12 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: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 4d699d8 ] [ backporter's notes: skipped a few hunks modifying targets not present in the v1.12 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> Signed-off-by: Marco Iorio <marco.iorio@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: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 2b4f6df ] [ backporter's notes: reapplied the main hunk to the corresponding file, due to the different docs structure. Additionally backported the paragraph introduced in 9a01260, including the modification from the upstream commit. ] 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: 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: 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: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 2f7fbfe ] Signed-off-by: Quentin Monnet <quentin@isovalent.com> Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 4f1f08e ] The plague of copy/pasting strikes again... Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com> Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit c6fbeb1 ] [ backporter's notes: fixed trivial conflict accepting the combination of the changes. ] This target should completely ignore the existing contents of `cilium/values.yaml` since we always generate from the template files. Co-authored-by: Donia Chaiehloudj <donia.cld@isovalent.com> Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com> Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
giorio94
force-pushed
the
pr/v1.12-backport-2023-09-26
branch
from
913188a
to
b404516
Compare
|
/test-backport-1.12 Job 'Cilium-PR-K8s-1.18-kernel-4.9' has 1 failure but they might be new flake since it also hit 1 known flake: #27118 (97.27% similarity) |
julianwiedmann
approved these changes
Oct 2, 2023
julianwiedmann
removed
the
dont-merge/discussion
|
/test-1.18-4.9 Hit #27118 |
2 tasks
|
Manually run again the build commits steps locally, as timed out in the CI. |
giorio94
added
the
ready-to-merge
maintainer-s-little-helper
bot
removed
the
ready-to-merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ℹ️ Skipped the hunk modifying the
echo-dpl.yamlfile, as not present in the v1.12 branch.changes. Please review carefully.
ℹ️ Skipped the hunk related to bpf_tests, as not present in the v1.12 branch.
ℹ️ Adapted the hunk to the different variable name.
ℹ️ Skipped a few hunks modifying targets not present in the v1.12 branch.
ℹ️ Reapplied the main hunk to the corresponding file, due to the different docs structure. Additionally backported the paragraph introduced in 9a01260, including the modification from the upstream commit.
❌ Skipped the backport of c2bf891 as the modified file is not present in the v1.12 branch.
ℹ️ Picked Documentation/requirements.txt verbatim from the main branch to keep it consistent across versions, as we nonetheless use the same image in CI.
cilium/values.yamltarget to.PHONY#28225 (@nbusseneau)Once this PR is merged, you can update the PR labels via:
or with