Skip to content

Conversation

mhofstetter
Copy link
Member

@mhofstetter mhofstetter commented Nov 24, 2023

Currently, when a shared Ingress resource (managed by Cilium) gets deleted via k8s foreground deletion (e.g. kubectl delete ingress ... --cascade=foreground) the corresponding shared CiliumEnvoyConfig in Ciliums namespace gets rewritten empty.

This breaks all shared Ingresses from working.

The reason is an error in the condition that checks which Ingresses should be taken into account when building the model for the shared CiliumEnvoyConfig.

The condition checks the DeletionTimeStamp (set via foreground deletion) from the modified Ingress instead of the the one within the loop. In case of the foreground deletion this always evaluates to true (exclude) - hence no entries in the CEC.

This commit fixes the condition to check for any ongoing deletion on the Ingress that gets checked within the loop.

Fixes: #21386
Fixes: #29306

@mhofstetter mhofstetter added kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. area/servicemesh GH issues or PRs regarding servicemesh feature/k8s-ingress labels Nov 24, 2023
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/fix-ingress-foreground-deletion branch from 1ee885c to d78cfe6 Compare November 24, 2023 13:58
Currently, when a shared Ingress resource (managed by Cilium) gets deleted
via k8s foreground deletion (e.g. `kubectl delete ingress ... --cascade=foreground`)
the corresponding shared CiliumEnvoyConfig in Ciliums namespace gets
rewritten empty.

This breaks all other shared Ingresses from working.

The reason is an error in the condition that checks which Ingresses should be
taken into account when building the model for the shared CiliumEnvoyConfig.

The condition checks the `DeletionTimeStamp` (set via foreground deletion) from the
modified Ingres instead of the one within the loop. In case of the foreground
deletion this always evaluates to `true` - hence no entries in the CEC.

This commit fixes the condition to check for any ongoing deletion on the Ingress
that gets checked within the loop.

Fixes: cilium#21386
Fixes: cilium#29306

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit moves the write of the log message about the deletion of a
shared CiliumEnvoyConfig into the condition that checks whether it's a
dedicated Ingress.

This way, when deleting a shared Ingress, the irritating log message isn't
displayed.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/fix-ingress-foreground-deletion branch from d78cfe6 to c73a60a Compare November 24, 2023 14:13
@mhofstetter mhofstetter marked this pull request as ready for review November 24, 2023 14:17
@mhofstetter mhofstetter requested a review from a team as a code owner November 24, 2023 14:17
@mhofstetter mhofstetter requested a review from meyskens November 24, 2023 14:17
@mhofstetter
Copy link
Member Author

/test

Copy link
Contributor

@meyskens meyskens left a comment

Choose a reason for hiding this comment

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

LGTM

@mhofstetter mhofstetter added the affects/v1.13 This issue affects v1.13 branch label Nov 24, 2023
@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 Nov 24, 2023
@aanm aanm added this pull request to the merge queue Nov 24, 2023
Merged via the queue into cilium:main with commit 4f34e2c Nov 24, 2023
@mhofstetter mhofstetter deleted the pr/mhofstetter/fix-ingress-foreground-deletion branch November 24, 2023 17:35
@joamaki joamaki mentioned this pull request Nov 29, 2023
14 tasks
@joamaki joamaki added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 labels Nov 29, 2023
@github-actions github-actions bot added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects/v1.13 This issue affects v1.13 branch area/servicemesh GH issues or PRs regarding servicemesh backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. feature/k8s-ingress kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
Status: Released
Development

Successfully merging this pull request may close these issues.

cilium-ingress autogenerated CEC definition removed on k8s ingress deletion
4 participants