----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60439/#review179310 -----------------------------------------------------------
This is a great start, but I would prefer if we wouldn't directly mount an existing source directory into the container and bootstrap, configure and build in it (this is also broken for e.g., in-source builds). What about instead modify mounting the Mesos source tree into the container, then inside the container cloning from it, and building in the clean checkout? This woudl not only minimize the risk of destructive side-effects outside the container, but also e.g., make sure that we do not by accident pick up uncommitted changes. This is the approach we also followed in `support/mesos-tidy`. I left some first comments, but will look again once the handling of the source tree is under control. support/jenkins/websitebot.sh Lines 24 (patched) <https://reviews.apache.org/r/60439/#comment254355> Let's quote, : "${WORKSPACE:?"Environment variable 'WORKSPACE' must be set"}" support/jenkins/websitebot.sh Lines 26-27 (patched) <https://reviews.apache.org/r/60439/#comment254358> Let's quote, export MESOS_DIR="$WORKSPACE/mesos" export MESOS_SITE_DIR="$WORKSPACE/mesos-site" support/jenkins/websitebot.sh Lines 29 (patched) <https://reviews.apache.org/r/60439/#comment254356> Let's quote, pushd "$MESOS_DIR" support/jenkins/websitebot.sh Lines 31 (patched) <https://reviews.apache.org/r/60439/#comment254354> If the source tree contains uncommitted changes, the output will not reproducibly correspond to this SHA1. I'd suggest to instead clone the source tree instead. support/jenkins/websitebot.sh Lines 38 (patched) <https://reviews.apache.org/r/60439/#comment254359> Let's quote, pushd "$MESOS_SITE_DIR" support/mesos-website.sh Lines 25 (patched) <https://reviews.apache.org/r/60439/#comment253995> Let's put this into quotes, : "${MESOS_SITE_DIR:?"Environment variable 'MESOS_SITE_DIR' must be set"}" support/mesos-website.sh Lines 27 (patched) <https://reviews.apache.org/r/60439/#comment253993> Let's quote here, pushd "$MESOS_DIR" support/mesos-website.sh Lines 33 (patched) <https://reviews.apache.org/r/60439/#comment253992> Not crucial here right now, but let's put this string into single quotes, see https://github.com/koalaman/shellcheck/wiki/SC2064. Also please consider working on getting this image published in the Mesos dockerhub org so we can pull from there instead of having the rebuild the same image over and over again. See e.g., `support/mesos-tidy.sh` how we already do this for other images. support/mesos-website.sh Lines 43 (patched) <https://reviews.apache.org/r/60439/#comment253994> Let's be consistent with other command substitutions in this file, and also add quotes, docker run \ --rm \ -e LOCAL_USER_ID="$(id -u "$USER")" \ -v "$MESOS_DIR":/SRC \ -v "$MESOS_SITE_DIR"/content:/mesos/site/publish \ $TAG support/mesos-website/Dockerfile Lines 1-11 (patched) <https://reviews.apache.org/r/60439/#comment254000> We now have one Docker image to run tests in CI (via `docker_build.sh`), one to run clang-tidy checks (`mesos-tidy/Dockerfile`), and are adding another one here to build the website. We should try to reduce the number of these images or remove duplication, e.g., this image could be in principle implemented as a small addon on top of `mesos/mesos-tidy` FROM mesos/mesos-tidy:latest LABEL Description="This image is used for generating Mesos web site." # Install ruby and doxygen tools ... support/mesos-website/build.sh Lines 35 (patched) <https://reviews.apache.org/r/60439/#comment253955> Could we make this consistent with other CI scripts? In `docker_build.sh` we use `make -j6` w/o e.g., hitting a memory limit, in `mesos-tidy/entrypoint.sh` where we do not build the most expensive parts of Mesos we even pick `make -j $(nproc)`. - Benjamin Bannier On June 29, 2017, 11:59 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60439/ > ----------------------------------------------------------- > > (Updated June 29, 2017, 11:59 p.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent > huang. > > > Bugs: MESOS-7625 > https://issues.apache.org/jira/browse/MESOS-7625 > > > Repository: mesos > > > Description > ------- > > These scripts are expected to be run by ASF CI on any commit to > the mesos repo. The directory layout is similar to tidybot CI job. > > > Diffs > ----- > > support/jenkins/websitebot.sh PRE-CREATION > support/mesos-website.sh PRE-CREATION > support/mesos-website/Dockerfile PRE-CREATION > support/mesos-website/build.sh PRE-CREATION > support/mesos-website/entrypoint.sh PRE-CREATION > > > Diff: https://reviews.apache.org/r/60439/diff/1/ > > > Testing > ------- > > Tested by running a CI job pointing to a branch containing this patch. > > > Thanks, > > Vinod Kone > >
