Closed Bug 1339151 Opened 3 years ago Closed 3 years ago

Use different tasks for signed and unsigned bits in release promotion

Categories

(Release Engineering :: Release Automation: Other, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rail, Assigned: rail)

References

Details

Attachments

(3 files, 1 obsolete file)

After we switch to Tc based CI builds we have to use different tasks for signed (installers, MARs, etc) and unsigned (tests.zip, simbols, etC) bits in releasetasks.

The changes should ride the trains.

At least `task_for_revision()` https://hg.mozilla.org/build/tools/file/tip/lib/python/kickoff/__init__.py#l161 should change the behaviour to read the routes from somewhere else. As a possible solution we can use in-tree configs and checkout them run-time in releaserunner.
Blocks: 1334917
Attached file releasetasks
Attachment #8837167 - Flags: feedback?(jlund)
Attached patch configs.diff (obsolete) — Splinter Review
Attachment #8837168 - Flags: feedback?(jlund)
Attached patch tools.diffSplinter Review
Attachment #8837169 - Flags: feedback?(jlund)
This approach also requires splitting some beetmover configs.
Comment on attachment 8837168 [details] [diff] [review]
configs.diff

Review of attachment 8837168 [details] [diff] [review]:
-----------------------------------------------------------------

hm, so signed and unsigned routes are the same except for linux in jamun?
Attachment #8837168 - Flags: feedback?(jlund) → feedback+
Comment on attachment 8837168 [details] [diff] [review]
configs.diff

Review of attachment 8837168 [details] [diff] [review]:
-----------------------------------------------------------------

ah so this is what we want until we actually change things. And that's why jamun is already different as this is your staging env?
Comment on attachment 8837169 [details] [diff] [review]
tools.diff

Review of attachment 8837169 [details] [diff] [review]:
-----------------------------------------------------------------

::: buildfarm/release/release-runner.py
@@ +279,5 @@
>      # TODO: to be moved under kickoff soon, once new relpro sanity is in place
>      # bug 1282959
>      platforms = kwargs.get('en_US_config', {}).get('platforms', {})
>      for platform in platforms.keys():
> +        task_id = platforms.get(platform).get('signed_task_id', {})

I know this is existing logic but do we want to allow for no taskid and instead return an empty dict? Or should we fail here on key error?

::: lib/python/kickoff/build_status.py
@@ +70,5 @@
>                  # Tasks are always completed if they are referenced in the index
>                  # https://docs.taskcluster.net/reference/core/index
> +                # Assuming that the signed tasks are completed after their
> +                # unsigned counterparts
> +                task_id = signed_task_for_revision(

where is this method implemented?
Attachment #8837169 - Flags: feedback?(jlund) → feedback+
Attachment #8837167 - Flags: feedback?(jlund) → feedback+
(In reply to Jordan Lund (:jlund) from comment #5)
> hm, so signed and unsigned routes are the same except for linux in jamun?

Correct.

(In reply to Jordan Lund (:jlund) from comment #6)
> ah so this is what we want until we actually change things. And that's why
> jamun is already different as this is your staging env?

Correct.

(In reply to Jordan Lund (:jlund) from comment #7)
> >      for platform in platforms.keys():
> > +        task_id = platforms.get(platform).get('signed_task_id', {})
> 
> I know this is existing logic but do we want to allow for no taskid and
> instead return an empty dict? Or should we fail here on key error?

Good point. I'll change this.

 
> ::: lib/python/kickoff/build_status.py
> @@ +70,5 @@
> >                  # Tasks are always completed if they are referenced in the index
> >                  # https://docs.taskcluster.net/reference/core/index
> > +                # Assuming that the signed tasks are completed after their
> > +                # unsigned counterparts
> > +                task_id = signed_task_for_revision(
> 
> where is this method implemented?

Bah. That was my initial implementation. I'll fix that.

Thank you for the feedback!
I'd like to land the configs, so I can play with other patches on jamun/bm83
Attachment #8837168 - Attachment is obsolete: true
Comment on attachment 8837709 [details]
Bug 1339151 - Remove leadind "index." from routes

https://reviewboard.mozilla.org/r/112758/#review114344

have fun!
Attachment #8837709 - Flags: review?(jlund) → review+
Comment on attachment 8837709 [details]
Bug 1339151 - Remove leadind "index." from routes

https://reviewboard.mozilla.org/r/112758/#review114982

*stamp*
Attachment #8837709 - Flags: review?(aki) → review+
Priority: -- → P1
Attachment #8837709 - Flags: review+ → review?(aki)
Comment on attachment 8837709 [details]
Bug 1339151 - Remove leadind "index." from routes

https://reviewboard.mozilla.org/r/112758/#review117190
Attachment #8837709 - Flags: review?(aki) → review+
Attachment #8837709 - Flags: review+ → review?(aki)
Attachment #8837709 - Flags: review?(aki) → review+
Depends on: 1343953
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.