Closed Bug 1052016 Opened 10 years ago Closed 10 years ago

B2G mochitests attempt to connect to 2.pool.ntp.org & 3.pool.ntp.org during automation

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(firefox32 wontfix, firefox33 wontfix, firefox34 fixed, b2g-v1.3 wontfix, b2g-v1.3T wontfix, b2g-v1.4 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.1 S2 (15aug)
Tracking Status
firefox32 --- wontfix
firefox33 --- wontfix
firefox34 --- fixed
b2g-v1.3 --- wontfix
b2g-v1.3T --- wontfix
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: emorley, Assigned: emorley)

References

Details

Attachments

(1 file)

For tests jobs in automation to meet the visibility policy (https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy), they must not make external connections. Bug 1039019 is trying to enable MOZ_DISABLE_NONLOCAL_CONNECTIONS for B2G mochitests (carrying on the work started in bug 995417), however I'm seeing the following attempted connections: https://tbpl.mozilla.org/php/getParsedLog.php?id=45336486&tree=Try b2g_emulator_vm try opt test mochitest-6 2.pool.ntp.org (198.55.111.50) https://tbpl.mozilla.org/php/getParsedLog.php?id=45333715&tree=Try b2g_emulator_vm try opt test mochitest-7 3.pool.ntp.org (198.60.22.240) MXR shows: http://mxr.mozilla.org/mozilla-central/search?string=2.pool.ntp.org&case=1&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central http://mxr.mozilla.org/mozilla-central/source/b2g/app/b2g.js#886 http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/Sntp.jsm#292 These were introduced by bug 874771. We should set the servers to dummy localhost addresses and/or set the refresh time to something high (9999999 seconds etc), so we don't make these external connections in automation (only). This is what we do for all other services that make such connections (updates, safebrowsing, ...). This will likely mean adding prefs to http://mxr.mozilla.org/mozilla-central/source/testing/profiles/prefs_general.js
882 // SNTP preferences. ... 884 pref("network.sntp.refreshPeriod", 86400); // In seconds. 885 pref("network.sntp.pools", // Servers separated by ';'. 886 "0.pool.ntp.org;1.pool.ntp.org;2.pool.ntp.org;3.pool.ntp.org");
Gene, Shao was the original author here but his Bugzilla account is disabled. As the reviewer of bug 874771, what would be your preferred approach? :-)
Flags: needinfo?(airpingu)
Depends on: 1053118
No longer depends on: 1053118
Attached patch Patch v1 (deleted) — Splinter Review
Assignee: nobody → emorley
Status: NEW → ASSIGNED
Flags: needinfo?(airpingu)
Comment on attachment 8472253 [details] [diff] [review] Patch v1 Both Gene & Shao have left since bug 874771, so we may have to be a bit more flexible on reviewer :-) The try run for this is green with MOZ_DISABLE_NONLOCAL_CONNECTIONS set: https://tbpl.mozilla.org/?tree=Try&rev=67cdda9e6b14 There doesn't appear to be a way to disable the SNTP time updates from occurring without also disabling using NITZ (bug 874771 comment 46) - the knobs we're able to twiddle are those passed in here: http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/RadioInterfaceLayer.js#1982 1982 this._sntp = new Sntp(this.setClockBySntp.bind(this), 1983 Services.prefs.getIntPref("network.sntp.maxRetryCount"), 1984 Services.prefs.getIntPref("network.sntp.refreshPeriod"), 1985 Services.prefs.getIntPref("network.sntp.timeout"), 1986 Services.prefs.getCharPref("network.sntp.pools").split(";"), 1987 Services.prefs.getIntPref("network.sntp.port")); Which have these defaults: http://mxr.mozilla.org/mozilla-central/source/b2g/app/b2g.js#882 882 // SNTP preferences. 883 pref("network.sntp.maxRetryCount", 10); 884 pref("network.sntp.refreshPeriod", 86400); // In seconds. 885 pref("network.sntp.pools", // Servers separated by ';'. 886 "0.pool.ntp.org;1.pool.ntp.org;2.pool.ntp.org;3.pool.ntp.org"); 887 pref("network.sntp.port", 123); 888 pref("network.sntp.timeout", 30); // In seconds. Setting refreshPeriod to zero disables later refreshing, but from what I can tell doesn't disable the initial update that is caused when a data connection becomes available: http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/Sntp.jsm#25 http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/Sntp.jsm#185 Setting |network.sntp.pools| to an empty string would result in us hitting the hardcoded defaults in Sntp.jsm (bug 1053118): http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/Sntp.jsm#289 http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/Sntp.jsm#48 So I've just used "%(server)s". I've set |network.sntp.maxRetryCount| to zero, since the value in b2g.js is 10 - and that seems a little excessive, given that we know that port 123 won't be open on localhost. I'm up for setting the retry to 1, if you think it's worth at least covering the retry path once during tests in automation.
Attachment #8472253 - Flags: review?(nfroyd)
Comment on attachment 8472253 [details] [diff] [review] Patch v1 Review of attachment 8472253 [details] [diff] [review]: ----------------------------------------------------------------- r=me with maxRetryCount set to 1; a comment about why it's 1 would be desirable, too, I think. We'll definitely exercise the retry path in our automation setup, which seems valuable.
Attachment #8472253 - Flags: review?(nfroyd) → review+
(In reply to Nathan Froyd (:froydnj) from comment #6) > r=me with maxRetryCount set to 1; a comment about why it's 1 would be > desirable, too, I think. We'll definitely exercise the retry path in our > automation setup, which seems valuable. Done :-) https://hg.mozilla.org/integration/mozilla-inbound/rev/692265fa9e80
Flags: needinfo?(ryanvm)
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: