aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 37107: Add a tool and environment for building Aurora packages.
Date Wed, 05 Aug 2015 14:46:56 GMT


> 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 <dev@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 <dev@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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message