Closed Bug 1577110 Opened 5 years ago Closed 5 years ago

Switch signing jobs for geckodriver to toolchain builds

Categories

(Testing :: geckodriver, task, P1)

Version 3
task

Tracking

(firefox78 fixed)

RESOLVED FIXED
mozilla78
Tracking Status
firefox78 --- fixed

People

(Reporter: whimboo, Assigned: catlee)

References

(Blocks 3 open bugs)

Details

Attachments

(2 files, 3 obsolete files)

With bug 1427849 and bug 1546299 we got signed builds for geckodriver based on the binary as produced in the Firefox build jobs, and located in the common test archive.

Once bug 1534533 landed, we will be able to change the signing jobs to no longer depend/require a full build, but just the geckodriver toolchain task. This will give us signed builds way faster, and solves all known issues for releasing geckodriver from Taskcluster.

This work has to be done before we can stop producing geckodriver builds via the build job.

Blocks: 1580622

Johann, just to get a confirmation from you in regards of signing toolchain build jobs, mind replying to comment 0 if that would be possible? Mainly that we keep the chain of trust, and are allowed to sign those builds. Thanks.

Flags: needinfo?(jlorenzo)

Yes, it looks feasible to me! Toolchain tasks (like this one[1]) outputs both public/chain-of-trust.json and public/chain-of-trust.json.sig, so Chain of Trust is able to trace this task back to the tree. Therefore, it will be happy.

Please let me know if need more information.

[1] https://tools.taskcluster.net/groups/MFD7X6NMTpmWY3gKrYaSDA/tasks/NjKrxvOBS5avXZoCt8LQmg/runs/0/artifacts

Flags: needinfo?(jlorenzo)

Great. So I had a look into this issue and tried to change that signing job to depend on the toolchain build job but failed. Mainly because of the way how it has been implemented with transforms. It's way different compared to normal jobs.

Would you mind giving me a hint in how to change the code so it no longer depends on the geckodriver-repack job but uses the builds as generated by the geckodriver toolchain job?

https://searchfox.org/mozilla-central/source/taskcluster/ci/geckodriver-signing/kind.yml
https://searchfox.org/mozilla-central/source/taskcluster/taskgraph/transforms/geckodriver_signing.py

https://searchfox.org/mozilla-central/source/taskcluster/ci/toolchain/geckodriver.yml

The geckodriver-repack job wouldn't be needed anymore, and could also be removed once that switch has been done.

Flags: needinfo?(jlorenzo)

I'm testing locally by comparing before and after the patch with

./mach taskgraph full --json --target-kind geckodriver-signing --tasks '.*geckodriver-signing.*' > f.json

It's pretty clear that this can be made to work, but I'm not sure how
we want to handle the nightly choices here. Toolchains don't really
respect nightly in the way that builds do.

jlorenzo: take a look at my commit and see what needs to change? Current diff for one task looks like:

-  "geckodriver-signing-linux-nightly/opt": {
+  "geckodriver-signing-linux32/opt": {
     "attributes": {
       "always_target": false,
-      "build_platform": "linux-nightly",
+      "build_platform": "linux32",
       "build_type": "opt",
       "kind": "geckodriver-signing",
-      "nightly": true,
       "repackage_type": "repackage-signing",
       "run_on_projects": [
-        "all"
+        "trunk"
       ],
       "shipping_phase": null,
       "shipping_product": null
     },
     "dependencies": {},
     "kind": "geckodriver-signing",
-    "label": "geckodriver-signing-linux-nightly/opt",
+    "label": "geckodriver-signing-linux32/opt",
     "optimization": null,
     "release_artifacts": [
       "public/geckodriver.tar.gz",
@@ -34,7 +33,7 @@
       },
       "extra": {
         "index": {
-          "rank": 0
+          "rank": 1569621433
         },
         "parent": "",
         "treeherder": {
@@ -48,15 +47,15 @@
             "platform": "linux32"
           },
           "symbol": "s",
-          "tier": 2
+          "tier": 1
         },
         "treeherder-platform": "linux32/opt"
       },
       "metadata": {
-        "description": "Signing Geckodriver for build 'linux-nightly/opt' ([Treeherder push](https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=1e0685b06c61f350da1973d419223883ba64d3e2))",
-        "name": "geckodriver-signing-linux-nightly/opt",
-        "owner": "btara@mozilla.com",
-        "source": "https://hg.mozilla.org/mozilla-central/file/1e0685b06c61f350da1973d419223883ba64d3e2/taskcluster/ci/geckodriver-signing"
+        "description": "Signing Geckodriver for build 'linux32/opt' ([Treeherder push](https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=d23a0a0ffa93fe273c571592bdbff6aa68f6ca2b))",
+        "name": "geckodriver-signing-linux32/opt",
+        "owner": "dluca@mozilla.com",
+        "source": "https://hg.mozilla.org/mozilla-central/file/d23a0a0ffa93fe273c571592bdbff6aa68f6ca2b/taskcluster/ci/geckodriver-signing"
       },
       "payload": {
         "maxRunTime": 600,
@@ -69,7 +68,7 @@
               "public/geckodriver.tar.gz"
             ],
             "taskId": {
-              "task-reference": "<geckodriver-repack>"
+              "task-reference": "<toolchain>"
             },
             "taskType": "repackage"
           }
@@ -78,37 +77,36 @@
       "priority": "medium",
       "provisionerId": "scriptworker-prov-v1",
       "routes": [
-        "tc-treeherder.v2.mozilla-central.1e0685b06c61f350da1973d419223883ba64d3e2.36626"
+        "tc-treeherder.v2.mozilla-central.d23a0a0ffa93fe273c571592bdbff6aa68f6ca2b.36627"
       ],
       "scopes": [
-        "project:releng:signing:cert:nightly-signing"
+        "project:releng:signing:cert:dep-signing"
       ],
       "tags": {
-        "createdForUser": "btara@mozilla.com",
+        "createdForUser": "dluca@mozilla.com",
         "kind": "geckodriver-signing",
-        "label": "geckodriver-signing-linux-nightly/opt",
+        "label": "geckodriver-signing-linux32/opt",
         "retrigger": "false"
       },
-      "workerType": "signing-linux-v1"
+      "workerType": "depsigning"
     }
   },

We used the dependency on nightly jobs before to lower the amount of geckodriver builds to be signed because geckodriver gets built for each and every Firefox build. Given that we have a toolchain job now, which only gets run when source code changes happen (this is right or?), it shouldn't matter anymore. The amount of jobs is kinda low.

Blocks: 1573798

Chatted with :whimboo and :nalexander. Nick will update the ticket with a summary.

Flags: needinfo?(jlorenzo)

This flattens geckodriver-{repack,signing} into one task that takes
a toolchain-geckodriver-* task as input. It's very close to a
general "sign toolchain artifacts" task.

Open questions:

  • I've called it geckodriver-signing2 for now, and not removed the
    other tasks. Happy to rename whatever we like.
  • Can we sign stuff in public/build on macOS? I can link the error,
    which I think is a missing mkdir -p .../public/build. (It's just
    failing to tar a result.)
  • We can bikeshed where to put these on Treeherder at some point. The
    tasks right now are with the build platforms, so I put them there.
    I think a "Signed toolchain" row (platform) makes more sense, but I
    don't really care.

A try build before I squashed is at:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a65c83402515975c381998b615dcfd7bf46bafec

Apparently I can't ask for feedback, so I asked for review. Questions welcome, folks!

As discussed with Andreas today we do not want to hold back the 0.26.0 release. We can live with the MSVCRT dependency until the 0.26.1 or 0.27.0 release.

Blocks: 1584911
No longer blocks: 1573798

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #10)

As discussed with Andreas today we do not want to hold back the 0.26.0 release. We can live with the MSVCRT dependency until the 0.26.1 or 0.27.0 release.

OK, that's grand for me. Hopefully I'll get more significant feedback from Johan and Aki sometime this week.

As it looks like due to bug 1588081 we won't be able to switch the signing jobs to built on the toolchain jobs. The notarization service only works for Nightly and Release keys.

If that is the case we might have to get rid of the toolchain jobs, and update the normal builds for geckodriver for bug 1442253, and bug 1584530. :/

Depends on: 1588081

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #12)

As it looks like due to bug 1588081 we won't be able to switch the signing jobs to built on the toolchain jobs. The notarization service only works for Nightly and Release keys.

That was actually a wrong information. So lets forget about this comment.

Aki, were you able to make some steps forward in having a solution here? When it comes to the next release, which we actually want to have soon, it would be great to have this working. Thanks!

Flags: needinfo?(aki)

See the latest comments in bug 1588081. We have workarounds; if we want a fully notarized geckodriver someone needs to create a notarizable package format.

Flags: needinfo?(aki)

Could we for now just switch the job dependency to the toolchain task, and then implement the specific package format? Having this bug open causes a lot of trouble for us when releasing geckodriver. Or is this causing a lot of extra work?

Flags: needinfo?(aki)

I believe it'll work, give it a try.

Flags: needinfo?(aki)

Note that Chris started working on that by end of last week. Here a recent try build which already looks kinda promising:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8a8fd0953aefdbbdd9228753d9d172478dc4fe12

Assignee: nobody → catlee
Status: NEW → ASSIGNED

Unfortunately, I started working on this without having looked at nalexander's patches. It would have saved me a lot of time!

Ultimately, this is pretty close to D47518. I worked around the mac signing issue by changing the output directory of the toolchain builds to remove the 'build/' intermediate directory. I had to add an extra comment in the build script to allow force-rebuilding of the toolchains, since the toolchain optimization hashing doesn't take artifact paths into account.

I think this should allow geckodriver builds to happen on autoland and central. Signing will happen on central per push I think.

I did need to change the archive format for windows and mac, to .zip and .tar.bz2 respectively.

This does not solve the macOS notarization issue.

No longer depends on: 1588081

(In reply to Chris AtLee [:catlee] from comment #19)

Ultimately, this is pretty close to D47518. I worked around the mac signing issue by changing the output directory of the toolchain builds to remove the 'build/' intermediate directory. I had to add an extra comment in the build script to allow force-rebuilding of the toolchains, since the toolchain optimization hashing doesn't take artifact paths into account.

This is great to hear Chris! Thanks a lot for your time and helping us with that part!

I think this should allow geckodriver builds to happen on autoland and central. Signing will happen on central per push I think.

I did need to change the archive format for windows and mac, to .zip and .tar.bz2 respectively.

Oh, this is exactly what we would need for our releases: https://github.com/mozilla/geckodriver/releases. So that's perfect.

This does not solve the macOS notarization issue.

Ok, that can be delayed and will now block the general release from TC meta bug.

Priority: -- → P1
Attachment #9144086 - Attachment is obsolete: true

If a task has explicitly specified artifact paths, don't additionally specify
the default paths. If the task has private artifacts, having a directory
that uploads public artifacts seems like an attractive nuissance.

Pushed by catlee@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b76e3e988fdd Switch geckodriver signing to use toolchain tasks r=tomprince
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78

We probably need to switch to the dep-signing pool, or switch the scope to nightly-signing / release-signing for the m-c bustage.

Ah, it's a combination of linux-signing pool in the transform, along with no shippable or nightly attribute in the task, I'm guessing.

Yeah, the intent was to use nightly signing for the toolchains on m-c. I'll try and figure out how to make that happen.

Flags: needinfo?(catlee)
Pushed by catlee@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/09443cbbdfc9 Switch geckodriver signing to use toolchain tasks r=tomprince
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
Regressions: 1636589
Blocks: 1636907
Pushed by mozilla@hocat.ca: https://hg.mozilla.org/integration/autoland/rev/33277064c343 Don't add default toolchain artifact directory if already specified; r=glandium

Looks like we still have one thing to land here...

Status: RESOLVED → REOPENED
Flags: needinfo?(catlee) → needinfo?(mozilla)
Resolution: FIXED → ---
Target Milestone: mozilla78 → ---
Attachment #9146416 - Attachment description: Bug 1577110: Don't add default toolchain artifact directory if already specified; r?glandium → Bug 1577110: Don't add default toolchain artifact directory if already specified; r=glandium
Pushed by mozilla@hocat.ca: https://hg.mozilla.org/integration/autoland/rev/72f6e2d07667 Don't add default toolchain artifact directory if already specified; r=glandium
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
Blocks: 1558497
Regressions: 1639955
Attachment #9098710 - Attachment is obsolete: true
Attachment #9097058 - Attachment is obsolete: true

To fix the regression for external tools, which expect the location of the geckodriver binary artifact at public/build/, we agreed on backing out the following changeset that was optional for this bug anyway:

https://hg.mozilla.org/mozilla-central/rev/72f6e2d07667

Once done please do NOT reopen this bug. I will file a follow-up bug for the additional fix. Thanks.

Flags: needinfo?(mozilla) → needinfo?(sheriffs)

Actually a follow-up patch was provided on bug 1639955 that should have fixed the regression now. So there is no need to backout this specified changeset anymore.

Flags: needinfo?(sheriffs)
Regressions: 1640961
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: