> On Oct. 3, 2016, 11:22 a.m., Joshua Cohen wrote:
> > I don't think that the registry to resolve against should be an argument to 
> > the docker helper, as this should likely be controlled by Aurora operators, 
> > not by Aurora users. When we originally discussed this, we talked about 
> > having the docker registry to use by a trait of the cluster, was that not 
> > feasible?
> 
> Santhosh Kumar Shanmugham wrote:
>     I totally agree. Unfortunately I could not find a way to make pystachio 
> work that way. There are two stages in the parsing of a pystachio config. The 
> first one is where all the context is built and the second stage where the 
> context is used to bind any Ref to concrete values. Due to the restrictions 
> pystachio puts on the names for Ref address (alphanumeric only), resolving 
> tags during the binding stage, limits the values that can be used for 'name' 
> and 'tag'. (For example: 'name' cannot have '/' in them and 'tag' cannot have 
> '.' in them, which is obviously a serious shortcoming). 
>     
>     The side-effect, however is that at this stage we are unaware of the 
> 'cluster' value to look up any details from 'CLUSTERS'. Hence I have opted to 
> do the resolution in the first stage by explicitly passing in the registry 
> address. The only way I see around this problem involves modifying pystachio 
> to allow more values for Ref objects. Please share your thoughts.
>     
>     See 
> https://github.com/wickman/pystachio/blob/master/pystachio/naming.py#L60
> 
> Santhosh Kumar Shanmugham wrote:
>     Another solution is to encode these special characters using the only 
> special character that is allowed '-'. Hence the mapping would look like,
>     \ => -pys-slash-
>     . => -pys-dot-
>     and so on. 
>     
>     I vote for expanding the character limit for Ref.
> 
> Zameer Manji wrote:
>     Can you elaborate on what you mean by unware of the 'cluster' value? I 
> don't understand why you can't do from apache.aurora.common.clusters import 
> CLUSTERS and get the registry from that. That way we don't need to specify 
> the registry value in the helper.
>     
>     I think expanding Ref would be nice but it doesn't seem feasible because 
> the structured binding of pystachio invovles `.`.
>     
>     I think we should tread carefully here and consider a better solution.
> 
> Santhosh Kumar Shanmugham wrote:
>     'CLUSTERS' can potentially contain multiple cluster definitions. Which of 
> these do we pick is determined by the value of the 'MesosJob.cluster' 
> variable inside the AuroraConfig. But since the AuroraConfig is not yet 
> built, there is no way to know which one to pick. That is what I mean by 
> 'unaware of cluster'. Hence pystachio has the concept of binding to allow for 
> manipoulating the variables post-parsing. Unfortunately, the Ref object is 
> too restrictive.
>     
>     +1 on expanding Ref.
>     
>     I am open to suggestions here.
> 
> Zameer Manji wrote:
>     If you want to expand Ref, I suggest putting this review on hold, and 
> filing a bug report on the github repo. I believe wickman and jsirois have 
> the keys to the repo so they can figure out if this solution is viable or not.
> 
> Santhosh Kumar Shanmugham wrote:
>     Opened a new issue for Pystachio - 
> https://github.com/wickman/pystachio/issues/26

Pull request: https://github.com/wickman/pystachio/pull/27


- Santhosh Kumar


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


On Oct. 3, 2016, 7:05 p.m., Santhosh Kumar Shanmugham wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52479/
> -----------------------------------------------------------
> 
> (Updated Oct. 3, 2016, 7:05 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 398338966ba715fae523ee4d12afbba4c0eb6bbb 
>   RELEASE-NOTES.md 49c03e85ae4c2e3ebc8af89e9ce41df9fd52d6cd 
>   docs/features/containers.md 8af38e3af989351925a4130da1ce7a0f5853f0bf 
>   examples/jobs/hello_docker_engine.aurora 
> 3c830e8f99f07732ae985b0aad114c3346888765 
>   examples/vagrant/provision-dev-cluster.sh 
> 66b99cfbd55ab7becfa9534213cc5ea9bdbb3a74 
>   src/main/python/apache/aurora/config/schema/base.py 
> baea660a6ea9f3f7213f98f8bfdb95d912083ac0 
>   src/test/python/apache/aurora/config/BUILD 
> 52b60409e1de2f69f181a83720ebe1c649b49166 
>   src/test/python/apache/aurora/config/test_base.py 
> b354f0804ce70682e8ecb9fb3a3d8fe736fd1cc5 
>   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
> 
>

Reply via email to