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

Proxy deployment: Change probes to https port #1378

Merged
merged 3 commits into from
Aug 29, 2019

Conversation

chicocvenancio
Copy link
Contributor

When left on http and http to https redirection is configured the probes may be redirected to the wrong port.

When left on http and http to https redirection is configured the probes may be redirected to the wrong port.
@consideRatio
Copy link
Member

Thanks for this insight and PR!

There is quite a bit of complexity going on here... Hmm... I'm not confident at all yet about what should be done or if something should be done.

Questions raised in my mind:

  • What is the proxy-https service pointing to? Is it https termination followed by the proxy-public service?
  • I've learned that any response in the range of >=200 and <400 will be a OK sign. But, that they will follow the redirect if assuming it on the same host, I think.
  • It is possible to specify if it should be a http or https request in the probe, that may be relevant to do as well if this PR would go through.
  • What if you don't have internal https termination, but that is handled before arrival to the pod, then having this set would break something perhaps?

Did you experience the probes failing due to this somehow? I can imagine that...

  • you got an additional redirect that was followed, but the bounce was unnecessary and could had been slimmed away
  • the probe was redirected and not followed, leading to a potentially success without actually asking the _chp_healthz endpoint?
  • something else happened?

@chicocvenancio
Copy link
Contributor Author

@consideRatio Thanks for this through and quick response.

  • something else happened?

Basically with #1341 chp will redirect http to https on 443 with manual secrets to work with the external (to k8s) ingress. This means the proxy probes fail with timeouts.

What if you don't have internal https termination, but that is handled before arrival to the pod, then having this set would break something perhaps?

Sorry, you're right. I'll make this only use https in the manual https scenario.

@consideRatio
Copy link
Member

consideRatio commented Aug 27, 2019

Related documentation

Note how the livenessProbe and readinessProbe, being of the k8s type probe has the httpGet field of the k8s type HTTPGetAction that has a field called scheme that defaults to "HTTP" but could be changed to "HTTPS". That may be required. I'm not that confident on this.

❤️ for your work to get this right @chicocvenancio !

@chicocvenancio
Copy link
Contributor Author

@consideRatio, by the docs it seems you're right. I could swear something magically detected https when I was testing this yesterday, but it is better to be explicit anyway.

@consideRatio
Copy link
Member

LGTM! Thank you @chicocvenancio ❤️

@consideRatio consideRatio merged commit 7087fdb into jupyterhub:master Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants