1138989 - Integrate Autoland testing with version-control-tools test framework

Status: RESOLVED FIXED

(Conduit Graveyard :: Transplant, defect, P1)

Description

[Dan Minor [:dminor]]

Autoland is now located in the version-control-tools repository, but maintains a separate testing infrastructure.

It should be integrated with the vct test framework, which among other things, will make it easier to test interactions between mozreview and autoland.

Comment 1

[Dan Minor [:dminor]]

Attachment: MozReview Request: bz://1138989/dminor

/r/4545 - autoland: move autoland docker images to vcttesting framework (bug 1138989); r=gps

Pull down this commit:

hg pull review -r 1c64211e998b8bd8a4805b152857bae43032f6c0

Comment 2

[Gregory Szorc [:gps]]

Comment on attachment 8572015 [details]
MozReview Request: bz://1138989/dminor

https://reviewboard.mozilla.org/r/4543/#review3757

This code is mostly solid.

My main concern is about performance. I went through hell to ensure Apache and MySQL for BMO didn't consume more memory than necessary. I suspect the stock Apache and Postgres settings in this patch will add considerable resource utilization to tests.

Along that vein, I'm not keen on running the Autoland containers for every test. There are many tests that won't touch the Autoland service, so running the containers is just wasteful. Perhaps we should have running these containers behind an environment variable which is only set on tests that actually touch the Autoland service? This could be a follow-up (perhaps as part of the same series).

::: testing/docker/builder-autoland/Dockerfile
(Diff revision 1)
> +CMD ${AUTOLAND_HOME}/start-autoland.sh

It is recommended to use ["arg0", "arg1"] for CMD so shells don't get involved.

::: testing/docker/builder-autolanddb/Dockerfile
(Diff revision 1)
> +FROM postgres:latest

The version should be pinned so things don't get out of sync. IIRC "latest" evaluates to whatever "latest" was the last time someone did a `docker pull`. So, this won't upgrade versions until the builder is rebuilt.

::: testing/vcttesting/docker.py
(Diff revision 1)
> +                    links=[],
> +                    port_bindings={})

I don't think you need to explicitly declare empty values as those are the default.

::: testing/vcttesting/docker.py
(Diff revision 1)
> +                    binds={os.path.abspath(os.path.dirname(self._state_path)):
> +                           '/mnt/mozreview'},

This scares me a bit. Nowhere else do we mount things from the host into the containers.

What's the justification for doing this?

Comment 3

[Dan Minor [:dminor]]

https://reviewboard.mozilla.org/r/4543/#review3765

I investigated the memory use of the autolanddb and autoland containers: autolanddb shows a rss of 2277376 and autoland a rss of 11051008. For comparison, bmodb has a rss of 35880960 and bmoweb has a rss of 163704832. These numbers are from the /sys/fs/cgroup/memory/docker/<id>/memory.stat file.

As a quick and non-scientific experiment on my laptop, running:
time ./run-mercurial-tests.py -j 2 hgext/reviewboard/tests/*.t

Takes 8m55.604s with 3 containers and 10m38.764s with 5 containers.

Comment 4

[Dan Minor [:dminor]]

https://reviewboard.mozilla.org/r/4543/#review3767

> This scares me a bit. Nowhere else do we mount things from the host into the containers.
> 
> What's the justification for doing this?

I use this to be able to write the autoland and apache logs to the mozreview "instance" directory. This is handy for debugging but if you'd prefer to not open this particular can of worms I can remove it (and just hack it back in temporarily if I need it for something.)

Comment 5

[Dan Minor [:dminor]]

Comment on attachment 8572015 [details]
MozReview Request: bz://1138989/dminor

/r/4545 - autoland: move autoland docker images to vcttesting framework (bug 1138989); r=gps
/r/4627 - Add autoland port as argument to mozreview start mach command
/r/4629 - Move remaining autoland docker files to testing
/r/4631 - Address review comments

Pull down these commits:

hg pull review -r 67516d0be5e23ca1e93b47801105b7d173bd4f2a

Comment 6

[Dan Minor [:dminor]]

Comment on attachment 8572015 [details]
MozReview Request: bz://1138989/dminor

/r/4545 - autoland: move autoland docker images to vcttesting framework (bug 1138989); r=gps
/r/4627 - Add autoland port as argument to mozreview start mach command
/r/4629 - Move remaining autoland docker files to testing
/r/4631 - Address review comments

Pull down these commits:

hg pull review -r 67516d0be5e23ca1e93b47801105b7d173bd4f2a

Comment 7

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/4627/#review3805

Ship It!

Comment 8

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/4629/#review3807

Ship It!

Comment 9

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/4631/#review3809

::: testing/vcttesting/docker.py
(Diff revision 1)
>                      binds={os.path.abspath(os.path.dirname(self._state_path)):
>                             '/mnt/mozreview'},

You forgot to clean up this reference to the mount point.

Comment 10

[Dan Minor [:dminor]]

Comment on attachment 8572015 [details]
MozReview Request: bz://1138989/dminor

/r/4545 - autoland: move autoland docker images to vcttesting framework (bug 1138989); r=gps
/r/4627 - Add autoland port as argument to mozreview start mach command
/r/4629 - Move remaining autoland docker files to testing
/r/4631 - Address review comments
/r/4721 - Only start autoland instances if autoland port is specified
/r/4723 - Remove volume binding

Pull down these commits:

hg pull review -r 4812fac60315df6dd9df539e5d4c5ea300272210

Comment 11

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/4721/#review3871

Ship It!

Comment 12

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/4723/#review3873

Ship It!

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8572015 [details]
MozReview Request: bz://1138989/dminor

https://reviewboard.mozilla.org/r/4543/#review3875

Ship It!

Comment 14

[Dan Minor [:dminor]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/13287eb8684d

Comment 16

[Dan Minor [:dminor]]

Attachment: MozReview Request: autoland: move autoland docker images to vcttesting framework (bug 1138989); r=gps

Comment 17

[Dan Minor [:dminor]]

Attachment: MozReview Request: Remove volume binding

Comment 18

[Dan Minor [:dminor]]

Attachment: MozReview Request: Only start autoland instances if autoland port is specified

Comment 19

[Dan Minor [:dminor]]

Attachment: MozReview Request: Add autoland port as argument to mozreview start mach command

Comment 20

[Dan Minor [:dminor]]

Attachment: MozReview Request: Move remaining autoland docker files to testing

Comment 21

[Dan Minor [:dminor]]

Attachment: MozReview Request: Address review comments