Closed Bug 1330668 Opened 3 years ago Closed 3 years ago

Prepare date -> central beetmover, balrog, and funsize landing for tc-nightlies on central

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla53

People

(Reporter: jlund, Assigned: Callek)

References

Details

Attachments

(4 files, 2 obsolete files)

similar to: `Bug 1322041 - Prepare date -> central crash land in preparation of tc-nightlies on central`, we need to wrap up the remaining differences on date->central. This is pointed out by callek as:

<Callek> jlund: what's left: "Funsize routes" -- "Beetmover tasks" -- "Balrog tasks"
Depends on: 1330476
Comment on attachment 8826355 [details]
Bug 1330668 - Generate balrog_props.json for l10n repacks.

https://reviewboard.mozilla.org/r/104288/#review105050

This appears to match what we've got on date.
Attachment #8826355 - Flags: review?(aki) → review+
Assignee: nobody → bugspam.Callek
Comment on attachment 8826354 [details]
Bug 1330668 - Add routes used to trigger funsize.

https://reviewboard.mozilla.org/r/104286/#review105242

R+ if both of these are fixed, since the changes to the patch itself are trivial.

::: taskcluster/taskgraph/transforms/signing.py:65
(Diff revision 1)
>      # below transforms for defaults of various values.
>      Optional('treeherder'): task_description_schema['treeherder'],
> +
> +    # If True, adds a route which funsize uses to schedule generation of partial mar
> +    # files for updates. Expected to be added on nightly builds only.
> +    Optional('use_funsize_route'): bool,

This should use dashes (`use-funsize-route`)

::: taskcluster/taskgraph/transforms/signing.py:126
(Diff revision 1)
>              'run-on-projects': dep_job.attributes.get('run_on_projects'),
>              'treeherder': treeherder,
>          }
>  
> +        if job.get('use_funsize_route', False):
> +            task['routes'] = ["index.project.releng.funsize.{project}.level-{level}".format(

Will this require a new scope for each project,  `queue:route:index.project.releng.funsize.<project>.*`?

Would it be difficult to switch that around so that we can grant `queue:route:index.project.releng.funsize.level-N.*` to each level instead?  This relates to bug 1278986 where we're worried about users with access to a lot of repos having 1000's of individual scopes.

If this is easier to change now, before it's in production, that'd be great.  If it's already really hard to change and won't be appreciably harder later, then we can delay the change.
Attachment #8826354 - Flags: review?(dustin) → review+
Comment on attachment 8826356 [details]
Bug 1330668 - Schedule beetmover tasks.

https://reviewboard.mozilla.org/r/104290/#review105250

I don't totally understand this - especially the parts about manifests and artifact names.  But I think that's OK.  You and the others hacking on date understand it.

My vague sense is, there should be only one beetmover kind, and the transform in beetmover_l10n.py should figure out based on the parent kind whether it should apply or not.  But I see that beetmover.py is already handling both the signed and unsigned cases, so maybe one kind handling four cases would be too much?

At any rate, this is fine as it is.

::: taskcluster/docs/kinds.rst:167
(Diff revision 2)
> +
> +beetmover
> +---------
> +
> +Beetmover, takes specific artifacts, "Beets", and pushes them to a location outside
> +of taskclusters task artifacts, (archive.mozilla.org as one place) and in the

Taskcluster's

::: taskcluster/docs/kinds.rst:176
(Diff revision 2)
> +--------------
> +
> +Beetmover L10n, takes specific artifacts, "Beets", and pushes them to a location outside
> +of taskclusters task artifacts, (archive.mozilla.org as one place) and in the
> +process determines the final location and a "pretty" name (versioned product name)
> +This seperate task uses logic specific to localized artifacts, such as including

Separate kind
   ^

::: taskcluster/taskgraph/transforms/beetmover.py:153
(Diff revision 2)
> +beetmover_description_schema = Schema({
> +    # the dependent task (object) for this beetmover job, used to inform beetmover.
> +    Required('dependent-task'): object,
> +
> +    # depname is used in taskref's to identify the taskID of the unsigned things
> +    # to do change to build or signed-build

remove todo
Attachment #8826356 - Flags: review?(dustin) → review+
Comment on attachment 8826433 [details]
Bug 1330668 - Schedule balrog submission tasks.

https://reviewboard.mozilla.org/r/104374/#review105282

I'm sort of echoing jonas's concern that we're proliferating kinds where that's not necessary.  I think in this case that refactoring to a single balrog kind would be trivial, but if it's somewhere between "trivial" and "impossible", then deferring to a mentored bug would be fine :)

::: taskcluster/ci/balrog-l10n/kind.yml:11
(Diff revision 1)
> +kind-dependencies:
> +  - beetmover-l10n

From what I can see, it would work find to have a single balrog kind here, that depends on both beetmover and beetmover-l10n.  There appear to be no conditionals on the kind name that distinguish the two.

::: taskcluster/taskgraph/transforms/balrog.py:55
(Diff revision 1)
> +        if 'signing' not in job['dependent-task'].label:
> +            # Skip making a balrog task for this
> +            continue

I kinda dislike substring matching on labels -- attributes would be better here.  But it's not the first place this occurs, and I have a TODO item to go back and find/fix them all.

::: taskcluster/taskgraph/transforms/balrog.py:78
(Diff revision 1)
> +        treeherder.setdefault('kind', 'build')
> +
> +        attributes = {
> +                'nightly': dep_job.attributes.get('nightly', False),
> +                'build_platform': dep_job.attributes.get('build_platform'),
> +                'build_type': dep_job.attributes.get('build_type'),

too many tabs?

::: taskcluster/taskgraph/transforms/task.py:321
(Diff revision 1)
> +            Required('taskId'): taskref_or_string,
> +
> +            # type of signing task (for CoT)
> +            Required('taskType'): basestring,

I'd prefer that these use dashes (oops, and I missed `taskType` above for beetmover, too), but I suppose its' OK since this is verbatim input to the workers.
Attachment #8826433 - Flags: review?(dustin) → review+
Comment on attachment 8826354 [details]
Bug 1330668 - Add routes used to trigger funsize.

https://reviewboard.mozilla.org/r/104286/#review105242

> Will this require a new scope for each project,  `queue:route:index.project.releng.funsize.<project>.*`?
> 
> Would it be difficult to switch that around so that we can grant `queue:route:index.project.releng.funsize.level-N.*` to each level instead?  This relates to bug 1278986 where we're worried about users with access to a lot of repos having 1000's of individual scopes.
> 
> If this is easier to change now, before it's in production, that'd be great.  If it's already really hard to change and won't be appreciably harder later, then we can delay the change.

:sfraser told me its not hard, so I'm switching it -- landing this change requires simon deploy a change to funsize to watch the new route though, so will coordinate with him before I land the same change on date.
Comment on attachment 8826356 [details]
Bug 1330668 - Schedule beetmover tasks.

https://reviewboard.mozilla.org/r/104290/#review105250

At the current design merging both will make an already complex transform set even more so. And I'm not a fan.
Comment on attachment 8826433 [details]
Bug 1330668 - Schedule balrog submission tasks.

https://reviewboard.mozilla.org/r/104374/#review105282

It seems to be trivial to my eyes.

> too many tabs?

yep fixed

> I'd prefer that these use dashes (oops, and I missed `taskType` above for beetmover, too), but I suppose its' OK since this is verbatim input to the workers.

left it as-is for now.
Attachment #8826353 - Attachment is obsolete: true
Attachment #8826433 - Attachment is obsolete: true
Comment on attachment 8827607 [details]
Bug 1330668 - Schedule balrog submission tasks.

https://reviewboard.mozilla.org/r/105230/#review106050
Attachment #8827607 - Flags: review?(dustin) → review+
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/98b1156574e3
Schedule balrog submission tasks. r=dustin a=me
Wes told me on IRC he'll make sure this bug reaches central before his EOD. I'm sending a needinfo to ensure that (he told me to do so last time this sort of request happened)
Flags: needinfo?(wkocher)
Pushed by Callek@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d84bef0d3b09
Add routes used to trigger funsize. r=dustin
https://hg.mozilla.org/integration/autoland/rev/023c67997091
Generate balrog_props.json for l10n repacks. r=aki
https://hg.mozilla.org/integration/autoland/rev/028c582db176
Schedule beetmover tasks. r=dustin
Flags: needinfo?(wkocher)
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.