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.15 Backports 2024-04-10 #31890

Merged
merged 14 commits into from Apr 11, 2024
Merged

v1.15 Backports 2024-04-10 #31890

merged 14 commits into from Apr 11, 2024

Conversation

@nbusseneau nbusseneau added kind/backports This PR provides functionality previously merged into master. backport/1.15 This PR represents a backport for Cilium 1.15.x of a PR that was merged to main. labels Apr 10, 2024
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.

LGTM for my commits, thanks a lot 👍

@qmonnet
Copy link
Member

qmonnet commented Apr 10, 2024

I suppose you resolved the conflicts by regenerating the images? Sorry for the additional work, I should maybe have marked this one as backport/author 😬

By the way, this commit misses your own sign-off tag, can you fix it please?

@nbusseneau
Copy link
Member Author

@qmonnet Yeah nah I actually intended to drop this one when I realized it was touching the images, but I messed up... will fix and remove from the PR, this is indeed one of the cases where manual backport is needed.

@nbusseneau nbusseneau force-pushed the pr/v1.15-backport-2024-04-10-05-56 branch from 3f94073 to 290a52d Compare April 10, 2024 16:54
@qmonnet qmonnet removed their request for review April 10, 2024 16:55
@nbusseneau
Copy link
Member Author

By the way, this commit misses your own sign-off tag, can you fix it please?

Huh, so I'm just noticing now that my automation for adding Sign-off is not working for commits I fixed during the backporting process. This is new (the process is new, so...), thanks for spotting it!

[ upstream commit 3d95fbc ]

Signed-off-by: Chance Zibolski <chance.zibolski@gmail.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@nbusseneau nbusseneau force-pushed the pr/v1.15-backport-2024-04-10-05-56 branch from 290a52d to 608ba9e Compare April 10, 2024 17:18
vipul-21 and others added 9 commits April 10, 2024 19:24
[ upstream commit 76867e2 ]

Signed-off-by: Vipul Singh <vipul21sept@gmail.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit e22c108 ]

This commit updates tested azure k8s versions according to supported versions

Signed-off-by: Birol Bilgin <birol@cilium.io>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 4d05e9f ]

This commit is to make sure that the processed item in pod deletion
queue is removed by explicitly call Done() function as per suggestion
in godoc[^1].

The impact of not having this change will be increasing of memory in
cilium agent when the hubble metrics are enabled. This might take days
(if not weeks) to observe in a normal Cilium deployment due to low number
of Pod deletion events (i.e. in high churn environment, the memory will
be increasing in a faster pace).

Testing is done before and after the changes as per below.

Sample workload to simulate high number of pod deletion events

```yaml
apiVersion: batch/v1
kind: Job
metadata:
  name: pod-churn-job
spec:
  completions: 50000000
  parallelism: 100
  template:
    metadata:
      labels:
        app: pod-churn-job
    spec:
      containers:
      - name: churn-app
        image: sandeshkv92/highpodchurn:linux_amd64
      restartPolicy: Never
```

Before this change, the cilium agent memory keeps increasing from 150MB
to ~500MB in less than 3 hours, while with the same workload configured
and this change, the memory is quite stable for a longer period (e.g. 5
hours).

[^1]: https://pkg.go.dev/k8s.io/client-go@v0.29.3/util/workqueue#Type.Get

Fixes: 782f934
Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 0a79dbd ]

The new log format (e.g. text-ts) is added recently in the below commit,
so we need to allow it in regex. Additionally, text-ts is used as the
default value if not specified or invalid.

Fixes: a099bf1
Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit ecf6ff1 ]

    This commit adds BackendSlot value to the Service6Key.String
    and Service4Key.String methods. This is to prevent the
    service key from being deleted when the backend endpoint is deleted.

    Fixes: #29580

Signed-off-by: xyz-li <hui0787411@163.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 09572e2 ]

If os.OpenFile returns an error we shouldn't be writing to the returned
os.(*File) instance which might be nil.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 7aa2c74 ]

Files opened using os.Open{,File} need to be closed manually using
os.(*File).Close to avoid leaking os.(*File) instances and file
descriptors.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit d4ec036 ]

Signed-off-by: Dean <22192242+saintdle@users.noreply.github.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 559eb5a ]

Status was incorrectly reporting the number of generic leases twice,
rather than generic and lock leases. Let's fix it.

Fixes: c6eb358 ("etcd: switch lock session to lease manager")
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
julianwiedmann and others added 4 commits April 10, 2024 19:24
[ upstream commit 20fbf45 ]

Prior to the blamed commit we ignored any errors from the L4 port
extraction, and so traffic that's not supported by CT (eg ESP) would be
allowed through. Restore this behaviour by explicitly checking for
DROP_CT_UNKNOWN_PROTO.

Fixes: 76217a1 ("bpf: nat: Handle errors from snat_v(4|6)_prepare_state()")
Reported-by: Paul Chaignon <paul.chaignon@gmail.com>
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 3847833 ]

We are moving the governance docs out of the main Cilium repo and into the Cilium community repo. They are currently published in the community repo, and this commit will finish the move by deleting them from this main repo.

Changes includes deleting the governance docs from this location and updating links to point to the governance docs in the new location.

Fixes: cilium/community#78

Signed-off-by: Katie Struthers <99215338+katiestruthers@users.noreply.github.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 0976a1b ]

The Hubble OTel repo is going to be archived so it should be removed from the roadmap

Signed-off-by: Bill Mulligan <billmulligan516@gmail.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 9e89397 ]

Descendants and Ancestors cannot share the same
traversal method, because Descendants needs to be
able to select at least one in-trie key-prefix match
that may not be a full match for the argument key-prefix.
The old traversal method worked for the Descendants
method if there happened to be an exact match of the
argument key-prefix in the trie. These new tests ensure
that Descendants will still return a proper list of
Descendants even if there is not an exact match in the
trie.

Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@nbusseneau nbusseneau force-pushed the pr/v1.15-backport-2024-04-10-05-56 branch from 608ba9e to f207601 Compare April 10, 2024 17:24
@nbusseneau
Copy link
Member Author

Backport of #31227 introduces a change to the upgrade guide under Changed metrics. Just want to make sure all of this is correct and intended and we're not going to cause troubles for existing v1.15 users. cc @joestringer @vipul-21

@nbusseneau
Copy link
Member Author

/test-backport-1.15

@nbusseneau nbusseneau marked this pull request as ready for review April 10, 2024 17:29
@nbusseneau nbusseneau requested review from a team as code owners April 10, 2024 17:29
@nbusseneau nbusseneau requested a review from rolinh April 10, 2024 17:29
@joestringer
Copy link
Member

@nbusseneau as I understand, it should only be a net addition of a new label for one metric, so should not impact existing usage of the metric. Furthermore I wouldn't expect a significant change in cardinality.

@vipul-21
Copy link
Contributor

@nbusseneau the addition of a label to the metric should not impact the existing usage of the metric. The changes of commit looks good. Thanks !

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 commit looks good, thanks!

Copy link
Contributor

@lmb lmb left a comment

Choose a reason for hiding this comment

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

tophat

Copy link
Member

@tklauser tklauser 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 looks good, thanks Nicolas!

@sayboras sayboras merged commit 29b08af into v1.15 Apr 11, 2024
232 of 234 checks passed
@sayboras sayboras deleted the pr/v1.15-backport-2024-04-10-05-56 branch April 11, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.15 This PR represents a backport for Cilium 1.15.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