Closed Bug 1049595 Opened 10 years ago Closed 10 years ago

cope with signing server transition in buildbot configs

Categories

(Release Engineering :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bhearsum, Assigned: bhearsum)

References

Details

Attachments

(5 files, 4 obsolete files)

14.27 KB, patch
rail
: review+
catlee
: feedback+
bhearsum
: checked-in+
Details | Diff | Splinter Review
5.53 KB, patch
rail
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
3.54 KB, patch
rail
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
8.18 KB, patch
rail
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
5.43 KB, patch
rail
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
When bug 1046749 is done, we'll have new 10.9 signing servers up and running. We need to support them as a separate set of machines than 10.6. The tricky part here is that signing will fail until some in-tree changes are landed. This presents a difficult timing problem because the signing servers are currently defined at config time in Buildbot, and it will be very difficult to properly time the landing without closing the trees. Things get even trickier as it makes its way from inbound -> central and onto other branches. https://bugzilla.mozilla.org/show_bug.cgi?id=1046747#c9 talks about a process for landing like that.

However, I'm hoping we can do better here and find a way to choose the right group of servers at runtime. This would eliminate any races or timing issues and all need for a tree closure. It would also mean that changes that require the 10.9 servers could be pushed to try before they land on m-c, and changes that require the 10.6 servers could be pushed to try after the 10.9 changes land on m-c (eg, changes on mozilla-aurora or another branch).

I haven't looked into this much yet, but if we pass both sets of servers into the necessary factories, we might be able to do something in fileIsImportant to make this work - maybe by looking for the presence of %(app)s/app/macbuild/Contents/_CodeSignature/CodeResources in the revision (as much as I loathe to contact hg as part of such a function).

We need to be sure to alter both the DownloadToken steps and whatever code sets MOZ_SIGN_CMD in the environement.
Catlee had a good idea on IRC:
11:26 <@catlee> let signtool decide
11:26 <@catlee> pass them all in the environment
11:27 <@catlee> could do -H osxv2:signing2.domain.com
11:27 <@catlee> or something
11:27 < bhearsum> that's certainly better than contacting hg for every build
11:27 <@catlee> another use case for etcd?
11:27 <@catlee> or somesuch
11:28 < bhearsum> etcd?
11:28 <@catlee> https://github.com/coreos/etcd
11:28 < bhearsum> ah
11:28 < bhearsum> yes, that could help in the future :)
11:28 <@catlee> a configuration service
11:29 < bhearsum> well, unless i can come up something not nasty in buildbot, i'll take your suggestion above
11:31 <@catlee> I was kind of thinking we could do something like that to not require the osx machines to sign gpg stuff
11:31 <@catlee> and then realized that we only sign teeny text files with gpg
11:31 < bhearsum> oh yeah
11:32 < bhearsum> mar signing could also balance across all mac+linux servers with this change
11:32 < bhearsum> which wouldn't be a bad thing


Under this plan, we'd only have three groups of signing machines: dep, nightly, and release. A branch would be specified as one of those, and all of the servers (linux and both types of mac) would be passed along to signtool, which would figure out which servers are appropriate for a given format. Token generation isn't tied to any specific format or server, so it could happen on any of them.

I like this plan.
These patches aren't really tested yet, but that's going to take awhile. I wanted to throw these up for initial feedback while I do that, and also because I'm out tomorrow and then you're out next week. There's buildbotcustom and buildbot-configs patches coming, and eventually we'll need a Puppet patch to update the real masters.

This patch is pretty straightforward - it just adds support for optional formats in front of the host. I put them at the front instead of the rear because port is optional and I didn't feel like making it required.

I was originally using a comma as a delimiter, but that won't work because we pass signing servers to mozharness as a comma delimited list (https://github.com/mozilla/build-mozharness/blob/master/mozharness/mozilla/signing.py#L140).

Here's a sample invocation:
➜  signing git:(signtool-with-formats) ✗ python signtool.py -H dmg:mar:bar:80 -H mar:foo:80 -c host.cert -t /tmp/token -n /tmp/nonce -f dmg -f mar blah -v -H hutneo:22
2014-08-07 11:25:05,564 - in /home/bhearsum/repos/working/tools/release/signing
2014-08-07 11:25:05,564 - doing dmg signing
2014-08-07 11:25:05,564 - possible hosts are ["https://['hutneo', '22']", "https://['bar', '80']"]
2014-08-07 11:25:05,565 - doing mar signing
2014-08-07 11:25:05,565 - possible hosts are ["https://['hutneo', '22']", "https://['bar', '80']", "https://['foo', '80']"]
Attachment #8469315 - Flags: feedback?(catlee)
I think this change actually simplifies the Buildbot side a bit by unifying all of the signing servers into just 3 categories (dep/nightly/release).
Attachment #8469316 - Flags: feedback?(catlee)
Not tested at all yet, I'm sure there's bugs.
Attachment #8469317 - Flags: feedback?(catlee)
Comment on attachment 8469315 [details] [diff] [review]
update signtool to support formats as part of the host

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

::: release/signing/signtool.py
@@ +39,5 @@
>              p.close()
>  
>  
>  def main():
> +    allowed_formats = ("signcode", "gpg", "mar", "dmg", "dmgv2", "jar", "b2gmar")

wonder if we can drop b2gmar here...
Attachment #8469315 - Flags: feedback?(catlee) → feedback+
Attachment #8469316 - Flags: feedback?(catlee) → feedback+
Attachment #8469317 - Flags: feedback?(catlee) → feedback+
The only new things in this patch are:
* Fix bug in URL generation
* Remove "continue" statement used in testing
* Translate "dmgv2" format back to "dmg" before attempting signing

This patch should be able to land on its own as soon as it's reviewed. You can see a test run here: http://dev-master1.srv.releng.scl3.mozilla.com:8018/builders/OS%20X%2010.7%20mozilla-esr31%20build/builds/7 (the make_pkg step, specifically).

After everything else is landed we may want to rip out support for servers that don't specify formats.
Attachment #8469315 - Attachment is obsolete: true
Attachment #8470930 - Flags: review?(rail)
Rail, I assume you'll probably reread the bug for context, but the tl;dr for what we're doing here is getting signtool to select signing servers based on the formats they support, rather than their platform. This means that we pass all of them in MOZ_SIGN_CMD, and signtool.py decides which it should use. This lets us support signing on our existing 10.6 servers on some branches, and signing on our new 10.9 servers on others by changing MOZ_INTERNAL_SIGNING_COMMAND in https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/signing.mk#25. This also means that pushing to try will continue to work regardless of what branch you are on.

The only notable thing about this patch is it fixes release.py not to look for "mac-signing-servers" anymore. Landing this is going to require a bit of timing, because I need to land a puppet patch (incoming), wait for it to sync, and then land and reconfig for this patch and the buildbot-configs one - otherwise we'll be in a broken state. I'll coordinate with buildduty on that. I did a bunch of over the weekend testing on this. Here's some examples:
* A mac build talking to a linux server for gpg signing: http://dev-master1.srv.releng.scl3.mozilla.com:8018/builders/OS%20X%2010.7%20mozilla-beta%20build/builds/15/steps/make_upload/logs/stdio
* A linux build talking to a linux server for gpg signing: http://dev-master1.srv.releng.scl3.mozilla.com:8018/builders/Linux%20mozilla-beta%20build/builds/5/steps/make_upload/logs/stdio
* A mac build talking to a mac server for dmg signing (http://dev-master1.srv.releng.scl3.mozilla.com:8018/builders/OS%20X%2010.7%20mozilla-central%20nightly/builds/4/steps/make_pkg/logs/stdio) and then a linux server for gpg signing (http://dev-master1.srv.releng.scl3.mozilla.com:8018/builders/OS%20X%2010.7%20mozilla-central%20nightly/builds/4/steps/make_upload/logs/stdio)
* A linux build talking to a mac server for gpg signing: http://dev-master1.srv.releng.scl3.mozilla.com:8018/builders/Linux%20mozilla-esr31%20nightly/builds/3/steps/make_upload/logs/stdio

If you have a look at each of those sign commands, you'll see the list of signing servers is correct, so these aren't working just by fluke :).
Attachment #8469317 - Attachment is obsolete: true
Attachment #8470941 - Flags: review?(rail)
Attachment #8469316 - Flags: review?(rail)
Attached patch enable v2 signing servers (obsolete) — Splinter Review
This patch applies on top of the other puppet one, and will land after the other stuff lands cleanly. mac-v2-signing2 isn't up yet, so I may end up landing this without that in the list if we're ready to try out v2 signing before that server is up.
Attachment #8470970 - Flags: review?(rail)
Attachment #8469316 - Flags: review?(rail) → review+
Attachment #8470941 - Flags: review?(rail) → review+
Attachment #8470942 - Flags: review?(rail) → review+
Attachment #8470970 - Flags: review?(rail) → review+
Comment on attachment 8470930 [details] [diff] [review]
fixed version of signtool.py patch

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

::: release/signing/signtool.py
@@ +153,5 @@
> +        # The last two parts of a host is the actual hostname:port. Any parts
> +        # before that are formats - there could be 0..n formats so this is
> +        # tricky to split.
> +        parts = h.split(":")
> +        h = parts[-2:]

Can you use something like
host, port = parts[-2:]

@@ +160,5 @@
> +        if not fmts:
> +            fmts = formats
> +
> +        for f in fmts:
> +            format_urls[f].append("https://%s" % ":".join(h))

... and use them here without join()?
Attachment #8470930 - Flags: review?(rail) → review+
Attachment #8470942 - Flags: checked-in+
Comment on attachment 8470941 [details] [diff] [review]
tested version of buildbotcustom patch

This and the buildbot-configs patch will land after the Puppet changes sync around to all of the masters.
Attachment #8470941 - Flags: checked-in+
Attachment #8469316 - Flags: checked-in+
Attachment #8470930 - Flags: checked-in+
Everything except the second puppet patch is in production now. I needed two bustage fixes for stupid errors in puppet:
https://hg.mozilla.org/build/puppet/rev/a97a00779d51 (syntax)
https://hg.mozilla.org/build/puppet/rev/54ba6465a8ad (bad copy/paste)

Everything appears to be fine now, though. If that continues, I intend to land the other Puppet patch tomorrow, which will enable v2 signatures so that developers can start pushing their patches for this work to try.
The previous version of this patch suffers from the same bug as the other one (changes all token auth passwords to the nightly one).
Attachment #8470970 - Attachment is obsolete: true
Attachment #8471682 - Flags: review?(rail)
Attachment #8471682 - Flags: review?(rail) → review+
I accidentally removed the old ones in my previous patch, whoops.
Attachment #8472322 - Flags: review?(rail)
Attachment #8472322 - Flags: review?(rail) → review+
Attachment #8472322 - Attachment is obsolete: true
Comment on attachment 8471682 [details] [diff] [review]
fix v2 puppet patch

Waiting for to sync, then i'll reconfig for it.
Attachment #8471682 - Flags: checked-in+
This seems to have landed cleanly. I'm still waiting for a successful try build to double check that v2 signing is working, but we're all done here. If there's any issues we'll handle them in a follow-up.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Depends on: 1053813
No longer depends on: 1053813
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: