xds:Cleanup to reduce test flakiness (#11895)

* don't process resourceDoesNotExist for watchers that have been cancelled.

* Change test to use an ArgumentMatcher instead of expecting that only the final result will be sent since depending on timing there may be configs sent for clusters being removed with their entries as errors.
This commit is contained in:
Larry Safran 2025-02-14 10:23:54 -08:00 committed by GitHub
parent 5a7f350537
commit 41dd0c6d73
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 37 additions and 3 deletions

View File

@ -149,6 +149,7 @@ final class XdsDependencyManager implements XdsConfig.XdsClusterSubscriptionRegi
throwIfParentContextsNotEmpty(watcher); throwIfParentContextsNotEmpty(watcher);
} }
watcher.cancelled = true;
XdsResourceType<T> type = watcher.type; XdsResourceType<T> type = watcher.type;
String resourceName = watcher.resourceName; String resourceName = watcher.resourceName;
@ -597,6 +598,8 @@ final class XdsDependencyManager implements XdsConfig.XdsClusterSubscriptionRegi
implements ResourceWatcher<T> { implements ResourceWatcher<T> {
private final XdsResourceType<T> type; private final XdsResourceType<T> type;
private final String resourceName; private final String resourceName;
boolean cancelled;
@Nullable @Nullable
private StatusOr<T> data; private StatusOr<T> data;
@ -693,6 +696,10 @@ final class XdsDependencyManager implements XdsConfig.XdsClusterSubscriptionRegi
@Override @Override
public void onResourceDoesNotExist(String resourceName) { public void onResourceDoesNotExist(String resourceName) {
if (cancelled) {
return;
}
handleDoesNotExist(resourceName); handleDoesNotExist(resourceName);
xdsConfigWatcher.onResourceDoesNotExist(toContextString()); xdsConfigWatcher.onResourceDoesNotExist(toContextString());
} }
@ -752,6 +759,9 @@ final class XdsDependencyManager implements XdsConfig.XdsClusterSubscriptionRegi
@Override @Override
public void onResourceDoesNotExist(String resourceName) { public void onResourceDoesNotExist(String resourceName) {
if (cancelled) {
return;
}
handleDoesNotExist(checkNotNull(resourceName, "resourceName")); handleDoesNotExist(checkNotNull(resourceName, "resourceName"));
xdsConfigWatcher.onResourceDoesNotExist(toContextString()); xdsConfigWatcher.onResourceDoesNotExist(toContextString());
} }
@ -836,6 +846,9 @@ final class XdsDependencyManager implements XdsConfig.XdsClusterSubscriptionRegi
@Override @Override
public void onResourceDoesNotExist(String resourceName) { public void onResourceDoesNotExist(String resourceName) {
if (cancelled) {
return;
}
handleDoesNotExist(checkNotNull(resourceName, "resourceName")); handleDoesNotExist(checkNotNull(resourceName, "resourceName"));
maybePublishConfig(); maybePublishConfig();
} }
@ -857,6 +870,9 @@ final class XdsDependencyManager implements XdsConfig.XdsClusterSubscriptionRegi
@Override @Override
public void onResourceDoesNotExist(String resourceName) { public void onResourceDoesNotExist(String resourceName) {
if (cancelled) {
return;
}
handleDoesNotExist(checkNotNull(resourceName, "resourceName")); handleDoesNotExist(checkNotNull(resourceName, "resourceName"));
maybePublishConfig(); maybePublishConfig();
} }

View File

@ -88,6 +88,7 @@ import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.junit.runners.JUnit4; import org.junit.runners.JUnit4;
import org.mockito.ArgumentCaptor; import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatcher;
import org.mockito.ArgumentMatchers; import org.mockito.ArgumentMatchers;
import org.mockito.Captor; import org.mockito.Captor;
import org.mockito.InOrder; import org.mockito.InOrder;
@ -696,9 +697,9 @@ public class XdsDependencyManagerTest {
controlPlaneService.setXdsConfig(ADS_TYPE_URL_EDS, edsMap); controlPlaneService.setXdsConfig(ADS_TYPE_URL_EDS, edsMap);
// Verify that the config is updated as expected // Verify that the config is updated as expected
inOrder.verify(xdsConfigWatcher, timeout(1000)).onUpdate(xdsConfigCaptor.capture()); ClusterNameMatcher nameMatcher
XdsConfig config = xdsConfigCaptor.getValue(); = new ClusterNameMatcher(Arrays.asList("root", "clusterA21", "clusterA22"));
assertThat(config.getClusters().keySet()).containsExactly("root", "clusterA21", "clusterA22"); inOrder.verify(xdsConfigWatcher, timeout(1000)).onUpdate(argThat(nameMatcher));
} }
private Listener buildInlineClientListener(String rdsName, String clusterName) { private Listener buildInlineClientListener(String rdsName, String clusterName) {
@ -789,4 +790,21 @@ public class XdsDependencyManagerTest {
}); });
} }
} }
static class ClusterNameMatcher implements ArgumentMatcher<XdsConfig> {
private final List<String> expectedNames;
ClusterNameMatcher(List<String> expectedNames) {
this.expectedNames = expectedNames;
}
@Override
public boolean matches(XdsConfig xdsConfig) {
if (xdsConfig == null || xdsConfig.getClusters() == null) {
return false;
}
return xdsConfig.getClusters().size() == expectedNames.size()
&& xdsConfig.getClusters().keySet().containsAll(expectedNames);
}
}
} }