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.

Sign up for GitHub

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

Add load_assignment field in Cluster #3504

Merged
merged 4 commits into from
Jun 6, 2018
Merged

Add load_assignment field in Cluster #3504

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
91bc921
Add load_assignment field in Cluster
dio May 30, 2018
0001298
Add indication: address form depends on the cluster type
dio Jun 5, 2018
f5783e9
review comments: rephrase and add note
dio Jun 6, 2018
2ab4f13
Fix comment
dio Jun 6, 2018
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
4 changes: 4 additions & 0 deletions api/envoy/api/v2/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ api_proto_library(
visibility = [":friends"],
deps = [
":discovery",
":eds",
"//envoy/api/v2/auth:cert",
"//envoy/api/v2/cluster:circuit_breaker",
"//envoy/api/v2/cluster:outlier_detection",
Expand All @@ -70,6 +71,7 @@ api_proto_library(
"//envoy/api/v2/core:config_source",
"//envoy/api/v2/core:health_check",
"//envoy/api/v2/core:protocol",
"//envoy/api/v2/endpoint",
"//envoy/type:percent",
],
)
Expand All @@ -79,6 +81,7 @@ api_go_grpc_library(
proto = ":cds",
deps = [
":discovery_go_proto",
":eds_go_grpc",
"//envoy/api/v2/auth:cert_go_proto",
"//envoy/api/v2/cluster:circuit_breaker_go_proto",
"//envoy/api/v2/cluster:outlier_detection_go_proto",
Expand All @@ -87,6 +90,7 @@ api_go_grpc_library(
"//envoy/api/v2/core:config_source_go_proto",
"//envoy/api/v2/core:health_check_go_proto",
"//envoy/api/v2/core:protocol_go_proto",
"//envoy/api/v2/endpoint:endpoint_go_proto",
"//envoy/type:percent_go_proto",
],
)
Expand Down
18 changes: 17 additions & 1 deletion api/envoy/api/v2/cds.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import "envoy/api/v2/core/health_check.proto";
import "envoy/api/v2/core/protocol.proto";
import "envoy/api/v2/cluster/circuit_breaker.proto";
import "envoy/api/v2/cluster/outlier_detection.proto";
import "envoy/api/v2/eds.proto";
import "envoy/type/percent.proto";

import "google/api/annotations.proto";
Expand Down Expand Up @@ -41,7 +42,7 @@ service ClusterDiscoveryService {
// [#protodoc-title: Clusters]

// Configuration for a single upstream cluster.
// [#comment:next free field: 32]
// [#comment:next free field: 34]
message Cluster {
// Supplies the name of the cluster which must be unique across all clusters.
// The cluster name is used when emitting
Expand Down Expand Up @@ -157,6 +158,21 @@ message Cluster {
// then hosts is required.
repeated core.Address hosts = 7;

// Setting this is required for specifying members of
// :ref:`STATIC<envoy_api_enum_value_Cluster.DiscoveryType.STATIC>`,
// :ref:`STRICT_DNS<envoy_api_enum_value_Cluster.DiscoveryType.STRICT_DNS>`
// or :ref:`LOGICAL_DNS<envoy_api_enum_value_Cluster.DiscoveryType.LOGICAL_DNS>` clusters.
// This field supersedes :ref:`hosts<envoy_api_field_Cluster.hosts>` field.
//
// .. attention::
//
// Setting this allows CDS static/DNS assignments to contain embedded EDS equivalent
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"CDS static/DNS" is a bit confusing to read. Can you rephrase? Do you mean "Setting this allows non-EDS cluster types to contain..." ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated f5783e9

// :ref:`endpoint assignments<envoy_api_msg_ClusterLoadAssignment>`.
// Setting this overrides :ref:`hosts<envoy_api_field_Cluster.hosts>` values.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dio can you make a note to deprecate the hosts field and add to deprecated.md when you do the PR implementing this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, 2ab4f13. Thanks for reminding me.

//
// [#not-implemented-hide:]
ClusterLoadAssignment load_assignment = 33;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the discussion at https://github.com/envoyproxy/envoy/pull/3261/files#r188243657 still applies.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zuercher thanks, will update it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in 0001298.


// Optional :ref:`active health checking <arch_overview_health_checking>`
// configuration for the cluster. If no
// configuration is specified no health checking will be done and all cluster
Expand Down