Closed Bug 1316077 Opened 8 years ago Closed 7 years ago

Support buildbot-bridge in gecko tree config

Categories

(Taskcluster :: Services, defect, P1)

Product:
Taskcluster
Component:
Services
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla53

People

(Reporter: wcosta, Assigned: wcosta)

References

Details

Attachments

(3 files)

Bug 1316077 part 1: Support buildbot-bridge for talos tests.
58 bytes, text/x-review-board-request
dustin
: review+
Details
Bug 1316077 part 2: remove taskcluster scheduler from mozharness.
58 bytes, text/x-review-board-request
jlund
: review+
Details
Bug 1316077 part 3: Add in-tree config support to taskcluster_helper.
58 bytes, text/x-review-board-request
jlund
: review+
Details 
Add buildbot-bridge as a worker in the in tree configuration.
Comment on attachment 8812051 [details]
Bug 1316077 part 2: remove taskcluster scheduler from mozharness.

https://reviewboard.mozilla.org/r/93962/#review94118

::: testing/mozharness/mozharness/mozilla/taskcluster_helper.py:187
(Diff revision 1)
> -
>      def find_parent_task_id(self, task_id):
>          """ Returns the task_id of the parent task associated to the given task_id."""
>          # Find group id to associated to all related tasks
> -        task_group_id = self.get_task(task_id)['taskGroupId']
> -
> +        task = self.load_json_url(urljoin(self.QUEUE_URL, task_id))
> +        return task['dependencies'][0]

should we check/warn if len(task['dependencies']) > 1?
Comment on attachment 8812052 [details]
Bug 1316077 part 3: Add in-tree config support to taskcluster_helper.

https://reviewboard.mozilla.org/r/93964/#review94120

::: testing/mozharness/mozharness/mozilla/taskcluster_helper.py:257
(Diff revision 1)
>                  self.set_bbb_artifacts(
>                      task_id=parent_id,
>                      properties_file_path='public/build/buildbot_properties.json'
>                  )
> -
> +        # Case B: intree config
> +        elif 'installer_path' in properties:

I wonder if we could reduce the 4 condition blocks in set_parent_artficts down to 2. That would reduce a lot of the code duplication.

e.g.

```
# bad psuedo code
parent_id = properties.get('parent_task_id', self.find_parent_task_id(child_task_id))

installer_path = properties.get('installer_path', self.get_task(parent_id)['extra'].get('locations', {}).get('build'))

if installer_path:
    # use self.set_artifacts
else:
    # use self.set_bbb_artfiacts
```

what do you think?
Comment on attachment 8812051 [details]
Bug 1316077 part 2: remove taskcluster scheduler from mozharness.

https://reviewboard.mozilla.org/r/93962/#review94122
Attachment #8812051 - Flags: review?(jlund) → review+
Comment on attachment 8812052 [details]
Bug 1316077 part 3: Add in-tree config support to taskcluster_helper.

https://reviewboard.mozilla.org/r/93964/#review94124

just so it's clear I'm finished reviewing, I'm r- for now. feel free to push back and prove me ignorant :)
Attachment #8812052 - Flags: review?(jlund) → review-
Comment on attachment 8812050 [details]
Bug 1316077 part 1: Support buildbot-bridge for talos tests.

https://reviewboard.mozilla.org/r/93960/#review94204

I like this!  It's a very concise, clear implementation of something that I expected to be horrendously complex.

I think these test tasks will still have `extra.treeherder`, which will cause tc-treeherder to show them in treeherder in addition to the usual mechanism for buildbot jobs to appear in treeherder.  It would probably be sufficient to just remove `extra.treeherder` in `buildbot_bridge_setup`.

We will eventually want to control, on a suite-by-suite basis, whether a task runs in BBB or not.  Maybe `set_worker_implementation` should look at a test parameter to determine whether to use BBB?

Anyway, r+ for this (with typo fixed), as long as the talos jobs don't run by default when they land -- I think `run-on-projects: []` should do the trick.  My other two points will make good followup patches.

::: taskcluster/taskgraph/transforms/tests/make_task_description.py:472
(Diff revision 2)
> +    branch = config.params['project']
> +    platform, build_type = test['build-platform'].split('/')
> +    test_name = test.get('talos-try-name', test['test-name'])
> +    mozharness = test['mozharness']
> +
> +    if test['e10s'] and not test_name.endswith('-e10z'):

s/z/s/ (typo)
Attachment #8812050 - Flags: review?(dustin) → review+
Blocks: 1311814
Comment on attachment 8812052 [details]
Bug 1316077 part 3: Add in-tree config support to taskcluster_helper.

https://reviewboard.mozilla.org/r/93964/#review94656

awesome! thanks for patching. Case 1 and B are the same lines of code but I think this is a large improvement.
Attachment #8812052 - Flags: review?(jlund) → review+
Pushed by wcosta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/55c69395d608
part 1: Support buildbot-bridge for talos tests. r=dustin
https://hg.mozilla.org/integration/autoland/rev/3c5e7423e4eb
part 2: remove taskcluster scheduler from mozharness. r=jlund
https://hg.mozilla.org/integration/autoland/rev/6fd955031403
part 3: Add in-tree config support to taskcluster_helper. r=jlund
https://hg.mozilla.org/mozilla-central/rev/55c69395d608
https://hg.mozilla.org/mozilla-central/rev/3c5e7423e4eb
https://hg.mozilla.org/mozilla-central/rev/6fd955031403
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Depends on: 1320760
Reopening because one of it's dependencies is still open.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Blocks: 132491
No longer blocks: 132491
Blocks: 1324911
Status: REOPENED → RESOLVED
Closed: 8 years ago → 7 years ago
Resolution: --- → FIXED
Component: Integration → Services
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: