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.14 Backports 2023-07-26 #27097

Merged
merged 8 commits into from Jul 26, 2023
Merged

v1.14 Backports 2023-07-26 #27097

merged 8 commits into from Jul 26, 2023

Conversation

nbusseneau
Copy link
Member

@nbusseneau nbusseneau commented Jul 26, 2023

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

for pr in 26721 26736 26857 27002 27014 27066; do contrib/backporting/set-labels.py $pr done 1.14; done

or with

make add-labels BRANCH=v1.14 ISSUES=26721,26736,26857,27002,27014,27066

julianwiedmann and others added 8 commits July 26, 2023 16:10
[ upstream commit ebbe02b ]

The nexthop and default IP routes don't require any endpoint-specific
information. So instead of re-inserting the identical routes for every
processed endpoint, do it just once per active EgressGW policy that uses
the local node as gateway.

Also remove the `gwc.localNodeConfiguredAsGateway` condition from the
per-EP helper, we now already check this in the caller.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 9b9e7c2 ]

To delete stale EgressGW-specific IP routes, we currently iterate over all
interfaces on the node. If an interface is not in active use by the
EgressGW, we fetch any old EgressGW routes and delete them.

But under the covers, the netlink.RouteListFiltered() will *fetch* all
routes and only then apply the filter. Considering the potential number of
links on a worker node with many pods, this can add up to quite a bit of
overhead.

Take a different approach instead - fetch all routes just *once*, and then
simply delete all those EgressGW-associated routes that don't point to an
active EgressGW interface.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit fabbc5d ]

route.ReplaceRule() internally fetches the whole set of IP rules in the
system. So calling addEgressIpRule() for every EgressGW-eligible endpoint
causes quite a bit of churn.

Instead fetch the rules just once per EgressGW policy (filtered for the
policy's routing table). Then check for each of the policy's endpoints
whether its IP rule already exists, and insert any rule that is missing.

Note that there is further potential for improvement here - ideally we
would fetch the whole rule set just once, dynamically filter it down to
each policy's routing table, and only match the policy's endpoints against
those specific rules.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit c40d2db ]

The current message looks copy&pasted from the previous error case. Adjust
the text to describe what operation actually failed.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit f979bd8 ]

Without specifying a filter, we're only getting the routes from the default
table. So explicitly get the routes from all tables for the reconciliation
of stale EgressGW IP routes.

Fixes: 9b9e7c2 ("egressgw: improve removal of IP routes")
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 7a9447d ]

Use Ariane to trigger all the required workflows so that they get marked
as skipped when they get skipped.

Fixes #26968

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 944dddf ]

To generate the gRPC API reference, we copy the "api" repository at the
root of the repository to "Documentation/_api". This step is required
everywhere we need to build the docs:

  - Locally, we run it through the "copy-api" target in
    Documentation/Makefile, before generating the HTML.
  - Same thing for the Netlify preview, where "copy-api" is a dependency
    for the "html-netlify" target.
  - However, on ReadTheDocs, where we generate and host the online
    documentation, we do not perform this step; nor do we use the
    Makefile at all.

As a workaround, let's simplify the way we access the API reference.
Instead of copying the docs, just symlink them from the Documentation
directory.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 4772468 ]

- Use `kubeProxyReplacement=true` as `partial` and `strict` settings
  have been deprecated in v1.14.
- Specify the same Helm values for Helm and Cilium-CLI instructions.
- Move the Cilium CLI installation instructions out of the tabs. It's
  required for both Helm and Cilium-CLI instructions.

Fixes: #27060

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@nbusseneau nbusseneau added kind/backports This PR provides functionality previously merged into master. backport/1.14 This PR represents a backport for Cilium 1.14.x of a PR that was merged to main. labels Jul 26, 2023
@nbusseneau nbusseneau marked this pull request as ready for review July 26, 2023 16:13
@nbusseneau nbusseneau requested review from a team as code owners July 26, 2023 16:13
@nbusseneau
Copy link
Member Author

/test-backport-1.14

@nbusseneau
Copy link
Member Author

Note for maintainers:

After merge, the following checks will need to be unmarked as required on v1.14:

  • Runtime Test (agent)
  • Runtime Test (datapath)
  • Runtime Test (privileged)
  • gateway-api-conformance-test (experimental)
  • gateway-api-conformance-test (standard)
  • ingress-conformance-test (With Default Ingress Controller, disabled, dedicated, true)
  • ingress-conformance-test (With Shared LB, disabled, shared, false)
  • ingress-conformance-test (With XDP, native, dedicated, false)
  • ingress-conformance-test (Without XDP, disabled, dedicated, false)
  • integration-test

And the following checks will need to be marked as required instead:

  • Cilium Runtime
  • ConformanceGatewayAPI
  • ConformanceIngress
  • IntegrationTests

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.

i guess we need to update branch protection rules for v1.14 once this PR gets merged? cc @joestringer

@michi-covalent
Copy link
Contributor

michi-covalent commented Jul 26, 2023

ah i missed your comment. you are way ahead of me 🥰

@joestringer joestringer added dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs and removed dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs labels Jul 26, 2023
@joestringer joestringer merged commit 176b3db into v1.14 Jul 26, 2023
189 of 190 checks passed
@joestringer joestringer deleted the pr/v1.14-backport-2023-07-26 branch July 26, 2023 18:43
@joestringer
Copy link
Member

Merged and applied the new branch protection settings. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.14 This PR represents a backport for Cilium 1.14.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants