You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Michael Park <mp...@apache.org> on 2018/02/01 19:20:48 UTC

Review Request 65464: WIP: Introduced `mesos-build`, along with pre-built docker images.

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

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


Re: Review Request 65464: WIP: Introduced `mesos-build`, along with pre-built docker images.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65464/#review196649
-----------------------------------------------------------



PASS: Mesos patch 65464 was successfully built and tested.

Reviews applied: `['65463', '65464']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65464

- Mesos Reviewbot Windows


On Feb. 1, 2018, 7: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, 7: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
> 
>


Re: Review Request 65464: WIP: Introduced `mesos-build`, along with pre-built docker images.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 65464: Introduced `mesos-build` along with pre-built docker images.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65464/#review196770
-----------------------------------------------------------



FAIL: Failed to apply the dependent review: 65463.

Failed command: `python.exe .\support\apply-reviews.py -n -r 65463`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65464

Relevant logs:

- [apply-review-65463-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65464/logs/apply-review-65463-stdout.log):

```
error: patch failed: support/jenkins/buildbot.sh:21
error: support/jenkins/buildbot.sh: patch does not apply
```

- Mesos Reviewbot Windows


On Feb. 1, 2018, 7: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, 7:20 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The layout of `mesos-build` is similar to `mesos-tidy`.
> 
> The `mesos-build.sh` will be the user-facing driver, and `buildbot.sh`
> invokes `mesos-build.sh` along with the clean-up (`docker rmi`) phase.
> The `mesos-build` directory contains docker files and a common
> `entrypoint.sh`. The docker images are built with all of the
> dependencies required, and the various testing configurations are set
> within `entrypoint.sh`.
> 
> 
> Diffs
> -----
> 
>   support/jenkins/buildbot.sh dab4b72b645eaca382a70acef4220fa04af96e36 
>   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/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 65464: Introduced `mesos-build` along with pre-built docker images.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65464/#review196796
-----------------------------------------------------------



PASS: Mesos patch 65464 was successfully built and tested.

Reviews applied: `['65464']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65464

- Mesos Reviewbot Windows


On Feb. 4, 2018, 8:22 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65464/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2018, 8:22 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The layout of `mesos-build` is similar to `mesos-tidy`.
> 
> The `mesos-build.sh` will be the user-facing driver, and `buildbot.sh`
> invokes `mesos-build.sh` along with the clean-up (`docker rmi`) phase.
> The `mesos-build` directory contains docker files and a common
> `entrypoint.sh`. The docker images are built with all of the
> dependencies required, and the various testing configurations are set
> within `entrypoint.sh`.
> 
> 
> Diffs
> -----
> 
>   support/jenkins/buildbot.sh dab4b72b645eaca382a70acef4220fa04af96e36 
>   support/mesos-build.sh PRE-CREATION 
>   support/mesos-build/centos-7.dockerfile PRE-CREATION 
>   support/mesos-build/enable-devtoolset-4.sh 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/4/
> 
> 
> Testing
> -------
> 
> https://builds.apache.org/job/Mesos-Buildbot-Test/103/
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 65464: Introduced `mesos-build` along with pre-built docker images.

Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65464/
-----------------------------------------------------------

(Updated Feb. 4, 2018, 12:22 p.m.)


Review request for mesos and Benjamin Bannier.


Repository: mesos


Description
-------

The layout of `mesos-build` is similar to `mesos-tidy`.

The `mesos-build.sh` will be the user-facing driver, and `buildbot.sh`
invokes `mesos-build.sh` along with the clean-up (`docker rmi`) phase.
The `mesos-build` directory contains docker files and a common
`entrypoint.sh`. The docker images are built with all of the
dependencies required, and the various testing configurations are set
within `entrypoint.sh`.


Diffs (updated)
-----

  support/jenkins/buildbot.sh dab4b72b645eaca382a70acef4220fa04af96e36 
  support/mesos-build.sh PRE-CREATION 
  support/mesos-build/centos-7.dockerfile PRE-CREATION 
  support/mesos-build/enable-devtoolset-4.sh 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/4/

Changes: https://reviews.apache.org/r/65464/diff/3-4/


Testing (updated)
-------

https://builds.apache.org/job/Mesos-Buildbot-Test/103/


Thanks,

Michael Park


Re: Review Request 65464: Introduced `mesos-build` along with pre-built docker images.

Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65464/
-----------------------------------------------------------

(Updated Feb. 3, 2018, 5:14 p.m.)


Review request for mesos and Benjamin Bannier.


Changes
-------

Fixed CentOS builds.


Repository: mesos


Description
-------

The layout of `mesos-build` is similar to `mesos-tidy`.

The `mesos-build.sh` will be the user-facing driver, and `buildbot.sh`
invokes `mesos-build.sh` along with the clean-up (`docker rmi`) phase.
The `mesos-build` directory contains docker files and a common
`entrypoint.sh`. The docker images are built with all of the
dependencies required, and the various testing configurations are set
within `entrypoint.sh`.


Diffs (updated)
-----

  support/jenkins/buildbot.sh dab4b72b645eaca382a70acef4220fa04af96e36 
  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/3/

Changes: https://reviews.apache.org/r/65464/diff/2-3/


Testing (updated)
-------

https://builds.apache.org/job/Mesos-Buildbot-Test/101/


Thanks,

Michael Park