aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Santhosh Kumar Shanmugham <santhoshkuma...@gmail.com>
Subject Re: Review Request 52479: Resolve docker tags to concrete identifiers for DockerContainerizer
Date Mon, 24 Oct 2016 05:10:56 GMT


> On Oct. 14, 2016, 11:05 a.m., Joshua Cohen wrote:
> > docs/features/containers.md, line 71
> > <https://reviews.apache.org/r/52479/diff/6/?file=1533866#file1533866line71>
> >
> >     "Starting with the 0.17.0 release..."

Done.


> On Oct. 14, 2016, 11:05 a.m., Joshua Cohen wrote:
> > src/test/python/apache/aurora/client/binding_helpers/BUILD, line 15
> > <https://reviews.apache.org/r/52479/diff/6/?file=1533875#file1533875line15>
> >
> >     Same here, re: BUILD

Dropped.


> On Oct. 14, 2016, 11:05 a.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/client/docker/BUILD, line 15
> > <https://reviews.apache.org/r/52479/diff/6/?file=1533872#file1533872line15>
> >
> >     ditto here re: BUILD file.

Dropped.


> On Oct. 14, 2016, 11:05 a.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/client/binding_helpers/BUILD, line 15
> > <https://reviews.apache.org/r/52479/diff/6/?file=1533868#file1533868line15>
> >
> >     This BUILD file should not exist. See https://github.com/apache/aurora/blob/master/docs/development/thermos.md
for a reference to how BUILD files are structured for our python code.

Dropped.


> On Oct. 14, 2016, 11:05 a.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/client/docker/docker_client.py, line 38
> > <https://reviews.apache.org/r/52479/diff/6/?file=1533874#file1533874line38>
> >
> >     I thought we were going to drop the auth stuff?

We are dropping AuthZ but keeping AuthN. It is not possible to communicate with the Docker
Registry without proper AuthN token. I have dropped the change to read credentials from the
docker config file to authorize access to restricted repositories/images.

Docker Registry is made of 2 components - Registry service and Auth service. Each request
to the Registry service will need an AuthN token, which can be got by solving the `auth challenge`
(by issuing a GET to the Auth Service with the provided parameters returned by the registry).
Technically, it is possible to run a Docker Registry without an Auth Service, in which case
there is no need to present any AuthN token.

I can see 4 different uses cases:
1. 'Public Registry (ex: DockerHub)' - this has both Registry service and Auth service, and
we will need to provide AuthN token to make any progress. (I believe this will the most common
usecase).
2. 'Private Registry (ex: registry.example.com) without Auth Service' - will work without
an AuthN token.
3. 'Private Registry (ex: registry.example.com) with Auth Service' - same as 1
4. 'Private Registry (ex: registry.example.com) with Auth Service and ACLs on repositories'
- similar to 1 but we will need to present the credentials to the Auth Service to get a AuthN/AuthZ
token (its also AuthZ since the Auth service takes care of checking the process access).

Having some working against a public registry is an MVP. If there is interest for the 4th
kind, we can add AuthZ support as well (which was in the initial implementation).


> On Oct. 14, 2016, 11:05 a.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/client/docker/docker_client.py, line 90
> > <https://reviews.apache.org/r/52479/diff/6/?file=1533874#file1533874line90>
> >
> >     If we're only using the `Docker-Content-Digest` header from the response and
dropping the body on the floor, can we just send a `HEAD` request instead of a full `GET`?

Good suggestion. Changing all the request to `HEAD` instead of `GET`, expect for the request
to get the authentication token where the token is returned in the body of the response.


> On Oct. 14, 2016, 11:05 a.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/client/docker/docker_client.py, line 99
> > <https://reviews.apache.org/r/52479/diff/6/?file=1533874#file1533874line99>
> >
> >     If we do drop auth support, can move this to right after we make the request.

We cannot drop the authentication support. See comment above.


> On Oct. 14, 2016, 11:05 a.m., Joshua Cohen wrote:
> > src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh, line 545
> > <https://reviews.apache.org/r/52479/diff/6/?file=1533885#file1533885line545>
> >
> >     I'd say just use the binding helper as the default for the docker containerizer
test case rather than re-running all the tests again just to test the binding helper.

Done.


> On Oct. 14, 2016, 11:05 a.m., Joshua Cohen wrote:
> > build-support/packer/build.sh, line 64
> > <https://reviews.apache.org/r/52479/diff/6/?file=1533865#file1533865line64>
> >
> >     What's the purpose of this restart?

I had it initially to make sure that the docker engine was running before the end-to-end test,
but here it is not needed. Dropping all the checks to make sure that docker is running, since
it is too defensive. Replacing it with just a `docker run` command.


> On Oct. 14, 2016, 11:05 a.m., Joshua Cohen wrote:
> > build-support/packer/build.sh, line 69
> > <https://reviews.apache.org/r/52479/diff/6/?file=1533865#file1533865line69>
> >
> >     I believe this script is running as root, so this will create the directory
as `/root/.docker-registry-data`, is that what we want?

Dropped. Having the registry use the default path for image cache.


- Santhosh Kumar


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52479/#review152572
-----------------------------------------------------------


On Oct. 12, 2016, 9:41 p.m., Santhosh Kumar Shanmugham wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52479/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2016, 9:41 p.m.)
> 
> 
> Review request for Aurora, George Sirois, Joshua Cohen, John Sirois, and Brian Wickman.
> 
> 
> Bugs: AURORA-1014
>     https://issues.apache.org/jira/browse/AURORA-1014
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Resolve docker tags to concrete identifiers for DockerContainerizer
> 
> Docker tags are mutable and can point to different different images
> at different points in time. This makes a job launched with a Docker
> image to be mutable across restarts of the job. This breaks Aurora's
> guarantee of job immutability (except via job updates).
> 
> This change introduces a binding helper, that resolves docker name:tag
> to a concrete registry/name@digest identifier. Identifying
> docker images via a content-addressable digest is available via the
> Docker Registry v2, that is a prerequisite for this feature.
> 
> 
> Diffs
> -----
> 
>   3rdparty/python/requirements.txt 2fb6f4c76c0da9f7958f9b161e701eb894125481 
>   RELEASE-NOTES.md 03349b0b687bade48a2d19712dcb63802f3ef273 
>   build-support/packer/build.sh f5157a60dcabde3e4cd8f6655f1010771f14b702 
>   docs/features/containers.md 8af38e3af989351925a4130da1ce7a0f5853f0bf 
>   examples/jobs/hello_docker_engine.aurora 3c830e8f99f07732ae985b0aad114c3346888765 
>   src/main/python/apache/aurora/client/binding_helpers/BUILD PRE-CREATION 
>   src/main/python/apache/aurora/client/binding_helpers/__init__.py PRE-CREATION 
>   src/main/python/apache/aurora/client/binding_helpers/docker_helper.py PRE-CREATION

>   src/main/python/apache/aurora/client/cli/client.py fa0c2648c5ff7ea6c9d949cf8cd9b9795d452e98

>   src/main/python/apache/aurora/client/docker/BUILD PRE-CREATION 
>   src/main/python/apache/aurora/client/docker/__init__.py PRE-CREATION 
>   src/main/python/apache/aurora/client/docker/docker_client.py PRE-CREATION 
>   src/test/python/apache/aurora/client/binding_helpers/BUILD PRE-CREATION 
>   src/test/python/apache/aurora/client/binding_helpers/__init__.py PRE-CREATION 
>   src/test/python/apache/aurora/client/binding_helpers/test_docker_helper.py PRE-CREATION

>   src/test/python/apache/aurora/client/docker/BUILD PRE-CREATION 
>   src/test/python/apache/aurora/client/docker/__init__.py PRE-CREATION 
>   src/test/python/apache/aurora/client/docker/test_docker_client.py PRE-CREATION 
>   src/test/python/apache/aurora/config/BUILD 52b60409e1de2f69f181a83720ebe1c649b49166

>   src/test/sh/org/apache/aurora/e2e/http/http_example.aurora c71fb81490863b44827bf090f6e5a6b28da94b93

>   src/test/sh/org/apache/aurora/e2e/http/http_example_bad_healthcheck.aurora 4fa387d5184968e456bf4c8388496f0514454cb1

>   src/test/sh/org/apache/aurora/e2e/http/http_example_updated.aurora b279b4f679cc6843e99806bcbf57350ba1c9a6e0

>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh c93be9b4b4c76186445ae13eb9424bbf9de9b202

> 
> Diff: https://reviews.apache.org/r/52479/diff/
> 
> 
> Testing
> -------
> 
> build-support/jenkins/build.sh
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message