Implement openh264 update manifests in Balrog

RESOLVED FIXED

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: benjamin, Assigned: bhearsum)

Tracking

(Blocks 5 bugs)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

Reporter

Description

5 years ago
This is the server component of bug 1009816. Firefox will download the Cisco OpenH264 plugin from Cisco, but we want to control the update schedule:

Requirements:

* We need to be able to verify the bits to a Mozilla-trusted source (Mozilla update server)
* We expect the plugin API to change for a while yet, and so we're going to need to serve different Cisco plugins to different release channels
* It would be nice to be able to issue out-of-band updates to the plugin that didn't require a full Firefox respin. This may not be a 100% requirement, if we're willing to respin a Firefox dot-release.
* After somebody updates Firefox, they may have an "old" OpenH264 plugin and we need to decide whether it's ok to keep using it or not (blocklisting, perhaps a separate bug)

Meeting about this later today, but right now my recommended approach is to try and shoehorn this into AUS responses somehow.

The approach here may also affect how we provide updates for the Adobe EME plugin and others in the future.
Reporter

Comment 1

5 years ago
Inputs and outputs details:

Overview: Every build of Firefox will have exactly one OpenH264 plugin that it is "supposed" to use.

Firefox client inputs: The client will fetch "a manifest" (hopefully using existing AUS) giving the version/buildid/architecture of Firefox.

Manifest output: a URL and hash

Developer input: A list of platform-specific mappings. First wins:
* Each mapping has a Firefox version range and optionally a buildid range mapped to a cisco plugin build (download URL and hash). For example:

{
  "WINNT-x86_MSVC": [
    {
      "minVersion": "35.0a1",
      "minBuildID": "20141104", // We switched to a new API on the 1104 nightly. This is riding the trains
      "url": "http://download-cdn.cisco.com/openh264/rev2.zip",
      "hash": "sha256:2222222asd"
    },
    {
      "minVersion": "35.0a1",
      "maxVersion": "35.0a1",
      "maxBuildID": "20141103", // The old API 
      "url": "http://download-cdn.cisco.com/openh264/rev1.zip",
      "hash": "sha256:ASDRFYAUEWR"
    },
    {
      "minVersion": "32.0a1",
      "maxVersion": "34.*",
      "minBuildID": "20140613",
      "url": "http://download-cdn.cisco.com/openh264/rev1.zip",
      "hash": "sha256:ASDRFYAUEWR"
    }
  ]
}

We don't expect frequent updates to OpenH264, so committing this information to a repo would be fine for now. We also don't currently expect to deal with special requests like a required Windows service pack or whatnot, but I'm sure that will eventually be a problem and we'll cross that bridge when we get to it.
Reporter

Comment 2

5 years ago
Also: When there is a Firefox update available, it would be a bonus to be able to tell Firefox about a new OpenH264 plugin so that we can download it in advance. This is not a requirement.
Ben & I met to discuss this today. We propose: When Firefox makes a app-update request to Balrog, we carry out a request for openh264 which is internal to Balrog, and combine both results (format tbd). 

Behind the scenes:
* Balrog works by matching requests to a rule, which specify a release blob to use to create an xml response. All the parts of the path can be matched on
* we would define some rules with Product=openh264 which capture the restrictions (eg comment #1), and point to openh264 blobs (one per plugin version)
* we would add an openh264 blob schema to stores file url, hash type & value, and version
* we would add support in existing release blobs for definition of otherProductRequests, eg ['openh264']. This would go into to Firefox blobs, starting with those for Nightly updates
* when a rule points to this type of release, Balrog will make a 'request' against itself where it substitutes the product name (ie s/Firefox/openh264/'). All the other parts of the path will be forwarded, for maximum future proofing
* that request will resolve the appropriate openh264 rule and return an xml fragment, which will be included in the response. We would ensure this happens even if throttling has yielded no Firefox update
* we can handle the 'new firefox + new plugin' case by also making a request using the new firefox version & buildid
* this should be easily extensible to multiple values in otherProductRequests

To support this we would need
* some refactoring
  * move xml generation out of AUS.py and onto blob classes
  * clean up business logic to be more modular
* add code to combine xml fragments
* add otherProductRequests attribute to v2 schema
* add new schema for openh264 blobs
* add comparison operators for matching version & buildID in rules
* look at if we need any new API calls for submitting/modifying rules/blobs

I'm not going to guess at a time to achieve this, but Ben and I feel nothing there looks intrinsically difficult. It would take some dedicated time to get done, so it would help to clarify the timeline. If we're aiming for Firefox 33.0, would we need this in 4 weeks time ? ie early July; one week left this cycle, and half a cycle with m-c on 33.0, leaving time to test before uplift to Aurora. Is that a fair guess ?

Before we start using Balrog for Beta we have other work to do (bug 933414), as well as some detailed verification. Ideally that would be completed by mid-cycle for Aurora 33.0 (Aug 11), so may overlap with any followup work for openh264.
Reporter

Comment 4

5 years ago
Sounds like a plan!

> get done, so it would help to clarify the timeline. If we're aiming for
> Firefox 33.0, would we need this in 4 weeks time ? ie early July; one week
> left this cycle, and half a cycle with m-c on 33.0, leaving time to test
> before uplift to Aurora. Is that a fair guess ?

Yes.
Reporter

Updated

5 years ago
See Also: → 1009816
Reporter

Updated

5 years ago
Summary: Implement openh264 update manifests, server component → Implement openh264 update manifests in AUS
Assignee

Updated

5 years ago
Component: General Automation → Balrog: Backend
QA Contact: catlee → bhearsum
Assignee

Updated

5 years ago
Depends on: 1021018
Assignee

Updated

5 years ago
Depends on: 1021021
Assignee

Updated

5 years ago
Depends on: 1021022
Assignee

Updated

5 years ago
Depends on: 1021029
Reporter

Comment 5

5 years ago
I had a conversation with rstrong today about this and he brought up one additional concern, which is that we currently don't send the AUS ping from the client when app update is disabled, even though we may still want OpenH264 updates.

He's going to look into this and we may decide to do two things:
* go with a separate ping to AUS using all of the same information but for a different "product"
* change the client to send the AUS ping in more circumstances

I don't think that materially affects the work here except that the subrequest from the main Firefox AUS ping to the separate OpenH264 ping may not be required.
One issue we may need to figure out is how downstream linux distros will get these updates. Most of the distros disable Firefox's automatic updates and rely on their own package distributions to deliver updates. If there's no update ping, how will the users get an updated h264 plugin?
Reporter

Comment 7

5 years ago
Yeah, that's what comment 5 was about. That may depend on whether the distro is already an H.264 licensee in which case they will ship this as a builtin package somehow. If they aren't, then they may either want to download from cisco/install in the profile or have a separate repo for cisco code somehow.

Comment 8

5 years ago
Do we have an ETA for a patch here?
(In reply to Andreas Gal :gal from comment #8)
> Do we have an ETA for a patch here?

bsmedberg was going to come back to catlee with the required timeline, but we were assuming to have this patch ready around the end of July.
Reporter

Comment 10

5 years ago
I am not involved in the timeline discussion. That was a conversation with catless/mreavy/shell.
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #10)
> I am not involved in the timeline discussion. That was a conversation with
> catless/mreavy/shell.

When we met with mreavy, she took as an action to follow up with you to figure out a schedule for when this would be required for initial testing. I guess we can do that here now!

We have some ideas about the proper way to get this done, and some ideas about quick and dirty ways to get this done. We'd much prefer to do this properly, but it all depends on the timeline.

When do you need this stood up for initial testing? I know we're targeting Firefox 33 for final release.
Reporter

Comment 12

5 years ago
I presumed that we could deal with the client work in bug 1009816 without a direct dependency here: that we could easily mock responses for local testing.

I hate integration testing at the last minute but I think we're already at the point where we'd be doing that anyway... we only have 4 weeks left in 33 nightly.
Blocks: 1009816
I'd like to begin work on bug 1009816 since it's wanted for FF33. 

Would it be possible to have answers to the following questions?

Will we use the same AUS ping that we do for updates? The new elements will appear within the existing structure? 

Either way, could you give me a sample file?  If it's inside the existing schema, please make the sample also include a full and/or partial update.  Will everything be in XML? Or JSON inside XML (Like Comment 1 suggests)? Or?

Also just a heads up for Bug 1009816 Comment 8. I suggest that we keep it simple for v1 as per bsmedberg suggested in v1 and then possibly add this in a follow up bug with a new field in addition to the hash after FF33.
Flags: needinfo?(bhearsum)
catlee provided me with best guess info on irc. Canceling request.
Flags: needinfo?(bhearsum)
(In reply to Brian R. Bondy [:bbondy] from comment #14)
> catlee provided me with best guess info on irc. Canceling request.

Yeah, we haven't figured out exactly what the XML will look like yet, I think. That will probably be dealt with in bug 1021021.
Assignee

Updated

5 years ago
No longer depends on: 1021029
This is a horrible patch that lets us return the necessary data for Firefox to process h264 updates. Once the dependent bugs are fixed we can rip this out and burn it.

I was originally going to stuff the xml into fileUrls, but that didn't work because those are checked against whitelisted domains. Those checks require "urlparse" to work, which obviously doesn't when fed arbitary text. So, ftpFilenames it is.

Below is an example blob I used when testing locally. I managed to upload it fine through the web interface. My suggested workflow for updating these is to delete and then re-upload. It will result in brief unavailability, but it's a PITA to edit existing blobs because we don't have UI for that, just an API.

{
    "schema_version": 3,
    "name": "HackyH264Blob",
    "hashFunction": "sha512",
    "ftpFilenames": {
        "completes": {
            "Linux_x86_64-gcc3": "<updates><addons><addon id=\"openh264-plugin@cisco.com\" URL=\"http://download.cdn.cisco.com/openh264/win32/openh264-1.1.zip\" hashFunction=\"SHA512\" hashValue=\"ABCDEF123456\" size=\"123\" version=\"1.1\" /></addons></updates>",
            "Linux_x86-gcc3": "",
            "Darwin_x86-gcc3-u-i386-x86_64": "",
            "Darwin_x86_64-gcc3-u-i386-x86_64": "",
            "WINNT_x86-msvc": ""
        }
    }
}
Attachment #8453787 - Flags: review?(nthomas)
Comment on attachment 8453787 [details] [diff] [review]
horrible temporary hack to support h264 endpoints

Looks like we should be prefixing each ftpFilename entry with '<?xml version="1.0"?>' too.
Attachment #8453787 - Flags: review?(nthomas) → review+

Comment 18

5 years ago
Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/a6ff677d28d08e8c4e0bf28b2921bd4b4a3b50ea
bug 1013354: Implement openh264 update manifests in AUS - hacky version until we're able to do it right. r=nthomas
(In reply to Nick Thomas [:nthomas] from comment #17)
> Comment on attachment 8453787 [details] [diff] [review]
> horrible temporary hack to support h264 endpoints
> 
> Looks like we should be prefixing each ftpFilename entry with '<?xml
> version="1.0"?>' too.

Good catch. Turns out that the test doesn't catch that comparing two minidom objects doesn't test for that, to my surprise.

Here's the patch as landed. Passes all tests, for whatever that's worth given the above.
Attachment #8454424 - Flags: checked-in+
I'll be testing the hacky version of this patch in dev today, and once I verify it I'll get it pushed to production.
Assignee

Updated

5 years ago
Attachment #8453787 - Attachment is obsolete: true
Assignee

Updated

5 years ago
Depends on: 1037440
Blocks: 1037767
Blocks: 1039490
Blocks: 1039555
(In reply to Ben Hearsum [:bhearsum] from comment #20)
> I'll be testing the hacky version of this patch in dev today, and once I
> verify it I'll get it pushed to production.

For posterity, this patch was put into production last week. We've been serving data at URLs like https://aus4.mozilla.org/update/3/GMP/1.0/12345/Linux_x86_64-gcc3/en-US/release/win/default/default/update.xml.

This bug is now tracking getting rid of the short term hack.
Blocks: 1039839
Blocks: 1040060
Per mreavy, we need to support different plugins by version now. I'd hoped we'd have this bug fixed properly before we needed that. In the meantime, this should do it.

When it gets pushed to production, existing plugin downloads will break briefly until I update the HackyH264Blob with new data. I'll try to keep that downtime extremely brief (< 1min hopefully).
Attachment #8461600 - Flags: review?(rail)
Comment on attachment 8461600 [details] [diff] [review]
find xml via version+buildTarget

I like the version name!
Attachment #8461600 - Flags: review?(rail) → review+

Comment 24

5 years ago
Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/d765bebc9d284426c0f11b2f567ec9f093dc7b7b
bug 1013354: Implement openh264 update manifests in AUS - allow for version+buildTarget in hacky h264 implementation
Assignee

Updated

5 years ago
Depends on: 1043455
Comment on attachment 8461600 [details] [diff] [review]
find xml via version+buildTarget

Checked in. bug 1043455 for the push to prod.
Attachment #8461600 - Flags: checked-in+
Assignee

Updated

5 years ago
Blocks: 1043911
Summary: Implement openh264 update manifests in AUS → Implement openh264 update manifests in Balrog
I'm planning on landing the non-hacky support for this next week.

This attachment is one of the new blobs we'll need to return the correct update responses. We had problems at the last uplift where we stopped returning results because they were tied to specific version numbers like "33.0a2". My intention is to have a new blob each time we need new GMP builds.

Here's my first idea for what rules could look like:
product: GMP, version: <=33.0, priority=95, mapping=foo
product: GMP, version: <=34.0, priority=93, mapping=bar
product: GMP, version: >=35.0, priority=91, mapping=baz

And then when we add a new version mid-cycle on mozilla-central:
product: GMP, version: >=35.0, buildid: >=201410010000, priority=92, mapping=blah

I feel like there's a simpler or better way to express this, but I can't come up with at the moment.

I'm not really sure what the best way to name the gmp blobs is, though. They're not necessarily tied to a specific Gecko version (even though we don't have any overlap at the moment), and we'll have other plugins in them in the future - so we shouldn't put any h264 specific things in the name.
Attachment #8492302 - Flags: feedback?(nthomas)
Attachment #8492302 - Flags: feedback?(catlee)
Comment on attachment 8492302 [details]
openh264 blob for Firefox/Gecko 33

This seems fine to me as a general approach, but I was having trouble reasoning about the priorities until I inverted the test for the older builds, and tweaked the priorities:

product: GMP, version: >=35.0, buildid: >=201411001000, priority=92, mapping=blurgh    (new rule)
product: GMP, version: >=35.0, buildid: >=201410010000, priority=91, mapping=blah
product: GMP, version: >=35.0,                          priority=90, mapping=baz
product: GMP, version: >=34.0,                          priority=80, mapping=bar
product: GMP, version: >=33.0,                          priority=70, mapping=foo

I've tried to leave more space between priorities, but like this they could be consecutive. Gecko branches presumably just work (TM) if GMP follows the trains, at least until we need a fix for beta/release users only ?

As for blob naming, I think we'll end up with a blob per vendor because of the conditions on buildid (aka gecko code matching).
Attachment #8492302 - Flags: feedback?(nthomas) → feedback+
(In reply to Nick Thomas [:nthomas] from comment #27)
> Comment on attachment 8492302 [details]
> example of proper h264 blob
> 
> This seems fine to me as a general approach, but I was having trouble
> reasoning about the priorities until I inverted the test for the older
> builds, and tweaked the priorities:
> 
> product: GMP, version: >=35.0, buildid: >=201411001000, priority=92,
> mapping=blurgh    (new rule)
> product: GMP, version: >=35.0, buildid: >=201410010000, priority=91,
> mapping=blah
> product: GMP, version: >=35.0,                          priority=90,
> mapping=baz
> product: GMP, version: >=34.0,                          priority=80,
> mapping=bar
> product: GMP, version: >=33.0,                          priority=70,
> mapping=foo

This looks much cleaner, thanks! It seems like we can probably age off the rules with buildid specified shortly after the mozilla-central version bumps. If someone is eg, two weeks out of date on Nightly, I'm not sure I care about what plugins we serve them.

> I've tried to leave more space between priorities, but like this they could
> be consecutive. Gecko branches presumably just work (TM) if GMP follows the
> trains, at least until we need a fix for beta/release users only ?

Good point about needing a fix for just beta or release users. Though, do we really care about special casing version=34.0, channel=beta at that point? Nightly and Aurora should be 36.0 and 35.0, so they shouldn't be on 34.0 anymore...

> As for blob naming, I think we'll end up with a blob per vendor because of
> the conditions on buildid (aka gecko code matching).

Hm, if we need a blob per vendor we'll need some tweaks to the code from bug 1021021. Right now there's no way to serve updates for multiple vendors unless they're in the same blob. Another possible way to do this would be to have a blob for every interesection needed. Eg, given vendors V1 and V2 and buildids 2000 through 2005, you may need:
* V1 and V2 work with any buildid on a branch at buildid 2000 (one rule, priority 70, pointing to this blob)
* V1 requires new version, V2 stays the same at buildid 2001 (add a rule to point at previous blob for buildid<=2000, priority 92, repoint original rule at new blob)
* V1 stays the same, V2 updates at buildid 2002 (add a rule to point at previous blob for buildid<=2001, priority 91)

...etc.

This doesn't scale well when you have too many vendors or a lot of updates, but it could get us by without code changes for a time.
re: gmp blob naming - I filed bug 1074395 about improving openh264 plugin versioning. That doesn't fully address the naming issues, but it might make it easier to figure out what to call them until we have a second plugin to serve.
Assignee

Updated

5 years ago
Attachment #8492302 - Attachment description: example of proper h264 blob → openh264 blob for Firefox/Gecko 33
Once the latest Balrog changes are pushed to prod, I will be uploading the previous blob and the current blob to it and pointing rules at them. I've precreated the rules, currently pointing at No-Update. In case anyone's curious, here's what they look like currently:
mysql> select backgroundRate, priority, product, version, mapping from rules where product="GMP";
+----------------+----------+---------+---------+-----------+
| backgroundRate | priority | product | version | mapping   |
+----------------+----------+---------+---------+-----------+
|            100 |       10 | GMP     | >=33.0  | No-Update |
|            100 |       20 | GMP     | >=34.0  | No-Update |
+----------------+----------+---------+---------+-----------+

(I went with really low priorities for these rules, since we're going with +10 every 6 weeks for now. We'll have to adjust this later when we run out of numbers, of course.)
Assignee: nobody → bhearsum
Status: NEW → ASSIGNED
Juan, this bug is going to change the way OpenH264 plugins are served to Firefox when it lands. It should be a no-op for clients, but I'd really like to get QA to verify that after it's landed. The change also involves IT, so I'd like to organize a time that works for everyone (in case we need to back out).

Verification should involve starting Firefox with a fresh profile and making sure the OpenH264 plugin is downloaded and installed. This should be done for at least the following platforms
* Linux 32-bit
* Linux 64-bit
* Windows 32-bit
* Mac 64-bit (and if you have access to 32-bit Mac machines, that would be nice to have, too)

On at least the following versions:
* The latest Firefox 33.0 Beta
* The latest Firefox Aurora
* The latest Firefox Nightly

Doing most of the testing with en-US is fine, but doing one of the above combinations with a localized build would be good, too.

The most likely time for the push is the Pacific morning (day TBD), but we can haggle on that if it doesn't work for QE.

Let me know if you have any questions or need more info!
Flags: needinfo?(jbecerra)
Flags: needinfo?(jbecerra)
Verified based on comment 31 on all versions, including localized builds. No issues found.
Testing details are available here: https://etherpad.mozilla.org/OpenH264-UpdateManifests-Balrog
Comment on attachment 8454424 [details] [diff] [review]
hacky h264 w/ xml version set

With bug 1021021 landing, I backed out the hacky patches and deleted the HackyH264Blob.
Attachment #8454424 - Flags: checked-in+ → checked-in-
Assignee

Updated

5 years ago
Attachment #8461600 - Flags: checked-in+ → checked-in-
I ended up going with GMP-Firefox33-201410010830 and GMP-Firefox34plus-201410010830 as names for the blobs for now. I also had to fix the vendor id (gmp-openh264 should've been gmp-gmpopenh264). This is all landed now though, and everything works great!

With this bug fixed, we shouldn't hit another instance of bug 1062259 at the next uplift.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Assignee

Updated

5 years ago
Attachment #8492302 - Flags: feedback?(catlee)
You need to log in before you can comment on or make changes to this bug.