Skip to content

Commit

Permalink
xds_override_host LB: make subchannel keys consistent with the filter (…
Browse files Browse the repository at this point in the history 
…grpc#32224)
  • Loading branch information
@eugeneo
eugeneo authored Jan 27, 2023
1 parent 843cf42 commit 42a25af
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 38 deletions.
4 changes: 2 additions & 2 deletions src/core/ext/filters/client_channel/lb_policy/xds/xds_override_host.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ absl::StatusOr<ServerAddressList> XdsOverrideHostLb::UpdateAddressMap(
// Skip draining hosts if not in the override status set.
continue;
}
auto key = grpc_sockaddr_to_string(&address.address(), false);
auto key = grpc_sockaddr_to_uri(&address.address());
if (key.ok()) {
addresses_for_map.emplace(std::move(*key), status);
}
Expand Down Expand Up @@ -550,7 +550,7 @@ absl::StatusOr<ServerAddressList> XdsOverrideHostLb::UpdateAddressMap(
RefCountedPtr<XdsOverrideHostLb::SubchannelWrapper>
XdsOverrideHostLb::AdoptSubchannel(
ServerAddress address, RefCountedPtr<SubchannelInterface> subchannel) {
auto key = grpc_sockaddr_to_string(&address.address(), false);
auto key = grpc_sockaddr_to_uri(&address.address());
if (!key.ok()) {
return subchannel;
}
Expand Down
20 changes: 12 additions & 8 deletions test/core/client_channel/lb_policy/lb_policy_test_lib.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,8 +463,12 @@ class LoadBalancingPolicyTest : public ::testing::Test {
// A fake CallState implementation, for use in PickArgs.
class FakeCallState : public LbCallStateInternal {
public:
explicit FakeCallState(std::map<UniqueTypeName, std::string> attributes)
: attributes_(attributes) {}
explicit FakeCallState(
const std::map<UniqueTypeName, absl::string_view>& attributes) {
for (const auto& p : attributes) {
attributes_.emplace(p.first, std::string(p.second));
}
}

~FakeCallState() override {
for (void* allocation : allocations_) {
Expand Down Expand Up @@ -697,7 +701,7 @@ class LoadBalancingPolicyTest : public ::testing::Test {
WaitForRoundRobinListChange(
absl::Span<const absl::string_view> old_addresses,
absl::Span<const absl::string_view> new_addresses,
const std::map<UniqueTypeName, std::string>& call_attributes = {},
const std::map<UniqueTypeName, absl::string_view>& call_attributes = {},
size_t num_iterations = 3, SourceLocation location = SourceLocation()) {
gpr_log(GPR_INFO, "Waiting for expected RR addresses...");
RefCountedPtr<LoadBalancingPolicy::SubchannelPicker> retval;
Expand Down Expand Up @@ -757,7 +761,7 @@ class LoadBalancingPolicyTest : public ::testing::Test {
// Does a pick and returns the result.
LoadBalancingPolicy::PickResult DoPick(
LoadBalancingPolicy::SubchannelPicker* picker,
const std::map<UniqueTypeName, std::string>& call_attributes = {}) {
const std::map<UniqueTypeName, absl::string_view>& call_attributes = {}) {
ExecCtx exec_ctx;
FakeMetadata metadata({});
FakeCallState call_state(call_attributes);
Expand All @@ -767,7 +771,7 @@ class LoadBalancingPolicyTest : public ::testing::Test {
// Requests a pick on picker and expects a Queue result.
void ExpectPickQueued(
LoadBalancingPolicy::SubchannelPicker* picker,
const std::map<UniqueTypeName, std::string>& call_attributes = {},
const std::map<UniqueTypeName, absl::string_view>& call_attributes = {},
SourceLocation location = SourceLocation()) {
ASSERT_NE(picker, nullptr);
auto pick_result = DoPick(picker, call_attributes);
Expand All @@ -786,7 +790,7 @@ class LoadBalancingPolicyTest : public ::testing::Test {
// automatically to represent a complete call with no backend metric data.
absl::optional<std::string> ExpectPickComplete(
LoadBalancingPolicy::SubchannelPicker* picker,
const std::map<UniqueTypeName, std::string>& call_attributes = {},
const std::map<UniqueTypeName, absl::string_view>& call_attributes = {},
std::unique_ptr<LoadBalancingPolicy::SubchannelCallTrackerInterface>*
subchannel_call_tracker = nullptr,
SourceLocation location = SourceLocation()) {
Expand Down Expand Up @@ -822,7 +826,7 @@ class LoadBalancingPolicyTest : public ::testing::Test {
// list of addresses, or nullopt if a non-complete pick was returned.
absl::optional<std::vector<std::string>> GetCompletePicks(
LoadBalancingPolicy::SubchannelPicker* picker, size_t num_picks,
const std::map<UniqueTypeName, std::string>& call_attributes = {},
const std::map<UniqueTypeName, absl::string_view>& call_attributes = {},
std::vector<
std::unique_ptr<LoadBalancingPolicy::SubchannelCallTrackerInterface>>*
subchannel_call_trackers = nullptr,
Expand Down Expand Up @@ -872,7 +876,7 @@ class LoadBalancingPolicyTest : public ::testing::Test {
void ExpectRoundRobinPicks(
LoadBalancingPolicy::SubchannelPicker* picker,
absl::Span<const absl::string_view> addresses,
const std::map<UniqueTypeName, std::string>& call_attributes = {},
const std::map<UniqueTypeName, absl::string_view>& call_attributes = {},
size_t num_iterations = 3, SourceLocation location = SourceLocation()) {
auto picks = GetCompletePicks(picker, num_iterations * addresses.size(),
call_attributes, nullptr, location);
Expand Down
56 changes: 28 additions & 28 deletions test/core/client_channel/lb_policy/xds_override_host_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,11 @@ TEST_F(XdsOverrideHostTest, OverrideHost) {
auto picker = ExpectStartupWithRoundRobin(kAddresses);
ASSERT_NE(picker, nullptr);
// Check that the host is overridden
std::map<UniqueTypeName, std::string> call_attributes{
{XdsOverrideHostTypeName(), "127.0.0.1:442"}};
std::map<UniqueTypeName, absl::string_view> call_attributes{
{XdsOverrideHostTypeName(), kAddresses[1]}};
EXPECT_EQ(ExpectPickComplete(picker.get(), call_attributes), kAddresses[1]);
EXPECT_EQ(ExpectPickComplete(picker.get(), call_attributes), kAddresses[1]);
call_attributes[XdsOverrideHostTypeName()] = std::string("127.0.0.1:441");
call_attributes[XdsOverrideHostTypeName()] = kAddresses[0];
EXPECT_EQ(ExpectPickComplete(picker.get(), call_attributes), kAddresses[0]);
EXPECT_EQ(ExpectPickComplete(picker.get(), call_attributes), kAddresses[0]);
}
Expand All @@ -149,7 +149,7 @@ TEST_F(XdsOverrideHostTest, SubchannelNotFound) {
"ipv4:127.0.0.1:441", "ipv4:127.0.0.1:442", "ipv4:127.0.0.1:443"};
auto picker = ExpectStartupWithRoundRobin(kAddresses);
ASSERT_NE(picker, nullptr);
std::map<UniqueTypeName, std::string> call_attributes{
std::map<UniqueTypeName, absl::string_view> call_attributes{
{XdsOverrideHostTypeName(), "no such host"}};
ExpectRoundRobinPicks(picker.get(), kAddresses, call_attributes);
}
Expand All @@ -160,8 +160,8 @@ TEST_F(XdsOverrideHostTest, SubchannelsComeAndGo) {
auto picker = ExpectStartupWithRoundRobin(kAddresses);
ASSERT_NE(picker, nullptr);
// Check that the host is overridden
std::map<UniqueTypeName, std::string> call_attributes{
{XdsOverrideHostTypeName(), "127.0.0.1:442"}};
std::map<UniqueTypeName, absl::string_view> call_attributes{
{XdsOverrideHostTypeName(), kAddresses[1]}};
ExpectRoundRobinPicks(picker.get(), {kAddresses[1]}, call_attributes);
// Some other address is gone
EXPECT_EQ(ApplyUpdate(BuildUpdate({kAddresses[0], kAddresses[1]},
Expand Down Expand Up @@ -206,8 +206,8 @@ TEST_F(XdsOverrideHostTest, FailedSubchannelIsNotPicked) {
auto picker = ExpectStartupWithRoundRobin(kAddresses);
ASSERT_NE(picker, nullptr);
// Check that the host is overridden
std::map<UniqueTypeName, std::string> pick_arg{
{XdsOverrideHostTypeName(), "127.0.0.1:442"}};
std::map<UniqueTypeName, absl::string_view> pick_arg{
{XdsOverrideHostTypeName(), kAddresses[1]}};
EXPECT_EQ(ExpectPickComplete(picker.get(), pick_arg), kAddresses[1]);
auto subchannel = FindSubchannel(kAddresses[1]);
ASSERT_NE(subchannel, nullptr);
Expand All @@ -232,8 +232,8 @@ TEST_F(XdsOverrideHostTest, ConnectingSubchannelIsQueued) {
auto picker = ExpectStartupWithRoundRobin(kAddresses);
ASSERT_NE(picker, nullptr);
// Check that the host is overridden
std::map<UniqueTypeName, std::string> pick_arg{
{XdsOverrideHostTypeName(), "127.0.0.1:442"}};
std::map<UniqueTypeName, absl::string_view> pick_arg{
{XdsOverrideHostTypeName(), kAddresses[1]}};
EXPECT_EQ(ExpectPickComplete(picker.get(), pick_arg), kAddresses[1]);
auto subchannel = FindSubchannel(kAddresses[1]);
ASSERT_NE(subchannel, nullptr);
Expand Down Expand Up @@ -262,8 +262,8 @@ TEST_F(XdsOverrideHostTest, DrainingState) {
ExpectRoundRobinPicks(picker.get(), {kAddresses[0], kAddresses[2]});
ExpectQueueEmpty();
// Draining subchannel is returned
std::map<UniqueTypeName, std::string> pick_arg{
{XdsOverrideHostTypeName(), "127.0.0.1:442"}};
std::map<UniqueTypeName, absl::string_view> pick_arg{
{XdsOverrideHostTypeName(), kAddresses[1]}};
EXPECT_EQ(ExpectPickComplete(picker.get(), pick_arg), kAddresses[1]);
ApplyUpdateWithHealthStatuses(
{{kAddresses[0], XdsHealthStatus::HealthStatus::kUnknown},
Expand All @@ -281,8 +281,8 @@ TEST_F(XdsOverrideHostTest, DrainingSubchannelIsConnecting) {
auto picker = ExpectStartupWithRoundRobin(kAddresses);
ASSERT_NE(picker, nullptr);
// Check that the host is overridden
std::map<UniqueTypeName, std::string> pick_arg{
{XdsOverrideHostTypeName(), "127.0.0.1:442"}};
std::map<UniqueTypeName, absl::string_view> pick_arg{
{XdsOverrideHostTypeName(), kAddresses[1]}};
EXPECT_EQ(ExpectPickComplete(picker.get(), pick_arg), kAddresses[1]);
ApplyUpdateWithHealthStatuses(
{{kAddresses[0], XdsHealthStatus::HealthStatus::kUnknown},
Expand Down Expand Up @@ -326,8 +326,8 @@ TEST_F(XdsOverrideHostTest, DrainingToHealthy) {
ASSERT_NE(picker, nullptr);
ExpectRoundRobinPicks(picker.get(), {kAddresses[0], kAddresses[2]});
ExpectQueueEmpty();
std::map<UniqueTypeName, std::string> pick_arg{
{XdsOverrideHostTypeName(), "127.0.0.1:442"}};
std::map<UniqueTypeName, absl::string_view> pick_arg{
{XdsOverrideHostTypeName(), kAddresses[1]}};
EXPECT_EQ(ExpectPickComplete(picker.get(), pick_arg), kAddresses[1]);
ApplyUpdateWithHealthStatuses(
{{kAddresses[0], XdsHealthStatus::HealthStatus::kHealthy},
Expand All @@ -353,13 +353,13 @@ TEST_F(XdsOverrideHostTest, OverrideHostStatus) {
ASSERT_NE(picker, nullptr);
ExpectRoundRobinPicks(picker.get(), {kAddresses[0], kAddresses[1]});
EXPECT_EQ(ExpectPickComplete(picker.get(),
{{XdsOverrideHostTypeName(), "127.0.0.1:441"}}),
{{XdsOverrideHostTypeName(), kAddresses[0]}}),
kAddresses[0]);
EXPECT_EQ(ExpectPickComplete(picker.get(),
{{XdsOverrideHostTypeName(), "127.0.0.1:442"}}),
{{XdsOverrideHostTypeName(), kAddresses[1]}}),
kAddresses[1]);
EXPECT_EQ(ExpectPickComplete(picker.get(),
{{XdsOverrideHostTypeName(), "127.0.0.1:443"}}),
{{XdsOverrideHostTypeName(), kAddresses[2]}}),
kAddresses[2]);
// UNKNOWN excluded - first chanel does not get overridden
ApplyUpdateWithHealthStatuses(
Expand All @@ -371,12 +371,12 @@ TEST_F(XdsOverrideHostTest, OverrideHostStatus) {
ASSERT_NE(picker, nullptr);
ExpectRoundRobinPicks(picker.get(), {kAddresses[0], kAddresses[1]});
ExpectRoundRobinPicks(picker.get(), {kAddresses[0], kAddresses[1]},
{{XdsOverrideHostTypeName(), "127.0.0.1:441"}});
{{XdsOverrideHostTypeName(), kAddresses[0]}});
EXPECT_EQ(ExpectPickComplete(picker.get(),
{{XdsOverrideHostTypeName(), "127.0.0.1:442"}}),
{{XdsOverrideHostTypeName(), kAddresses[1]}}),
kAddresses[1]);
EXPECT_EQ(ExpectPickComplete(picker.get(),
{{XdsOverrideHostTypeName(), "127.0.0.1:443"}}),
{{XdsOverrideHostTypeName(), kAddresses[2]}}),
kAddresses[2]);
// HEALTHY excluded - second chanel does not get overridden
ApplyUpdateWithHealthStatuses(
Expand All @@ -388,13 +388,13 @@ TEST_F(XdsOverrideHostTest, OverrideHostStatus) {
ASSERT_NE(picker, nullptr);
ExpectRoundRobinPicks(picker.get(), {kAddresses[0], kAddresses[1]});
EXPECT_EQ(ExpectPickComplete(picker.get(),
{{XdsOverrideHostTypeName(), "127.0.0.1:441"}}),
{{XdsOverrideHostTypeName(), kAddresses[0]}}),
kAddresses[0]);
EXPECT_EQ(ExpectPickComplete(picker.get(),
{{XdsOverrideHostTypeName(), "127.0.0.1:442"}}),
{{XdsOverrideHostTypeName(), kAddresses[1]}}),
kAddresses[1]);
ExpectRoundRobinPicks(picker.get(), {kAddresses[0], kAddresses[1]},
{{XdsOverrideHostTypeName(), "127.0.0.1:443"}});
{{XdsOverrideHostTypeName(), kAddresses[2]}});
// DRAINING excluded - third chanel does not get overridden
ApplyUpdateWithHealthStatuses(
{{kAddresses[0], XdsHealthStatus::HealthStatus::kUnknown},
Expand All @@ -405,13 +405,13 @@ TEST_F(XdsOverrideHostTest, OverrideHostStatus) {
ASSERT_NE(picker, nullptr);
ExpectRoundRobinPicks(picker.get(), {kAddresses[0], kAddresses[1]});
EXPECT_EQ(ExpectPickComplete(picker.get(),
{{XdsOverrideHostTypeName(), "127.0.0.1:441"}}),
{{XdsOverrideHostTypeName(), kAddresses[0]}}),
kAddresses[0]);
EXPECT_EQ(ExpectPickComplete(picker.get(),
{{XdsOverrideHostTypeName(), "127.0.0.1:442"}}),
{{XdsOverrideHostTypeName(), kAddresses[1]}}),
kAddresses[1]);
ExpectRoundRobinPicks(picker.get(), {kAddresses[0], kAddresses[1]},
{{XdsOverrideHostTypeName(), "127.0.0.1:443"}});
{{XdsOverrideHostTypeName(), kAddresses[2]}});
}

} // namespace
Expand Down

0 comments on commit 42a25af

Please sign in to comment.