xds: populate envoy RetryPolicy with no retryOn to resolver (#8511)

Envoy RetryPolicy with empty retryOn should not be ignored as no retry config when selecting Route config. Therefore, if xDS update for a route contains a RetryPolicy that has no RetryOn value that we support, but the virtual host config does, xds client should choose the Envoy RetryPolicy from the route (even with no RetryOn), rather than choosing the one from virtual host, and try to convert it into grpc RetryPolicy, and end up with no retry.

xds/src/main/java/io/grpc/xds/ClientXdsClient.java

@@ -1273,13 +1273,10 @@ final class ClientXdsClient extends AbstractXdsClient {
       retryableStatusCodesBuilder.add(code);
     }
     List<Code> retryableStatusCodes = retryableStatusCodesBuilder.build();
-    if (!retryableStatusCodes.isEmpty()) {
-      return StructOrError.fromStruct(
-          RetryPolicy.create(
-              maxAttempts, retryableStatusCodes, initialBackoff, maxBackoff,
-              /* perAttemptRecvTimeout= */ null));
-    }
-    return null;
+    return StructOrError.fromStruct(
+        RetryPolicy.create(
+            maxAttempts, retryableStatusCodes, initialBackoff, maxBackoff,
+            /* perAttemptRecvTimeout= */ null));
   }
 
   @VisibleForTesting

xds/src/main/java/io/grpc/xds/XdsNameResolver.java

@@ -182,13 +182,14 @@ final class XdsNameResolver extends NameResolver {
   @VisibleForTesting
   static Map<String, ?> generateServiceConfigWithMethodConfig(
       @Nullable Long timeoutNano, @Nullable RetryPolicy retryPolicy) {
-    if (timeoutNano == null && retryPolicy == null) {
+    if (timeoutNano == null
+        && (retryPolicy == null || retryPolicy.retryableStatusCodes().isEmpty())) {
       return Collections.emptyMap();
     }
     ImmutableMap.Builder<String, Object> methodConfig = ImmutableMap.builder();
     methodConfig.put(
         "name", Collections.singletonList(Collections.emptyMap()));
-    if (retryPolicy != null) {
+    if (retryPolicy != null && !retryPolicy.retryableStatusCodes().isEmpty()) {
       ImmutableMap.Builder<String, Object> rawRetryPolicy = ImmutableMap.builder();
       rawRetryPolicy.put("maxAttempts", (double) retryPolicy.maxAttempts());
       rawRetryPolicy.put("initialBackoff", Durations.toString(retryPolicy.initialBackoff()));

xds/src/test/java/io/grpc/xds/ClientXdsClientDataTest.java

@@ -552,7 +552,8 @@ public class ClientXdsClientDataTest {
         .setRetryPolicy(builder.build())
         .build();
     struct = ClientXdsClient.parseRouteAction(proto, filterRegistry, false);
-    assertThat(struct.getStruct().retryPolicy()).isNull();
+    assertThat(struct.getStruct().retryPolicy()).isNotNull();
+    assertThat(struct.getStruct().retryPolicy().retryableStatusCodes()).isEmpty();
 
     // base_interval unset
     builder

xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java

@@ -988,6 +988,8 @@ public class XdsNameResolverTest {
     RetryPolicy retryPolicy = RetryPolicy.create(
         4, ImmutableList.of(Code.UNAVAILABLE, Code.CANCELLED), Durations.fromMillis(100),
         Durations.fromMillis(200), null);
+    RetryPolicy retryPolicyWithEmptyStatusCodes = RetryPolicy.create(
+        4, ImmutableList.<Code>of(), Durations.fromMillis(100), Durations.fromMillis(200), null);
 
     // timeout only
     String expectedServiceConfigJson = "{\n"
@@ -1001,6 +1003,11 @@ public class XdsNameResolverTest {
     assertThat(XdsNameResolver.generateServiceConfigWithMethodConfig(timeoutNano, null))
         .isEqualTo(expectedServiceConfig);
 
+    // timeout and retry with empty retriable status codes
+    assertThat(XdsNameResolver.generateServiceConfigWithMethodConfig(
+            timeoutNano, retryPolicyWithEmptyStatusCodes))
+        .isEqualTo(expectedServiceConfig);
+
     // retry only
     expectedServiceConfigJson = "{\n"
         + "  \"methodConfig\": [{\n"
@@ -1021,6 +1028,7 @@ public class XdsNameResolverTest {
     assertThat(XdsNameResolver.generateServiceConfigWithMethodConfig(null, retryPolicy))
         .isEqualTo(expectedServiceConfig);
 
+
     // timeout and retry
     expectedServiceConfigJson = "{\n"
         + "  \"methodConfig\": [{\n"
@@ -1043,12 +1051,16 @@ public class XdsNameResolverTest {
         .isEqualTo(expectedServiceConfig);
 
     // no timeout and no retry
-    // timeout and retry
     expectedServiceConfigJson = "{}";
     expectedServiceConfig =
         (Map<String, ?>) JsonParser.parse(expectedServiceConfigJson);
     assertThat(XdsNameResolver.generateServiceConfigWithMethodConfig(null, null))
         .isEqualTo(expectedServiceConfig);
+
+    // retry with emtry retriable status codes only
+    assertThat(XdsNameResolver.generateServiceConfigWithMethodConfig(
+            null, retryPolicyWithEmptyStatusCodes))
+        .isEqualTo(expectedServiceConfig);
   }
 
   @Test