feat(regsitry): Add support for private registry#81
Conversation
| --no-install-recommends \ | ||
| && curl -sSL https://bootstrap.pypa.io/get-pip.py | python - pip==8.1.1 \ | ||
| && pip install --disable-pip-version-check --no-cache-dir docker-py==1.8.1 \ | ||
| && pip install --disable-pip-version-check --no-cache-dir git+https://github.com/kmala/docker-py.git@56016b806cd730d3de74712c161f57208e7473f7 \ |
There was a problem hiding this comment.
this will be changed before merging once docker/docker-py#1133 is merged.
There was a problem hiding this comment.
Did this get included in docker-py 1.9?
There was a problem hiding this comment.
No..they tagged it for 1.10...i am thinking of building from commit..what do you say?
| --no-install-recommends \ | ||
| && curl -sSL https://bootstrap.pypa.io/get-pip.py | python - pip==8.1.1 \ | ||
| && pip install --disable-pip-version-check --no-cache-dir docker-py==1.8.1 \ | ||
| && pip install --disable-pip-version-check --no-cache-dir git+https://github.com/docker/docker-py.git@9b63bed6a0b5185b043e85df8c49d86d2c048aa1 \ |
There was a problem hiding this comment.
any way we can just run off v1.8.1 with your patch instead of off a dev release of docker-py? Maybe even just bump to v1.9.0?
There was a problem hiding this comment.
we can do a docker login and then do docker push but that would mean we are populating the credentials onto the cluster hosts and i am not sure how it will behave when two concurrent pushes happen.
There was a problem hiding this comment.
?
I'm just talking about changing this line to use docker-py 1.9.0. Is there some background context I'm missing that's blocking us from upgrading straight to 1.9.0 which includes your patch?
There was a problem hiding this comment.
My patch is not in 1.9.0...it will be present in 1.10.0.
docker/docker-py#1133 (comment)
|
Every time I see more code dumped into deploy.py, the more I feel like we need some unit tests for dockerbuilder. In my eyes deploy.py is starting to become its own project at this point. |
| } | ||
| return client.push(repo, tag=tag, stream=True, auth_config=auth_config) | ||
| else: | ||
| return client.push(repo, tag=tag, stream=True, insecure_registry=True) |
There was a problem hiding this comment.
Do we need the insecure_registry portion here anymore given the work @bacongobbler did?
There was a problem hiding this comment.
@bacongobbler needs to do more PR's in the dockerbuilder and controller before it works fully where DEIS_REGISTRY_SERVICE_HOST and DEIS_REGISTRY_SERVICE_PORT should be changed.
There was a problem hiding this comment.
Ah - I did a PR that pulls out some of insecure_registry stuff but not fixing the env vars in Controller deis/controller#933
There was a problem hiding this comment.
for what it's worth, deis pull and dockerfile builds work just fine on workflow-dev for me as well as for aaron and jack when they reviewed it manually. If you're finding different results then please file a bug and bring it to my attention.
There was a problem hiding this comment.
I just wanted to clean up that code is all :D
6 participants
No description provided.