960201 - Implement ghetto "gaia-try" by allowing test jobs to operate on arbitrary gaia.json

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Jonathan Griffin (:jgriffin)]

Since we've agreed to push back implementation of a robust gaia-try until the TaskCluster project is up and running (see https://bugzilla.mozilla.org/show_bug.cgi?id=914632#c9), we've had a couple of developers request that we implement a kind of ghetto gaia-try as a temporary hack, in which test jobs on try could operate on arbitrary gaia.json files that may point to github branches.

I believe the implementation cost of this would be fairly low.  We need:

1 - sec review on opening up try server jobs to github.  Done!  See bug 914651.
2 - opening network flows from try to github (if not already available)
3 - git installed on ubuntu VM slves.  Done!  See bug 914627.
4 - updating the mozharness scripts that run gaia test jobs to be able to handle additional options in gaia.json.

If we agree to only support tests on b2g desktop builds, we would not need to change the build jobs at all, since the test jobs do not use gaia from the build products, but rather generate their own gaia profile.

Are there any objections or obstacles to this plan?

Comment 1

[[e-mail] (:bhearsum)]

https://bugzilla.mozilla.org/show_bug.cgi?id=914651#c3 reads to me like OpSec signed off on try jobs pulling from github.com/mozilla repositories, but not arbitrary other repos (based on the comments about committer agreement/level 1). If that's the case, I think we need some wrapper around the code that clones gaia to enforce that.

Comment 2

[Jonathan Griffin (:jgriffin)]

You are possibly correct, although I should note that this kind of gaia-try would still require level 1 commit access to mozilla's hg try; you couldn't initiate a try run just by changing something in github alone.

Limiting this to github.com/mozilla(-b2g) would very much reduce its usefulness, and I don't think it really adds any meaningful security, since nothing happens without pushing to try anyway.

Comment 3

[hwine]

(In reply to Jonathan Griffin (:jgriffin) from comment #0)
> I believe the implementation cost of this would be fairly low.  We need:
> 
> 1 - sec review on opening up try server jobs to github.  Done!  See bug
> 914651.

That bug has been reopened to clarify which machines need the github access.

Comment 4

[Jonathan Griffin (:jgriffin)]

Attachment: Add ghetto gaia try support

This is a ghetto patch for ghetto try!  I'm going to play around with it on ash.

Comment 5

[Jonathan Griffin (:jgriffin)]

This works like a charm on linux slaves; we'll see if it runs on osx too.

Comment 6

[Jonathan Griffin (:jgriffin)]

Attachment: Add git support to gaia.json

Fixed a minor error that I saw on ash.

Comment 7

[Jonathan Griffin (:jgriffin)]

Attachment: Add git dict to gaia.json for gaia try,

This changes gaia.json to include an optional gaia dict; to use it for gaia try, you would specify a remote and a branch OR revision, like so:

{
    revision: 621040ee199cdde3bdd2aeaf2a016ff7806875bb, 
    repo_path: /integration/gaia-central,
    git: {
        remote: https://github.com/jonallengriffin/gaia.git,
        branch: gaia_unit_crash,
        revision: 
    }
}

The limitations are that this will only affect gaia tests (gaia-integration, gaia-unit, and gaia-ui) on b2g desktop builds.  It won't be honored for legacy tests (like mochitests) or any tests running on emulators.  We can expand this later to work with those if we think we need to.

Comment 8

[Jonathan Griffin (:jgriffin)]

Attachment: Add git dict to gaia.json for gaia try,

Er, that was a patch against ash, here's the correct one.

Comment 9

[Jonathan Griffin (:jgriffin)]

I'll need to update bump_gaia_json.py as well.

Comment 10

[Jonathan Griffin (:jgriffin)]

Attachment: Add git support to gaia.json

This adds mozharness support for ghetto gaia try.  Ideally, we'd make a git VCS class to support git cloning, but until then, this approach should work good enough for try (I hope).  Tested fine on ash.  See for instance:

https://tbpl.mozilla.org/php/getParsedLog.php?id=34176647&tree=Ash&full=1

(look for 'Running pull step')

Trying not to add a review to aki's queue right before he leaves for PTO, but feel free to redirect if this should go elsewhere.

Comment 11

[[e-mail] (:bhearsum)]

Comment on attachment 8371987 [details] [diff] [review]
Add git support to gaia.json

Review of attachment 8371987 [details] [diff] [review]:
-----------------------------------------------------------------

Seems fine for the most part - do you have any logs from sample runs that I can have a look at it?

::: mozharness/mozilla/gaia.py
@@ +51,5 @@
>  
> +            # git clone
> +            git_cmd = self.query_exe('git')
> +            cmd = [git_cmd,
> +                   'clone',

Why are you using plain git instead of vcs_checkout? You can override its default vcs by passing vcs='git' (https://hg.mozilla.org/build/mozharness/file/5af0139c7e39/mozharness/base/vcs/vcsbase.py#l54).

Comment 12

[Jonathan Griffin (:jgriffin)]

(In reply to Ben Hearsum [:bhearsum] from comment #11)
> Comment on attachment 8371987 [details] [diff] [review]
> Add git support to gaia.json
> 
> Review of attachment 8371987 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Seems fine for the most part - do you have any logs from sample runs that I
> can have a look at it?

Yes, see https://tbpl.mozilla.org/php/getParsedLog.php?id=34176647&tree=Ash&full=1

> 
> ::: mozharness/mozilla/gaia.py
> @@ +51,5 @@
> >  
> > +            # git clone
> > +            git_cmd = self.query_exe('git')
> > +            cmd = [git_cmd,
> > +                   'clone',
> 
> Why are you using plain git instead of vcs_checkout? You can override its
> default vcs by passing vcs='git'
> (https://hg.mozilla.org/build/mozharness/file/5af0139c7e39/mozharness/base/
> vcs/vcsbase.py#l54).

Because we don't have any git class in mozharness.  We have only gittool, but that's for git shares which aren't appropriate here, AFAICT.  In order to use vcs='git', we'd need a GitVCS class that's the equivalent of http://hg.mozilla.org/build/mozharness/file/5af0139c7e39/mozharness/base/vcs/mercurial.py, and I didn't want to write one just for this use case.  But we eventually should!

Comment 13

[[e-mail] (:bhearsum)]

Comment on attachment 8371987 [details] [diff] [review]
Add git support to gaia.json

(In reply to Jonathan Griffin (:jgriffin) from comment #12)
> (In reply to Ben Hearsum [:bhearsum] from comment #11)
> > Comment on attachment 8371987 [details] [diff] [review]
> > Add git support to gaia.json
> > 
> > Review of attachment 8371987 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Seems fine for the most part - do you have any logs from sample runs that I
> > can have a look at it?
> 
> Yes, see
> https://tbpl.mozilla.org/php/getParsedLog.php?id=34176647&tree=Ash&full=1
> 
> > 
> > ::: mozharness/mozilla/gaia.py
> > @@ +51,5 @@
> > >  
> > > +            # git clone
> > > +            git_cmd = self.query_exe('git')
> > > +            cmd = [git_cmd,
> > > +                   'clone',
> > 
> > Why are you using plain git instead of vcs_checkout? You can override its
> > default vcs by passing vcs='git'
> > (https://hg.mozilla.org/build/mozharness/file/5af0139c7e39/mozharness/base/
> > vcs/vcsbase.py#l54).
> 
> Because we don't have any git class in mozharness.  We have only gittool,
> but that's for git shares which aren't appropriate here, AFAICT.  In order
> to use vcs='git', we'd need a GitVCS class that's the equivalent of
> http://hg.mozilla.org/build/mozharness/file/5af0139c7e39/mozharness/base/vcs/
> mercurial.py, and I didn't want to write one just for this use case.  But we
> eventually should!

Per IRC, gittool doesn't help because gaia will come from various different sources, and gittool isn't smart enough to combine them.

I looked over your test run and it seems good. Ship it!

Comment 14

[Jonathan Griffin (:jgriffin)]

Will land on Monday (so I can monitor for bustage).

Comment 15

[Jonathan Griffin (:jgriffin)]

Attachment: Add git support to gaia.json

New version of the patch, which only does an hg purge of the 'gaia' dir if an .hg repo exists, otherwise it just deletes the directory.

This change is needed because sometimes the 'gaia' dir from a previous test run will be an hg repo, and sometimes a github one.  This should also fix bug 970166, which I believe is caused by my experiments running the first version of this patch on ash.

Comment 16

[[e-mail] (:bhearsum)]

Comment on attachment 8373724 [details] [diff] [review]
Add git support to gaia.json

Review of attachment 8373724 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozharness/mozilla/gaia.py
@@ +46,5 @@
>                  branch = None
>  
> +        if git:
> +            if os.path.exists(dest):
> +                self.rmtree(dest)

Can we do something smarter here for git, too, and only clobber if it isn't already a git dir? Or does that not make sense?

@@ +74,5 @@
> +
> +        else:
> +            # purge the repo if it already exists
> +            if os.access(dest, os.F_OK):
> +                if os.access(os.path.join(dest, '.hg'), os.F_OK):

Just curious - is there a reason you're using os.access instead of os.path.exists?

Comment 17

[Jonathan Griffin (:jgriffin)]

(In reply to Ben Hearsum [:bhearsum] from comment #16)
> Comment on attachment 8373724 [details] [diff] [review]
> Add git support to gaia.json
> 
> Review of attachment 8373724 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mozharness/mozilla/gaia.py
> @@ +46,5 @@
> >                  branch = None
> >  
> > +        if git:
> > +            if os.path.exists(dest):
> > +                self.rmtree(dest)
> 
> Can we do something smarter here for git, too, and only clobber if it isn't
> already a git dir? Or does that not make sense?

We normally will want to clobber anyway, since the target will likely be a different github repo, even if it's git.  But, we could skip the clobber if it's a git dir and the remote is the same.  I wouldn't expect that to happen often...it will only occur on repeated try runs off the same user repo if we happen to get the same slave.

> 
> @@ +74,5 @@
> > +
> > +        else:
> > +            # purge the repo if it already exists
> > +            if os.access(dest, os.F_OK):
> > +                if os.access(os.path.join(dest, '.hg'), os.F_OK):
> 
> Just curious - is there a reason you're using os.access instead of
> os.path.exists?

No reason, I can change it.

Comment 18

[[e-mail] (:bhearsum)]

Comment on attachment 8373724 [details] [diff] [review]
Add git support to gaia.json

Review of attachment 8373724 [details] [diff] [review]:
-----------------------------------------------------------------

Given your comments, I'm okay with this as is. Feel free to address either of the things mentioned though.

Comment 19

[Jonathan Griffin (:jgriffin)]

Attachment: Add git support to gaia.json

Addressing both review comments; will test this on ash before landing.

Comment 20

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8374284 [details] [diff] [review]
Add git support to gaia.json

Review of attachment 8374284 [details] [diff] [review]:
-----------------------------------------------------------------

Carry r+ forward.

Comment 21

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/build/mozharness/rev/754b925ab7b8

Comment 22

[Jonathan Griffin (:jgriffin)]

Pushed a follow-up fix for a typo (thanks aki!): http://hg.mozilla.org/build/mozharness/rev/f3e61d3d1ee2

Comment 23

[Jonathan Griffin (:jgriffin)]

Marking as fixed even though there's still an open dependency, as it isn't a strict blocker.