Use robustclone and modern mercurial when cloning from interactive loaner

RESOLVED FIXED in Firefox 51

Status

defect
P1
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: ahal, Assigned: ahal)

Tracking

(Blocks 1 bug)

Version 3
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Currently the wizard will simply shell out to 'hg'. We should instead be using the new docker recipes that gps added.
The above patch should get the images set up, I believe. But I haven't been able to test it out yet due to an error when building the parent images (we still need to push those to docker hub manually):

[~/hg/mozilla-central]$ sudo ./mach taskcluster-build-image ubuntu1204-test
Sending build context to Docker daemon 18.94 kB
Step 1 : FROM ubuntu:12.04
Trying to pull repository docker.io/library/ubuntu ... 
12.04: Pulling from docker.io/library/ubuntu

765826873799: Pull complete 
e7a187926114: Pull complete 
fd01d4f3de3b: Pull complete 
c704fce22a3c: Pull complete 
Digest: sha256:45bf6eb4403c7171dc497a7105c2ef3acd12f16b4b80256b067fc2a8183c6348
Status: Downloaded newer image for docker.io/ubuntu:12.04
 ---> 60df6678b255
Step 2 : MAINTAINER Jonas Finnemann Jensen <jopsen@gmail.com>
 ---> Running in 0b70447d8ae2
 ---> dacb69c82b99
Removing intermediate container 0b70447d8ae2
Step 3 : RUN useradd -d /home/worker -s /bin/bash -m worker
 ---> Running in ec6a02e09132
The command '/bin/sh -c useradd -d /home/worker -s /bin/bash -m worker' returned a non-zero code: 12
Traceback (most recent call last):
  File "/home/ahal/hg/mozilla-central/taskcluster/mach_commands.py", line 268, in build_image
    build_image(image_name)
  File "/home/ahal/hg/mozilla-central/taskcluster/taskgraph/docker.py", line 100, in build_image
    docker.build_from_context(docker_bin, context_path, name, tag)
  File "/home/ahal/hg/mozilla-central/taskcluster/taskgraph/util/docker.py", line 157, in build_from_context
    raise Exception('error building image')
Exception: error building image
Gps already had a bug for adding install-mercurial.sh and run-task to the desktop-test image. I won't obsolete this patch just yet though, as it contains some things that are still missing over there. Such as updating ubuntu1604-test and adding the 'run-task' recipe.
Depends on: 1291365
Priority: -- → P1
The dependent bug got resolved. What are the next steps for this bug?
I think the image changes are in place. I just haven't converted tests to run from `run-task` yet. But I don't think that blocks this.
The next step is to write the patch :).

Note, this bug doesn't have anything to do with running the tests. It's more about choosing option 3 (clone gecko) from the wizard on an interactive loaner. It's definitely low priority, so I haven't worked on it. But the patch should be easy, I should just do it.
Attachment #8778284 - Attachment is obsolete: true
Comment on attachment 8789032 [details]
Bug 1292561 - Use robustcheckout from the wizard on interactive loaners,

https://reviewboard.mozilla.org/r/77306/#review75614

::: taskcluster/scripts/tester/run-wizard:80
(Diff revision 1)
> -    repo = os.environ['GECKO_HEAD_REPOSITORY']
> -    rev = os.environ['GECKO_HEAD_REV']
> -    clone_path = os.path.expanduser(os.path.join('~', 'gecko'))
> -
> -    # try is too large to clone, instead clone central and pull
> -    # in changes from try
> -    if "hg.mozilla.org/try" in repo:
> -        central = 'http://hg.mozilla.org/mozilla-central'
> -        call(['hg', 'clone', '-U', central, clone_path])
> -        call(['hg', 'pull', '-u', '-r', rev, repo], cwd=clone_path)
> +    base_repo = os.environ['GECKO_HEAD_REPOSITORY'].rstrip('/')
> +    dest = os.path.expanduser(os.path.join('~', 'gecko'))
> +
> +    # Switch to mozilla-unified because robustcheckout works best with it.
> +    if base_repo.rsplit('/', 1)[1] in ('mozilla-central', 'try'):
> +        base_repo = b'https://hg.mozilla.org/mozilla-unified'
> +
> +    # Specify method to checkout a revision. This defaults to revisions as
> +    # SHA-1 strings, but also supports symbolic revisions like `tip` via the
> +    # branch flag.

This code is mostly copied from the "run-task" binary. It might be worthwhile to pull this out into a common library, but I wanted to avoid making a new binary as that would be 3 layers of subprocesses. For now I just took the lazy route with some duplication.

Also not related to this patch, but I think the 'run-task' binary should also switch to mozilla-unified if base_repo is 'try'.
Comment on attachment 8789032 [details]
Bug 1292561 - Use robustcheckout from the wizard on interactive loaners,

https://reviewboard.mozilla.org/r/77306/#review75634

This is mostly good.

I'm fine duplicating the code for now because when we eventually have checkouts on the test tasks we can presumably use the same code for putting the repo in place. Although, since you won't be using run-task, I guess that makes it a bit harder. Hmmm. Still, I'm not inclined to split up run-task into Python modules just yet. I'd like to see how long we can keep it an isolated and minimal script...

::: taskcluster/scripts/tester/run-wizard:84
(Diff revision 1)
> -    # try is too large to clone, instead clone central and pull
> -    # in changes from try
> +    if base_repo.rsplit('/', 1)[1] in ('mozilla-central', 'try'):
> +        base_repo = b'https://hg.mozilla.org/mozilla-unified'

Assuming this code will only run from a Firefox repo, I think we should **always** use the mozilla-unified repo. The simple reason is it will clone faster because it is smaller.

(This is a side-effect of how the repos are currently encoded on the server. Some day I hope to re-encode mozilla-central and friends to make them smaller.)

::: taskcluster/scripts/tester/run-wizard:100
(Diff revision 1)
> +        revision = os.environ['GECKO_HEAD_REF']
>      else:
> -        call(['hg', 'clone', '-u', rev, repo, clone_path])
> -    print("Finished cloning to {} at revision {}.".format(
> -                clone_path, rev))
> +        print('revision is not specified for checkout')
> +        return 1
> +
> +    call([

I landed some code on autoland earlier today that adds certificate fingerprint pinning for extra protection. We need at least a TODO to track adding that.

If you decide to implement it, you'll need to define the "taskclusterProxy" feature on the task and the task will need a scope to obtain the TC secret. See https://hg.mozilla.org/integration/autoland/rev/22093eae5f63d2a81669b7c8cd0d4fe7ae372147, https://hg.mozilla.org/integration/autoland/rev/d7ba1c09a00adf69dce37833887b28bf03926ae2, and https://hg.mozilla.org/integration/autoland/rev/b82d339b725b0a0cbe01bf609b13d2cc6a303755.

Since the fingerprint pinning is really a paranoia thing and since this script is only for running interactive wizards, a TODO is probably fine.
Attachment #8789032 - Flags: review?(gps) → review+
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9abe6825c426
Use robustcheckout from the wizard on interactive loaners, r=gps
https://hg.mozilla.org/mozilla-central/rev/9abe6825c426
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.