Skip to content

ingress: cleanup resources on changed ingress class field #28886

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 6, 2023
Merged

ingress: cleanup resources on changed ingress class field #28886

merged 1 commit into from
Nov 6, 2023

Conversation

mhofstetter
Copy link
Member

@mhofstetter mhofstetter commented Oct 31, 2023 •
edited
Loading

If an Ingress resource with ingressClassName: cilium is changed to a different value, the corresponding resources (CEC, Endpoints & Service aren't removed (mode dedicated) or the shared CiliumEnvoyConfig isn't updated (mode shared).

Therefore, this commit reflects the changes on the the corresponding resources when the ingressClassName of an Ingress gets updated from cilium to something else.

Fixes: #23781

@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 Oct 31, 2023
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/ingressclass-change branch 2 times, most recently from 18df246 to 07c9e63 Compare October 31, 2023 07:41
@mhofstetter
ingress: update resources on changed ingress class field
890e0da 
If an Ingress resource with `ingressClass: cilium` is changed to
a different value, the corresponding resources (CEC, Endpoints &
Service` aren't removed (mode dedicated) or the shared CiliumEnvoyConfig
isn't updated (mode shared).

Therefore, this commit reflects the changes on the corresponding
resources when the `ingressClass` of an Ingress gets updated from
`cilium` to something else.

Fixes: cilium#23781

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/ingressclass-change branch from 07c9e63 to 890e0da Compare October 31, 2023 08:15
@mhofstetter
Copy link
Member Author

/test

@mhofstetter mhofstetter marked this pull request as ready for review October 31, 2023 10:39
@mhofstetter mhofstetter requested a review from a team as a code owner October 31, 2023 10:39
@mhofstetter mhofstetter requested a review from meyskens October 31, 2023 10:39
@mhofstetter mhofstetter changed the title ingress: update resources on changed ingress class field ingress: cleanup resources on changed ingress class field Oct 31, 2023
meyskens
meyskens approved these changes Nov 3, 2023
View reviewed changes
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

@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 3, 2023
@mhofstetter mhofstetter added needs-backport/1.13 affects/v1.13 This issue affects v1.13 branch and removed needs-backport/1.13 labels Nov 3, 2023
@aanm aanm merged commit c2674ae into cilium:main Nov 6, 2023
@mhofstetter mhofstetter deleted the pr/mhofstetter/ingressclass-change branch November 6, 2023 14:10
@jibi jibi mentioned this pull request Nov 7, 2023
Merged
15 tasks
@jibi jibi 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 7, 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 Nov 8, 2023
@thorn3r thorn3r mentioned this pull request Nov 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@meyskens meyskens meyskens approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
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
cilium v1.14.4
Status: Released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: Cilium does not clean up config on ingressClass change
4 participants
@mhofstetter @meyskens @jibi @aanm