Skip to content

Commit

Permalink
feature: zone aware eskip.LBEndpoints
Browse files Browse the repository at this point in the history
Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>
  • Loading branch information
szuecs committed Jun 19, 2024
1 parent 5a13740 commit 5a062c1
Show file tree
Hide file tree
Showing 17 changed files with 120 additions and 66 deletions.
4 changes: 2 additions & 2 deletions dataclients/kubernetes/ingressv1.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func convertPathRuleV1(
r := &eskip.Route{
Id: routeID(ns, name, host, prule.Path, svcName),
BackendType: eskip.LBBackend,
LBEndpoints: eps,
LBEndpoints: eskip.NewLBEndpoints(eps),
LBAlgorithm: getLoadBalancerAlgorithm(metadata, defaultLoadBalancerAlgorithm),
HostRegexps: hostRegexp,
}
Expand Down Expand Up @@ -397,7 +397,7 @@ func (ing *ingress) convertDefaultBackendV1(
return &eskip.Route{
Id: routeID(ns, name, "", "", ""),
BackendType: eskip.LBBackend,
LBEndpoints: eps,
LBEndpoints: eskip.NewLBEndpoints(eps),
LBAlgorithm: getLoadBalancerAlgorithm(i.Metadata, ing.defaultLoadBalancerAlgorithm),
}, true, nil
}
Expand Down
4 changes: 2 additions & 2 deletions dataclients/kubernetes/routegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func applyServiceBackend(ctx *routeGroupContext, backend *definitions.SkipperBac
}

r.BackendType = eskip.LBBackend
r.LBEndpoints = eps
r.LBEndpoints = eskip.NewLBEndpoints(eps)
r.LBAlgorithm = ctx.defaultLoadBalancerAlgorithm
if backend.Algorithm != loadbalancer.None {
r.LBAlgorithm = backend.Algorithm.String()
Expand Down Expand Up @@ -261,7 +261,7 @@ func applyBackend(ctx *routeGroupContext, backend *definitions.SkipperBackend, r
}
}

r.LBEndpoints = backend.Endpoints
r.LBEndpoints = eskip.NewLBEndpoints(backend.Endpoints)
r.LBAlgorithm = ctx.defaultLoadBalancerAlgorithm
if backend.Algorithm != loadbalancer.None {
r.LBAlgorithm = backend.Algorithm.String()
Expand Down
2 changes: 1 addition & 1 deletion eskip/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func Copy(r *Route) *Route {
c.BackendType = r.BackendType
c.Backend = r.Backend
c.LBAlgorithm = r.LBAlgorithm
c.LBEndpoints = make([]string, len(r.LBEndpoints))
c.LBEndpoints = make([]*LBEndpoint, len(r.LBEndpoints))
copy(c.LBEndpoints, r.LBEndpoints)
return c
}
Expand Down
22 changes: 17 additions & 5 deletions eskip/copy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestCopy(t *testing.T) {
checkFilter(t, c.Filters[i], r.Filters[i])
}

r.LBEndpoints[0] = "test-slice-identity"
r.LBEndpoints[0] = &LBEndpoint{Address: "test-slice-identity"}
if c.LBEndpoints[0] == r.LBEndpoints[0] {
t.Error("failed to copy LB endpoints")
}
Expand Down Expand Up @@ -132,7 +132,10 @@ func TestCopy(t *testing.T) {
},
BackendType: LBBackend,
LBAlgorithm: "roundRobin",
LBEndpoints: []string{"10.0.0.1:80", "10.0.0.2:80"},
LBEndpoints: []*LBEndpoint{
{Address: "10.0.0.1:80"},
{Address: "10.0.0.2:80"},
},
}

c := Copy(r)
Expand All @@ -154,7 +157,10 @@ func TestCopy(t *testing.T) {
},
BackendType: LBBackend,
LBAlgorithm: "roundRobin",
LBEndpoints: []string{"10.0.1.1:80", "10.0.1.2:80"},
LBEndpoints: []*LBEndpoint{
{Address: "10.0.1.1:80"},
{Address: "10.0.1.2:80"},
},
}, {
Id: "route2",
Predicates: []*Predicate{
Expand All @@ -169,7 +175,10 @@ func TestCopy(t *testing.T) {
},
BackendType: LBBackend,
LBAlgorithm: "roundRobin",
LBEndpoints: []string{"10.0.2.1:80", "10.0.2.2:80"},
LBEndpoints: []*LBEndpoint{
{Address: "10.0.2.1:80"},
{Address: "10.0.2.2:80"},
},
}, {
Id: "route3",
Predicates: []*Predicate{
Expand All @@ -184,7 +193,10 @@ func TestCopy(t *testing.T) {
},
BackendType: LBBackend,
LBAlgorithm: "roundRobin",
LBEndpoints: []string{"10.0.3.1:80", "10.0.3.2:80"},
LBEndpoints: []*LBEndpoint{
{Address: "10.0.3.1:80"},
{Address: "10.0.3.2:80"},
},
}}

c := CopyRoutes(r)
Expand Down
12 changes: 7 additions & 5 deletions eskip/eq.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ func eqArgs(left, right []interface{}) bool {
return true
}

func eqStrings(left, right []string) bool {
func eqLBEndpoints(left, right []*LBEndpoint) bool {
if len(left) != len(right) {
return false
}

for i := range left {
if left[i] != right[i] {
if left[i].Address != right[i].Address && left[i].Zone != right[i].Zone {
return false
}
}
Expand Down Expand Up @@ -103,7 +103,7 @@ func eq2(left, right *Route) bool {
return false
}

if !eqStrings(lc.LBEndpoints, rc.LBEndpoints) {
if !eqLBEndpoints(lc.LBEndpoints, rc.LBEndpoints) {
return false
}

Expand Down Expand Up @@ -255,9 +255,11 @@ func Canonical(r *Route) *Route {
case LBBackend:
// using the LB fields only when apply:
c.LBAlgorithm = r.LBAlgorithm
c.LBEndpoints = make([]string, len(r.LBEndpoints))
c.LBEndpoints = make([]*LBEndpoint, len(r.LBEndpoints))
copy(c.LBEndpoints, r.LBEndpoints)
sort.Strings(c.LBEndpoints)
sort.Slice(c.LBEndpoints, func(i, j int) bool {
return c.LBEndpoints[i].Address < c.LBEndpoints[j].Address
})
}

// Name and Namespace stripped
Expand Down
14 changes: 7 additions & 7 deletions eskip/eq_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,14 @@ func TestEq(t *testing.T) {
}, {
title: "non-eq lb endpoint count",
routes: []*Route{
{BackendType: LBBackend, LBEndpoints: []string{"", ""}},
{BackendType: LBBackend, LBEndpoints: []string{""}},
{BackendType: LBBackend, LBEndpoints: []*LBEndpoint{nil, nil}},
{BackendType: LBBackend, LBEndpoints: []*LBEndpoint{nil}},
},
}, {
title: "non-eq lb endpoints",
routes: []*Route{
{BackendType: LBBackend, LBEndpoints: []string{"https://one.example.org"}},
{BackendType: LBBackend, LBEndpoints: []string{"https://two.example.org"}},
{BackendType: LBBackend, LBEndpoints: []*LBEndpoint{{Address: "https://one.example.org"}}},
{BackendType: LBBackend, LBEndpoints: []*LBEndpoint{{Address: "https://two.example.org"}}},
},
}, {
title: "all eq",
Expand All @@ -116,14 +116,14 @@ func TestEq(t *testing.T) {
Filters: []*Filter{{Name: "foo", Args: []interface{}{3, 4}}},
BackendType: LBBackend,
LBAlgorithm: "random",
LBEndpoints: []string{"https://one.example.org", "https://two.example.org"},
LBEndpoints: []*LBEndpoint{{Address: "https://one.example.org"}, {Address: "https://two.example.org"}},
}, {
Id: "foo",
Predicates: []*Predicate{{Name: "Foo", Args: []interface{}{1, 2}}},
Filters: []*Filter{{Name: "foo", Args: []interface{}{3, 4}}},
BackendType: LBBackend,
LBAlgorithm: "random",
LBEndpoints: []string{"https://one.example.org", "https://two.example.org"},
LBEndpoints: []*LBEndpoint{{Address: "https://one.example.org"}, {Address: "https://two.example.org"}},
}},
expect: true,
}, {
Expand Down Expand Up @@ -282,7 +282,7 @@ func TestCanonical(t *testing.T) {
expect: &Route{BackendType: NetworkBackend, Backend: "https://www.example.org"},
}, {
title: "clear LB when different type",
route: &Route{LBEndpoints: []string{"https://one.example.org", "https://two.example.org"}},
route: &Route{LBEndpoints: []*LBEndpoint{{Address: "https://one.example.org"}, {Address: "https://two.example.org"}}},
expect: &Route{},
}} {
t.Run(test.title, func(t *testing.T) {
Expand Down
43 changes: 40 additions & 3 deletions eskip/eskip.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ type Route struct {

// LBEndpoints stores one or more backend endpoint in case of
// load balancing backends.
LBEndpoints []string
LBEndpoints []*LBEndpoint

// Name is deprecated and not used.
Name string
Expand All @@ -330,6 +330,43 @@ type Route struct {
Namespace string
}

func NewLBEndpoints(eps []string) []*LBEndpoint {
if len(eps) == 0 {
return nil
}
result := make([]*LBEndpoint, len(eps))

for i := 0; i < len(eps); i++ {
result[i] = &LBEndpoint{
Address: eps[i],
}
}

return result
}

func LBEndpointString(eps []*LBEndpoint) []string {
if len(eps) == 0 {
return nil
}
result := make([]string, len(eps))

for i := 0; i < len(eps); i++ {
result[i] = eps[i].String()
}

return result
}

type LBEndpoint struct {
Address string
Zone string
}

func (ep LBEndpoint) String() string {
return ep.Address
}

type RoutePredicate func(*Route) bool

// RouteInfo contains a route id, plus the loaded and parsed route or
Expand Down Expand Up @@ -402,7 +439,7 @@ func (r *Route) Copy() *Route {
}

if len(r.LBEndpoints) > 0 {
c.LBEndpoints = make([]string, len(r.LBEndpoints))
c.LBEndpoints = make([]*LBEndpoint, len(r.LBEndpoints))
copy(c.LBEndpoints, r.LBEndpoints)
}

Expand Down Expand Up @@ -561,7 +598,7 @@ func newRouteDefinition(r *parsedRoute) (*Route, error) {
rd.Shunt = r.shunt
rd.Backend = r.backend
rd.LBAlgorithm = r.lbAlgorithm
rd.LBEndpoints = r.lbEndpoints
rd.LBEndpoints = NewLBEndpoints(r.lbEndpoints)

switch {
case r.shunt:
Expand Down
4 changes: 2 additions & 2 deletions eskip/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func newJSONRoute(r *Route) *jsonRoute {
Type: cr.BackendType.String(),
Address: cr.Backend,
Algorithm: cr.LBAlgorithm,
Endpoints: cr.LBEndpoints,
Endpoints: LBEndpointString(cr.LBEndpoints),
}
}

Expand Down Expand Up @@ -94,7 +94,7 @@ func (r *Route) UnmarshalJSON(b []byte) error {
}
case LBBackend:
r.LBAlgorithm = jr.Backend.Algorithm
r.LBEndpoints = jr.Backend.Endpoints
r.LBEndpoints = NewLBEndpoints(jr.Backend.Endpoints)
if len(r.LBEndpoints) == 0 {
r.LBEndpoints = nil
}
Expand Down
2 changes: 1 addition & 1 deletion eskip/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestMarshalUnmarshalJSON(t *testing.T) {
},
{
"lb backend",
[]*Route{{Id: "beef", BackendType: LBBackend, LBAlgorithm: "yolo", LBEndpoints: []string{"localhost"}}},
[]*Route{{Id: "beef", BackendType: LBBackend, LBAlgorithm: "yolo", LBEndpoints: []*LBEndpoint{{Address: "localhost"}}}},
`[{"id":"beef","backend":{"type":"lb","algorithm":"yolo","endpoints":["localhost"]}}]`,
},
{
Expand Down
28 changes: 14 additions & 14 deletions eskip/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func TestLBBackend(t *testing.T) {
code: `* -> <"https://example.org">`,
expectedResult: []*Route{{
BackendType: LBBackend,
LBEndpoints: []string{"https://example.org"},
LBEndpoints: []*LBEndpoint{{Address: "https://example.org"}},
}},
}, {
title: "multiple endpoints, default algorithm",
Expand All @@ -264,10 +264,10 @@ func TestLBBackend(t *testing.T) {
"https://example3.org">`,
expectedResult: []*Route{{
BackendType: LBBackend,
LBEndpoints: []string{
"https://example1.org",
"https://example2.org",
"https://example3.org",
LBEndpoints: []*LBEndpoint{
{Address: "https://example1.org"},
{Address: "https://example2.org"},
{Address: "https://example3.org"},
},
}},
}, {
Expand All @@ -276,7 +276,7 @@ func TestLBBackend(t *testing.T) {
expectedResult: []*Route{{
BackendType: LBBackend,
LBAlgorithm: "algFoo",
LBEndpoints: []string{"https://example.org"},
LBEndpoints: []*LBEndpoint{{Address: "https://example.org"}},
}},
}, {
title: "multiple endpoints, default algorithm",
Expand All @@ -287,10 +287,10 @@ func TestLBBackend(t *testing.T) {
expectedResult: []*Route{{
BackendType: LBBackend,
LBAlgorithm: "algFoo",
LBEndpoints: []string{
"https://example1.org",
"https://example2.org",
"https://example3.org",
LBEndpoints: []*LBEndpoint{
{Address: "https://example1.org"},
{Address: "https://example2.org"},
{Address: "https://example3.org"},
},
}},
}, {
Expand All @@ -303,10 +303,10 @@ func TestLBBackend(t *testing.T) {
Filters: []*Filter{{Name: "foo"}},
BackendType: LBBackend,
LBAlgorithm: "algFoo",
LBEndpoints: []string{
"https://example1.org",
"https://example2.org",
"https://example3.org",
LBEndpoints: []*LBEndpoint{
{Address: "https://example1.org"},
{Address: "https://example2.org"},
{Address: "https://example3.org"},
},
}},
}} {
Expand Down
2 changes: 1 addition & 1 deletion eskip/string.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func lbBackendString(r *Route) string {
b.WriteString(", ")
}
b.WriteByte('"')
b.WriteString(ep)
b.WriteString(ep.String())
b.WriteByte('"')
}
b.WriteByte('>')
Expand Down
9 changes: 6 additions & 3 deletions eskip/string_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,16 @@ func TestRouteString(t *testing.T) {
BackendType: DynamicBackend},
`* -> filter0("Line 1\r\nLine 2") -> <dynamic>`,
}, {
&Route{Method: "GET", BackendType: LBBackend, LBEndpoints: []string{"http://127.0.0.1:9997"}},
&Route{Method: "GET", BackendType: LBBackend, LBEndpoints: []*LBEndpoint{{Address: "http://127.0.0.1:9997"}}},
`Method("GET") -> <"http://127.0.0.1:9997">`,
}, {
&Route{Method: "GET", LBAlgorithm: "random", BackendType: LBBackend, LBEndpoints: []string{"http://127.0.0.1:9997"}},
&Route{Method: "GET", LBAlgorithm: "random", BackendType: LBBackend, LBEndpoints: []*LBEndpoint{{Address: "http://127.0.0.1:9997"}}},
`Method("GET") -> <random, "http://127.0.0.1:9997">`,
}, {
&Route{Method: "GET", LBAlgorithm: "random", BackendType: LBBackend, LBEndpoints: []string{"http://127.0.0.1:9997", "http://127.0.0.1:9998"}},
&Route{Method: "GET", LBAlgorithm: "random", BackendType: LBBackend, LBEndpoints: []*LBEndpoint{
{Address: "http://127.0.0.1:9997"},
{Address: "http://127.0.0.1:9998"},
}},
`Method("GET") -> <random, "http://127.0.0.1:9997", "http://127.0.0.1:9998">`,
}, {
// test slash escaping
Expand Down
4 changes: 2 additions & 2 deletions loadbalancer/algorithm.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func (a Algorithm) String() string {
func parseEndpoints(r *routing.Route) error {
r.LBEndpoints = make([]routing.LBEndpoint, len(r.Route.LBEndpoints))
for i, e := range r.Route.LBEndpoints {
scheme, host, err := snet.SchemeHost(e)
scheme, host, err := snet.SchemeHost(e.String())
if err != nil {
return err
}
Expand All @@ -337,7 +337,7 @@ func setAlgorithm(r *routing.Route) error {
initialize = algorithms[t]
}

r.LBAlgorithm = initialize(r.Route.LBEndpoints)
r.LBAlgorithm = initialize(eskip.LBEndpointString(r.Route.LBEndpoints))
return nil
}

Expand Down
Loading

0 comments on commit 5a062c1

Please sign in to comment.