Closed Bug 1138989 Opened 6 years ago Closed 6 years ago

Integrate Autoland testing with version-control-tools test framework

Categories

(Conduit :: Transplant, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dminor, Assigned: dminor)

Details

(Whiteboard: [autoland M2])

Attachments

(6 files, 1 obsolete file)

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.
Attached file MozReview Request: bz://1138989/dminor (obsolete) —
/r/4545 - autoland: move autoland docker images to vcttesting framework (bug 1138989); r=gps

Pull down this commit:

hg pull review -r 1c64211e998b8bd8a4805b152857bae43032f6c0
Attachment #8572015 - Flags: review?(gps)
Assignee: nobody → dminor
Status: NEW → ASSIGNED
Attachment #8572015 - Flags: review?(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?
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.
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 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
Attachment #8572015 - Flags: review?(gps)
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
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 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
Attachment #8572015 - Flags: review?(gps) → review+
Comment on attachment 8572015 [details]
MozReview Request: bz://1138989/dminor

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

Ship It!
https://hg.mozilla.org/hgcustom/version-control-tools/rev/13287eb8684d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Attachment #8572015 - Attachment is obsolete: true
Attachment #8619644 - Flags: review+
Attachment #8619645 - Flags: review+
Attachment #8619646 - Flags: review+
Attachment #8619647 - Flags: review+
Attachment #8619648 - Flags: review+
Attachment #8619649 - Flags: review+
Product: Tree Management → MozReview
Product: MozReview → Conduit
You need to log in before you can comment on or make changes to this bug.