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

Handle multiple potentially overlapping wildcard ingress domains in istio. #9183

Conversation

ScheererJ
Copy link
Contributor

How to categorize this PR?

/area networking
/area robustness
/kind enhancement

What this PR does / why we need it:

Handle multiple potentially overlapping wildcard ingress domains in istio.

With PR #9038, nginx was moved behind the istio ingress gateway. The same PR also enabled management of multiple ingress domains via the Garden resource.
Unfortunately, istio is a bit picky about overlapping domains in certain scenarios. This could lead to failing reconciliation of the nginx-ingress managed resource.
Therefore, to prevent those situations this change splits the virtual services into separate instances per host so that istio does not complain and simply does the right thing.

Which issue(s) this PR fixes:
None.

Special notes for your reviewer:

Release note:

Multiple ingress domains in `.spec.runtimeCluster.ingress.domains` can now overlap without triggering reconciliation issues.

@gardener-prow gardener-prow bot added area/networking Networking related area/robustness Robustness, reliability, resilience related kind/enhancement Enhancement, improvement, extension labels Feb 19, 2024
@gardener-prow gardener-prow bot added cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 19, 2024
…stio.

With PR gardener#9038, nginx was moved behind the istio ingress gateway. The same
PR also enabled management of multiple ingress domains via the `Garden`
resource.
Unfortunately, istio is a bit picky about overlapping domains in certain
scenarios. This could lead to failing reconciliation of the nginx-ingress
managed resource.
Therefore, to prevent those situations this change splits the virtual
services into separate instances per host so that istio does not complain
and simply does the right thing.
@ScheererJ ScheererJ force-pushed the fix/handle-overlapping-ingress-domains branch from faa6843 to d97920c Compare February 19, 2024 17:32
@ScheererJ
Copy link
Contributor Author

/retest

@axel7born
Copy link
Contributor

/lgtm

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Feb 20, 2024
Copy link
Contributor

gardener-prow bot commented Feb 20, 2024

LGTM label has been added.

Git tree hash: 30099f42ec30ffe388743d36988a32833a5ad6b2

@plkokanov
Copy link
Contributor

/lgtm
/approve

Copy link
Contributor

gardener-prow bot commented Feb 20, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: plkokanov

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 20, 2024
@gardener-prow gardener-prow bot merged commit 8c8e440 into gardener:master Feb 20, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/networking Networking related area/robustness Robustness, reliability, resilience related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants