cloudfoundry-community / stackdriver-tools

Stackdriver Nozzle for Cloud Foundry Loggregator, Host Monitoring Agents BOSH Release
Apache License 2.0
21 stars 13 forks source link

Dockerfile and scripts to automate tile building. #171

Closed fluffle closed 6 years ago

fluffle commented 6 years ago

This change is Reviewable

johnsonj commented 6 years ago

Super awesome to package this up like this. A comment on how it could be made to do less for a tighter iteration cycle but even without that change it's a big benefit.


Review status: 0 of 3 files reviewed at latest revision, 2 unresolved discussions.


scripts/build-custom-tile-docker.sh, line 37 at r1 (raw file):

fi

bosh2 --version

Did you experiment with this setup being part of the Dockerfile? Everything above this line should be a 'one-and-done' while below is the iteration workflow of building new tiles. You will also need to drop the ARG VERSION but it should be able to be done via -e


scripts/build-custom-tile.sh, line 11 at r1 (raw file):

export VERSION="0.0.1-custom.$(git rev-parse --short HEAD)"
docker build -t tile --build-arg "VERSION=${VERSION}" .
docker run --rm -v "${PWD}:/mnt" tile cp "/tmp/tile-build/stackdriver-tools/product/stackdriver-nozzle-custom-${VERSION}.pivotal" /mnt

If you moved the code around, i'd expect this command to read:

docker build -t tile .
docker run --rm -e VERSION -v "${PWD}:/mnt" tile /tmp/ /tmp/tile-build/stackdriver-tools/scripts/build-custom-tile-docker.sh && cp "/tmp/tile-build/stackdriver-tools/product/stackdriver-nozzle-custom-${VERSION}.pivotal" /mnt

Or maybe move the cp "/tmp/tile-build/stackdriver-tools/product/stackdriver-nozzle-custom-${VERSION}.pivotal" to build-custom-tile-docker.sh


Comments from Reviewable

fluffle commented 6 years ago

Review status: 0 of 3 files reviewed at latest revision, 2 unresolved discussions.


scripts/build-custom-tile-docker.sh, line 37 at r1 (raw file):

Previously, johnsonj (Jeff Johnson) wrote…
Did you experiment with this setup being part of the Dockerfile? Everything above this line should be a 'one-and-done' while below is the iteration workflow of building new tiles. You will also need to drop the `ARG VERSION` but it should be able to be done via `-e`

I have an aversion to Dockerfiles that are basically shell scripts, because that's what shell scripts are for ;-)

The problem is that development iteration usually means updates to the code being reflected in the build container, which in turn requires the COPY from the Dockerfile to be re-run. But that can be solved by changing the tool to have three discrete steps "setup", "build" and "clean", and autogenerating Dockerfiles to DTRT for the first two. The workflow now goes:

scripts/custom-tile setup
# hack code
scripts/custom-tile build
# fix bugs
scripts/custom-tile build
# .... etc
scripts/custom-tile clean

scripts/build-custom-tile.sh, line 11 at r1 (raw file):

Previously, johnsonj (Jeff Johnson) wrote…
If you moved the code around, i'd expect this command to read: ```bash docker build -t tile . docker run --rm -e VERSION -v "${PWD}:/mnt" tile /tmp/ /tmp/tile-build/stackdriver-tools/scripts/build-custom-tile-docker.sh && cp "/tmp/tile-build/stackdriver-tools/product/stackdriver-nozzle-custom-${VERSION}.pivotal" /mnt ``` Or maybe move the `cp "/tmp/tile-build/stackdriver-tools/product/stackdriver-nozzle-custom-${VERSION}.pivotal"` to `build-custom-tile-docker.sh`

See other comment about the COPY being necessary to update code inside the container.


Comments from Reviewable

johnsonj commented 6 years ago

Reviewed 4 of 5 files at r2. Review status: all files reviewed at latest revision, 1 unresolved discussion.


scripts/custom-tile, line 1 at r2 (raw file):

#! /bin/bash

really handy wrapper- thank you!


Comments from Reviewable