> On Aug. 5, 2015, 1:13 p.m., Jake Farrell wrote: > > build-artifact.sh, line 32 > > <https://reviews.apache.org/r/37107/diff/1/?file=1032087#file1032087line32> > > > > sudo should not be needed and should be removed. > > > > Would be good to add a check to ensure docker is running and available > > before starting build > > > > ``` > > function error_exit { > > echo "$1" >&2 > > exit "${2:-1}" > > } > > > > docker info > /dev/null 2>&1 || error_exit "Docker is not running." > > ``` > > > > and also adding in a cleanup task to remove any exited containers > > > > ``` > > # Remove any containers that have a status of 'Exited' > > if [[ "$(docker ps -a | grep Exited | wc -l)" -gt "0" ]]; then > > docker rm $(docker ps -a | grep Exited | awk '{print $1}') > > fi > > ```
> sudo should not be needed and should be removed. Done. > Would be good to add a check to ensure docker is running and available before > starting build Seems like over-engineering, since `docker run` is the first non-trivial thing being done. > and also adding in a cleanup task to remove any exited containers Isn't that a bit heavy-handed? It would blow away other containers on the system that the user might care about. > On Aug. 5, 2015, 1:13 p.m., Jake Farrell wrote: > > build-artifact.sh, line 34 > > <https://reviews.apache.org/r/37107/diff/1/?file=1032087#file1032087line34> > > > > sudo docker build -t "aurora-$IMAGE_NAME" "$BUILDER_DIR" Done, but baked 'aurora-' into `IMAGE_NAME`. > On Aug. 5, 2015, 1:13 p.m., Jake Farrell wrote: > > build-artifact.sh, line 39 > > <https://reviews.apache.org/r/37107/diff/1/?file=1032087#file1032087line39> > > > > add --rm flag Thanks, that's the kind of useful trick i was after :-) > On Aug. 5, 2015, 1:13 p.m., Jake Farrell wrote: > > builder/deb/ubuntu-trusty/Dockerfile, line 14 > > <https://reviews.apache.org/r/37107/diff/1/?file=1032088#file1032088line14> > > > > Add MAINTAINER Apache Aurora <d...@aurora.apache.org> Honest question - isn't that mostly intended for containers that are shared, e.g. on Docker Hub? I'm trying to understand what value it will provide here but i'm coming up blank. > On Aug. 5, 2015, 1:13 p.m., Jake Farrell wrote: > > builder/deb/ubuntu-trusty/build.sh, line 23 > > <https://reviews.apache.org/r/37107/diff/1/?file=1032089#file1032089line23> > > > > does not need to be a copy, symlink would work here That actually doesn't work, since dpkg-buildpackage writes to this directory. I'm open to alternatives. > On Aug. 5, 2015, 1:13 p.m., Jake Farrell wrote: > > builder/deb/ubuntu-trusty/build.sh, line 25 > > <https://reviews.apache.org/r/37107/diff/1/?file=1032089#file1032089line25> > > > > dpkg-buildpackage -uc -b -d -tc > > > > also could automake the changelog with something like > > > > ``` > > dch -v $(cat .auroraversion)~${1} -u low --maintmaint \ > > "Apache Aurora package builder <d...@aurora.apache.org> $(date -R)" > > ``` > dpkg-buildpackage -uc -b -d -tc Thanks, great pointers! What's the motivation behind `-d`? `-d Do not check build dependencies and conflicts.` > dch Awesome! Done. On Aug. 5, 2015, 1:13 p.m., Bill Farner wrote: > > Thoughts on changing the layout to a similar pattern as what we use in our > > main repo? > > > > ``` > > ??? README.md > > ??? build-artifact.sh > > ??? build-support > > ? ??? jenkins > > ? ??? release > > ??? docs > > ??? packaging > > ??? deb > > ??? docker > > ? ??? centos > > ? ??? ubuntu > > ??? rpm > > ``` Re: directories/files not yet present - once we have content for them, sure. Re: `/packaging` - i avoided that simply because `aurora-packaging/packaging` seemed funky and redundant. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37107/#review94236 ----------------------------------------------------------- On Aug. 5, 2015, 1:21 a.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37107/ > ----------------------------------------------------------- > > (Updated Aug. 5, 2015, 1:21 a.m.) > > > Review request for Aurora, Benjamin Staffin, Maxim Khutornenko, and Steve > Salevan. > > > Bugs: AURORA-1410 > https://issues.apache.org/jira/browse/AURORA-1410 > > > Repository: aurora-packaging > > > Description > ------- > > There's definitely more work to be done here, but this is my proposed > structure. > > Noteworthy files/dirs: > - `/specs`: package build specifications > - `/builder`: build environments and scripts > - `/build-artifact.sh`: build tool and definition of the 'contract' that > builders must adhere to > > To make this work, i had to make some changes to the package defs we have: > - **deb and rpm**: reverted some post-0.9.0 build target changes, so i could > build 0.9.0 packages > - **rpm**: removed fetch/build of mesos to produce eggs, using our dists on > svn instead > > > **Note** - this is my first time playing with docker, `rpmbuild`, and > `dpkg-buildpackage`. Be gentle, but don't hesitate to point out mistakes i'm > making! > > > Diffs > ----- > > .gitignore PRE-CREATION > build-artifact.sh PRE-CREATION > builder/deb/ubuntu-trusty/Dockerfile PRE-CREATION > builder/deb/ubuntu-trusty/build.sh PRE-CREATION > builder/rpm/centos-7/Dockerfile PRE-CREATION > builder/rpm/centos-7/build.sh PRE-CREATION > specs/debian/pants.ini > specs/debian/rules d9814008e749c5dee3c6b0a7d0d852e123e29987 > specs/rpm/Makefile 1833a25f5040efdaf62b2c95c97b027acc09fef8 > specs/rpm/aurora.spec 5ec516f0cedaa5a1944c513d6303f4ce58d0bcd2 > > Diff: https://reviews.apache.org/r/37107/diff/ > > > Testing > ------- > > Successfully ran: > ``` > ./build-artifact.sh builder/deb/ubuntu-trusty > ~/Downloads/apache-aurora-0.9.0.tar.gz > ./build-artifact.sh builder/rpm/centos-7 > ~/Downloads/apache-aurora-0.9.0.tar.gz > ``` > > I have no idea if the rpms or debs produced _work_, but they are created. I > will follow up after actually trying to use them. > > > Thanks, > > Bill Farner > >
