> 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.
'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. - Santhosh Kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52479/#review151205 ----------------------------------------------------------- On Oct. 3, 2016, 6:22 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, 6:22 p.m.) > > > Review request for Aurora, George Sirois, Joshua Cohen, 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 > >
