From d6e36d820e6defa4e1357e01f8f46ae3535ce27c Mon Sep 17 00:00:00 2001 From: Luke Huang Date: Sat, 11 Apr 2020 02:56:37 +0800 Subject: [PATCH] Deflaky test for DnsResolverTest It's possible that private DNS setting is not in the state we expected when we tried to enable strict mode during tests. The problem here is that there are 2 setting Uris(mode and specifier) relating to strict mode, each of them might trigger private DNS setting changing evnet in ConnectivityService. Previously, we tried to enable strict mode with first set private DNS mode and then private DNS specifier. This may result in 2 consecutive private DNS changes events with very short intervals, which caused conflicts between DnsResolver / NetworkMonitor and lead to flaky tests. So 0. Use opportunistic as default mode if no default mode existed. 1. Change the order (mode and specifier) for enabling strict mode. 2. Change private DNS mode only when needed. (If original mode is "hostname", then we only need to set specifier) Bug: 153624005 Bug: 151122313 Bug: 150952393 Test: atest DnsResolverTest --rerun-until-failure 100 Test: forrest (git_master, cts/networking/gce-all) Test: forrest (git_rvc-dev, atest CtsNetTestCases) Test: forrest (git_rvc-dev, mts/dnsresolver/device-all) Change-Id: I224a6493c87cebaf0bf954c2644e2945ccd50db1 --- .../src/android/net/cts/DnsResolverTest.java | 35 ++--------- .../android/net/cts/MultinetworkApiTest.java | 23 ++----- .../android/net/cts/util/CtsNetUtils.java | 62 +++++++++++++++++-- 3 files changed, 66 insertions(+), 54 deletions(-) diff --git a/tests/cts/net/src/android/net/cts/DnsResolverTest.java b/tests/cts/net/src/android/net/cts/DnsResolverTest.java index 1cc49f900a..28753ffc41 100644 --- a/tests/cts/net/src/android/net/cts/DnsResolverTest.java +++ b/tests/cts/net/src/android/net/cts/DnsResolverTest.java @@ -86,7 +86,6 @@ public class DnsResolverTest extends AndroidTestCase { static final int CANCEL_RETRY_TIMES = 5; static final int QUERY_TIMES = 10; static final int NXDOMAIN = 3; - static final int PRIVATE_DNS_SETTING_TIMEOUT_MS = 6_000; private ContentResolver mCR; private ConnectivityManager mCM; @@ -107,32 +106,15 @@ public class DnsResolverTest extends AndroidTestCase { mExecutorInline = (Runnable r) -> r.run(); mCR = getContext().getContentResolver(); mCtsNetUtils = new CtsNetUtils(getContext()); - storePrivateDnsSetting(); + mCtsNetUtils.storePrivateDnsSetting(); } @Override protected void tearDown() throws Exception { - restorePrivateDnsSetting(); + mCtsNetUtils.restorePrivateDnsSetting(); super.tearDown(); } - private void storePrivateDnsSetting() { - // Store private DNS setting - mOldMode = Settings.Global.getString(mCR, Settings.Global.PRIVATE_DNS_MODE); - mOldDnsSpecifier = Settings.Global.getString(mCR, Settings.Global.PRIVATE_DNS_SPECIFIER); - } - - private void restorePrivateDnsSetting() throws InterruptedException { - // restore private DNS setting - Settings.Global.putString(mCR, Settings.Global.PRIVATE_DNS_MODE, mOldMode); - if ("hostname".equals(mOldMode)) { - Settings.Global.putString( - mCR, Settings.Global.PRIVATE_DNS_SPECIFIER, mOldDnsSpecifier); - mCtsNetUtils.awaitPrivateDnsSetting("restorePrivateDnsSetting timeout", - mCM.getActiveNetwork(), mOldDnsSpecifier, PRIVATE_DNS_SETTING_TIMEOUT_MS, true); - } - } - private static String byteArrayToHexString(byte[] bytes) { char[] hexChars = new char[bytes.length * 2]; for (int i = 0; i < bytes.length; ++i) { @@ -416,16 +398,13 @@ public class DnsResolverTest extends AndroidTestCase { final String msg = "RawQuery " + TEST_NX_DOMAIN + " with private DNS"; // Enable private DNS strict mode and set server to dns.google before doing NxDomain test. // b/144521720 - Settings.Global.putString(mCR, Settings.Global.PRIVATE_DNS_MODE, "hostname"); - Settings.Global.putString(mCR, - Settings.Global.PRIVATE_DNS_SPECIFIER, GOOGLE_PRIVATE_DNS_SERVER); + mCtsNetUtils.setPrivateDnsStrictMode(GOOGLE_PRIVATE_DNS_SERVER); for (Network network : getTestableNetworks()) { final Network networkForPrivateDns = (network != null) ? network : mCM.getActiveNetwork(); assertNotNull("Can't find network to await private DNS on", networkForPrivateDns); mCtsNetUtils.awaitPrivateDnsSetting(msg + " wait private DNS setting timeout", - networkForPrivateDns, GOOGLE_PRIVATE_DNS_SERVER, - PRIVATE_DNS_SETTING_TIMEOUT_MS, true); + networkForPrivateDns, GOOGLE_PRIVATE_DNS_SERVER, true); final VerifyCancelCallback callback = new VerifyCancelCallback(msg); mDns.rawQuery(network, TEST_NX_DOMAIN, CLASS_IN, TYPE_AAAA, FLAG_NO_CACHE_LOOKUP, executor, null, callback); @@ -688,9 +667,7 @@ public class DnsResolverTest extends AndroidTestCase { final Network[] testNetworks = getTestableNetworks(); // Set an invalid private DNS server - Settings.Global.putString(mCR, Settings.Global.PRIVATE_DNS_MODE, "hostname"); - Settings.Global.putString(mCR, - Settings.Global.PRIVATE_DNS_SPECIFIER, INVALID_PRIVATE_DNS_SERVER); + mCtsNetUtils.setPrivateDnsStrictMode(INVALID_PRIVATE_DNS_SERVER); final String msg = "Test PrivateDnsBypass " + TEST_DOMAIN; for (Network network : testNetworks) { // This test cannot be ran with null network because we need to explicitly pass a @@ -699,7 +676,7 @@ public class DnsResolverTest extends AndroidTestCase { // wait for private DNS setting propagating mCtsNetUtils.awaitPrivateDnsSetting(msg + " wait private DNS setting timeout", - network, INVALID_PRIVATE_DNS_SERVER, PRIVATE_DNS_SETTING_TIMEOUT_MS, false); + network, INVALID_PRIVATE_DNS_SERVER, false); final CountDownLatch latch = new CountDownLatch(1); final DnsResolver.Callback> errorCallback = diff --git a/tests/cts/net/src/android/net/cts/MultinetworkApiTest.java b/tests/cts/net/src/android/net/cts/MultinetworkApiTest.java index f123187d86..985e313a92 100644 --- a/tests/cts/net/src/android/net/cts/MultinetworkApiTest.java +++ b/tests/cts/net/src/android/net/cts/MultinetworkApiTest.java @@ -41,7 +41,6 @@ public class MultinetworkApiTest extends AndroidTestCase { private static final String TAG = "MultinetworkNativeApiTest"; static final String GOOGLE_PRIVATE_DNS_SERVER = "dns.google"; - static final int PRIVATE_DNS_SETTING_TIMEOUT_MS = 2_000; /** * @return 0 on success @@ -69,7 +68,7 @@ public class MultinetworkApiTest extends AndroidTestCase { mCM = (ConnectivityManager) getContext().getSystemService(Context.CONNECTIVITY_SERVICE); mCR = getContext().getContentResolver(); mCtsNetUtils = new CtsNetUtils(getContext()); - storePrivateDnsSetting(); + mCtsNetUtils.storePrivateDnsSetting(); } @Override @@ -77,18 +76,6 @@ public class MultinetworkApiTest extends AndroidTestCase { super.tearDown(); } - private void storePrivateDnsSetting() { - // Store private DNS setting - mOldMode = Settings.Global.getString(mCR, Settings.Global.PRIVATE_DNS_MODE); - mOldDnsSpecifier = Settings.Global.getString(mCR, Settings.Global.PRIVATE_DNS_SPECIFIER); - } - - private void restorePrivateDnsSetting() { - // restore private DNS setting - Settings.Global.putString(mCR, Settings.Global.PRIVATE_DNS_MODE, mOldMode); - Settings.Global.putString(mCR, Settings.Global.PRIVATE_DNS_SPECIFIER, mOldDnsSpecifier); - } - private Network[] getTestableNetworks() { final ArrayList testableNetworks = new ArrayList(); for (Network network : mCM.getAllNetworks()) { @@ -239,17 +226,15 @@ public class MultinetworkApiTest extends AndroidTestCase { // Enable private DNS strict mode and set server to dns.google before doing NxDomain test. // b/144521720 try { - Settings.Global.putString(mCR, Settings.Global.PRIVATE_DNS_MODE, "hostname"); - Settings.Global.putString(mCR, - Settings.Global.PRIVATE_DNS_SPECIFIER, GOOGLE_PRIVATE_DNS_SERVER); + mCtsNetUtils.setPrivateDnsStrictMode(GOOGLE_PRIVATE_DNS_SERVER); for (Network network : getTestableNetworks()) { // Wait for private DNS setting to propagate. mCtsNetUtils.awaitPrivateDnsSetting("NxDomain test wait private DNS setting timeout", - network, GOOGLE_PRIVATE_DNS_SERVER, PRIVATE_DNS_SETTING_TIMEOUT_MS, true); + network, GOOGLE_PRIVATE_DNS_SERVER, true); runResNnxDomainCheck(network.getNetworkHandle()); } } finally { - restorePrivateDnsSetting(); + mCtsNetUtils.restorePrivateDnsSetting(); } } } diff --git a/tests/cts/net/util/java/android/net/cts/util/CtsNetUtils.java b/tests/cts/net/util/java/android/net/cts/util/CtsNetUtils.java index 824146fedf..f39b184914 100644 --- a/tests/cts/net/util/java/android/net/cts/util/CtsNetUtils.java +++ b/tests/cts/net/util/java/android/net/cts/util/CtsNetUtils.java @@ -17,6 +17,7 @@ package android.net.cts.util; import static android.Manifest.permission.NETWORK_SETTINGS; +import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OPPORTUNISTIC; import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; @@ -28,6 +29,7 @@ import static org.junit.Assert.fail; import android.annotation.NonNull; import android.content.BroadcastReceiver; import android.content.Context; +import android.content.ContentResolver; import android.content.Intent; import android.content.IntentFilter; import android.net.ConnectivityManager; @@ -39,6 +41,7 @@ import android.net.NetworkInfo; import android.net.NetworkInfo.State; import android.net.NetworkRequest; import android.net.wifi.WifiManager; +import android.provider.Settings; import android.system.Os; import android.system.OsConstants; import android.util.Log; @@ -59,6 +62,7 @@ public final class CtsNetUtils { private static final int SOCKET_TIMEOUT_MS = 2000; private static final int PRIVATE_DNS_PROBE_MS = 1_000; + public static final int PRIVATE_DNS_SETTING_TIMEOUT_MS = 6_000; public static final int HTTP_PORT = 80; public static final String TEST_HOST = "connectivitycheck.gstatic.com"; public static final String HTTP_REQUEST = @@ -69,15 +73,19 @@ public final class CtsNetUtils { public static final String NETWORK_CALLBACK_ACTION = "ConnectivityManagerTest.NetworkCallbackAction"; - private Context mContext; - private ConnectivityManager mCm; - private WifiManager mWifiManager; + private final Context mContext; + private final ConnectivityManager mCm; + private final ContentResolver mCR; + private final WifiManager mWifiManager; private TestNetworkCallback mCellNetworkCallback; + private String mOldPrivateDnsMode; + private String mOldPrivateDnsSpecifier; public CtsNetUtils(Context context) { mContext = context; mCm = (ConnectivityManager) mContext.getSystemService(Context.CONNECTIVITY_SERVICE); mWifiManager = (WifiManager) mContext.getSystemService(Context.WIFI_SERVICE); + mCR = context.getContentResolver(); } // Toggle WiFi twice, leaving it in the state it started in @@ -249,9 +257,51 @@ public final class CtsNetUtils { return s; } + public void storePrivateDnsSetting() { + // Store private DNS setting + mOldPrivateDnsMode = Settings.Global.getString(mCR, Settings.Global.PRIVATE_DNS_MODE); + mOldPrivateDnsSpecifier = Settings.Global.getString(mCR, + Settings.Global.PRIVATE_DNS_SPECIFIER); + // It's possible that there is no private DNS default value in Settings. + // Give it a proper default mode which is opportunistic mode. + if (mOldPrivateDnsMode == null) { + mOldPrivateDnsSpecifier = ""; + mOldPrivateDnsMode = PRIVATE_DNS_MODE_OPPORTUNISTIC; + Settings.Global.putString(mCR, + Settings.Global.PRIVATE_DNS_SPECIFIER, mOldPrivateDnsSpecifier); + Settings.Global.putString(mCR, Settings.Global.PRIVATE_DNS_MODE, mOldPrivateDnsMode); + } + } + + public void restorePrivateDnsSetting() throws InterruptedException { + if (mOldPrivateDnsMode == null || mOldPrivateDnsSpecifier == null) { + return; + } + // restore private DNS setting + if ("hostname".equals(mOldPrivateDnsMode)) { + setPrivateDnsStrictMode(mOldPrivateDnsSpecifier); + awaitPrivateDnsSetting("restorePrivateDnsSetting timeout", + mCm.getActiveNetwork(), + mOldPrivateDnsSpecifier, true); + } else { + Settings.Global.putString(mCR, Settings.Global.PRIVATE_DNS_MODE, mOldPrivateDnsMode); + } + } + + public void setPrivateDnsStrictMode(String server) { + // To reduce flake rate, set PRIVATE_DNS_SPECIFIER before PRIVATE_DNS_MODE. This ensures + // that if the previous private DNS mode was not "hostname", the system only sees one + // EVENT_PRIVATE_DNS_SETTINGS_CHANGED event instead of two. + Settings.Global.putString(mCR, Settings.Global.PRIVATE_DNS_SPECIFIER, server); + final String mode = Settings.Global.getString(mCR, Settings.Global.PRIVATE_DNS_MODE); + // If current private DNS mode is "hostname", we only need to set PRIVATE_DNS_SPECIFIER. + if (!"hostname".equals(mode)) { + Settings.Global.putString(mCR, Settings.Global.PRIVATE_DNS_MODE, "hostname"); + } + } + public void awaitPrivateDnsSetting(@NonNull String msg, @NonNull Network network, - @NonNull String server, int timeoutMs, - boolean requiresValidatedServers) throws InterruptedException { + @NonNull String server, boolean requiresValidatedServers) throws InterruptedException { CountDownLatch latch = new CountDownLatch(1); NetworkRequest request = new NetworkRequest.Builder().clearCapabilities().build(); NetworkCallback callback = new NetworkCallback() { @@ -266,7 +316,7 @@ public final class CtsNetUtils { } }; mCm.registerNetworkCallback(request, callback); - assertTrue(msg, latch.await(timeoutMs, TimeUnit.MILLISECONDS)); + assertTrue(msg, latch.await(PRIVATE_DNS_SETTING_TIMEOUT_MS, TimeUnit.MILLISECONDS)); mCm.unregisterNetworkCallback(callback); // Wait some time for NetworkMonitor's private DNS probe to complete. If we do not do // this, then the test could complete before the NetworkMonitor private DNS probe