----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65464/#review196694 -----------------------------------------------------------
Fix it, then Ship it! Thanks for these patches mpark. I left some comments, mostly around reducing the size of the images the CI would need to download. Before rolling this out we should make sure we have some automation in tool keeping the images up to date from source. Like discussed offline we might be able to leverage dockerhub's infra for that. support/mesos-build.sh Lines 33 (patched) <https://reviews.apache.org/r/65464/#comment276487> Nit: I think we don't use markdown in logging output, maybe use single quotes around `mesos-build` here. support/mesos-build/centos-7.dockerfile Lines 18 (patched) <https://reviews.apache.org/r/65464/#comment276494> Just as a heads-up, while We also use this in the `mesos-tidy` image, the syntax is actually deprecated upstream by now, https://docs.docker.com/engine/reference/builder/#maintainer-deprecated. support/mesos-build/centos-7.dockerfile Lines 21 (patched) <https://reviews.apache.org/r/65464/#comment276477> This seems even worse than the unpinned dependencies we have below since we might pick up random, disruptive upstream changes. Can we just get rid of this? It seems to not be required. support/mesos-build/centos-7.dockerfile Lines 22-24 (patched) <https://reviews.apache.org/r/65464/#comment276478> Let's use a single layer for this, i.e., just chain the commands with `&&`. support/mesos-build/centos-7.dockerfile Lines 42 (patched) <https://reviews.apache.org/r/65464/#comment276479> Let's clean up local caches in this layer, i.e., yum clean all && \ rm -rf /var/cache/yum support/mesos-build/centos-7.dockerfile Lines 47 (patched) <https://reviews.apache.org/r/65464/#comment276480> Let's remove the downloaded script in this layer, i.e., add rm -f /tmp/install-cmake.sh support/mesos-build/centos-7.dockerfile Lines 52 (patched) <https://reviews.apache.org/r/65464/#comment276476> Let's use `COPY` here, https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#add-or-copy. support/mesos-build/entrypoint.sh Lines 53 (patched) <https://reviews.apache.org/r/65464/#comment276485> Quote this. support/mesos-build/entrypoint.sh Lines 77 (patched) <https://reviews.apache.org/r/65464/#comment276486> Quote this. support/mesos-build/ubuntu-16.04.dockerfile Lines 18 (patched) <https://reviews.apache.org/r/65464/#comment276495> Just as a heads-up, while We also use this in the `mesos-tidy` image, the syntax is actually deprecated upstream by now, https://docs.docker.com/engine/reference/builder/#maintainer-deprecated. support/mesos-build/ubuntu-16.04.dockerfile Lines 42 (patched) <https://reviews.apache.org/r/65464/#comment276484> We don't seem to need this. support/mesos-build/ubuntu-16.04.dockerfile Lines 44 (patched) <https://reviews.apache.org/r/65464/#comment276481> Let's also get rid of the dowloaded lists, i.e., add rm -rf /var/lib/apt/lists support/mesos-build/ubuntu-16.04.dockerfile Lines 49 (patched) <https://reviews.apache.org/r/65464/#comment276482> Let's remove the downloaded script in this layer, i.e., add rm -f /tmp/install-cmake.sh support/mesos-build/ubuntu-16.04.dockerfile Lines 55 (patched) <https://reviews.apache.org/r/65464/#comment276483> Let's use `COPY` here, https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#add-or-copy. - Benjamin Bannier On Feb. 1, 2018, 8:20 p.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65464/ > ----------------------------------------------------------- > > (Updated Feb. 1, 2018, 8:20 p.m.) > > > Review request for mesos and Benjamin Bannier. > > > Repository: mesos > > > Description > ------- > > Introduced `mesos-build`, along with pre-built docker images. > > > Diffs > ----- > > support/jenkins/buildbot.sh 7f78509699b036df025c314fe913158f54402014 > support/mesos-build.sh PRE-CREATION > support/mesos-build/centos-7.dockerfile PRE-CREATION > support/mesos-build/entrypoint.sh PRE-CREATION > support/mesos-build/ubuntu-16.04.dockerfile PRE-CREATION > > > Diff: https://reviews.apache.org/r/65464/diff/1/ > > > Testing > ------- > > > Thanks, > > Michael Park > >
