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

RESOLVED FIXED

Status

enhancement
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: aki, Assigned: aki)

Tracking

unspecified
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(5 attachments)

+ in-tree changes to use them.
Comment hidden (mozreview-request)
Blocks: 1397762
Comment hidden (mozreview-request)
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.
Attachment #8951491 - Flags: feedback?(mtabara)
Attachment #8951491 - Flags: feedback?(bhearsum)

Comment 4

a year ago
mozreview-review
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?
Assignee

Comment 5

a year ago
mozreview-review-reply
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 hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Attachment #8952327 - Flags: review?(mtabara)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 16

a year ago
mozreview-review
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',
> -    ],
> -}

!
Attachment #8951491 - Flags: review?(bhearsum) → review+
Assignee

Comment 17

a year ago
mozreview-review-reply
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

a year ago
mozreview-review
Comment on attachment 8953008 [details]
bug 1438735 - balrogscript 2.0.0.

https://reviewboard.mozilla.org/r/222266/#review229178
Attachment #8953008 - Flags: review?(mtabara) → review+
Attachment #8952327 - Flags: review?(mtabara) → review+
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 hidden (mozreview-request)

Comment 21

a year ago
Pushed by asasaki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/42a3b4c11354
balrog scriptworker push and schedule support. r=bhearsum
Posted patch fix-balrogSplinter Review
Attachment #8954170 - Flags: review?
Attachment #8954170 - Flags: review? → review+

Comment 23

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/42a3b4c11354
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Attachment #8954472 - Flags: review?(bhearsum)
Attachment #8954472 - Flags: review?(bhearsum) → review+
You need to log in before you can comment on or make changes to this bug.