code review 132590043: dashboard: optimize linux docker image size

dashboard: optimize linux docker image size

[proppy, 2014-09-05 01:24:00]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go.tools/

[proppy, 2014-09-05 01:28:34]

On 2014/09/05 01:24:00, proppy wrote:
> Hello mailto:golang-codereviews@googlegroups.com,
> 
> I'd like you to review this change to
> https://code.google.com/p/go.tools/

Before:
REPOSITORY          TAG                 IMAGE ID            CREATED            
VIRTUAL SIZE
bar                 latest              2a4fb04b434c        6 seconds ago      
536 MB

After:
REPOSITORY          TAG                 IMAGE ID            CREATED            
VIRTUAL SIZE
foo                 latest              df1c7989de7e        6 minutes ago      
402.1 MB

[bradfitz, 2014-09-05 01:48:09]

Nice. I'm kinda surprised we only lost 136 MB, though.

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
File dashboard/env/linux-x86-base/Dockerfile (right):

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
dashboard/env/linux-x86-base/Dockerfile:15: ENV GOPATH /gopath
this doesn't need to survive to the child.  can this just be in
build-go-builder.sh and then unset at the end of it?

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
dashboard/env/linux-x86-base/Dockerfile:16: ENV GOROOT /goroot
likewise.  I don't think we ever use this.  we specify a working path of "/"
explicitly, and then the builder looks at filepath.Join(that, "goroot")

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
File dashboard/env/linux-x86-base/scripts/build-go-builder.sh (right):

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
dashboard/env/linux-x86-base/scripts/build-go-builder.sh:11:
PREFIX=${PREFIX:-/usr/local}
who would set this otherwise?

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
dashboard/env/linux-x86-base/scripts/build-go-builder.sh:13: (cd $GO_TOOLS && hg
pull -r f9e37a924785 -u && GOBIN=$PREFIX/bin /goroot/bin/go install
code.google.com/p/go.tools/dashboard/builder)
maybe this f9e37a924785 should and the b8ff above should be ENV vars (perhaps
prefixed as DOCKER_BUILDER_VER and DOCKER_GO_FOR_BUILDER_VER with comments?) it
the outer file?

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
dashboard/env/linux-x86-base/scripts/build-go-builder.sh:15: rm -fR $GOROOT/bin
$GOROOT/pkg $GOPATH/bin $GOPATH/pkg
I think you can nuke all of $GOPATH, not just $GOPATH/pkg

[proppy, 2014-09-05 19:33:15]

PTAL

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
File dashboard/env/linux-x86-base/Dockerfile (right):

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
dashboard/env/linux-x86-base/Dockerfile:15: ENV GOPATH /gopath
On 2014/09/05 01:48:09, bradfitz wrote:
> this doesn't need to survive to the child.  can this just be in
> build-go-builder.sh and then unset at the end of it?

Done.

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
dashboard/env/linux-x86-base/Dockerfile:16: ENV GOROOT /goroot
On 2014/09/05 01:48:09, bradfitz wrote:
> likewise.  I don't think we ever use this.  we specify a working path of "/"
> explicitly, and then the builder looks at filepath.Join(that, "goroot")

Done.

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
File dashboard/env/linux-x86-base/scripts/build-go-builder.sh (right):

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
dashboard/env/linux-x86-base/scripts/build-go-builder.sh:11:
PREFIX=${PREFIX:-/usr/local}
On 2014/09/05 01:48:09, bradfitz wrote:
> who would set this otherwise?

Done.

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
dashboard/env/linux-x86-base/scripts/build-go-builder.sh:13: (cd $GO_TOOLS && hg
pull -r f9e37a924785 -u && GOBIN=$PREFIX/bin /goroot/bin/go install
code.google.com/p/go.tools/dashboard/builder)
On 2014/09/05 01:48:09, bradfitz wrote:
> maybe this f9e37a924785 should and the b8ff above should be ENV vars (perhaps
> prefixed as DOCKER_BUILDER_VER and DOCKER_GO_FOR_BUILDER_VER with comments?)
it
> the outer file?

Done.

https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
dashboard/env/linux-x86-base/scripts/build-go-builder.sh:15: rm -fR $GOROOT/bin
$GOROOT/pkg $GOPATH/bin $GOPATH/pkg
On 2014/09/05 01:48:09, bradfitz wrote:
> I think you can nuke all of $GOPATH, not just $GOPATH/pkg

Done.

[proppy, 2014-09-05 20:13:32]

On 2014/09/05 19:33:15, proppy wrote:
> PTAL
> 
>
https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
> File dashboard/env/linux-x86-base/Dockerfile (right):
> 
>
https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
> dashboard/env/linux-x86-base/Dockerfile:15: ENV GOPATH /gopath
> On 2014/09/05 01:48:09, bradfitz wrote:
> > this doesn't need to survive to the child.  can this just be in
> > build-go-builder.sh and then unset at the end of it?
> 
> Done.
> 
>
https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
> dashboard/env/linux-x86-base/Dockerfile:16: ENV GOROOT /goroot
> On 2014/09/05 01:48:09, bradfitz wrote:
> > likewise.  I don't think we ever use this.  we specify a working path of "/"
> > explicitly, and then the builder looks at filepath.Join(that, "goroot")
> 
> Done.
> 
>
https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
> File dashboard/env/linux-x86-base/scripts/build-go-builder.sh (right):
> 
>
https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
> dashboard/env/linux-x86-base/scripts/build-go-builder.sh:11:
> PREFIX=${PREFIX:-/usr/local}
> On 2014/09/05 01:48:09, bradfitz wrote:
> > who would set this otherwise?
> 
> Done.
> 
>
https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
> dashboard/env/linux-x86-base/scripts/build-go-builder.sh:13: (cd $GO_TOOLS &&
hg
> pull -r f9e37a924785 -u && GOBIN=$PREFIX/bin /goroot/bin/go install
> code.google.com/p/go.tools/dashboard/builder)
> On 2014/09/05 01:48:09, bradfitz wrote:
> > maybe this f9e37a924785 should and the b8ff above should be ENV vars
(perhaps
> > prefixed as DOCKER_BUILDER_VER and DOCKER_GO_FOR_BUILDER_VER with comments?)
> it
> > the outer file?
> 
> Done.
> 
>
https://codereview.appspot.com/132590043/diff/60002/dashboard/env/linux-x86-b...
> dashboard/env/linux-x86-base/scripts/build-go-builder.sh:15: rm -fR
$GOROOT/bin
> $GOROOT/pkg $GOPATH/bin $GOPATH/pkg
> On 2014/09/05 01:48:09, bradfitz wrote:
> > I think you can nuke all of $GOPATH, not just $GOPATH/pkg
> 
> Done.

PTAL, also udpated nacl and added check rule in Makefile.

[bradfitz, 2014-09-05 20:17:32]

LGTM once the tests all pass.

Also, wait for https://codereview.appspot.com/132650043 to go in first and then
update this CL to reference that CL's hg hash.

[proppy, 2014-09-05 22:31:41]

On 2014/09/05 20:17:32, bradfitz wrote:
> LGTM once the tests all pass.
> 
> Also, wait for https://codereview.appspot.com/132650043 to go in first and
then
> update this CL to reference that CL's hg hash.

PTAL nacl build were broken, I moved the symlink stuff to the shell script.

[proppy, 2014-09-05 22:46:09]

On 2014/09/05 22:31:41, proppy wrote:
> On 2014/09/05 20:17:32, bradfitz wrote:
> > LGTM once the tests all pass.
> > 
> > Also, wait for https://codereview.appspot.com/132650043 to go in first and
> then
> > update this CL to reference that CL's hg hash.
> 
> PTAL nacl build were broken, I moved the symlink stuff to the shell script.

so both linux and nacl builder pass the tests:
```
$ make check
docker build -t gobuilders/linux-x86-base .
...
docker run gobuilders/linux-x86-base /usr/local/bin/builder -rev=50ac9eded6ad
-buildroot=/ -v -report=false linux-amd64-temp
...
ALL TESTS PASSED
```

```

$ make check
docker build -t gobuilders/linux-x86-nacl .
...
docker run gobuilders/linux-x86-nacl /usr/local/bin/builder -rev=59b1bb4bf045
-buildroot=/ -v -cmd=/usr/local/bin/build-command.pl -report=false
nacl-amd64p32-temp
...
ALL TESTS PASSED
...
```

Should I use the same base CL for the linux and nacl check rule?

[bradfitz, 2014-09-05 22:53:07]

Yeah, both should use the same Go (1.3.1's hash?) and same builder (latest).



On Fri, Sep 5, 2014 at 3:46 PM, <proppy@google.com> wrote:

> On 2014/09/05 22:31:41, proppy wrote:
>
>> On 2014/09/05 20:17:32, bradfitz wrote:
>> > LGTM once the tests all pass.
>> >
>> > Also, wait for https://codereview.appspot.com/132650043 to go in
>>
> first and
>
>> then
>> > update this CL to reference that CL's hg hash.
>>
>
>  PTAL nacl build were broken, I moved the symlink stuff to the shell
>>
> script.
>
> so both linux and nacl builder pass the tests:
> ```
> $ make check
> docker build -t gobuilders/linux-x86-base .
> ...
> docker run gobuilders/linux-x86-base /usr/local/bin/builder
> -rev=50ac9eded6ad -buildroot=/ -v -report=false linux-amd64-temp
> ...
> ALL TESTS PASSED
> ```
>
> ```
>
> $ make check
> docker build -t gobuilders/linux-x86-nacl .
> ...
> docker run gobuilders/linux-x86-nacl /usr/local/bin/builder
> -rev=59b1bb4bf045 -buildroot=/ -v -cmd=/usr/local/bin/build-command.pl
> -report=false nacl-amd64p32-temp
> ...
> ALL TESTS PASSED
> ...
> ```
>
> Should I use the same base CL for the linux and nacl check rule?
>
> https://codereview.appspot.com/132590043/
>

[proppy, 2014-09-05 23:22:09]

On 2014/09/05 22:53:07, bradfitz wrote:
> Yeah, both should use the same Go (1.3.1's hash?) and same builder (latest).
> 
> 
> 
> On Fri, Sep 5, 2014 at 3:46 PM, <mailto:proppy@google.com> wrote:
> 
> > On 2014/09/05 22:31:41, proppy wrote:
> >
> >> On 2014/09/05 20:17:32, bradfitz wrote:
> >> > LGTM once the tests all pass.
> >> >
> >> > Also, wait for https://codereview.appspot.com/132650043 to go in
> >>
> > first and
> >
> >> then
> >> > update this CL to reference that CL's hg hash.
> >>
> >
> >  PTAL nacl build were broken, I moved the symlink stuff to the shell
> >>
> > script.
> >
> > so both linux and nacl builder pass the tests:
> > ```
> > $ make check
> > docker build -t gobuilders/linux-x86-base .
> > ...
> > docker run gobuilders/linux-x86-base /usr/local/bin/builder
> > -rev=50ac9eded6ad -buildroot=/ -v -report=false linux-amd64-temp
> > ...
> > ALL TESTS PASSED
> > ```
> >
> > ```
> >
> > $ make check
> > docker build -t gobuilders/linux-x86-nacl .
> > ...
> > docker run gobuilders/linux-x86-nacl /usr/local/bin/builder
> > -rev=59b1bb4bf045 -buildroot=/ -v -cmd=/usr/local/bin/build-command.pl
> > -report=false nacl-amd64p32-temp
> > ...
> > ALL TESTS PASSED
> > ...
> > ```
> >
> > Should I use the same base CL for the linux and nacl check rule?
> >
> > https://codereview.appspot.com/132590043/
> >

PTAL updated revs

[bradfitz, 2014-09-08 22:35:33]

LGTM


On Fri, Sep 5, 2014 at 4:22 PM, <proppy@google.com> wrote:

> On 2014/09/05 22:53:07, bradfitz wrote:
>
>> Yeah, both should use the same Go (1.3.1's hash?) and same builder
>>
> (latest).
>
>
>
>  On Fri, Sep 5, 2014 at 3:46 PM, <mailto:proppy@google.com> wrote:
>>
>
>  > On 2014/09/05 22:31:41, proppy wrote:
>> >
>> >> On 2014/09/05 20:17:32, bradfitz wrote:
>> >> > LGTM once the tests all pass.
>> >> >
>> >> > Also, wait for https://codereview.appspot.com/132650043 to go in
>> >>
>> > first and
>> >
>> >> then
>> >> > update this CL to reference that CL's hg hash.
>> >>
>> >
>> >  PTAL nacl build were broken, I moved the symlink stuff to the shell
>> >>
>> > script.
>> >
>> > so both linux and nacl builder pass the tests:
>> > ```
>> > $ make check
>> > docker build -t gobuilders/linux-x86-base .
>> > ...
>> > docker run gobuilders/linux-x86-base /usr/local/bin/builder
>> > -rev=50ac9eded6ad -buildroot=/ -v -report=false linux-amd64-temp
>> > ...
>> > ALL TESTS PASSED
>> > ```
>> >
>> > ```
>> >
>> > $ make check
>> > docker build -t gobuilders/linux-x86-nacl .
>> > ...
>> > docker run gobuilders/linux-x86-nacl /usr/local/bin/builder
>> > -rev=59b1bb4bf045 -buildroot=/ -v
>>
> -cmd=/usr/local/bin/build-command.pl
>
>> > -report=false nacl-amd64p32-temp
>> > ...
>> > ALL TESTS PASSED
>> > ...
>> > ```
>> >
>> > Should I use the same base CL for the linux and nacl check rule?
>> >
>> > https://codereview.appspot.com/132590043/
>> >
>>
>
> PTAL updated revs
>
> https://codereview.appspot.com/132590043/
>

[bradfitz, 2014-09-08 22:35:43]

*** Submitted as
https://code.google.com/p/go/source/detail?r=5001f42c2fca&repo=tools ***

dashboard: optimize linux docker image size

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://codereview.appspot.com/132590043

Committer: Brad Fitzpatrick <bradfitz@golang.org>