Traefik v2 support #145
Merged
Traefik v2 support #145
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
- Renamed frontends and backedns to routers and services, respectively.
- Traefik API paths at /api/providers/{provider_name} no longer work, so
search /api/http/{services|routers} instead
- Traefik file provider doesn't like arbitrary data (any more?), so have put
JupyterHub's 'data' object into the dynamic configuration file in separate
root keys.
To Do:
- Haven't touched the consul or etcd providers, other than to rename
'frontends' and 'backends', as above.
- Test, test, test.
Additionally, have renamed TraefikTomlProxy to TraefikFileProviderProxy, and
edited relevant traefik_utils functions so the proxy provider should now work
with either yaml or toml files. (need to test).
- jupyterhub_config.py will now need to reference the proxy class
'traefik_file', e.g.:-
c.JupyterHub.proxy_class = 'traefik_file'
c.TraefikFileProviderProxy = '/path/to/rules.toml'
…ng initial work done in previous commit:- alexleach@41a5ef3 Also, see issue: jupyterhub#97 - Relevant documentation has been updated in README.md, docs/source/file.md (renamed from toml.md) - KV providers (consul and etcd) have been updated to use new Traefik KV paths, as per:- https://doc.traefik.io/traefik/reference/dynamic-configuration/kv/ - requirements.txt now makes toml an optional dependency as well as ruamel.yaml. This latter module is required by jupyterhub anyway, so should already be present on a system running JupyterHub. - The (thoroughly excellent) test system has had a bit of an overhaul:- - There were issues with repeated launching and shutting down of external `consul` and `etcd` servers, causing test failures. Added code to gracefully shutdown consul and wait for servers to launch (within `tests/conftest.py`) before continuing with the tests. - `traefik` v2 no longer has a 'storeconfig' command. Therefore, to load a pre-baked configuration file into the KV stores, have had to resort to loading configurations with either `etcdctl txn` or `consul kv import` commands. - The external file provider now watches a directory instead of a file, so have added a pre-baked dynamic_config directory (and file), where the rules.toml file will be saved and managed by the TraefikFileProviderProxy. - Removed the previous traefik_{consul_config,etcd_config}.toml files, (which acted as traefik static configuration files), and instead applied the static KV configuration using the CLI. - Refactored some of the text fixtures to try and re-use fixtures and make them (hopefully) a bit easier to follow. - Have duplicated the file_proxy and external_file_proxy pytest fixtures to test both toml and yaml files. (Would have preferred to parametrize the existing fixtures to avoid duplicating them, but couldn't figure out how). - `tests/test_traefik_api_auth.py` - Have had to make the test give traefik more of a chance to read its dynamic config before running the test. Previously, in traefik v1, the api authentication would be configured in the static config. Now the api 'middleware' is configured in the dynamic config, the previous wait condition of just waiting for the port to come up didn't give traefik enough time to set up the API authentication on that entrypoint / router. - All tests now pass on my dev system, woohoo!
documentation, examples and performance test suite. These are replaced by `traefik_file` and `TraefikFileProviderProxy`, respectively.
…elation to issue: jupyterhub#97 Although all tests passed on my test system, the github workflow tests failed with the etcd tests, for the following reasons:- - Hadn't actually tested etcd-3.4.15, the default version installed by the install script. I'd used the default ubuntu version, 3.2.26. I'm not sure if this caused issues (maybe?, see next point), but some new warnings are emitted about the log parameters for instance. - The tests mainly failed due to what I thought was a nasty race-condition between successive TraefikEtcdProxy test fixture calls. The `proxy()` fixture in `tests/test_proxy.py` doesn't appear to fully destroy the dependent Proxy classes between test runs. In the case of TraefikEtcdProxy, this leaves the etcd3/grpc client open through the end of one test and into the next test. Then the next TraefikEtcdProxy test gets a connection error. I don't know why only one grpc client is allowed - is this related to the etcd version? - but regardless, the less than ideal solution is to manually call `TraefikEtcdProxy.kv_client.close()` during the teardown of the external_etcd* test runs. This is also manually called now by `TraefikEtcdProxy.stop()`, when `should_start=True`. (This took me days to figure out!!) Some further modifications to the code include:- - Changed the KV store structure for jupyterhub-specific information. All jupyterhub routespecs, targets and data are now stored as:- jupyterhub/ -/routes/{escaped_routespec} = {target} -/targets/{escaped_target} = {data} N.B. I think this can be condensed to one single request. Atm, to get the {data} for a routespec, two KV get requests are required: 1. to get the {target}, and 2. to get the {data} using that {target}. The {target} is also in the traefik configuration, so it seems like unnecessary duplication of information and redirection. - Added `log_level` and `traefik_log_level` config parameters to the base Proxy class. The first sets the log level for the logger (and handler) of the Proxy class. The latter sets traefik's log level, if `should_start=True`. - General tidy up, removing excessive debug statements and commented-out code.
077a625, and later. The change in this commit made `jupyterhub_traefik/install.py` not download any versions of traefik, consul or etcd where the checksums were not present. Further, (and this was a change made in a later commit faa2832), the default version of etcd was updated to 3.4.15 and although this was present in the checksums, for some reason it did not install properly in the github workflow. Finally, updated some assert statements in tests/proxytest.py, to instead run `assert_equal(val, cmp)`, which makes the pytest error message easier to follow.
`proxy.kv_separator` in the wrong place, causing faulty target lookups in the etcd database.
… routers. As mentioned in jupyterhub#133 (comment) configurable options have been added to the base TraefikProxy class:- - TraefikProxy.traefik_tls Setting this to `True ` will set the traefik tls option on each jupyterhub-configured traefik router - TraefikProxy.default_entrypoint A string that should match the traefik entryPoint, for which each jupyterhub-configured service will be attached. - TraefikProxy.traefik_entrypoints A list of strings, where each entrypoint should match traefik-configured entrypoints. If empty, then just use the default_entrypoint I'm not sure if this is the best way to do this to be honest. It looks like jupyterhub's `bind_url` predicates use of a single port anyway. So, is only a default_entrypoint necessary? Maybe these configuration options aren't required and should just be figured out from a combination of the other jupyterhub configuration variables (e.g. is bind_url http or https), and pulled from existing jupyterhub configuration variables anyway?
…with the
traefik entrypoint.
* Removed the import of all TraefikProxy derived classes in
jupyterhub_traefik_proxy/__init__.py. Only one needed at a time, never
all of them.
* When using an external traefik proxy, the TraefikProxy classes will now
query traefik's API for the entrypoint that matches the assigned
public_url. See `TraefikProxy._get_traefik_entrypoint()`.
* Only a single entrypoint is allowed by this manner (reverting suggested
change in previous commit 3501d08) to
have multiple, configurable entrypoints.
* Done away with configurable option `traefik_tls`, instead determining it
from the scheme of `public_url`, i.e. http or https.
* Fixed docstrings in kv_proxy.py, if sphinx complained.
* Removed the `debug` Boolean trait from TraefikProxy, instead relying on
`traefik_log_level` and `log_level`, for setting traefik's log level, and
the TraefikProxy log level, respectively.
* Documentation fixes: updated docs/sphinxext/autodoc_traits.py to match
master, at https://github.com/jupyterhub/autodoc-traits/
* Implemented various suggested changes by @GeorgianaElena in
jupyterhub#133
* Removed the standalone configuration loading and dumping functions from
traefik_utils, leaving them only in TraefikConfigFileHandler.
* Updated test suite so works when testing https URLs (without validation,
as traefik will self-sign certificates) as well as http URLs. Currently,
proxytest.py will do either http or https. Should it run both?
Remove old, commented-out code, as per @GeorgianaElena's review Co-authored-by: Georgiana Elena <[email protected]>
it's not true that jupyterhub requires it
it was deprecated from collections in 3.3
Closed
|
While there are still some changes from #133 that I'd rather not add, none are really dealbreakers and all tests are passing, which is great! I propose we actually merge this as-is and work on incremental improvements in main. My first task will be #148 to make it less painful to run through the tests |
|
Thank you @minrk! |
This was referenced Feb 7, 2023
Closed
Closed
|
Thanks @minrk @GeorgianaElena and @dolfinus ! Glad to see this being approved and merged 😄 |
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.
Pick up #133