Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support mixed v2/v3 xDS request/responses #12913

Merged
merged 38 commits into from
Sep 22, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
28dcba3
first version
Sep 1, 2020
0b27712
add comment
Sep 1, 2020
e3f3485
fix no watcher case
Sep 1, 2020
1190cf2
fix no watcher case
Sep 1, 2020
afc5263
Merge branch 'master' of https://github.com/envoyproxy/envoy into mix…
Sep 1, 2020
d5f8dcc
Merge branch 'master' of https://github.com/envoyproxy/envoy into mix…
Sep 1, 2020
17a1002
clean up
Sep 2, 2020
333d68f
clean up
Sep 2, 2020
ca8e67b
clean up
Sep 2, 2020
a4deb6b
Merge branch 'master' of https://github.com/envoyproxy/envoy into mix…
Sep 2, 2020
53887ac
clean up
Sep 2, 2020
de87fd6
support v2 response to v3 request
Sep 3, 2020
8153e6c
fix spelling
Sep 3, 2020
56edcaf
move impl out of include header
Sep 8, 2020
d4d68be
fix watcher empty
Sep 8, 2020
fd72d9f
fix watcher empty
Sep 8, 2020
3a374b7
type url map be singleton
Sep 11, 2020
1f0c8b6
clean up
Sep 11, 2020
cf0e1ec
add runtime flag
Sep 15, 2020
e0475fd
add runtime flag
Sep 15, 2020
d0ba29a
clean up
Sep 15, 2020
504d403
clean up
Sep 15, 2020
8ea1fac
unit test runtime
Sep 15, 2020
0633926
merge
Sep 16, 2020
314d127
add doc
Sep 17, 2020
1371da9
add helper function
Sep 17, 2020
a8eae1d
refactor
Sep 17, 2020
1278fda
format
Sep 17, 2020
ca3ece4
clean up
Sep 17, 2020
bfd7e87
test coverage
Sep 18, 2020
804721d
type util test
Sep 19, 2020
ca31bf9
format
Sep 20, 2020
fd37479
format
Sep 21, 2020
4617ca2
add doc
Sep 21, 2020
1d9c95c
format
Sep 21, 2020
8c4170a
Merge branch 'master' of https://github.com/envoyproxy/envoy into mix…
Sep 21, 2020
eded175
change doc
Sep 22, 2020
5d14694
docs
Sep 22, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions source/common/config/api_type_oracle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,26 @@ ApiTypeOracle::getEarlierVersionMessageTypeName(const std::string& message_type)
}
return absl::nullopt;
}

const absl::optional<std::string> ApiTypeOracle::getEarlierTypeUrl(const std::string& type_url) {
chaoqin-li1123 marked this conversation as resolved.
Show resolved Hide resolved
char delimeter = '/';
size_t type_name_start = type_url.size();
for (size_t i = 0; i < type_url.size(); i++) {
chaoqin-li1123 marked this conversation as resolved.
Show resolved Hide resolved
if (type_url[i] == delimeter) {
type_name_start = i;
}
}
if (type_name_start == type_url.size()) {
return {};
}
std::string message_type = type_url.substr(type_name_start + 1);
chaoqin-li1123 marked this conversation as resolved.
Show resolved Hide resolved
absl::optional<std::string> old_message_type =
ApiTypeOracle::getEarlierVersionMessageTypeName(message_type);
if (old_message_type) {
chaoqin-li1123 marked this conversation as resolved.
Show resolved Hide resolved
return "type.googleapis.com/" + *old_message_type;
chaoqin-li1123 marked this conversation as resolved.
Show resolved Hide resolved
}
return {};
}

chaoqin-li1123 marked this conversation as resolved.
Show resolved Hide resolved
} // namespace Config
} // namespace Envoy
2 changes: 2 additions & 0 deletions source/common/config/api_type_oracle.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ class ApiTypeOracle {

static const absl::optional<std::string>
getEarlierVersionMessageTypeName(const std::string& message_type);

static const absl::optional<std::string> getEarlierTypeUrl(const std::string& type_url);
};

} // namespace Config
Expand Down
24 changes: 16 additions & 8 deletions source/common/config/grpc_mux_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,17 +116,25 @@ ScopedResume GrpcMuxImpl::pause(const std::vector<std::string> type_urls) {
void GrpcMuxImpl::onDiscoveryResponse(
std::unique_ptr<envoy::service::discovery::v3::DiscoveryResponse>&& message,
ControlPlaneStats& control_plane_stats) {
const std::string& type_url = message->type_url();
std::string type_url = message->type_url();
ENVOY_LOG(debug, "Received gRPC message for {} at version {}", type_url, message->version_info());
if (message->has_control_plane()) {
control_plane_stats.identifier_.set(message->control_plane().identifier());
}
// If this type url is not watched, try older version of type url.
if (api_state_.count(type_url) == 0) {
ENVOY_LOG(warn, "Ignoring the message for type URL {} as it has no current subscribers.",
type_url);
// TODO(yuval-k): This should never happen. consider dropping the stream as this is a
// protocol violation
return;
absl::optional<std::string> old_type_url =
ApiTypeOracle::getEarlierTypeUrl(message->type_url());
chaoqin-li1123 marked this conversation as resolved.
Show resolved Hide resolved
if (old_type_url && api_state_.count(*old_type_url)) {
type_url = *old_type_url;
ENVOY_LOG(debug, "v3 {} converted to v2 {}.", message->type_url(), *old_type_url);
} else {
// TODO(yuval-k): This should never happen. consider dropping the stream as this is a
// protocol violation
ENVOY_LOG(warn, "Ignoring the message for type URL {} as it has no current subscribers.",
type_url);
return;
}
}
if (api_state_[type_url].watches_.empty()) {
// update the nonce as we are processing this response.
Expand Down Expand Up @@ -164,10 +172,10 @@ void GrpcMuxImpl::onDiscoveryResponse(
OpaqueResourceDecoder& resource_decoder =
api_state_[type_url].watches_.front()->resource_decoder_;
for (const auto& resource : message->resources()) {
if (type_url != resource.type_url()) {
if (message->type_url() != resource.type_url()) {
throw EnvoyException(
fmt::format("{} does not match the message-wide type URL {} in DiscoveryResponse {}",
resource.type_url(), type_url, message->DebugString()));
resource.type_url(), message->type_url(), message->DebugString()));
}
resources.emplace_back(
new DecodedResourceImpl(resource_decoder, resource, message->version_info()));
Expand Down
8 changes: 8 additions & 0 deletions source/common/config/new_grpc_mux_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ void NewGrpcMuxImpl::onDiscoveryResponse(
ENVOY_LOG(debug, "Received DeltaDiscoveryResponse for {} at version {}", message->type_url(),
message->system_version_info());
auto sub = subscriptions_.find(message->type_url());
// If this type url is not watched, try older version type url.
if (sub == subscriptions_.end()) {
absl::optional<std::string> old_type_url =
chaoqin-li1123 marked this conversation as resolved.
Show resolved Hide resolved
ApiTypeOracle::getEarlierTypeUrl(message->type_url());
if (old_type_url) {
sub = subscriptions_.find(*old_type_url);
}
}
if (sub == subscriptions_.end()) {
ENVOY_LOG(warn,
"Dropping received DeltaDiscoveryResponse (with version {}) for non-existent "
Expand Down
88 changes: 88 additions & 0 deletions test/common/config/grpc_mux_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,94 @@ TEST_F(GrpcMuxImplTest, BadLocalInfoEmptyNodeName) {
"ads: node 'id' and 'cluster' are required. Set it either in 'node' config or via "
"--service-node and --service-cluster options.");
}

// Send discovery request with v2 resource type_url, receive discovery response with v3 resource
// type_url.
TEST_F(GrpcMuxImplTest, WatchV2ResourceV3) {
setup();
InSequence s;
TestUtility::TestOpaqueResourceDecoderImpl<envoy::config::endpoint::v3::ClusterLoadAssignment>
resource_decoder("cluster_name");
const std::string& v2_type_url = Config::TypeUrl::get().ClusterLoadAssignment;
const std::string& v3_type_url =
"type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment";
NiceMock<MockSubscriptionCallbacks> foo_callbacks;
auto foo_sub = grpc_mux_->addWatch(v2_type_url, {"x", "y"}, foo_callbacks, resource_decoder);
NiceMock<MockSubscriptionCallbacks> bar_callbacks;
auto bar_sub = grpc_mux_->addWatch(v2_type_url, {"y", "z"}, bar_callbacks, resource_decoder);
EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_));
expectSendMessage(v2_type_url, {"y", "z", "x"}, "", true);
grpc_mux_->start();

{
// Send resource with v3 type url, should invoke onConfigUpdate.
auto response = std::make_unique<envoy::service::discovery::v3::DiscoveryResponse>();
response->set_type_url(v3_type_url);
response->set_version_info("1");
envoy::config::endpoint::v3::ClusterLoadAssignment load_assignment;
load_assignment.set_cluster_name("x");
response->add_resources()->PackFrom(load_assignment);
EXPECT_CALL(bar_callbacks, onConfigUpdate(_, "1")).Times(0);
EXPECT_CALL(foo_callbacks, onConfigUpdate(_, "1"))
.WillOnce(Invoke([&load_assignment](const std::vector<DecodedResourceRef>& resources,
const std::string&) {
EXPECT_EQ(1, resources.size());
const auto& expected_assignment =
dynamic_cast<const envoy::config::endpoint::v3::ClusterLoadAssignment&>(
resources[0].get().resource());
EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment));
}));
expectSendMessage(v2_type_url, {"y", "z", "x"}, "1");
grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response));
}

{
chaoqin-li1123 marked this conversation as resolved.
Show resolved Hide resolved
auto response = std::make_unique<envoy::service::discovery::v3::DiscoveryResponse>();
response->set_type_url(v3_type_url);
response->set_version_info("2");
envoy::config::endpoint::v3::ClusterLoadAssignment load_assignment_x;
load_assignment_x.set_cluster_name("x");
response->add_resources()->PackFrom(load_assignment_x);
envoy::config::endpoint::v3::ClusterLoadAssignment load_assignment_y;
load_assignment_y.set_cluster_name("y");
response->add_resources()->PackFrom(load_assignment_y);
envoy::config::endpoint::v3::ClusterLoadAssignment load_assignment_z;
load_assignment_z.set_cluster_name("z");
response->add_resources()->PackFrom(load_assignment_z);
EXPECT_CALL(bar_callbacks, onConfigUpdate(_, "2"))
.WillOnce(Invoke([&load_assignment_y, &load_assignment_z](
const std::vector<DecodedResourceRef>& resources, const std::string&) {
EXPECT_EQ(2, resources.size());
const auto& expected_assignment =
dynamic_cast<const envoy::config::endpoint::v3::ClusterLoadAssignment&>(
resources[0].get().resource());
EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment_y));
const auto& expected_assignment_1 =
dynamic_cast<const envoy::config::endpoint::v3::ClusterLoadAssignment&>(
resources[1].get().resource());
EXPECT_TRUE(TestUtility::protoEqual(expected_assignment_1, load_assignment_z));
}));
EXPECT_CALL(foo_callbacks, onConfigUpdate(_, "2"))
.WillOnce(Invoke([&load_assignment_x, &load_assignment_y](
const std::vector<DecodedResourceRef>& resources, const std::string&) {
EXPECT_EQ(2, resources.size());
const auto& expected_assignment =
dynamic_cast<const envoy::config::endpoint::v3::ClusterLoadAssignment&>(
resources[0].get().resource());
EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment_x));
const auto& expected_assignment_1 =
dynamic_cast<const envoy::config::endpoint::v3::ClusterLoadAssignment&>(
resources[1].get().resource());
EXPECT_TRUE(TestUtility::protoEqual(expected_assignment_1, load_assignment_y));
}));
expectSendMessage(v2_type_url, {"y", "z", "x"}, "2");
grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response));
}

expectSendMessage(v2_type_url, {"x", "y"}, "2");
expectSendMessage(v2_type_url, {}, "2");
}

} // namespace
} // namespace Config
} // namespace Envoy
39 changes: 31 additions & 8 deletions test/common/config/new_grpc_mux_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,17 +162,40 @@ TEST_F(NewGrpcMuxImplTest, ConfigUpdateWithNotFoundResponse) {
response->set_type_url(type_url);
response->set_system_version_info("1");

response->add_resources();
response->mutable_resources()->at(0).set_name("not-found");
response->mutable_resources()->at(0).add_aliases("domain1.test");
envoy::config::route::v3::VirtualHost vhost;
vhost.set_name("vhost_1");

grpc_mux_->onDiscoveryResponse(std::move(response), control_plane_stats_);
response->add_resources()->mutable_resource()->PackFrom(vhost);
}

const auto& subscriptions = grpc_mux_->subscriptions();
auto sub = subscriptions.find(type_url);
// Watch v2 resource type_url, receive discovery response with v3 resource type_url.
TEST_F(NewGrpcMuxImplTest, V3ResourceResponseV2ResourceWatch) {
setup();

EXPECT_TRUE(sub != subscriptions.end());
watch->update({});
// Watch for v2 resource type_url.
const std::string& type_url = Config::TypeUrl::get().ClusterLoadAssignment;
auto watch = grpc_mux_->addWatch(type_url, {}, callbacks_, resource_decoder_);

EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_));
grpc_mux_->start();
{
auto response = std::make_unique<envoy::service::discovery::v3::DeltaDiscoveryResponse>();
response->set_system_version_info("1");
envoy::config::endpoint::v3::ClusterLoadAssignment load_assignment;
load_assignment.set_cluster_name("x");
response->add_resources()->mutable_resource()->PackFrom(load_assignment);
// Send response that contains resource with v3 type url.
response->set_type_url("type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment");
EXPECT_CALL(callbacks_, onConfigUpdate(_, _, "1"))
.WillOnce(Invoke([&load_assignment](const std::vector<DecodedResourceRef>& added_resources,
const Protobuf::RepeatedPtrField<std::string>&,
const std::string&) {
EXPECT_EQ(1, added_resources.size());
EXPECT_TRUE(
TestUtility::protoEqual(added_resources[0].get().resource(), load_assignment));
}));
grpc_mux_->onDiscoveryResponse(std::move(response), control_plane_stats_);
}
}

} // namespace
Expand Down
25 changes: 25 additions & 0 deletions test/integration/ads_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,31 @@ TEST_P(AdsIntegrationTest, Failure) {
makeSingleRequest();
}

// Validate that xds can support a mix of v2 and v3 type url.
TEST_P(AdsIntegrationTest, MixV2V3TypeUrlInDiscoveryResponse) {
initialize();
// Discovery response with v3 type url.
sendDiscoveryResponse<envoy::config::cluster::v3::Cluster>(
"type.googleapis.com/envoy.config.cluster.v3.Cluster", {buildCluster("cluster_0")},
{buildCluster("cluster_0")}, {}, "1", false);
// Discovery response with v2 type url.
sendDiscoveryResponse<envoy::config::endpoint::v3::ClusterLoadAssignment>(
Config::TypeUrl::get().ClusterLoadAssignment, {buildClusterLoadAssignment("cluster_0")},
{buildClusterLoadAssignment("cluster_0")}, {}, "1");
// Discovery response with v3 type url.
sendDiscoveryResponse<envoy::config::listener::v3::Listener>(
"type.googleapis.com/envoy.config.listener.v3.Listener",
{buildListener("listener_0", "route_config_0")},
{buildListener("listener_0", "route_config_0")}, {}, "1", false);
// Discovery response with v2 type url.
sendDiscoveryResponse<envoy::config::route::v3::RouteConfiguration>(
Config::TypeUrl::get().RouteConfiguration, {buildRouteConfig("route_config_0", "cluster_0")},
{buildRouteConfig("route_config_0", "cluster_0")}, {}, "1");
test_server_->waitForCounterGe("listener_manager.listener_create_success", 1);
// Validate that we can process a request.
makeSingleRequest();
}

// Validate that the request with duplicate listeners is rejected.
TEST_P(AdsIntegrationTest, DuplicateWarmingListeners) {
initialize();
Expand Down