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-08-22 #27629

Merged
merged 22 commits into from Aug 25, 2023
Merged

v1.14 Backports 2023-08-22 #27629

merged 22 commits into from Aug 25, 2023

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Aug 22, 2023

PRs skipped due to conflicts:

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

for pr in 26641 26964 27222 27342 27306 27346 27230 27365 27397 27353 27453 27495 27409 27309 27457 27251 26742 27537 27511 27496 27475 27561; do contrib/backporting/set-labels.py $pr done 1.14; done

or with

make add-labels BRANCH=v1.14 ISSUES=26641,26964,27222,27342,27306,27346,27230,27365,27397,27353,27453,27495,27409,27309,27457,27251,26742,27537,27511,27496,27475,27561

@tklauser tklauser 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 Aug 22, 2023
@tklauser tklauser marked this pull request as ready for review August 22, 2023 13:35
@tklauser tklauser requested review from a team as code owners August 22, 2023 13:35
@tklauser
Copy link
Member Author

/test-backport-1.14

Copy link
Member

@giorio94 giorio94 left a comment

Choose a reason for hiding this comment

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

My commits look good. Thanks!

Copy link
Member

@mhofstetter mhofstetter left a comment

Choose a reason for hiding this comment

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

LGTM - thank you!

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.

Looks good for my commit, thanks 👍

Copy link
Contributor

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

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

Lookin' good, thanks!

@doniacld
Copy link
Contributor

doniacld commented Aug 24, 2023

@tklauser tklauser removed the request for review from doniacld August 24, 2023 09:11
@tklauser
Copy link
Member Author

The manual backport was merged after I've created this PR, thus it wasn't picked up. Also the labels on the original PR caused the PR to be picked up by backporter tooling, see #27660 (comment). I've now dropped the commits and rebased this branch on latest v1.14.

@tklauser tklauser force-pushed the pr/v1.14-backport-2023-08-22 branch from d26bf30 to d5895e6 Compare August 24, 2023 09:15
@tklauser
Copy link
Member Author

/test-backport-1.14

@tklauser
Copy link
Member Author

tklauser commented Aug 25, 2023

/test-backport-1.14

Picked up a merge conflict between b4eef35 and backport of db0e2c2 (/cc @ti-mo). Rebased and resolved conflict by folding in logical instruction count calculation and logging into the loop (akin to version on main).

@tklauser tklauser mentioned this pull request Aug 25, 2023
3 tasks
ti-mo and others added 22 commits August 25, 2023 21:04
[ upstream commit db0e2c2 ]

As it says on the tin, echo instruction count for all programs, both if the
Collection fails to load, as well as at the end of the last line of the
verifier log on a successful load.

Example output of successful load:
```
=== RUN   TestVerifier/bpf_host_1
    verifier_test.go:216: tail_srv6_reply: processed 277 insns (limit 1000000) ... mark_read 8 (201 unverified insns)
...
```

Example output of unsuccessful load:

```
=== RUN   TestVerifier/bpf_lxc_1
    verifier_test.go:201: Full verifier log at bpf_lxc_1_verifier.log
    verifier_test.go:203: BPF unverified instruction count per program:
    verifier_test.go:205: tail_handle_ipv4: 1048 insns
```

Also output the original `err` containing the name of the prog that caused
the verifier error, which was accidentally removed by 1833964
("test/verifier: don't dump full log into test output"). It's now
documented why it's included.

Signed-off-by: Timo Beckers <timo@isovalent.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit 8a3148a ]

Signed-off-by: vakr <vakr@microsoft.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit e2ae22e ]

This commit brings various minor improvements to the intro and
prerequisites for the Ingress Controller documentation:

- Add a link to Kubernetes' documentation for Ingress, given that we
  never define or explain what Kubernetes Ingress is
- Instruct users to "enable" kube-proxy replacement, given that the
  values "strict" or "partial" are deprecated.
- Mention NodePort as an alternative to kube-proxy replacement, givent
  that this is the only feature from KPR which is actually required for
  Ingress.
- Use the Helm value instead of the agent flag for the L7 proxy
  configuration, simply to remain consistent with the item above (KPR).
- Remove the requirement on Kubernetes 1.19+, given that 1.19 is now the
  minimum version supported by Cilium.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit cd80d2c ]

Signed-off-by: Richard Lavoie <lavoie.richard@gmail.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit b2c5deb ]

We link to Cilium Slack at multiple locations in Cilium's documentation.
The way we do it is far from consistent:

- We usually provide a direct link to the Slack instance
- ... Which, over time, resulted in at least 13 occurrences of the same
  URL https://cilium.herokuapp.com
- Sometimes, we create an indirect link to a "_slack" target, pointing
  to the gettinghelp.rst page
- Some other times, we use other indirect links pointing to the
  glossary.
- At some locations, we also mention that users can get invites to the
  Slack instance. As far as I know, these invites are no longer
  necessary.

Let's try to clean this up.

- Create a target link in the RST epilogue in conf.py, so we can
  reference it from any location.
- Call this link everywhere we need, using `Cilium Slack`_ in a
  consistent fashion.
- Format most channel names as inline literals.
- Improve formatting (not the content) of some paragraphs that already
  get updates for Slack references: rewrap, remove trailing spaces, etc.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit 4461616 ]

cilium/cilium-cli#1854 is released.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit 4440b3e ]

As of August 08 2023, K8s version 1.22 is no longer available for GKE
clusters.

Therefore, this commit removes the version from the CI GKE matrix
configuration.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit e1f8389 ]

Let's add an explicit dependency to the wait-for-images job, so that
subsequent jobs are not started until the images are actually available.
This also prevents starting the full matrix jobs if the images don't
get ready in time. Finally, let's adapt status reporting, so that
we properly report a failure in that case.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit 679a162 ]

Given that tunnel map entries are currently inserted without
specifying the cluster ID, we should do the same during the removal
phase. Otherwise, we fail to remove the entries if the node is
associated with a non-zero cluster ID. Let's additionally update
the test so that it would catch future regressions.

Fixes: cda8767 ("bpf: Make tunnel map APIs aware of ClusterID")
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit e25bd05 ]

PR #24649 removed the deprecated support for adding fallback
HTTP log tags if the access log entry wasn't of type HTTP,
Kafka or GenericL7.

This removal leads to panics when the l7Tags function (`nil`)
is called while trying to enrich the log entry.

Therefore, this commit changes the behaviour by defaulting to an
noop l7Tags function if no specific L7 information are present.

Fixes: #27442

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit b7947bc ]

Signed-off-by: ishuar <ishansharma887@gmail.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit 0cd6932 ]

Signed-off-by: chentanjun <tanjunchen20@gmail.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit 5e1dff0 ]

This is to make sure that we are having a correct attached routes in gateway
status. The translation logic is already implemented such filters, but it
was overlooked in Gateway status.

For the below spec, the attached routes for both listeners was showing as 1,
the expected behavior should be 0 for http-non-attached listener, and 1 for
http listener. Similar scenario should be applied for port number.

```yaml

Signed-off-by: Tobias Klauser <tobias@cilium.io>
---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
  name: sample
spec:
  gatewayClassName: cilium
  listeners:
    - name: http-unattached
      port: 8080
      protocol: HTTP
      allowedRoutes:
        kinds:
          - kind: HTTPRoute
        namespaces:
          from: All
    - name: http
      port: 80
      protocol: HTTP
      allowedRoutes:
        kinds:
          - kind: HTTPRoute
        namespaces:
          from: All
---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
  name: http-route-4
spec:
  parentRefs:
    - kind: Gateway
      name: sample
      sectionName: http
  rules:
    - backendRefs:
        - name: infra-backend-v1
          port: 8080
```

Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit f70b7ed ]

Resource name in k8s cannot be longer than 63 character, so this commit
is to make sure that we don't have longer name due to cilium gateway prefix
(e.g. cilium-gateway-) for related resources.

```
2023-08-07T03:23:44.780547229Z level=error msg="Unable to create Service" controller=gateway error="Service \"cilium-gateway-gateway-with-two-listeners-and-one-attached-route\" is invalid: metadata.name: Invalid value: \"cilium-gateway-gateway-with-two-listeners-and-one-attached-route\": must be no more than 63 characters" resource=gateway-conformance-infra/gateway-with-two-listeners-and-one-attached-route subsys=gateway-controller
```

Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit 4cb2a4b ]

AKS supported k8s versions changed.
This commit updates tested k8s versions for AKS

Signed-off-by: Birol Bilgin <birol@cilium.io>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
…ypes

[ upstream commit ec05fd4 ]

Fixes #24584

Signed-off-by: Jan Jansen <jan.jansen@gdata.de>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit a331acd ]

Cached maps come with a controller that retries Update/Delete
operations in case an error occurred while synchronizing the given
entry with the kernel. Currently, it relies on the `outstandingErrors`
counter to determine whether reconciliation is necessary, as well as
if any entry still needs to be processed in a subsequent operation.

Yet, it is possible that this counter gets out of sync, in particular
in case the given error is resolved automatically when a subsequent
operation acting on the same key succeed. If this happens, the
reconciliation function will never complete successfully, as there
will continue to be an error that cannot be resolved (as it no
longer exists). The given controller will this continue failing
forever until the agent gets restarted.

Let's fix this inferring the number of outstanding errors from the
cache itself. The `outstandingErrors` variable is preserved (although
converted to a boolean) to avoid iterating over the full cache in case
it is known that no error occurred. Still, if this flag gets out of
sync, the only consequence will be that the cache is iterated once
to determine that there's actually no failure, ensuring that the
reconciliation logic still converges properly.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit 270be30 ]

We've been using a custom spelling filter to make sure that "WireGuard"
is spelled correctly, with the proper case. As it turns out, passing
this filter to the Sphinx configuration, in the conf.py file, makes
Sphinx re-read all sources and re-write all output files, as can be
observed when running sphinx-build multiple times, without suppressing
the output:

    $ sphinx-build -M html . _build
    [...]
    updating environment: [config changed ('spelling_filters')] 472 added, 30 changed, 0 removed
    [...]

This is because in conf.py, we pass the filter directly as a function.
When Sphinx writes its environment.pickle file to keep track of the
configuration in use, it discards values that cannot be serialised,
including the filter, "<class 'cilium_spellfilters.WireGuardFilter'>",
of instance "type" [0]. So the value for the configuration option
"spelling_filters" is not saved, and as Sphinx believes that the
configuration has changed, it reads and rebuilds everything.

In fact, the issue has been reported before, and solved in the
spellchecker [1]. We need to set the configuration with a string instead
of the direct function object, and the extension is able to load it from
there. Let's adjust accordingly, to save cycles when building the docs
more than once.

[0] https://github.com/sphinx-doc/sphinx/blob/v7.1.2/sphinx/config.py#L323
[1] sphinx-contrib/spelling#40

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit 164332f ]

v0.1.9 is the first release of certgen 1.5 years. As a patch release, it
doesn't contain any major changes but only updates to its dependencies
and the Go version that it's built with.

See release notes for details:
https://github.com/cilium/certgen/releases/tag/v0.1.9

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit 957e953 ]

The comment is 5 years old and no longer valid, removing and updating
the CRD.

Signed-off-by: Alex Waring <ajmwaring@gmail.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit 8b9eff9 ]

Signed-off-by: ishuar <ishansharma887@gmail.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit 603235d ]

While loading, the package 'pkg/cgroups/manager' sets the global variable 'cgroupRoot' from the 'pkg/cgroups' package.
This occurs before the configuration is fully loaded and the correct path is set in the 'pkg/cgroups' package.

This variable is later used by the 'validateCgroupPath' function.
As a result, the 'validateCgroupPath' function always works with the default value instead of the user-defined one.

Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@tklauser tklauser force-pushed the pr/v1.14-backport-2023-08-22 branch from b625c0c to 60840a0 Compare August 25, 2023 19:05
@tklauser
Copy link
Member Author

tklauser commented Aug 25, 2023

/test-backport-1.14

Corrected a stray log line in the conflict resolution for #27629 (comment)

Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

I briefly reviewed the remaining backported commits, LGTM.

@joestringer joestringer merged commit 45d9cf6 into v1.14 Aug 25, 2023
207 of 224 checks passed
@joestringer joestringer deleted the pr/v1.14-backport-2023-08-22 branch August 25, 2023 21:17
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