Now the xds resources are dynamically managed in resourceStore in xdsClient. The types is a xdsResourceType, singleton.
There is no longer hardcoded static list of known resource types, the subscription list is the source of truth.
AbstractXdsClient that manages AdsStream will only accept the xds resource types that has already has watchers subscribed to, same behaviour as before.
This fixes a regression in commit e1ad984. I'd create a test, but the
NPE gets thrown away in the context of the current test setup so can't
be created as quickly as we'd like to fix this. I have manually tested
in a custom reproduction to confirm it resolves the NPE.
Seen at b/248326695
```
java.lang.AssertionError: java.lang.NullPointerException
at io.grpc.xds.ClientXdsClient$1.uncaughtException(ClientXdsClient.java:89)
at io.grpc.SynchronizationContext.drain(SynchronizationContext.java:97)
at io.grpc.SynchronizationContext.execute(SynchronizationContext.java:127)
at io.grpc.xds.ClientXdsClient.cancelXdsResourceWatch(ClientXdsClient.java:327)
at io.grpc.xds.ClusterResolverLoadBalancer$ClusterResolverLbState$EdsClusterState.shutdown(ClusterResolverLoadBalancer.java:378)
at io.grpc.xds.ClusterResolverLoadBalancer$ClusterResolverLbState.shutdown(ClusterResolverLoadBalancer.java:206)
at io.grpc.util.GracefulSwitchLoadBalancer.shutdown(GracefulSwitchLoadBalancer.java:195)
at io.grpc.xds.ClusterResolverLoadBalancer.shutdown(ClusterResolverLoadBalancer.java:141)
at io.grpc.xds.CdsLoadBalancer2$CdsLbState.shutdown(CdsLoadBalancer2.java:136)
at io.grpc.xds.CdsLoadBalancer2.shutdown(CdsLoadBalancer2.java:110)
at io.grpc.util.GracefulSwitchLoadBalancer.shutdown(GracefulSwitchLoadBalancer.java:195)
at io.grpc.xds.ClusterManagerLoadBalancer$ChildLbState.shutdown(ClusterManagerLoadBalancer.java:256)
at io.grpc.xds.ClusterManagerLoadBalancer.shutdown(ClusterManagerLoadBalancer.java:138)
at io.grpc.internal.AutoConfiguredLoadBalancerFactory$AutoConfiguredLoadBalancer.shutdown(AutoConfiguredLoadBalancerFactory.java:164)
at io.grpc.internal.ManagedChannelImpl.shutdownNameResolverAndLoadBalancer(ManagedChannelImpl.java:381)
at io.grpc.internal.ManagedChannelImpl.access$8200(ManagedChannelImpl.java:118)
at io.grpc.internal.ManagedChannelImpl$DelayedTransportListener.transportTerminated(ManagedChannelImpl.java:2174)
at io.grpc.internal.DelayedClientTransport$3.run(DelayedClientTransport.java:122)
at io.grpc.SynchronizationContext.drain(SynchronizationContext.java:95)
at io.grpc.SynchronizationContext.execute(SynchronizationContext.java:127)
at io.grpc.internal.ManagedChannelImpl$RealChannel.shutdown(ManagedChannelImpl.java:1057)
at io.grpc.internal.ManagedChannelImpl.shutdown(ManagedChannelImpl.java:817)
at io.grpc.internal.ManagedChannelImpl.shutdownNow(ManagedChannelImpl.java:837)
at io.grpc.internal.ManagedChannelImpl.shutdownNow(ManagedChannelImpl.java:117)
at io.grpc.internal.ForwardingManagedChannel.shutdownNow(ForwardingManagedChannel.java:52)
at io.grpc.internal.ManagedChannelOrphanWrapper.shutdownNow(ManagedChannelOrphanWrapper.java:65)
at io.grpc.testing.integration.GrpclbFallbackTestClient.tearDown(GrpclbFallbackTestClient.java:178)
at io.grpc.testing.integration.GrpclbFallbackTestClient.main(GrpclbFallbackTestClient.java:67)
Caused by: java.lang.NullPointerException
at io.grpc.xds.ClientXdsClient.handleResourceResponse(ClientXdsClient.java:179)
at io.grpc.xds.AbstractXdsClient$AbstractAdsStream.handleRpcResponse(AbstractXdsClient.java:358)
at io.grpc.xds.AbstractXdsClient$AdsStreamV3$1$1.run(AbstractXdsClient.java:511)
at io.grpc.SynchronizationContext.drain(SynchronizationContext.java:95)
... 26 more
```
* xds: security code refactoring/renaming
1) move certprovider package under security
2) refactor inner Factory into CertProviderClientSslContextProviderFactory and CertProviderServerSslContextProviderFactory
3) Make CertProviderClientSslContextProvider and CertProviderServerSslContextProvider non-public
4) use only public (non package private) types like SslContextProvider (instead of CertProviderClientSslContextProvider etc)
Mainly refactor work to make type specific xds resources generic, e.g.
1. Define abstract class XdsResourceType to be extended by pluggable new resources. It mainly contains abstract method doParse() to parse unpacked proto messges and produce a ResourceUpdate. The common unpacking proto logic is in XdsResourceType default method parse()
2. Move the parsing/processing logics to specific XdsResourceType. Implementing:
XdsListenerResource for LDS
XdsRouteConfigureResource for RDS
XdsClusterResource for CDS
XdsEndpointResource for EDS
3. The XdsResourceTypes are singleton. To process for each XdsClient, context is passed in parameters, defined by XdsResourceType.Args.
4. Watchers will use generic APIs to subscribe to resource watchXdsResource(XdsResourceType, resourceName, watcher). Watcher and ResourceSubscribers becomes java generic class.
Removes the option of skipping the update of the priority LB state when
the failover timer is pending.
This consistency facilitates a future change weher we delay child LB
status updates if the priority LB is performing an update. The upcoming
priority LB policy gRFC also does not require this update to ever be
skipped.
The log id is an incrementing value starting from 0. That means the same
binary on two different machines will have the same hashes for each
consecutive Channel. That was not at all the intension of CHANNEL_ID.
From gRFC A42:
> This can be used in similar situations to where Envoy uses
> connection_properties to hash on the source IP address.
No logic changes, just cleans up warnings to make spotting real problems easier.
Remove "public" declarations on interfaces
Remove duplicate semicolons (Java lines ending in ";;")
Remove unneeded import
Change non-javadoc comment to not start with "/**"
Remove unneeded explicit type declarations from generics
Fix broken javadoc links
It's been 17 months since the check was introduced, which is plenty for
the migration. Leaving ignoreRefreshNameResolutionCheck() in-place to
let users delete their call sites. We'll remove the method after a few
releases.
Fixes#9409
This fixes builds including dependencies from Maven that use
io.grpc:grpc-services or io.grpc:grpc-xds. It resolves this error:
```
no such target '@io_grpc_grpc_java//services:services': target 'services' not declared in package 'services' defined by services/BUILD.bazel and referenced by '@maven//:io_grpc_grpc_services'
```
Fixes#9419
- Reduce nesting level by using `continue`
- Rearrange the order when it's possible to bail out early
- Add comments explaining what case it is, and the logic behind it
Introduces a new acceptResolvedAddresses() to the LoadBalancer.
This will now be the preferred way to handle addresses from the NameResolver. The existing handleResolvedAddresses() will eventually be deprecated.
The new method returns a boolean based on the LoadBalancers ability to use the provided addresses. If it does not accept them, false is returned. LoadBalancer implementations using the new method should no longer implement the canHandleEmptyAddressListFromNameResolution(), which will eventually be removed, along with handleResolvedAddresses().
Backward compatibility will be maintained so existing load balancers using handleResolvedAddresses() will continue to work.
Additionally the previously deprecated handleResolvedAddressGroups() method is removed.
%s is fairly safe (requires a Formattable to use Locale), so %d is the
main risk item. Places that really didn't need to use String.format()
were converted to plain string concatenation. Logging locations were
generally converted to using the log infrastructure's delayed
formatting, which is generally locale-sensitive but we're okay with
that. That wasn't done in okhttp, however, because Android frequently
doesn't use MessageFormat so we'd lose the parameters. Everywhere else
was explicitly defined to be Locale.US, to be consistent independent of
the default system locale.
The two checker tasks run quickly so don't gain much from UP-TO-DATE,
but it is convenient to not see them in the noise (checkUpperBoundDeps
in particular). Gradle only performs UP-TO-DATE checks (on the inputs)
if the task has both inputs and outputs defined.
The biggest saving was for distZip/distTar/shadowDistZip/shadowDistTar
which were using the same name for the non-shadow and shadow versions.
Thus the output file would always be out-of-date because it had been
rewritten and was invalid. This is worrisome because we could have
"randomly" been using the shadow Zip/Tar at times and the non-shadow
ones at others, although I think in practice the shadow tasks always run
last and so those are the files we'd see. Changing the classifier avoids
the colliding file names. These tasks took ~7 seconds, so incremental
builds are considerably shorter now.
INVALID_ARGUMENT is propagated to the data plane if no previous config
is available. INVALID_ARGUMENT is reserved for application use; LBs
should pretty much use UNAVAILABLE exclusively.
While most of the changes are in xds, there do not appear to be likely
xds code paths that would propagate a bad status to the data plane.
Internal policies either don't use parseLoadBalancingPolicyConfig() and
instead have their configuration objects constructed directly or are
constructed transitively through the cluster manager which uses INTERNAL
if there's a child failure. There was a worrisome hole before this
commit for StatusRuntimeExceptions received by the cluster manager, but
the audit didn't find any locations throwing such an exception.
User-selected policies produce a NACK and are protected from the
existing xds client watcher paths. The worst that appears could happen
is the channel could panic (which uses INTERNAL) if a bug let a bad
configuration through.
As defined in the gRFC [A53: Option for Ignoring xDS Resource Deletion](https://github.com/grpc/proposal/blob/master/A53-xds-ignore-resource-deletion.md).
This includes semi-related changes:
* Refactor ClientXdsClientTestBase: extract verify methods for golden resources
* Parameterize ClientXdsClientV2Test and ClientXdsClientV3Test with ignoreResourceDeletion enabled and disabled
* Add FORCE_INFO and FORCE_WARNING levels to XdsLogLevel
This can avoid creating an additional 736 tasks (previously 502 out of
1591 were not created). That's not all that important as the build time
is essentially the same, but this lets us see the poor behavior of the
protobuf plugin in our own project and increase our understanding of how
to avoid task creation when developing the plugin. Of the tasks still
being created, protobuf is the highest contributor with 165 tasks,
followed by maven-publish with 76 and appengine with 53. The remaining
59 are from our own build, but indirectly caused by maven-publish.
This dramatically shortens build time, even for full builds. A full
assemble of xds on my laptop goes from 1m 46s to 33s at least because
errorprone is disabled for the protos.
Previously if LDS/RDS were missing or improperly configured RPCs would
fail with "UNAVAILABLE: NameResolver returned no usable address errors".
That is very confusing and not helpful for debugging.
Ideally we'd also include the node id in this error message, but that's
a bit more involved and this is a huge improvement even without it.
b/237539851
all: Update netty to 4.1.77.Final and netty_tcnative to 2.0.53.Final
Also switches to a non-release version of rules_jvm_external to allow Bazel build to work with artifact classifiers.
This moves our depedencies into a plain file that can be read and
updated by tooling. While the current tooling is not particularly better
than just using gradle-versions-plugin, it should put us on better
footing. gradle-versions-plugin is actually pretty nice, but will be
incompatible with Gradle 8, so we need to wait a bit to see what the
future holds.
Left libraries as an alias for libs to reduce the commit size and make
it easier to revert if we don't end up liking this approach.
We're using Gradle 7.3.3 where it was an incubating fetaure. But in
Gradle 7.4 is became stable.
* xds: Custom LB configs to support UDPA TypeStruct
The legacy com.github.udpa.udpa.type.v1.TypedStruct proto should be supported in addition to the current com.github.udpa.udpa.type.v1.TypedStruct one.
Co-authored-by: Sergii Tkachenko <hi@sergii.org>
There are still some cases for xdstp processing, but they are to percent
encoding replacement strings. Those seem better to leave running since
it looks like it they could be triggered even with federation disabled
in the bootstrap processing.
Not updating the example WORKSPACE because it doesn't have any
Bazel-enabled build that depends on xds and so doesn't need the
additional repository dependencies.
Fixes#9162
Support for the is_optional logic in Cluster Specifier Plugins:
if unsupported Cluster Specifier Plugin is optional,
don't NACK, and skip any routes that point to it.
WrrLocalityLoadBalancer should remove the locality weights attribute from Resolved addresses after using the information. Not propagating this attribute will make it impossible for another child wrr_locality from working. This is not a supported situation and this change make the failure happen earlier and to be more obvious as to the cause.
remove OrcaOobHelperWrapper layer. Use OrcaOobUtil.updateListener() to set OrcaOobReportListener per each subchannel, not per helper. OrcaOobReportListener is per helper+subchannel unique.
Orca stats are created when creating helper.createSubchannels(), overriding subchannel attributes to store orcaState in the orcaHelper created subchannels
moved orcaMetrics to service project and renamed to MetricRecorder, added internal accessor. Maybe we can combine metricsRecorder and callMetricRecorder, they looks almost the same things.
OrcaServiceImpl depends on metricRecorder, not visa versa.
Instead of providing round robin or least request configurations directly, ClientXdsClient now wraps them in a WRR locality config.
ClusterResolverLoadBalancer passes this configuration directly to PriorityLoadBalancer to use as the endpoint LB policy it provides to ClusterImplLoadBalancer. A new ResolvedAddresses attribute is also set that has all the locality weights. This is needed by WrrLocalityLoadBalancer when it configures WeightedTargetLoadBalancer.
Renames the LegacyLoadBalancerConfigFactory to just LoadBalancerConfigFactory and gives it responsibility for both the legacy and the new LB config mechanism.
The new configuration mechanism is explained in gRFC A52: https://github.com/grpc/proposal/pull/298
1. move orca from xds and from service to io.grpc.xds.orca new package
2. keep CallMetricsRecorder and InternalCallMetricsRecorder in service
3. Added APIs for recording utilization/requestCost/cpuUtilization/memoryUtilzation for per-query requests, added internal data structure equivalent to OrcaLoadReport
This LB is the parent for weighted_target and will configure it based on the child policy it gets in its configuration and locality weights that come in a ResolvedAddresses attribute.
Described in [A52: gRPC xDS Custom Load Balancer Configuration](https://github.com/grpc/proposal/pull/298)
Remove unused xds/third_party/istio/src/main/proto/security/proto/providers/google/meshca.proto
and xds/src/generated/main/grpc/com/google/security/meshca/v1/MeshCertificateServiceGrpc.java
generated from it.
Proto updates:
- cncf/xds: Sort xds/import.sh protos alphabetically
- cncf/xds: Sync protos to cncf/xds@d92e9ce (commit 2021-12-16, corresponding to
envoy cl/440193522). It's a no-op for used protos, but helpful to import the
latest matcher.proto
- cncf/xds: Import xds/type/matcher/v3/matcher.proto with dependencies
- envoyproxy/protoc-gen-validate: Sync protos to
envoyproxy/protoc-gen-validate@dfcdc5e (commit 2022-03-10, corresponding to
envoy cl/440193522) to pick up ignore_empty field required for the following
envoy sync
- envoyproxy/envoy Sync protos to envoyproxy/envoy@e33f444 (commit 2022-04-07,
cl/440193522). This is the minimal version needed to pick up
ClusterSpecifierPlugin.is_optional. a. Generated code:
AggregatedDiscoveryServiceGrpc was regenerated from the updated proto. This
is a no-op, just a minor change to the docblocks. b. Deprecated fields had to
be taken care of manually, see "Manual updates to the code" below.
- envoyproxy/envoy Sync protos to the latest imported version
envoyproxy/envoy@5d74719 (commit 2022-04-08, cl/443359189). Not needed for
anything specific, just the last version, and was easy to import.
Manual updates to the code as the result of envoyproxy/envoy@e33f444 sync:
- Deprecated ConfigSource.path replaced with the ConfigSource.path_config_source
in test fake resources. The ConfigSource.path isn't in active code paths, so
no prod code changes needed.
- Suppress CertificateValidationContext.match_subject_alt_names deprecations in
test files. Surprisingly, we don't report deprecations in prod files, despite
the fact this field is used in prod code a few times.
* api: add support for SocketAddress types in ManagedChannelProvider
also add support for SocketAddress types in NameResolverProvider
Use scheme in target URI to select a NameRseolverProvider and get
that provider's supported SocketAddress types.
implement selection in ManagedChannelRegistry of appropriate
ManagedChannelProvider based on NameResolver's SocketAddress types
This refactoring is done in preparation of a larger change where LB configuration will be provided in the xDS Cluster proto message load_balancing_policy field. This field will allow for the configuration of custom LB policies with arbitrary configuration data.
- Instead of directly creating Java configuration objects, the client delegates to a new factory class to generate JSON configurations
- This factory is considered a "legacy" one as a separate factory will be introduced to build configs based on the new load_balancing_policy field
- The client will use a LoadBalancerProvider to parse the generated config to assure it is valid.
- Overlapping LB config validation that exists both in ClientXdsClient and LB providers will be removed from the client.
This is a second attempt at #8996 that was reverted by #9092.
The initial PR was reverted because the change caused the duplicate CDS update detection in ClientXdsClient to fail. This was because equality checking of PolicySelection instances cannot be relied on. This PR uses the JSON config instead - CdsLoadBalancer2 will handle the conversion from JSON config to PolicySelection.
changes in priority:
Keep track of whether a child has seen TRANSIENT_FAILURE more recently than IDLE or READY, and use this to decide whether to restart the failover timer when a child reports CONNECTING. This ensures that we properly start the failover timer when the ring_hash child policy transitions from IDLE to CONNECTING at startup.
Behaviour change also affects address updates the current priority from CONNECTING to CONNECTING, previously it reports one CONNECTING, right now it does not report and wait there due to failover timer in effect. This helps to try the next priority.
* Revert "- Change config builder to a static factory class. - Remove validation and default value logic that already exists in providers from the factory. - Using the PolicySelection in CdsUpdate instead of the JSON config."
This reverts commit 54c72b945e.
* Revert "xds: ClientXdsClient to provide LB config in JSON"
This reverts commit 4903b44a82.
- Remove validation and default value logic that already exists in providers from the factory.
- Using the PolicySelection in CdsUpdate instead of the JSON config.
This refactoring is done in preparation of a larger change where LB
configuration will be provided in the xDS Cluster proto message
load_balancing_policy field. This field will allow for the configuration
of custom LB policies with arbitrary configuration data.
- Instead of directly creating Java configuration objects, the client
delegates to a new builder to generate JSON configurations
- This factory is considered a "legacy" one as a separate factory will
be introduced to build configs based on the new load_balancing_policy
field
- The client will use a LoadBalancerProvider to parse the generated
config to assure it is valid.
- CdsLoadBalancer2 will parse to config again to produce the LB config
object passed down to child LBs.
This would limit LRS stream creation to one per second, even if the
old stream was considered good as it received a response. This is the
same change as made to ADS in 957079194a.
b/224833499
1. Unnecessary fully qualified names
Currently in XdsCredentialsRegistry, the child classes are referred by
their fully qualified names i.e.
'io.grpc.xds.internal.GoogleDefaultXdsCredentialsProvider' instead of
importing GoogleDefaultXdsCredentialsProvider and just using
GoogleDefaultXdsCredentialsProvider.class.
2. Use immutable interfaces instead of the generic collection interface
i.e. ImmutableMap instead of just Map.
These improvements are related to #8924.
Just using 100ms is mostly sufficient, but could potentially still flake
on the start()s that should return successfully. Waiting for the
XdsServingStatusListener to be called greatly reduces the amount of
processing needing to be done for start() to react and thus should
greatly avoid flakiness.
This would limit ADS stream creation to one per second, even if the
old stream was considered good as it received a response. This shouldn't
really ever trigger, and if it does 1 QPS is probably still too high.
But 1 QPS is _substantially_ better than a closed loop and there's very
few additional signals we could use to avoid resetting the backoff.
b/224833499
Terry Wilson
yifeizhuang
sanjaypujare
Eric Anderson
apolcyn
Terry Wilson
Larry Safran
Sergii Tkachenko
cfredri4
Ran
Anirudh Ramachandra
Kurt Alfred Kluever