Add support for storing build images in gitlab registries

[rochaporto]

Add support for gitlab token_url requests and accept gitlab registry structures, where any number of levels in the image name is possible.

Example:
binder/images/build-:tag

Where binder is the group, images the project and the rest the image name and tag in the 'images' registry. Subgroups can be added creating additional path levels. The previous logic assumed two levels only, which matches the docker.io behavior.

This change implies the image_prefix will only contain the image name, not a repo dns name. So 'rochaporto/myimage' is ok, but not 'docker.io/rochaporto/myimage'. If needed i could look into some additional logic for this, but given urllib.parse expects a scheme it won't be direct.

Fixes #965

[betatim]

Thanks for digging into this. Every time I end up looking at this code I think we should refactor it a bit and add some tests because I find it hard to "run it in my head" to check the various scenarios of different registries (and then which cases we already handle via what kind of workaround...). So below some code that I think reproduces the values we'd see in a setup like mybinder.org:

In [1]: image_prefix = "gcr.io/binder-prod/r2d-f18835fd-"

In [3]: '{prefix}{build_slug}:{ref}'.format(prefix=image_prefix, build_slug="some-escaped-url", ref="1234")
Out[3]: 'gcr.io/binder-prod/r2d-f18835fd-some-escaped-url:1234'

In [5]: image_name = '{prefix}{build_slug}:{ref}'.format(prefix=image_prefix, build_slug="some-escaped-url", ref="1234")

In [6]: '/'.join(image_name.split('/')[-2:]).split(':', 1)
Out[6]: ['binder-prod/r2d-f18835fd-some-escaped-url', '1234']

In [7]: image_name.split(':', 1)
Out[7]: ['gcr.io/binder-prod/r2d-f18835fd-some-escaped-url', '1234']

This means your hunch that what is passed to get_image_manifest() is indeed just the name like binder-prod/r2d-f18835fd-some-escaped-url. But the new code doesn't produce the same result. I'm trying to remember why we aren't using image_name.split('/', 1)[1].split(':', 1).

I think this would work for the gcr.io case and also for the GitLab registry case.

[rochaporto]

Thanks for digging into this. Every time I end up looking at this code I think we should refactor it a bit and add some tests because I find it hard to "run it in my head" to check the various scenarios of different registries (and then which cases we already handle via what kind of workaround...). So below some code that I think reproduces the values we'd see in a setup like mybinder.org:

In [1]: image_prefix = "gcr.io/binder-prod/r2d-f18835fd-"

In [3]: '{prefix}{build_slug}:{ref}'.format(prefix=image_prefix, build_slug="some-escaped-url", ref="1234")
Out[3]: 'gcr.io/binder-prod/r2d-f18835fd-some-escaped-url:1234'

In [5]: image_name = '{prefix}{build_slug}:{ref}'.format(prefix=image_prefix, build_slug="some-escaped-url", ref="1234")

In [6]: '/'.join(image_name.split('/')[-2:]).split(':', 1)
Out[6]: ['binder-prod/r2d-f18835fd-some-escaped-url', '1234']

In [7]: image_name.split(':', 1)
Out[7]: ['gcr.io/binder-prod/r2d-f18835fd-some-escaped-url', '1234']

This means your hunch that what is passed to get_image_manifest() is indeed just the name like binder-prod/r2d-f18835fd-some-escaped-url. But the new code doesn't produce the same result. I'm trying to remember why we aren't using image_name.split('/', 1)[1].split(':', 1).

I think this would work for the gcr.io case and also for the GitLab registry case.

Do you have a set of inputs->outputs to be used for testing? I'm happy to extract this logic to a separate function and add a few tests.

[betatim]

Not really :( The best I can think of is configs of existing deployments. For them we could feed in the current config, capture the output and then create a test based on this not changing.

The one posted above is what we use for the GKE cluster of mybinder.org.

For the OVH cluster we use this config https://github.com/jupyterhub/mybinder.org-deploy/blob/c7197ee387048d20838de3c2ab70135f4a83107c/config/ovh.yaml#L9-L14

This is the config of the GESIS cluster https://github.com/gesiscss/orc/blob/aa81bd432330547f76ee8ee8c5fcaf3c8735051e/gesisbinder/config.yaml#L32 (I think it uses DockerHub as storage)

The Pangeo BinderHub uses gcr.io as well https://github.com/pangeo-data/pangeo-binder/blob/7276aae9bf6077761255ef1f41243e6d391bf827/deploy/prod.yaml#L10 (so config wise a copy&paste from mybinder.org)

[betatim]

@bdrian do you want to make a new PR with your suggested patch and some of the test cases? This PR has been quiet for a while so I think it would be fine to start a new one with fresh momentum (and a slightly different approach).

[bdrian]

@betatim I'll submit a similar PR soon, just have to finish the local testing. The patch above is dumb on second thought.

[rochaporto]

Closing this one in favor of #986