748689 - Support OS deprecation in Balrog

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Nick Thomas [:nthomas] (UTC+12)]

In AUS2 we have this in the config file:

/*
 * Array that defines which %OS_VERSION% values are no longer supported.
 * Applies to all updates and uses the version of the update to be 
 * served to determine blocking (bug 666735). Previously only applied to
 * major updates, and used the version from the incoming URI (bug 418129)
 * Use of this array is in inc/patch.class.php.  
 *
 * The Array format is considered to be:
 * array(
 *      $Product => array(
 *          $Version => array(
 *              $OS_VERSION
 *          )
 *      )
 *  )
 *
 * $Product is the product name (Firefox, Thunderbird, etc. - %PRODUCT%).
 *
 * $Version is a string which identifies some set of releases
 *   '1.0.2'  - an exact version
 *   '1.0*'   - all versions starting '1.0' (via a regexp)
 *   '1.0b3+' - all versions from 1.0b3 onwards (via php's version_compare)
 *
 * $OS_VERSION is used in a string match (existence anywhere in passed
 * %OS_VERSION% triggers blocklisting of that OS).
 */
$unsupportedPlatforms = array(
    'Firefox'     =>  array(
        // Mac 10.2/10.3, Win < 2k, GTK < 2.10 - bug 418129
        '3.0b1+' => array(
            'Darwin 6',
            'Darwin 7',
            'Windows_95',
            'Windows_98',
            'Windows_NT 4',
            'GTK 2.0.',
            'GTK 2.1.',
            'GTK 2.2.',
            'GTK 2.3.',
            'GTK 2.4.',
            'GTK 2.5.',
            'GTK 2.6.',
            'GTK 2.7.',
            'GTK 2.8.',
            'GTK 2.9.'
        ),
        // Mac 10.4 - bug 640044
        // See index.php for PPC
        '4.0b1+' => array(
            'Darwin 8'
        ),
        // RHEL5 has too old libstdc++ - bug 655917
        // Fx6 will ship with --enable-stdcxx-compat
        '4.0*' => array(
            '.el5'
        ),
        '5.0*' => array(
            '.el5'
        ),
        // Too old freetype - bug 666735
        '7.0a2+' => array(
            'GTK 2.10.'
        ),
        // Block Win2000, XP RTM & SP1 after switching to MSVC2010
        '13.0a1+' => array (
            'Windows_NT 5.0',
            'Windows_NT 5.1.0',
            'Windows_NT 5.1.1',
        ),
    ),

This is used in patch.class.php's isSupported(), called in index.php after we have looked up the update offer, and know what the version offered would be. We need to have this functionality in Balrog too.

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

I think this should be pretty easy, if you don't mind changing the model a bit.

For each deprecated OS you write a rule with osVersion set to the sub-string you need to block, and map it to the last compatible build. (Plus set a higher priority than your regular rapid beta/release updates, possibly product, channel if you need to be specific.) We would have to change the comparison in getRulesMatchingQuery()
 https://github.com/mozilla/balrog/blob/c76a711cc3/auslib/db.py#L649
from a direct comparison to an IN or similar (AUSv2 is doing php's strpos()). We don't want to enumerate all the many values for all of osVersion as rules, just pick out the key characters. (ie Darwin 7 but we don't care about which subversion).

AUS2 provides no update at all once the offered update is to a deprecated version, so Balrog at least lets us keep people as update-to-date as possible before dropping support. 

Regarding the + and * wildcards in AUS2 and potentially losing flexibility - I think a + entry turns into a permanent rule - we point at the previous release or beta. A * entry is written the same but it has a finite lifetime, getting removed when the new version which isn't blocked is released.

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

We'll need this before switch nightly updates to Balrog.

Comment 3

[Nick Thomas [:nthomas] (UTC+12)]

Attachment: Null update blob

Adding this as a release called 'No-Update' allows you to write rules that point at empty updates. Ideally it wouldn't need an actual list of platforms for the empty locale list, but that's fodder for making expandRelease() work better.

Comment 4

[[e-mail] (:bhearsum)]

Attachment: support os substring matching w/ multiple os versions in one rule

Comment 5

[[e-mail] (:bhearsum)]

Attachment: fully tested patch

I think this is ready to go now. I tested this locally by importing a dump from balrog dev, adjusting the priority of the existing rules to 90, and adding this rule (null columns removed for brevity):
+---------+----------+-----------+---------+----------+-----------------------------------------------------------+-------------+
| rule_id | priority | mapping   | product | channel  | osVersion                                                 | update_type |
+---------+----------+-----------+---------+----------+-----------------------------------------------------------+-------------+
|       9 |      100 | No-Update | Firefox | nightly* | GTK 2.8,GTK 2.17,Darwin 9,Windows_NT 5.0,Windows_NT 5.1.1 | minor       |
+---------+----------+-----------+---------+----------+-----------------------------------------------------------+-------------+

Then I ran update comparison tests against aus3.mozilla.org with the following URLs:

/update/3/Firefox/23.0a1/20130412030828/Linux_x86_64-gcc3/en-US/nightly/Linux%203.8.6-1-ARCH%20(GTK%202.8.17)/default/default/update.xml
/update/3/Firefox/23.0a1/20130412030828/Linux_x86_64-gcc3/en-US/nightly/Linux%202.6.32-5-amd64%20(GTK%202.17.1)/default/default/update.xml
/update/3/Firefox/16.0a2/20120827042008/Darwin_x86-gcc3-u-i386-x86_64/en-US/aurora/Darwin%209.8.0/default/default/update.xml
/update/3/Firefox/23.0a1/20130412030828/WINNT_x86-msvc/en-US/nightly/Windows_NT%205.0.0.0%20(x64)/default/default/update.xml
/update/3/Firefox/23.0a1/20130406030922/WINNT_x86-msvc/en-US/nightly/Windows_NT%205.1.1.0%20(x86)/default/default/update.xml

All returned an empty update.

This patch also fixes up new rule additions to work for build ID, OS Version, and a few other columns where keywords were set wrong.

One small concern I have is that the OS blocking rules' OS Version columns will get very wide - to the point where we're going to need to copy/paste somewhere else to make them readable. This is fixable through UI enhancements, but it makes me wonder if we're better off without special parsing in OS Version and enchancing the UI to make more rules easier to deal with. I don't have strong feelings either way, just a thought.

Comment 6

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 766028 [details] [diff] [review]
fully tested patch

This looks fine to me.

> One small concern I have is that the OS blocking rules' OS Version columns
> will get very wide - to the point where we're going to need to copy/paste
> somewhere else to make them readable. This is fixable through UI
> enhancements, but it makes me wonder if we're better off without special
> parsing in OS Version and enchancing the UI to make more rules easier to
> deal with. I don't have strong feelings either way, just a thought.

I see what you're saying, especially given the rule page is already really wide. Do you have anything specific in mind to making lots of rules easier to work with ? I think you can land this as is, but feel free to go further if you prefer.

Comment 7

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] from comment #6)
> Comment on attachment 766028 [details] [diff] [review]
> fully tested patch
> 
> This looks fine to me.
> 
> > One small concern I have is that the OS blocking rules' OS Version columns
> > will get very wide - to the point where we're going to need to copy/paste
> > somewhere else to make them readable. This is fixable through UI
> > enhancements, but it makes me wonder if we're better off without special
> > parsing in OS Version and enchancing the UI to make more rules easier to
> > deal with. I don't have strong feelings either way, just a thought.
> 
> I see what you're saying, especially given the rule page is already really
> wide. Do you have anything specific in mind to making lots of rules easier
> to work with ? I think you can land this as is, but feel free to go further
> if you prefer.

I don't have anything in mind, I think we'll have to come back to this at some point though. (Come back to either making more rules more manageable, or long os versions more readable).

Comment 8

[[e-mail] (:bhearsum)]

Attachment: make os version column longer

Hit a problem when trying to add os version rules - the column isn't long enough! The rule I constructed for all of the existing OS' in config-dist.php was 290 characters, so I figured that 1000 should be enough for the forseeable future...easy enough to increase again if needed.

I tested the downgrade case locally, which will truncate the data.

Comment 9

[[e-mail] (:bhearsum)]

So I don't lose them, here's what I'm planning on adding, leaving it applicable to all products, but only nightly* channels. Technically, we could use all channels here, but the next time we add an exception that needs to ride trains it'd be easy to accidentally apply it to all channels. I'm thinking that it's probably best to have one osVersion rule per train (nightly/aurora/beta/release/esr):
Darwin 6,Darwin 7,Darwin 8,Darwin 9,Windows_95,Windows_98,Windows_NT 4,Windows_NT 5.0,Windows_NT 5.1.0,Windows_NT 5.1.1,GTK 2.0.,GTK 2.1.,GTK 2.2.,GTK 2.3.,GTK 2.4.,GTK 2.5.,GTK 2.6.,GTK 2.7.,GTK 2.8.,GTK 2.9.,GTK 2.10.,GTK 2.11.,GTK 2.12.,GTK 2.13.,GTK 2.14.,GTK 2.15.,GTK 2.16.,GTK 2.17.

I left .el5 out because we don't have any updates pointing at Firefox 4/5.

Comment 10

[Nick Thomas [:nthomas] (UTC+12)]

Oh, I didn't grok that you were going to put all the deprecations in a single rule, except for channels. Presumably we'll append as we go too ? For some reason I'd always assumed adding a rule each time we deprecate some new set, particularly in the beta/release case where we can still point at the most recent release that is still supported.

Comment 11

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] from comment #10)
> Oh, I didn't grok that you were going to put all the deprecations in a
> single rule, except for channels. Presumably we'll append as we go too ? For
> some reason I'd always assumed adding a rule each time we deprecate some new


I could separate these out, but since they're all going to point to a null I figured I'd put them in the same rule. I think for future ones (even on nightly/aurora) we'll want individual rules so that we can point those users at something, at least for some period of time....

> set, particularly in the beta/release case where we can still point at the
> most recent release that is still supported.

Yeah, I think for betas/releases we'll have permarules to point people at their latest supported version.

Comment 12

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 767184 [details] [diff] [review]
make os version column longer

Sounds fine to me.

Comment 13

[[github robot]]

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

https://github.com/mozilla/balrog/commit/9a60cf9fb3f6d5f6b269dc1ea51a49636ce5554f
bug 748689: Support OS deprecation in Balrog - make osVersion column longer to accommodate many versions in one rule.

Comment 14

[[e-mail] (:bhearsum)]

Comment on attachment 767184 [details] [diff] [review]
make os version column longer

Landed and uprgaded the dev db.

Comment 15

[[github robot]]

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

https://github.com/mozilla/balrog/commit/adb45b60b2b9b31e0a765ad3f2dd4c8f943adc45
bug 748689: Support OS deprecation in Balrog. r=nthomas

Comment 16

[[e-mail] (:bhearsum)]

Comment on attachment 766028 [details] [diff] [review]
fully tested patch

I added the rule and I've pushed this now...I'll be running another snippet comparison after the dev environment picks this up, which should show a lot more passes. We'll want to run that again after we unthrottle on AUS3 though, because we'll get some fake passes due to that at the moment.

Comment 17

[[e-mail] (:bhearsum)]

Well, I re-ran the snippet comparison and didn't end up with any new passes. Aurora tests fail because we've got those being sent to auroratest on AUS3. Desktop nightlies still fail because of extra attributes. Fennec nightlies are failing because AUS3 is still serving 20130626 for some reason (bug 888302). And AFAICT, we don't have anybody on deprecated versions on the nightly channel, which makes sense.

So, the best we can do to verify this for now is manual tests. I tried a few things like:
https://aus4-dev.allizom.org/update/3/Firefox/10.0a1/20120222174716/Linux_x86-gcc3/en-US/nightly/Darwin%2010/default/default/update.xml vs. https://aus4-dev.allizom.org/update/3/Firefox/10.0a1/20120222174716/Linux_x86-gcc3/en-US/nightly/Darwin%208/default/default/update.xml (Darwin 8 vs Darwin 10)
https://aus4-dev.allizom.org//update/3/Firefox/23.0a1/20130412030828/WINNT_x86-msvc/en-US/nightly/Windows_NT%205.1.1.0%20%28x64%29/default/default/update.xml vs https://aus4-dev.allizom.org//update/3/Firefox/23.0a1/20130412030828/WINNT_x86-msvc/en-US/nightly/Windows_NT%205.2.1.0%20%28x64%29/default/default/update.xml (Windows 5.1.1.0 vs 5.2.1.0)
https://aus4-dev.allizom.org/update/3/Firefox/14.0a1/20111108031146/Linux_x86_64-gcc3/en-US/nightly/Linux%203.0.0-12-generic%20%28GTK%202.10.6%29/default/default/update.xml?force=1 vs https://aus4-dev.allizom.org/update/3/Firefox/14.0a1/20111108031146/Linux_x86_64-gcc3/en-US/nightly/Linux%203.0.0-12-generic%20%28GTK%202.24.6%29/default/default/update.xml?force=1 (GTK 2.10.6 vs GTK 2.24.6)

All of which work as I would expect them to.