Skip to content

Commit 96464c0

Browse files
SamzeSamze
committed
Propagate options-only route changes immediately in route-emitter
Factor in Route options and perform a route register without unregistering.
1 parent 8ab220a commit 96464c0

4 files changed

Lines changed: 209 additions & 1 deletion

File tree

‎src/code.cloudfoundry.org/route-emitter/routingtable/endpoint.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ func (info ExternalEndpointInfo) Hash() interface{} {
100100
return info
101101
}
102102

103+
func (info ExternalEndpointInfo) HashWithOptions() interface{} {
104+
return info.Hash()
105+
}
106+
103107
func (info ExternalEndpointInfo) MessageFor(e Endpoint, directInstanceRoute, _ bool) (*RegistryMessage, *tcpmodels.TcpRouteMapping, *RegistryMessage) {
104108
tlsHostPort := -1
105109
tlsContainerPort := -1
@@ -168,6 +172,13 @@ type routeHash struct {
168172
Protocol string
169173
}
170174

175+
// routeHashWithOptions extends routeHash to include options, normalized so
176+
// semantically identical option sets always compare equal.
177+
type routeHashWithOptions struct {
178+
routeHash
179+
Options string
180+
}
181+
171182
// route hash is used to find route differences
172183
// it needs to be dereferenced so that it can be used as a key in a hash map
173184
func (r Route) Hash() interface{} {
@@ -180,6 +191,29 @@ func (r Route) Hash() interface{} {
180191
}
181192
}
182193

194+
// HashWithOptions includes normalized options so diffRoutes detects options-only changes.
195+
func (r Route) HashWithOptions() interface{} {
196+
opts := string(r.Options)
197+
if len(r.Options) > 0 {
198+
var v interface{}
199+
if err := json.Unmarshal(r.Options, &v); err == nil {
200+
if b, err := json.Marshal(v); err == nil {
201+
opts = string(b)
202+
}
203+
}
204+
}
205+
return routeHashWithOptions{
206+
routeHash: routeHash{
207+
Hostname: r.Hostname,
208+
RouteServiceUrl: r.RouteServiceUrl,
209+
IsolationSegment: r.IsolationSegment,
210+
LogGUID: r.LogGUID,
211+
Protocol: r.Protocol,
212+
},
213+
Options: opts,
214+
}
215+
}
216+
183217
func (r Route) MessageFor(endpoint Endpoint, directInstanceAddress, emitEndpointUpdatedAt bool) (*RegistryMessage, *tcpmodels.TcpRouteMapping, *RegistryMessage) {
184218
generator := RegistryMessageFor
185219
if endpoint.IsDirectInstanceRoute(directInstanceAddress) {
@@ -199,6 +233,10 @@ func (r InternalRoute) Hash() interface{} {
199233
return r
200234
}
201235

236+
func (r InternalRoute) HashWithOptions() interface{} {
237+
return r.Hash()
238+
}
239+
202240
func (r InternalRoute) MessageFor(endpoint Endpoint, _, emitEndpointUpdatedAt bool) (*RegistryMessage, *tcpmodels.TcpRouteMapping, *RegistryMessage) {
203241
generator := InternalEndpointRegistryMessageFor
204242
msg := generator(endpoint, r, emitEndpointUpdatedAt)

‎src/code.cloudfoundry.org/route-emitter/routingtable/endpoint_utils_test.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package routingtable_test
22

33
import (
4+
"encoding/json"
5+
46
"code.cloudfoundry.org/bbs/models"
57
"code.cloudfoundry.org/route-emitter/routingtable"
68
"code.cloudfoundry.org/routing-info/tcp_routes"
@@ -50,6 +52,49 @@ var _ = Describe("LRP Utils", func() {
5052
})
5153
})
5254

55+
Describe("HashWithOptions", func() {
56+
It("treats semantically identical JSON options as equal regardless of key order", func() {
57+
routeAlpha := routingtable.Route{
58+
Hostname: "foo.example.com",
59+
Options: json.RawMessage(`{"b":2,"a":1}`),
60+
}
61+
routeBeta := routingtable.Route{
62+
Hostname: "foo.example.com",
63+
Options: json.RawMessage(`{"a":1,"b":2}`),
64+
}
65+
Expect(routeAlpha.HashWithOptions()).To(Equal(routeBeta.HashWithOptions()))
66+
})
67+
68+
It("distinguishes routes with different option values", func() {
69+
routeAlpha := routingtable.Route{
70+
Hostname: "foo.example.com",
71+
Options: json.RawMessage(`{"loadbalancing":"hash"}`),
72+
}
73+
routeBeta := routingtable.Route{
74+
Hostname: "foo.example.com",
75+
Options: json.RawMessage(`{"loadbalancing":"round-robin"}`),
76+
}
77+
Expect(routeAlpha.HashWithOptions()).NotTo(Equal(routeBeta.HashWithOptions()))
78+
})
79+
80+
It("distinguishes a route with options from one without", func() {
81+
withOptions := routingtable.Route{
82+
Hostname: "foo.example.com",
83+
Options: json.RawMessage(`{"loadbalancing":"hash"}`),
84+
}
85+
withoutOptions := routingtable.Route{
86+
Hostname: "foo.example.com",
87+
}
88+
Expect(withOptions.HashWithOptions()).NotTo(Equal(withoutOptions.HashWithOptions()))
89+
})
90+
91+
It("treats nil and empty options as equal", func() {
92+
withNil := routingtable.Route{Hostname: "foo.example.com", Options: nil}
93+
withEmpty := routingtable.Route{Hostname: "foo.example.com", Options: json.RawMessage{}}
94+
Expect(withNil.HashWithOptions()).To(Equal(withEmpty.HashWithOptions()))
95+
})
96+
})
97+
5398
Describe("NewEndpointsFromActual", func() {
5499
Context("when actual is not evacuating", func() {
55100
It("builds a map of container port to endpoint", func() {

‎src/code.cloudfoundry.org/route-emitter/routingtable/nats_routing_table_test.go‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package routingtable_test
22

33
import (
4+
"encoding/json"
45
"fmt"
56

67
mfakes "code.cloudfoundry.org/diego-logging-client/testhelpers"
@@ -650,6 +651,128 @@ var _ = Describe("RoutingTable", func() {
650651
})
651652
})
652653

654+
Context("when there is an existing routing key with options", func() {
655+
var desiredLRP *models.DesiredLRP
656+
657+
createDesiredLRPWithOptions := func(options json.RawMessage) *models.DesiredLRP {
658+
routingInfo := cfroutes.CFRoutes{
659+
{
660+
Hostnames: []string{hostname1, hostname2},
661+
Port: key.ContainerPort,
662+
Options: options,
663+
},
664+
}.RoutingInfo()
665+
routes := models.Routes{}
666+
for k, v := range routingInfo {
667+
routes[k] = v
668+
}
669+
return createDesiredLRPWithRoutes(key.ProcessGUID, 3, routes, logGuid, *currentTag, runInfo)
670+
}
671+
672+
BeforeEach(func() {
673+
tempTable := routingtable.NewRoutingTable(false, false, fakeMetronClient)
674+
desiredLRP = createDesiredLRPWithOptions(json.RawMessage(`{"loadbalancing":"hash"}`))
675+
tempTable.SetRoutes(logger, nil, desiredLRP)
676+
lrp := createActualLRP(key, endpoint1, domain)
677+
tempTable.AddEndpoint(logger, lrp)
678+
table.Swap(logger, tempTable, domains)
679+
})
680+
681+
Context("when only options change in an event", func() {
682+
BeforeEach(func() {
683+
afterDesiredLRP := createDesiredLRPWithOptions(json.RawMessage(`{"loadbalancing":"round-robin"}`))
684+
afterDesiredLRP.ModificationTag.Index++
685+
_, messagesToEmit = table.SetRoutes(logger, desiredLRP, afterDesiredLRP)
686+
})
687+
688+
It("registers the updated route without unregistering the old one", func() {
689+
expected := routingtable.MessagesToEmit{
690+
RegistrationMessages: []routingtable.RegistryMessage{
691+
routingtable.RegistryMessageFor(endpoint1, routingtable.Route{Hostname: hostname1, LogGUID: logGuid, Options: json.RawMessage(`{"loadbalancing":"round-robin"}`)}, false),
692+
routingtable.RegistryMessageFor(endpoint1, routingtable.Route{Hostname: hostname2, LogGUID: logGuid, Options: json.RawMessage(`{"loadbalancing":"round-robin"}`)}, false),
693+
},
694+
}
695+
Expect(messagesToEmit).To(MatchMessagesToEmit(expected))
696+
})
697+
})
698+
699+
Context("when only options change during sync", func() {
700+
BeforeEach(func() {
701+
tempTable := routingtable.NewRoutingTable(false, false, fakeMetronClient)
702+
afterDesiredLRP := createDesiredLRPWithOptions(json.RawMessage(`{"loadbalancing":"round-robin"}`))
703+
tempTable.SetRoutes(logger, nil, afterDesiredLRP)
704+
lrp := createActualLRP(key, endpoint1, domain)
705+
tempTable.AddEndpoint(logger, lrp)
706+
_, messagesToEmit = table.Swap(logger, tempTable, domains)
707+
})
708+
709+
It("registers the updated route without unregistering the old one", func() {
710+
expected := routingtable.MessagesToEmit{
711+
RegistrationMessages: []routingtable.RegistryMessage{
712+
routingtable.RegistryMessageFor(endpoint1, routingtable.Route{Hostname: hostname1, LogGUID: logGuid, Options: json.RawMessage(`{"loadbalancing":"round-robin"}`)}, false),
713+
routingtable.RegistryMessageFor(endpoint1, routingtable.Route{Hostname: hostname2, LogGUID: logGuid, Options: json.RawMessage(`{"loadbalancing":"round-robin"}`)}, false),
714+
},
715+
}
716+
Expect(messagesToEmit).To(MatchMessagesToEmit(expected))
717+
})
718+
})
719+
720+
Context("when options change and endpoints change simultaneously during sync", func() {
721+
BeforeEach(func() {
722+
tempTable := routingtable.NewRoutingTable(false, false, fakeMetronClient)
723+
afterDesiredLRP := createDesiredLRPWithOptions(json.RawMessage(`{"loadbalancing":"round-robin"}`))
724+
tempTable.SetRoutes(logger, nil, afterDesiredLRP)
725+
lrp := createActualLRP(key, endpoint2, domain)
726+
tempTable.AddEndpoint(logger, lrp)
727+
_, messagesToEmit = table.Swap(logger, tempTable, domains)
728+
})
729+
730+
It("registers the new endpoint with new options and unregisters the old endpoint with old options", func() {
731+
expected := routingtable.MessagesToEmit{
732+
RegistrationMessages: []routingtable.RegistryMessage{
733+
routingtable.RegistryMessageFor(endpoint2, routingtable.Route{Hostname: hostname1, LogGUID: logGuid, Options: json.RawMessage(`{"loadbalancing":"round-robin"}`)}, false),
734+
routingtable.RegistryMessageFor(endpoint2, routingtable.Route{Hostname: hostname2, LogGUID: logGuid, Options: json.RawMessage(`{"loadbalancing":"round-robin"}`)}, false),
735+
},
736+
UnregistrationMessages: []routingtable.RegistryMessage{
737+
routingtable.RegistryMessageFor(endpoint1, routingtable.Route{Hostname: hostname1, LogGUID: logGuid, Options: json.RawMessage(`{"loadbalancing":"hash"}`)}, false),
738+
routingtable.RegistryMessageFor(endpoint1, routingtable.Route{Hostname: hostname2, LogGUID: logGuid, Options: json.RawMessage(`{"loadbalancing":"hash"}`)}, false),
739+
},
740+
}
741+
Expect(messagesToEmit).To(MatchMessagesToEmit(expected))
742+
})
743+
})
744+
745+
Context("when options change and a hostname is removed in the same event", func() {
746+
BeforeEach(func() {
747+
routingInfo := cfroutes.CFRoutes{
748+
{
749+
Hostnames: []string{hostname1},
750+
Port: key.ContainerPort,
751+
Options: json.RawMessage(`{"loadbalancing":"round-robin"}`),
752+
},
753+
}.RoutingInfo()
754+
routes := models.Routes{}
755+
for k, v := range routingInfo {
756+
routes[k] = v
757+
}
758+
afterDesiredLRP := createDesiredLRPWithRoutes(key.ProcessGUID, 3, routes, logGuid, *newerTag, runInfo)
759+
_, messagesToEmit = table.SetRoutes(logger, desiredLRP, afterDesiredLRP)
760+
})
761+
762+
It("registers hostname1 with new options and unregisters hostname2 without a gap", func() {
763+
expected := routingtable.MessagesToEmit{
764+
RegistrationMessages: []routingtable.RegistryMessage{
765+
routingtable.RegistryMessageFor(endpoint1, routingtable.Route{Hostname: hostname1, LogGUID: logGuid, Options: json.RawMessage(`{"loadbalancing":"round-robin"}`)}, false),
766+
},
767+
UnregistrationMessages: []routingtable.RegistryMessage{
768+
routingtable.RegistryMessageFor(endpoint1, routingtable.Route{Hostname: hostname2, LogGUID: logGuid, Options: json.RawMessage(`{"loadbalancing":"hash"}`)}, false),
769+
},
770+
}
771+
Expect(messagesToEmit).To(MatchMessagesToEmit(expected))
772+
})
773+
})
774+
})
775+
653776
Context("when the routing key has an evacuating and instance endpoint", func() {
654777
BeforeEach(func() {
655778
tempTable := routingtable.NewRoutingTable(false, false, fakeMetronClient)

‎src/code.cloudfoundry.org/route-emitter/routingtable/routingtable.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,7 @@ func (t *internalRoutingTable) GetRoutingEvents() (TCPRouteMappings, MessagesToE
426426
type routeMapping interface {
427427
MessageFor(endpoint Endpoint, directInstanceAddress, emitEndpointUpdatedAt bool) (*RegistryMessage, *tcpmodels.TcpRouteMapping, *RegistryMessage)
428428
Hash() interface{}
429+
HashWithOptions() interface{}
429430
}
430431

431432
func httpRoutesFrom(lrp *models.DesiredLRP) map[RoutingKey][]routeMapping {
@@ -638,7 +639,8 @@ func diffRoutes(before, after []routeMapping) routesDiff {
638639
}
639640

640641
for routeHash := range newRoutes {
641-
if _, ok := existingRoutes[routeHash]; !ok {
642+
// Register routes that are new, or whose options changed.
643+
if _, ok := existingRoutes[routeHash]; !ok || existingRoutes[routeHash].HashWithOptions() != newRoutes[routeHash].HashWithOptions() {
642644
diff.added = append(diff.added, newRoutes[routeHash])
643645
}
644646
}

0 commit comments

Comments
 (0)