> On July 6, 2016, 11:50 p.m., Jie Yu wrote: > > Can you verify with Docker code that `docker inspect` for image actually > > return the v1 image manifest? Better to add a link to the comments. > > Kevin Klues wrote: > Docker inspect seems to return its own type which is a superset of the > fields listed in the v1 image manifest spec: > > Function to do the inspect: > > https://github.com/docker/docker/blob/b316bc42fe1ad8edc709bf28975fb3a52e766344/daemon/image_inspect.go > > ImageInspect Type: > > https://github.com/docker/docker/blob/972c6a7113bffc91c6206f13758951acafa2e461/vendor/src/github.com/docker/engine-api/types/types.go > > There doesn't seem to be any sort of "inheritance" going on where the > results of a `docker store` and the results of a `docker inspect` return > types that are based on on another. Probably part of the reason the two > outputs differ in terms of how they case the labels. > > Jie Yu wrote: > Looks like some fields use the same type, but yeah, there is no > inheritence. > > https://github.com/docker/docker/blob/b316bc42fe1ad8edc709bf28975fb3a52e766344/image/image.go#L21 > > https://github.com/docker/docker/blob/972c6a7113bffc91c6206f13758951acafa2e461/vendor/src/github.com/docker/engine-api/types/types.go#L116 > > Maybe we should model container.Config in our protobuf since this is > shared: > > https://github.com/docker/docker/blob/972c6a7113bffc91c6206f13758951acafa2e461/vendor/src/github.com/docker/engine-api/types/container/config.go#L36 > > and then, module ImageInspect result as well in src/docker/spec.proto?
That seems reasonable. The only field we specifically care about is `Config.Labels`, so we could even think about passing just the `config` into the `shouldInject()` function instead of a Manifest. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49598/#review141103 ----------------------------------------------------------- On July 6, 2016, 11:15 p.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49598/ > ----------------------------------------------------------- > > (Updated July 6, 2016, 11:15 p.m.) > > > Review request for mesos, Benjamin Mahler, Jie Yu, and Yubo Li. > > > Bugs: MESOS-5779 > https://issues.apache.org/jira/browse/MESOS-5779 > > > Repository: mesos > > > Description > ------- > > The `docker::spec::v1::ImageManifest` protobuf implements the > official v1 image manifest specification found at: > > https://github.com/docker/docker/blob/master/image/spec/v1.md > > The field names in this spec are all written in snake_case as are the > field names of the JSON representing the image manifest when reading > it from disk (for example after performing a `docker save`). As such, > the protobuf for ImageManifest also provides these fields in > snake_case. Unfortunately, the `docker inspect` command also provides > a method of retrieving the JSON for an image manifest, with one major > caveat -- it represents all of its top level keys in CamelCase. > > To allow both representations to be parsed in the same way, we > intercept the incoming JSON from either source (disk or `docker > inspect`) and convert it to a canonical snake_case representation. > > > Diffs > ----- > > src/docker/spec.cpp 2711578626dd1847f73cbf7bd3771f36e6755a99 > src/tests/containerizer/docker_tests.cpp > 00ccc2fc54a984f3a67573ce6fe7aee3e60ce3a2 > > Diff: https://reviews.apache.org/r/49598/diff/ > > > Testing > ------- > > `sudo GTEST_FILTER="*ROOT_DOCKER_CompareImageManifests" bin/mesos-tests.sh` > > > Thanks, > > Kevin Klues > >