Deflake and fix testIpClientClearingIpAddressState.

This test is flaky (at least when run on device) because often
IpClient is started before it has heard about the IP address that
the test adds to the interface. Then, when provisioning is
started, it succeeds immediately because due to historical
reasons the presence of an IPv4 address is sufficient to declare
a network provisioned.

Attempt to configure the address in a way that will avoid this
sort of race. Because IpClient does not expose information about
its inner state, or send any callbacks while it's stopped, do
this by tapping in to the same observer registry that the test
IpClient instance uses, and adding two addresses on the
assumption that when the second one is observed, all observers,
including IpClient, will have seen the first one be added.

Also, the test was not very realistic. It configured an IPv4
address, and then ran DHCP, and expected that the IPv4 address
would be cleared by ClearingIpAddressesState. But DHCP clears
the address as well, because netd only supports configuring one
IPv4 address per interface, so the test is not very useful. Use
IPv6 instead.

Bug: 139314310
Bug: 152723363
Test: atest NetworkStackIntegrationTests:IpClientIntegrationTest#testIpClientClearingIpAddressState --iterations 100
Merged-In: Ibf2fa0260df313c604ec0be3d04f49319f02d60e
Change-Id: Ibf2fa0260df313c604ec0be3d04f49319f02d60e

1 files changed, 80 insertions, 11 deletions

diff --git a/tests/integration/src/android/net/ip/IpClientIntegrationTest.java b/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
index a89f07d..79264a8 100644
--- a/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
+++ b/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
@@ -135,6 +135,7 @@ import com.android.networkstack.apishim.CaptivePortalDataShimImpl;
 import com.android.networkstack.apishim.ConstantsShim;
 import com.android.networkstack.apishim.ShimUtils;
 import com.android.networkstack.arp.ArpPacket;
+import com.android.server.NetworkObserver;
 import com.android.server.NetworkObserverRegistry;
 import com.android.server.NetworkStackService.NetworkStackServiceManager;
 import com.android.server.connectivity.ipmemorystore.IpMemoryStoreService;
@@ -168,6 +169,8 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Objects;
 import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
 /**
  * Tests for IpClient.
@@ -199,6 +202,7 @@ public class IpClientIntegrationTest {
     @Spy private INetd mNetd;
 
     private String mIfaceName;
+    private NetworkObserverRegistry mNetworkObserverRegistry;
     private HandlerThread mPacketReaderThread;
     private Handler mHandler;
     private TapPacketReader mPacketReader;
@@ -441,10 +445,10 @@ public class IpClientIntegrationTest {
         when(mContext.getSystemService(eq(Context.NETD_SERVICE))).thenReturn(netdIBinder);
         assertNotNull(mNetd);
 
-        final NetworkObserverRegistry reg = new NetworkObserverRegistry();
-        reg.register(mNetd);
-        mIpc = new IpClient(mContext, mIfaceName, mCb, reg, mNetworkStackServiceManager,
-                mDependencies);
+        mNetworkObserverRegistry = new NetworkObserverRegistry();
+        mNetworkObserverRegistry.register(mNetd);
+        mIpc = new IpClient(mContext, mIfaceName, mCb, mNetworkObserverRegistry,
+                mNetworkStackServiceManager, mDependencies);
     }
 
     private void expectAlarmCancelled(InOrder inOrder, OnAlarmListener listener) {
@@ -1202,6 +1206,19 @@ public class IpClientIntegrationTest {
         fail("No router solicitation received on interface within timeout");
     }
 
+    private void sendBasicRouterAdvertisement(boolean waitForRs) throws Exception {
+        final String dnsServer = "2001:4860:4860::64";
+        final ByteBuffer pio = buildPioOption(3600, 1800, "2001:db8:1::/64");
+        ByteBuffer rdnss = buildRdnssOption(3600, dnsServer);
+        ByteBuffer ra = buildRaPacket(pio, rdnss);
+
+        if (waitForRs) {
+            waitForRouterSolicitation();
+        }
+
+        mPacketReader.sendResponse(ra);
+    }
+
     // TODO: move this and the following method to a common location and use them in ApfTest.
     private static ByteBuffer buildPioOption(int valid, int preferred, String prefixString)
             throws Exception {
@@ -1485,6 +1502,41 @@ public class IpClientIntegrationTest {
         expectNat64PrefixUpdate(inOrder, null);
     }
 
+    private void addIpAddressAndWaitForIt(final String iface) throws Exception {
+        final CountDownLatch latch = new CountDownLatch(1);
+
+        final String addr1 = "192.0.2.99";
+        final String addr2 = "192.0.2.3";
+        final int prefixLength = 26;
+
+        // Add two IPv4 addresses to the specified interface, and proceed when the NetworkObserver
+        // has seen the second one. This ensures that every other NetworkObserver registered with
+        // mNetworkObserverRegistry - in particular, IpClient's - has seen the addition of the first
+        // address.
+        final LinkAddress trigger = new LinkAddress(addr2 + "/" + prefixLength);
+        NetworkObserver observer = new NetworkObserver() {
+            @Override
+            public void onInterfaceAddressUpdated(LinkAddress address, String ifName) {
+                if (ifName.equals(iface) && address.isSameAddressAs(trigger)) {
+                    latch.countDown();
+                }
+            }
+        };
+
+        mNetworkObserverRegistry.registerObserverForNonblockingCallback(observer);
+        try {
+            mNetd.interfaceAddAddress(iface, addr1, prefixLength);
+            mNetd.interfaceAddAddress(iface, addr2, prefixLength);
+            assertTrue("Trigger IP address " + addr2 + " not seen after " + TEST_TIMEOUT_MS + "ms",
+                    latch.await(TEST_TIMEOUT_MS, TimeUnit.MILLISECONDS));
+        } finally {
+            mNetworkObserverRegistry.unregisterObserver(observer);
+        }
+
+        // Wait for IpClient to process the addition of the address.
+        HandlerUtilsKt.waitForIdle(mIpc.getHandler(), TEST_TIMEOUT_MS);
+    }
+
     @Test
     public void testIpClientClearingIpAddressState() throws Exception {
         final long currentTime = System.currentTimeMillis();
@@ -1503,14 +1555,31 @@ public class IpClientIntegrationTest {
 
         // Pretend that something else (e.g., Tethering) used the interface and left an IP address
         // configured on it. When IpClient starts, it must clear this address before proceeding.
-        // TODO: test IPv6 instead, since the DHCP client will remove this address by replacing it
-        // with the new address.
-        mNetd.interfaceAddAddress(mIfaceName, "192.0.2.99", 26);
+        // The address must be noticed before startProvisioning is called, or IpClient will
+        // immediately declare provisioning success due to the presence of an IPv4 address.
+        // The address must be IPv4 because IpClient clears IPv6 addresses on startup.
+        //
+        // TODO: once IpClient gets IP addresses directly from netlink instead of from netd, it
+        // may be sufficient to call waitForIdle to see if IpClient has seen the address.
+        addIpAddressAndWaitForIt(mIfaceName);
 
-        // start IpClient again and should enter Clearing State and wait for the message from kernel
-        performDhcpHandshake(true /* isSuccessLease */, TEST_LEASE_DURATION_S,
-                true /* isDhcpLeaseCacheEnabled */, false /* shouldReplyRapidCommitAck */,
-                TEST_DEFAULT_MTU, false /* isDhcpIpConflictDetectEnabled */);
+        disableRouterSolicitationDelay();
+
+        ProvisioningConfiguration config = new ProvisioningConfiguration.Builder()
+                .withoutIpReachabilityMonitor()
+                .build();
+        mIpc.startProvisioning(config);
+
+        sendBasicRouterAdvertisement(true /*waitForRs*/);
+
+        // Check that the IPv4 addresses configured earlier are not in LinkProperties...
+        ArgumentCaptor<LinkProperties> captor = ArgumentCaptor.forClass(LinkProperties.class);
+        verify(mCb, timeout(TEST_TIMEOUT_MS)).onProvisioningSuccess(captor.capture());
+        assertFalse(captor.getValue().hasIpv4Address());
+
+        // ... or configured on the interface.
+        InterfaceConfigurationParcel cfg = mNetd.interfaceGetCfg(mIfaceName);
+        assertEquals("0.0.0.0", cfg.ipv4Addr);
     }
 
     @Test