First of several PRs to improve the e2e testing for the SSL credentials API.
Closes#35433
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35433 from matthewstevenson88:more-ssl-testing 2a0db7624e
PiperOrigin-RevId: 625326074
The pick_first policy creates a list of subchannels for each resolver update and then iterates over the list, attempting to connect to each subchannel in turn, until one of them succeeds. However, once a subchannel does succeed, the policy unrefs the other subchannels but still retains a bunch of now-unnecessary state in the subchannel list itself. This wastes a bunch of memory, especially now that petiole policies are delegating to pick_first. This PR contains a new pick_first implementation that stops retaining that state, which significantly reduces per-channel memory.
There is one behavior change here, which is that if we have a connected subchannel and we get a resolver update that no longer includes that address, we now go IDLE instead of proactively trying to connect to the new addresses.
Closes#34766
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/34766 from markdroth:pick_first_free_memory_after_connecting 7236b4321f
PiperOrigin-RevId: 623887639
[grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging GPR_ASSERT
Replacing GPR_ASSERT with absl CHECK
Will not be replacing CHECK with CHECK_EQ , CHECK_NE etc because there are too many callsites. Only a few - which fit into single - line regex will be changed. This would be small in number just to reduce the load later.
Replacing CHECK with CHECK_EQ , CHECK_NE etc could be done using Cider-V once these changes are submitted if we want to clean up later. Given that we have 5000+ instances of GPR_ASSERT to edit, Doing it manually is too much work for both the author and reviewer.
<!--
If you know who should review your pull request, please assign it to that
person, otherwise the pull request would get assigned randomly.
If your pull request is for a specific language, please add the appropriate
lang label.
-->
Closes#36224
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36224 from tanvi-jagtap:assert_end2end_general 0b0e940f5d
PiperOrigin-RevId: 623410297
[grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging GPR_ASSERT
Replacing GPR_ASSERT with absl CHECK
Will not be replacing CHECK with CHECK_EQ , CHECK_NE etc because there are too many callsites. Only a few - which fit into single - line regex will be changed. This would be small in number just to reduce the load later.
Replacing CHECK with CHECK_EQ , CHECK_NE etc could be done using Cider-V once these changes are submitted if we want to clean up later. Given that we have 5000+ instances of GPR_ASSERT to edit, Doing it manually is too much work for both the author and reviewer.
<!--
If you know who should review your pull request, please assign it to that
person, otherwise the pull request would get assigned randomly.
If your pull request is for a specific language, please add the appropriate
lang label.
-->
Closes#36268
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36268 from tanvi-jagtap:tjagtap_grpc_assert_end2end_xds d9d072c772
PiperOrigin-RevId: 623360481
As per https://github.com/grpc/proposal/pull/419, the experimental optional label `grpc.lb.locality` is added to the follow per-call metrics -
* grpc.client.attempt.duration
* grpc.client.attempt.sent_total_compressed_message_size
* grpc.client.attempt.rcvd_total_compressed_message_size
Closes#36254
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36254 from yashykt:OTelOptionalLabelsOnPerCall c5390c99a1
PiperOrigin-RevId: 622973959
See #36176. The only difference is a temporary shim for Secure credentials types, which was already discussed and approved separately.
Closes#36242
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36242 from drfloob:reland/36176 f07bebe289
PiperOrigin-RevId: 621879911
Forked from #35957
This PR refactors the credentials types to remove Secure and Insecure Channel and Call credentials types. We standardize on a `c_creds()` accessor method for all credentials types, which can now be treated uniformly. This notably removes special-case handling of insecure credentials.
The special code-paths for insecure creds are no longer necessary in the wake of #25586.
Closes#36176
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36176 from drfloob:fork/35957/creds-API fd64d59c23
PiperOrigin-RevId: 621008166
There were some failures in the Crl Directory related tests after https://github.com/grpc/grpc/pull/36031
This came down to https://github.com/grpc/grpc/pull/36031 adding some CRLs with bad qualities (invalid content/signatures, overriding issuer names) to the `test_creds/crl_data/crls` directory, which is used in the directory reloading tests. The tests began failing on some platforms because they were picking up these bad crls which were failing various checks, but the test was designed to assume that `test_creds/crl_data/crls` was a valid and good directory.
This PR moves the bad CRLs to their own directory to prevent this accidental mash-up of test data. It also adds debug logging to our custom verification stack.
Closes#36122
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36122 from gtcooke94:crl_fix 508dd1370d
PiperOrigin-RevId: 616280898
While we're here, also change the Echo service handler to mutate clients before handling the RPC, mainly so that it can be reasoned about in cases involving client cancellation.
PiperOrigin-RevId: 613029541
There are tests in another change I'm working on for which we need to do things like:
1) waiting for N RPCs to concurrently pile up on a server
2) doing 1) multiple times sequentially on the same server
PiperOrigin-RevId: 612934305
This breaks the following pieces out of the `grpc_client_channel` BUILD target:
- backend_metric_parser
- oob_backend_metric
- child_policy_handler
- backup_poller
- service_config_channel_arg_filter
- client_channel_channelz
- client_channel_internal_header
- subchannel_connector
- subchannel_pool_interface
- config_selector
- client_channel_service_config_parser
- retry_service_config_parser
- retry_throttle
The code left in the `grpc_client_channel` target will need more work to pull apart.
Closes#35879
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35879 from markdroth:client_channel_build_split f388a37edc
PiperOrigin-RevId: 608806548
As title. Pulling these additions out from a larger change.
Related: cl/563857636
Closes#35861
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35861 from apolcyn:xds_test_changes a67f64d93c
PiperOrigin-RevId: 607728140
This new directory combines code from the following locations:
- src/core/ext/filters/client_channel/resolver
- src/core/lib/resolver
Closes#35804
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35804 from markdroth:client_channel_resolver_reorg2 30660e6b00
PiperOrigin-RevId: 604665835
This new directory combines code from the following locations:
- src/core/ext/filters/client_channel/lb_policy
- src/core/lib/load_balancing
Closes#35786
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35786 from markdroth:client_channel_resolver_reorg 98554efb98
PiperOrigin-RevId: 604351832
<!--
If you know who should review your pull request, please assign it to that
person, otherwise the pull request would get assigned randomly.
If your pull request is for a specific language, please add the appropriate
lang label.
-->
Closes#35210
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35210 from yijiem:csm-service-label 6a6a7d1774
PiperOrigin-RevId: 597641393
It's not clear to me that this one unit test of very marginal importance warrants 8 bytes per channel.
Closes#35465
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35465 from ctiller:we-dont-need-this-really e7ee62ccb2
PiperOrigin-RevId: 596091614
There are a select few tests that are failing when building with OpenSSL102 - disable them until we can fix.
Closes#35354
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35354 from gtcooke94:fix_ossl_102 8708d6ce86
PiperOrigin-RevId: 595761932
The `DirectoryReloaderProvider` currently segfaults on construction if grpc_init() is not called before construction. This is because when creating the `DirectoryReloaderCrlProvider` we [call GetDefaultEventEngine](a58f3f2df5/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc (L152)), and getting the default event engine requires that `grpc_init` is called.
This PR adds a test that catches the segfault and adds `grpc_init` and `grpc_shutdown` to the ctor and dtor of `DirectoryReloaderCrlProvider` so that the test passes.
Closes#35247
COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35247 from gtcooke94:crl_provider_init_fix 25f3dc7f27
PiperOrigin-RevId: 589885254
<!--
If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.
If your pull request is for a specific language, please add the
appropriate
lang label.
-->
---------
Co-authored-by: Bradley Hess <bdhess@google.com>
Co-authored-by: AJ Heller <hork@google.com>
- Fix deadlock in load reporting tests.
- Add timeout to `WaitForLoadReport()`. (Note: this required changing
from `grpc::internal::Mutex` and friends to `grpc_core::Mutex` and
friends.)
- Fix balancer stream shutdown machinery.
- Change `ServerThread` to be a class instead of a struct.
Changes to fake resolver:
- Add `WaitForReresolutionRequest()` method to fake resolver response
generator to allow tests to tell when re-resolution has been requested.
- Change fake resolver response generator API to have only one mechanism
for injecting results, regardless of whether the result is an error or
whether it's triggered by a re-resolution.
Changes to grpclb_end2end_test:
- Change balancer interface such that instead of setting a list of
responses with fixed delays, the test can control exactly when each
response is set.
- Change balancer impl to always send the initial LB response, as
expected by the grpclb protocol.
- Change balancer impl to always read load reports, even if load
reporting is not expected to be enabled. (The latter case will still
cause the test to fail.) Reads are done in a different thread than
writes.
- Allow each test to directly control how many backends and balancers
are started and the client load reporting interval, so that (a) we don't
waste resources starting servers we don't need and (b) there is no need
to arbitrarily split tests across different test classes.
- Add timeouts to `WaitForAllBackends()` functionality, so that tests
will fail with a useful error rather than timing out.
- Improved ergonomics of various helper functions in the test framework.
In the process of making these changes, I found a couple of bugs:
- A bug in pick_first, which I fixed in #34885.
- A bug in grpclb, in which we were using the wrong condition to decide
whether to propagate a re-resolution request from the child policy,
which I've fixed in this PR. (This bug probably originated way back in
#18344.)
This should address a lot of the flakes seen in grpclb_e2e_test
recently.
EventEngine experiments, especially with `work_serializer_dispatch` tend
to cause callbacks to occur later than we've previously seen, so tests
that verify global data structures tend to become flakier when these are
introduced.
Here, the fix is waiting for EventEngine to be closed before starting
the new test.
Whilst here, make some adjustments to the test for better readability on
what's going on:
- if we fail a request to an echo service, we do not actually expect the
messages to match, so don't report that
- if we expect a value of 1 or 2, AnyOf is a better tool: it will report
the actual value too
---------
Co-authored-by: ctiller <ctiller@users.noreply.github.com>
This PR fixes a bug identified in #29667, where the TLS channel
credentials still require a trust bundle even if the user has explicitly
opted to not verify the server certificate. This PR is based on #29810.
Relands #34785, which was reverted in #34818.
The first commit is the revert. The second commit removes the gtest
dependency from the xds_server library, which should address the
testonly problem internally.
Matthew Stevenson
Mark D. Roth
Tanvi Jagtap
Eugene Ostroukhov
Craig Tiller
Yash Tibrewal
AJ Heller
apolcyn
Gregory Cooke
Vignesh Babu
Yijie Ma
Matthew Stevenson