[KubeIngressProxy] Add reuse_existing_services option #656
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dolfinus
force-pushed
the
ingress_use_existing_service
branch
3 times, most recently
from
694c2c9
to
88534e9
Compare
dolfinus
changed the title
consideRatio
added
KubeIngressProxy
dolfinus
force-pushed
the
ingress_use_existing_service
branch
2 times, most recently
from
91587f6
to
5a5f5c0
Compare
consideRatio
reviewed
Nov 8, 2022
dolfinus
force-pushed
the
ingress_use_existing_service
branch
4 times, most recently
from
f3b406b
to
6e21354
Compare
dolfinus
force-pushed
the
ingress_use_existing_service
branch
from
6e21354
to
b0d2871
Compare
Next release is a major version of KubeSpawner, and I think you have worked KubeIngressProxy significantly so that it could be fine to include a breaking change. Is this behavior the better default in your mind? If so, maybe we should switch to it. |
|
Well, I'm not sure it this issue with accessing Service from ExternalName is related only to my k8s instance or it is a common behavior. It's better to wait for other users to use this option before changing the defaults. |
consideRatio
approved these changes
Mar 23, 2023
|
Thank you @dolfinus!!! Sorry for the slow turnaround on reviewing your work |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If
KubeSpawner.services_enabledoption is set toTrue,KubeIngressProxywill get target URL in a formhttp://jupyterhub-user--servername.pod-namespace.svc.cluster.local:8888, but instead of reusing the existing service it creates ExternalName pointing tojupyterhub-user--servername.pod-namespace.svc.cluster.localDNS name.Sometimes it is useful, for example if pod is created in another namespace, and there is no way to create Ingress with rule pointing to another namespace.
But in other cases
ExternalNameis redundant, and may cause errors. In my case, ingress paths with ExternalName are quire unstable, 50% of runs end with an error "500 Redirect loop detected". k8s documentation saysYou may have trouble using ExternalName for some common protocols, including HTTP and HTTPS.Here I've added a special case for
make_ingress- iftargetis a valid k8s service name, and it has the same namespace as ingress, Ingress -> External -> Service is reduced to Ingress -> Service, and Service is reused instead of creating new one.This behavior is activated by option
KubeIngressProxy.reuse_existing_services=True, which is disabled by default to avoid backward compatibility issues.