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

[WIP] Automatic HTTPS and tests #1101

Closed

Conversation

consideRatio
Copy link
Member

@consideRatio consideRatio commented May 20, 2020

I'm still experimenting, but the end goal is to allow for automatic https without cert-manager etc.

@betatim betatim changed the title [not ready for review] Automatic HTTPS and tests [WIP] Automatic HTTPS and tests May 21, 2020
@betatim
Copy link
Member

betatim commented May 22, 2020

Scrolling through this to see what kind of things are being changed. I think some of it could be a separate PR that can be merged before (and we can debug it).

  • changing versions of k8s, helm, etc
    • what is the oldest version of k8s we want to support with BinderHub? What promises have we made in the past?
  • moving the location of the git crypt secret file and corresponding travis update (why is this needed?)
  • new feature(?) hub_url_local in the Python code. What does it do? Why is it needed? What will it break/replace? What are the alternatives?
  • what happens to the existing way of running BinderHub as a dev?
  • installing tini in the bhub docker image
  • changing the version of the kubernetes Python package used (why?)
  • updating CONTRIBUTING.md for changes in testing/minikube/install-hub (if needed)

Will we require every dev to setup pebble locally? Can we instead keep the current way of running BinderHub as a dev w/o pebble and add a second setup with pebble?

In the CI will we have one run without pebble (what we do now) and one with?

@consideRatio
Copy link
Member Author

@betatim I'd like to get back to these questions when this work has matured a bit, I share your concerns and will do my best to address what I can.

Would you like to join a session with Min / Simon friday next week, with a focus on the Z2JH testing infra that I suggest we mimic overall in this subsequent PR? https://doodle.com/poll/xtbtcgx6wivpmcwz?


While I would like to develop the related z2jh PR and this before we enter a review process, I felt an itch to answer your questions since you have invested your time to consider this PR already:

changing versions of k8s, helm, etc
what is the oldest version of k8s we want to support with BinderHub? What promises have we made in the past?

We only tested one k8s version in the past, and it was 1.13, currently 1.18 is available.

moving the location of the git crypt secret file and corresponding travis update (why is this needed?)

In z2jh, it seems like git-crypt was a stranded remnant that wasn't used any more, I think this is the case for this repo as well. It is only used from .gitattributes to encrypt the content of a folder named secrets, but there is no such folder, so I've deleted it for now as I'd like there to be no extra things that can confuse us going forward and get whats not documented documented.

new feature(?) hub_url_local in the Python code. What does it do? Why is it needed? What will it break/replace? What are the alternatives?

The hub_url configuration was passed to browsers, to binder, and more. By having hub_url_local we can avoid making "hairpin" requests where not needed where we exit to the internet and go straight back. The hairpin requests exiting and directly coming back caused issues with our test setups, so two reasons exists for implementing it. I'll document the case when it is more clear to me what is really needed, what isn't, and what makes most sense.

what happens to the existing way of running BinderHub as a dev?

I'm not sure what I suggest as I don't overview it fully overview it yet, I need to work more until I have a suggestion what I think make sense. I'd like there to be lightweight tests not involving k8s at all if possible for example.

Right now, I think there is "main", "auth", and "helm", where "main" and "auth" both install k8s + z2jh, while "helm" installs k8s + the binderhub helm chart that reference the z2jh helm chart as a dependency.

installing tini in the bhub docker image

I got tired of waiting 30 seconds for the old binderhub pod to shut down because the container isn't listening to the SIGTERM that comes 30 seconds before SIGKILL. It can perhaps be fixed by starting binderhub differently than python3 -m binderhub. There is a similar issue fixed in the z2jh refactoring PR for the secret-sync container running a python script part of the autohttps setup. I didn't fully get why Python chose to not shut the process down on SIGTERM signal, but it didn't.

changing the version of the kubernetes Python package used (why?)

Because it is two version behind the Python client already which is available at version 11, and those are in turn behind the official k8s versions - this should be bumped further i think, to ensure we don't loose track of kubernetes developments entirely. This is an easy thing to have in a separate PR, and I think it will work with the old version as well still.

updating CONTRIBUTING.md for changes in testing/minikube/install-hub (if needed)

Agree, its needed, and I'm aiming to do that. I've got help from @GeorgianaElena on my suggested z2jh contribution instructions, this is how it currently looks, but its also a WIP still: https://github.com/jupyterhub/zero-to-jupyterhub-k8s/blob/7da1ffdf302f01bc350e9a9b0d8b66f0c917ef15/CONTRIBUTING.md

Will we require every dev to setup pebble locally? Can we instead keep the current way of running BinderHub as a dev w/o pebble and add a second setup with pebble?

I'm not sure what I'd suggest yet, it will depend how simply the setup of pebble is I think, but BinderHub may benefit from this because you can get hub.binder.test and binder.test making CORS matters be more realistic as well.

In the CI will we have one run without pebble (what we do now) and one with?

This PR is currently in a state where it has disabled all but one test to focus on making sure that one test works. It is possible to keep testing non autohttps based setups as well of course.

@manics
Copy link
Member

manics commented May 22, 2020

new feature(?) hub_url_local in the Python code. What does it do? Why is it needed? What will it break/replace? What are the alternatives?

The hub_url configuration was passed to browsers, to binder, and more. By having hub_url_local we can avoid making "hairpin" requests where not needed where we exit to the internet and go straight back. The hairpin requests exiting and directly coming back caused issues with our test setups, so two reasons exists for implementing it. I'll document the case when it is more clear to me what is really needed, what isn't, and what makes most sense.

I've just realised I opened a similar PR a few months ago to fix what I thought was a problem unique to my K8s cluster, though it's basically the same issue as here but with a different underlying cause: #994

@consideRatio consideRatio force-pushed the binderhub-autohttps branch from 8abcf49 to 82cc5a8 Compare July 7, 2020 14:22
@consideRatio
Copy link
Member Author

I'm closing this PR as I don't intent to continue from this point but instead start fresh in another PR. I gained a lot of experience and insights that led up to the CI updates in #1188, so the new PR can be a lot less cluttered with changes.

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.

3 participants