fix master service endpoint system for multiple masters

[lavalamp]

Closes #6572.

@thockin I can't see any reason why the old system won't still work?

[lavalamp]

Also, @davidopp

[thockin]

Address structures are sorted by IP - what am I missing? I guess it could NOT sort, but that would potentially trigger false updates.

Why not just detect that you have too many and remove one at random. That should still converge, no?

[lavalamp]

We sort by IP? Why? What component does that?

[lavalamp]

I will make it sort and then delete the address(es) that come after it's own address. I think that has faster convergence properties than deleting a random one.

[thockin]

The sorting is done in the RepackSubsets function.

On Tue, Apr 28, 2015 at 5:33 PM, Daniel Smith notifications@github.com
wrote:

In pkg/master/publish.go
#7273 (comment)
:

• // First, determine if the endpoint is in the format we expect (one
• // subset, one port, N IP addresses).
• formatCorrect, foundIP := m.checkEndpointSubsetFormat(e, ip.String(), port)
• if !formatCorrect {

•   // Something is egregiously wrong, just re-make the endpoints record.
  

•   e.Subsets = []api.EndpointSubset{{
  

•       Addresses: []api.EndpointAddress{{IP: ip.String()}},
  

•       Ports:     []api.EndpointPort{{Port: port, Protocol: api.ProtocolTCP}},
  

•   }}
  

•   glog.Infof("Resetting endpoints for master service %q to %v", serviceName, e)
  

•   return m.endpointRegistry.UpdateEndpoints(ctx, e)
  

• } else if !foundIP {

•   // So we _always_ add our own IP address to the end, and if
  

•   // there are too many IP addresses, we remove from the
  

•   // beginning. Given the requirements stated at the top of this
  

•   // function, this should cause the list of IP addresses to
  

I will make it sort and then delete the address(es) that come after it's
own address. I think that has faster convergence properties than deleting a
random one.

—
Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/pull/7273/files#r29302403
.

[lavalamp]

Comments addressed, PTAL

[thockin]

It's minor, but this will make copies of every address struct. I've gotten into the habit (as suggested by r) to do

for i := range *addrs {
    addr := &(*addrs)[i]
    //...
}

[thockin]

wrap this loop with a test for len > masterCount?

[lavalamp]

Meh, the struct is like two words, it's hard for me to get excited about not copying it. Two word copy vs an indirection everywhere is not even a clear victory for the latter. That pattern is really only useful when you're worried about a reference escaping the loop. (or when the struct is huge, maybe)

[lavalamp]

Comments disputed or addressed :) PTAL

[thockin]

LGTM

[davidopp]

LGTM

[lavalamp]

Shippable is a flake; I can haz merge?