Skip to content

Adding weights to service entry endpoints#704

Merged
rshriram merged 5 commits intoistio:release-1.1from
rshriram:weight
Nov 12, 2018
Merged

Adding weights to service entry endpoints#704
rshriram merged 5 commits intoistio:release-1.1from
rshriram:weight

Conversation

@rshriram
Copy link
Member

@rshriram rshriram commented Nov 12, 2018

The endpoint weights correspond to Envoy's LB endpoint weights. This will allow service entries for remote cluster endpoints to carry weights.

Signed-off-by: Shriram Rajagopalan shriramr@vmware.com

@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Nov 12, 2018
@rshriram rshriram requested a review from ZackButcher November 12, 2018 15:24
Copy link
Member

@ymesika ymesika left a comment

Choose a reason for hiding this comment

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

/lgtm

@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rshriram, ymesika

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@istio-testing
Copy link
Collaborator

@ymesika: changing LGTM is restricted to assignees, and only istio/api repo collaborators may be assigned issues.

Details

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.


// The load balancing weight associated with the endpoint. Endpoints
// with higher weights will receive proportionally higher traffic.
uint32 weight = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this work in conjunction with route destination weights? When do you use which?

Copy link
Member Author

Choose a reason for hiding this comment

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

Route destination weights are for a set of endpoints.. a service version etc. This is within an envoy cluster (i.e. a route destination if you will), where you can have one single service instance receive more traffic than other service instances in the same version.

The weights here are used in the multicluster scenario where you use the ingress gateway address as the endpoint for a host (foo.ns1.global) in the remote cluster. If the service is present in many clusters, then you would have multiple ingrss gateway endpoints here. In such a scenario, we need a way to specify the weight associated with each gateway (proportional to the number of pods of that service in that cluster).

Copy link
Contributor

@ZackButcher ZackButcher left a comment

Choose a reason for hiding this comment

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

LGTM but this is just a proxy for the Envoy endpoint weight field then we should say something about the allowed values (1 to 128) in the comment on the field.


// The load balancing weight associated with the endpoint. Endpoints
// with higher weights will receive proportionally higher traffic.
uint32 weight = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't Envoy just go through a whole exercise in defining multi-dimensional weights for load balancing? Should we use the same here?

Copy link
Contributor

Choose a reason for hiding this comment

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

And to answer my own question: not yet. They still use a uint32 today, and must be in the range 1-128. We should probably add a comment about the allowed range here.

Copy link
Member Author

Choose a reason for hiding this comment

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

we can normalize in code across all endpoints in a cluster. No point in asking users to normalize


// The load balancing weight associated with the endpoint. Endpoints
// with higher weights will receive proportionally higher traffic.
uint32 weight = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't make sense - why would the weight be associated with the ServiceEntry ( the full set of endpoints) ? I think it needs to be part of the endpoint.

Copy link
Member Author

Choose a reason for hiding this comment

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

Its part of endppint

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, missed the indentation.
/lgtm

@rshriram rshriram merged commit 56c8213 into istio:release-1.1 Nov 12, 2018
@andraxylia
Copy link
Contributor

I see this has been merged already, but isn't the weight here actually capacity? If this is meant to be used to show how many Endpoints are behind a Gateway.

@rshriram
Copy link
Member Author

Yep. The weight here is capacity. But the term weight is more well understood - service has N endpoints each with a different weight.

rshriram added a commit to rshriram/api that referenced this pull request Nov 14, 2018
* add dynamic state to attribute list

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* Adding weights to service entry endpoints

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* undo
istio-testing pushed a commit that referenced this pull request Nov 14, 2018
* Adding weights to service entry endpoints (#704)

* add dynamic state to attribute list

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* Adding weights to service entry endpoints

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* undo

* update protolock

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
louiscryan pushed a commit to louiscryan/api that referenced this pull request Jan 16, 2019
…stio#706)

* Adding weights to service entry endpoints (istio#704)

* add dynamic state to attribute list

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* Adding weights to service entry endpoints

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* undo

* update protolock

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants