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)
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)
People
(Reporter: emorley, Assigned: emorley)
References
Details
Attachments
(1 file)
(deleted),
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•10 years ago
|
||
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");
Assignee | ||
Comment 2•10 years ago
|
||
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)
Gene left too.
Assignee | ||
Comment 4•10 years ago
|
||
Assignee: nobody → emorley
Status: NEW → ASSIGNED
Flags: needinfo?(airpingu)
Assignee | ||
Comment 5•10 years ago
|
||
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 6•10 years ago
|
||
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+
Assignee | ||
Comment 7•10 years ago
|
||
(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
Updated•10 years ago
|
Flags: needinfo?(ryanvm)
Comment 8•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 9•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/daa768a58750
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/4ab0d87178a5
status-b2g-v1.3:
--- → wontfix
status-b2g-v1.3T:
--- → wontfix
status-b2g-v1.4:
--- → fixed
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
status-firefox32:
--- → wontfix
status-firefox33:
--- → wontfix
status-firefox34:
--- → fixed
Flags: needinfo?(ryanvm)
Target Milestone: --- → 2.1 S2 (15aug)
You need to log in
before you can comment on or make changes to this bug.
Description
•