Skip to content

Commit

Permalink
xds: delete the permanent error logic in processing LDS updates in Xd…
Browse files Browse the repository at this point in the history 
…sServerWrapper (#9268) (#9278)
  • Loading branch information
@sanjaypujare
sanjaypujare committed Jun 15, 2022
1 parent 1b9876d commit 6de0e29
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 94 deletions.
9 changes: 4 additions & 5 deletions xds/src/main/java/io/grpc/xds/XdsServerBuilder.java
View file
Expand Up @@ -176,27 +176,26 @@ public interface XdsServingStatusListener {
private static class DefaultListener implements XdsServingStatusListener {
private final Logger logger;
private final String prefix;
boolean notServing;
boolean notServingDueToError;

DefaultListener(String prefix) {
logger = Logger.getLogger(DefaultListener.class.getName());
this.prefix = prefix;
notServing = true;
}

/** Log calls to onServing() following a call to onNotServing() at WARNING level. */
@Override
public void onServing() {
if (notServing) {
notServing = false;
if (notServingDueToError) {
notServingDueToError = false;
logger.warning("[" + prefix + "] Entering serving state.");
}
}

@Override
public void onNotServing(Throwable throwable) {
logger.warning("[" + prefix + "] " + throwable.getMessage());
notServing = true;
notServingDueToError = true;
}
}
}
19 changes: 2 additions & 17 deletions xds/src/main/java/io/grpc/xds/XdsServerWrapper.java
View file
Expand Up @@ -61,7 +61,6 @@
import java.net.SocketAddress;
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -445,12 +444,8 @@ public void run() {
if (stopped) {
return;
}
boolean isPermanentError = isPermanentError(error);
logger.log(Level.FINE, "{0} error from XdsClient: {1}",
new Object[]{isPermanentError ? "Permanent" : "Transient", error});
if (isPermanentError) {
handleConfigNotFound(error.asException());
} else if (!isServing) {
logger.log(Level.FINE, "Error from XdsClient", error);
if (!isServing) {
listener.onNotServing(error.asException());
}
}
Expand Down Expand Up @@ -719,16 +714,6 @@ private void maybeUpdateSelector() {
}
}
}

private boolean isPermanentError(Status error) {
return EnumSet.of(
Status.Code.INTERNAL,
Status.Code.INVALID_ARGUMENT,
Status.Code.FAILED_PRECONDITION,
Status.Code.PERMISSION_DENIED,
Status.Code.UNAUTHENTICATED)
.contains(error.getCode());
}
}

@VisibleForTesting
Expand Down
67 changes: 0 additions & 67 deletions xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTestMisc.java
View file
Expand Up @@ -199,56 +199,6 @@ public void run() {
assertThat(selectorManager.getSelectorToUpdateSelector()).isSameInstanceAs(NO_FILTER_CHAIN);
}

@Test
public void registerServerWatcher_notifyInternalError() throws Exception {
final SettableFuture<Server> start = SettableFuture.create();
Executors.newSingleThreadExecutor().execute(new Runnable() {
@Override
public void run() {
try {
start.set(xdsServerWrapper.start());
} catch (Exception ex) {
start.setException(ex);
}
}
});
xdsClient.ldsResource.get(5, TimeUnit.SECONDS);
xdsClient.ldsWatcher.onError(Status.INTERNAL);
verify(listener, timeout(5000)).onNotServing(any());
try {
start.get(START_WAIT_AFTER_LISTENER_MILLIS, TimeUnit.MILLISECONDS);
fail("Start should throw exception");
} catch (TimeoutException ex) {
assertThat(start.isDone()).isFalse();
}
assertThat(selectorManager.getSelectorToUpdateSelector()).isSameInstanceAs(NO_FILTER_CHAIN);
}

@Test
public void registerServerWatcher_notifyPermDeniedError() throws Exception {
final SettableFuture<Server> start = SettableFuture.create();
Executors.newSingleThreadExecutor().execute(new Runnable() {
@Override
public void run() {
try {
start.set(xdsServerWrapper.start());
} catch (Exception ex) {
start.setException(ex);
}
}
});
xdsClient.ldsResource.get(5, TimeUnit.SECONDS);
xdsClient.ldsWatcher.onError(Status.PERMISSION_DENIED);
verify(listener, timeout(5000)).onNotServing(any());
try {
start.get(START_WAIT_AFTER_LISTENER_MILLIS, TimeUnit.MILLISECONDS);
fail("Start should throw exception");
} catch (TimeoutException ex) {
assertThat(start.isDone()).isFalse();
}
assertThat(selectorManager.getSelectorToUpdateSelector()).isSameInstanceAs(NO_FILTER_CHAIN);
}

@Test
public void releaseOldSupplierOnChanged_noCloseDueToLazyLoading() throws Exception {
InetAddress ipLocalAddress = InetAddress.getByName("10.1.2.3");
Expand Down Expand Up @@ -327,23 +277,6 @@ public void releaseOldSupplierOnNotFound_verifyClose() throws Exception {
verify(tlsContextManager, times(1)).releaseServerSslContextProvider(eq(sslContextProvider1));
}

@Test
public void releaseOldSupplierOnPermDeniedError_verifyClose() throws Exception {
SslContextProvider sslContextProvider1 = mock(SslContextProvider.class);
when(tlsContextManager.findOrCreateServerSslContextProvider(eq(tlsContext1)))
.thenReturn(sslContextProvider1);
InetAddress ipLocalAddress = InetAddress.getByName("10.1.2.3");
localAddress = new InetSocketAddress(ipLocalAddress, PORT);
sendListenerUpdate(localAddress, tlsContext1, null,
tlsContextManager);
SslContextProviderSupplier returnedSupplier =
getSslContextProviderSupplier(selectorManager.getSelectorToUpdateSelector());
assertThat(returnedSupplier.getTlsContext()).isSameInstanceAs(tlsContext1);
callUpdateSslContext(returnedSupplier);
xdsClient.ldsWatcher.onError(Status.PERMISSION_DENIED);
verify(tlsContextManager, times(1)).releaseServerSslContextProvider(eq(sslContextProvider1));
}

@Test
public void releaseOldSupplierOnTemporaryError_noClose() throws Exception {
SslContextProvider sslContextProvider1 = mock(SslContextProvider.class);
Expand Down
9 changes: 4 additions & 5 deletions xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java
View file
Expand Up @@ -685,7 +685,7 @@ public void run() {
xdsClient.ldsWatcher.onError(Status.INTERNAL);
assertThat(selectorManager.getSelectorToUpdateSelector())
.isSameInstanceAs(FilterChainSelector.NO_FILTER_CHAIN);
assertThat(xdsClient.rdsWatchers).isEmpty();
RdsResourceWatcher saveRdsWatcher = xdsClient.rdsWatchers.get("rds");
verify(mockBuilder, times(1)).build();
verify(listener, times(2)).onNotServing(any(StatusException.class));
assertThat(sslSupplier0.isShutdown()).isFalse();
Expand All @@ -705,7 +705,6 @@ public void run() {
assertThat(ex.getCause()).isInstanceOf(IOException.class);
assertThat(ex.getCause().getMessage()).isEqualTo("error!");
}
RdsResourceWatcher saveRdsWatcher = xdsClient.rdsWatchers.get("rds");
assertThat(executor.forwardNanos(RETRY_DELAY_NANOS)).isEqualTo(1);
verify(mockBuilder, times(1)).build();
verify(mockServer, times(2)).start();
Expand Down Expand Up @@ -739,7 +738,7 @@ public void run() {
// not serving after serving
xdsClient.ldsWatcher.onResourceDoesNotExist(ldsResource);
assertThat(xdsClient.rdsWatchers).isEmpty();
verify(mockServer, times(3)).shutdown();
verify(mockServer, times(2)).shutdown();
when(mockServer.isShutdown()).thenReturn(true);
assertThat(selectorManager.getSelectorToUpdateSelector())
.isSameInstanceAs(FilterChainSelector.NO_FILTER_CHAIN);
Expand Down Expand Up @@ -777,7 +776,7 @@ public void run() {

assertThat(executor.numPendingTasks()).isEqualTo(1);
xdsClient.ldsWatcher.onResourceDoesNotExist(ldsResource);
verify(mockServer, times(4)).shutdown();
verify(mockServer, times(3)).shutdown();
verify(listener, times(4)).onNotServing(any(StatusException.class));
when(mockServer.isShutdown()).thenReturn(true);
assertThat(executor.numPendingTasks()).isEqualTo(0);
Expand All @@ -804,7 +803,7 @@ public void run() {
assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of());

xdsServerWrapper.shutdown();
verify(mockServer, times(5)).shutdown();
verify(mockServer, times(4)).shutdown();
assertThat(sslSupplier3.isShutdown()).isTrue();
when(mockServer.awaitTermination(anyLong(), any(TimeUnit.class))).thenReturn(true);
assertThat(xdsServerWrapper.awaitTermination(5, TimeUnit.SECONDS)).isTrue();
Expand Down

0 comments on commit 6de0e29

Please sign in to comment.