1438735 - no-bbb: add push+schedule logic to balrog scriptworkers

Status: RESOLVED FIXED

(Release Engineering :: Release Automation, enhancement)

Description

[Aki Sasaki (not active)]

+ in-tree changes to use them.

Comment 1

[Aki Sasaki (not active)]

Attachment: bug 1438735 - balrog scriptworker push and schedule support.

- add balrog push - this replaces the final portion of the updates builder.
  - I *believe* the rest of it is also obsolete...
- rename balrog transform to balrog_submit, because it's for balrog submission
  - make this default to the 'promote' phase. balrog and beetmover currently take the current
    phase, which isn't always the wanted behavior.
- rename balrog publish to balrog schedule
- add balrog secondary push and secondary scheduling, for RCs
- remove the release_updates transforms
- make the task.py balrog transforms smarter
- get rid of the release_balrog_publishing transforms; ad a generic worker_type transform
- add BALROG_ACTIONS to scriptworker.py
- add get_balrog_action_scope()
- remove the unused balrog channel scopes

Review commit: https://reviewboard.mozilla.org/r/220790/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/220790/

Comment 2

[Aki Sasaki (not active)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220790/diff/1-2/

Comment 3

[Aki Sasaki (not active)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

This patch might change if I find issues while debugging, but I think it's ready for a feedback pass. I'm not entirely sure about my dependencies and whether I'm doing the right thing by replacing the updates builder.

Comment 4

[[e-mail] (:bhearsum)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

https://reviewboard.mozilla.org/r/220790/#review227168

This largely looks good. I've got some suggestions about dependencies and naming below, and a couple of other questions.

::: taskcluster/ci/release-balrog-push/kind.yml:29
(Diff revision 2)
> +         mozilla-beta: scriptworker-prov-v1/balrogworker-v1
> +         mozilla-release: scriptworker-prov-v1/balrogworker-v1
> +         default: invalid/invalid
> +   worker:
> +      implementation: balrog
> +      balrog-action: push

I wonder if "submit" is better here, to avoid confusion with the ship phase?

Also, is this task intended to both submit the top level information (the ReleaseCreatorV4 stuff) and push to the test channels (the ReleasePusher stuff)?

::: taskcluster/ci/release-balrog-push/kind.yml:31
(Diff revision 2)
> +         default: invalid/invalid
> +   worker:
> +      implementation: balrog
> +      balrog-action: push
> +      require-mirrors: true
> +      platforms: ["linux", "linux64", "macosx64", "win32", "win64"]

Do these need to be -devedition for Deved?

::: taskcluster/ci/release-balrog-push/kind.yml:55
(Diff revision 2)
> +         channel-names:
> +            by-project:
> +               maple: ["beta", "beta-localtest", "beta-cdntest"]
> +               birch: ["release", "release-localtest", "release-cdntest"]
> +               mozilla-beta: ["beta", "beta-localtest", "beta-cdntest"]
> +               mozilla-release: ["release", "release-localtest", "release-cdntest"]

It looks like these args are coming from https://github.com/mozilla/build-tools/blob/master/scripts/build-promotion/balrog-release-pusher.py?

::: taskcluster/ci/release-balrog-push/kind.yml:57
(Diff revision 2)
> +               maple: ["beta", "beta-localtest", "beta-cdntest"]
> +               birch: ["release", "release-localtest", "release-cdntest"]
> +               mozilla-beta: ["beta", "beta-localtest", "beta-cdntest"]
> +               mozilla-release: ["release", "release-localtest", "release-cdntest"]
> +               default: []
> +         publish-rules:

I don't think you need this here -- this is only used for release scheduling.

::: taskcluster/ci/release-balrog-scheduling/kind.yml:14
(Diff revision 2)
> +   - taskgraph.transforms.worker_type:transforms
> +   - taskgraph.transforms.release_notifications:transforms
> +   - taskgraph.transforms.task:transforms
> +
> +kind-dependencies:
> +   - release-uptake-monitoring

It would be good to depend on update verify here, too, if cross-phase dependencies are acceptable. We generally should not be shipping if update verify is failing. Same for the secondary versions.

::: taskcluster/ci/release-balrog-scheduling/kind.yml:37
(Diff revision 2)
> +      description: Schedule Firefox publishing in balrog
> +      name: release-firefox_schedule_publishing_in_balrog
> +      shipping-product: firefox
> +      worker:
> +         product: firefox
> +         channel-names:

I don't think you're going to need these here -- this is only something needed when creating the top level blob bits (specifically, fileUrls).

::: taskcluster/ci/release-balrog-scheduling/kind.yml:51
(Diff revision 2)
> +               maple: [32]
> +               birch: [145]
> +               mozilla-beta: [32]
> +               mozilla-release: [145]
> +               default: []
> +         rules-to-update:

I don't think you'll need these either -- they're only used when pushing to test channels (which happpens in the promote phase.

::: taskcluster/ci/release-secondary-final-verify/kind.yml
(Diff revision 2)
>  loader: taskgraph.loader.transform:loader
>  
>  kind-dependencies:
>     - post-balrog-dummy
>     - post-beetmover-dummy
> -   - release-updates-builder

This will need to depend on the task that does the top level blob submission.

::: taskcluster/taskgraph/transforms/task.py:555
(Diff revision 2)
> +        Optional('rules-to-update'): optionally_keyed_by('project', [basestring]),
> +        Optional('archive-domain'): optionally_keyed_by('project', basestring),
> +        Optional('download-domain'): optionally_keyed_by('project', basestring),
>  
>          # list of artifact URLs for the artifacts that should be beetmoved
> -        Required('upstream-artifacts'): [{
> +        Optional('upstream-artifacts'): [{

Would it be better to have a separate transform for balrog instead of making this schema so loose?

Comment 5

[Aki Sasaki (not active)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

https://reviewboard.mozilla.org/r/220790/#review227168

> I wonder if "submit" is better here, to avoid confusion with the ship phase?
> 
> Also, is this task intended to both submit the top level information (the ReleaseCreatorV4 stuff) and push to the test channels (the ReleasePusher stuff)?

Renamed `submit` and `push` to `submit-locale` and `submit-toplevel`.

> Do these need to be -devedition for Deved?

Not sure, but I'll try that.

> It looks like these args are coming from https://github.com/mozilla/build-tools/blob/master/scripts/build-promotion/balrog-release-pusher.py?

They're coming from mozharness configs.

> I don't think you need this here -- this is only used for release scheduling.

I think this comment was meant for the scheduling kind?

> It would be good to depend on update verify here, too, if cross-phase dependencies are acceptable. We generally should not be shipping if update verify is failing. Same for the secondary versions.

Leaving that out for the moment. In the future we can have more flexible dependencies - optionally override failures with human signoffs or something.

> I don't think you're going to need these here -- this is only something needed when creating the top level blob bits (specifically, fileUrls).

Thanks!

> I don't think you'll need these either -- they're only used when pushing to test channels (which happpens in the promote phase.

Thanks!

> This will need to depend on the task that does the top level blob submission.

Fixed.

> Would it be better to have a separate transform for balrog instead of making this schema so loose?

The schema is loose, but then we feed into the `payload_builder` which has some of its own checks. We ignore `upstreamArtifacts` in non-`submit-locale`, and reference it directly in `submit-locale`, so I think we're ok. Then balrogscript has its own schema check... I think we're covered.

Comment 6

[Aki Sasaki (not active)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220790/diff/2-3/

Comment 7

[Aki Sasaki (not active)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220790/diff/3-4/

Comment 8

[Aki Sasaki (not active)]

Attachment: balrogscript patches

Comment 9

[Aki Sasaki (not active)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220790/diff/4-5/

Comment 10

[Aki Sasaki (not active)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220790/diff/5-6/

Comment 11

[Aki Sasaki (not active)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220790/diff/6-7/

Comment 12

[Aki Sasaki (not active)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220790/diff/7-8/

Comment 13

[Aki Sasaki (not active)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220790/diff/8-9/

Comment 14

[Aki Sasaki (not active)]

Attachment: bug 1438735 - balrogscript 2.0.0.

Review commit: https://reviewboard.mozilla.org/r/222266/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222266/

Comment 15

[Aki Sasaki (not active)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220790/diff/9-10/

Comment 16

[[e-mail] (:bhearsum)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

https://reviewboard.mozilla.org/r/220790/#review228176

::: taskcluster/taskgraph/transforms/balrog_submit.py:110
(Diff revision 10)
> -            'scopes': [server_scope] + channel_scopes,
>              'dependencies': {'beetmover': dep_job.label},
>              'attributes': attributes,
>              'run-on-projects': dep_job.attributes.get('run_on_projects'),
>              'treeherder': treeherder,
> -            'shipping-phase': job.get('shipping-phase', phase),
> +            'shipping-phase': job.get('shipping-phase', 'promote'),

Should this go in the kind instead?

::: taskcluster/taskgraph/util/scriptworker.py
(Diff revision 10)
> -        'balrog:channel:release-cdntest',
> -        'balrog:channel:esr',
> -        'balrog:channel:esr-localtest',
> -        'balrog:channel:esr-cdntest',
> -    ],
> -}

!

Comment 17

[Aki Sasaki (not active)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

https://reviewboard.mozilla.org/r/220790/#review228176

> Should this go in the kind instead?

I think so. However, the real fix is to separate beetmover and balrog jobs into separate jobs with specific phases, rather than dynamically populating that information. This underlying issue is what prevents us from running `push_firefox` at the beginning of >b2 releases. I think I'm going to leave this until we fix the underlying issue for real.

> !

Yes, we're dropping the unused channel scopes. They add complexity to the graph, and we're ignoring them on the balrogscript side.

Comment 18

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 8953008 [details]
bug 1438735 - balrogscript 2.0.0.

https://reviewboard.mozilla.org/r/222266/#review229178

Comment 19

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s b3116c9d46f94147e36c2541c3af3d6adaa1f0d2 -d 2fa454395a88: rebasing 448908:b3116c9d46f9 "bug 1438735 - balrog scriptworker push and schedule support. r=bhearsum" (tip)
merging taskcluster/ci/release-secondary-update-verify/kind.yml
merging taskcluster/ci/release-update-verify/kind.yml
merging taskcluster/docs/kinds.rst
merging taskcluster/taskgraph/target_tasks.py
merging taskcluster/taskgraph/util/scriptworker.py
warning: conflicts while merging taskcluster/ci/release-secondary-update-verify/kind.yml! (edit, then use 'hg resolve --mark')
warning: conflicts while merging taskcluster/ci/release-update-verify/kind.yml! (edit, then use 'hg resolve --mark')
warning: conflicts while merging taskcluster/taskgraph/target_tasks.py! (edit, then use 'hg resolve --mark')
warning: conflicts while merging taskcluster/taskgraph/util/scriptworker.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 20

[Aki Sasaki (not active)]

Comment on attachment 8951491 [details]
bug 1438735 - balrog scriptworker push and schedule support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220790/diff/10-11/

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/42a3b4c11354
balrog scriptworker push and schedule support. r=bhearsum

Comment 22

[Aki Sasaki (not active)]

Attachment: fix-balrog

Comment 23

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/42a3b4c11354

Comment 24

[Aki Sasaki (not active)]

Attachment: 5min-submitter.diff