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.

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

ingress: cleanup resources on changed ingress class field #28886

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

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 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
Member

@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 This PR / issue needs backporting to the v1.13 branch needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Nov 3, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.4 Nov 3, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.9 Nov 3, 2023
@mhofstetter mhofstetter added affects/v1.13 This issue affects v1.13 branch and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Nov 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Needs backport from main in 1.13.9 Nov 6, 2023
@aanm aanm merged commit c2674ae into cilium:main Nov 6, 2023
62 checks passed
@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 This PR / issue needs backporting to the v1.14 branch labels Nov 7, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.14 in 1.14.4 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

@meyskens meyskens meyskens approved these changes

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
1.14.4
Backport pending to v1.14
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