Closed Bug 1438735 Opened 7 years ago Closed 7 years ago

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

Categories

(Release Engineering :: Release Automation: Other, enhancement)

enhancement
Not set
normal

Tracking

(firefox60 fixed)

RESOLVED FIXED
Tracking Status
firefox60 --- fixed

People

(Reporter: mozilla, Assigned: mozilla)

References

Details

Attachments

(5 files)

+ in-tree changes to use them.
Blocks: 1397762
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 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 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.
Attached file balrogscript patches (deleted) —
Attachment #8952327 - Flags: review?(mtabara)
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+
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.
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)
Pushed by asasaki@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/42a3b4c11354 balrog scriptworker push and schedule support. r=bhearsum
Attached patch fix-balrog (deleted) — Splinter Review
Attachment #8954170 - Flags: review?
Attachment #8954170 - Flags: review? → review+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Attached patch 5min-submitter.diff (deleted) — Splinter Review
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.

Attachment

General

Created:
Updated:
Size: