Skip to content

Commit

Permalink
Issue #385: attach options to target instead of route
Browse files Browse the repository at this point in the history
When defining a route for a given target the options were stored on the
Route object which had the effect that the first route definition
defined the route options for all instances. This was done under the
assumption that the options for a route would be the same for all
targets.

However, it should be possible to define different options for different
targets of the same route for example during a migration from one
service to another. This patch enables that behavior.

Fixes #385
  • Loading branch information
magiconair committed Nov 28, 2017
1 parent ff58485 commit c8518c0
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 31 deletions.
10 changes: 5 additions & 5 deletions admin/api/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ func (h *RoutesHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
var routes []apiRoute
for _, host := range hosts {
for _, tr := range t[host] {
var opts []string
for k, v := range tr.Opts {
opts = append(opts, k+"="+v)
}

for _, tg := range tr.Targets {
var opts []string
for k, v := range tg.Opts {
opts = append(opts, k+"="+v)
}

ar := apiRoute{
Service: tg.Service,
Host: tr.Host,
Expand Down
33 changes: 27 additions & 6 deletions proxy/http_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,20 @@ func TestProxyStripsPath(t *testing.T) {
}
}

// TestProxyHost
func TestProxyHost(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprint(w, r.Host)
}))

// create a static route table so that we can see the effect
// of round robin distribution. The other tests generate the
// route table on the fly since order does not matter to them.
routes := "route add mock /hostdst http://a.com/ opts \"host=dst\"\n"
routes += "route add mock /hostcustom http://a.com/ opts \"host=foo.com\"\n"
routes += "route add mock /hostcustom http://b.com/ opts \"host=bar.com\"\n"
routes += "route add mock / http://a.com/"
tbl, _ := route.NewTable(routes)

proxy := httptest.NewServer(&HTTPProxy{
Transport: &http.Transport{
Dial: func(network, addr string) (net.Conn, error) {
Expand All @@ -131,10 +139,6 @@ func TestProxyHost(t *testing.T) {
},
},
Lookup: func(r *http.Request) *route.Target {
routes := "route add mock /hostdst http://a.com/ opts \"host=dst\"\n"
routes += "route add mock /hostcustom http://a.com/ opts \"host=foo.com\"\n"
routes += "route add mock / http://a.com/"
tbl, _ := route.NewTable(routes)
return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"])
},
})
Expand All @@ -151,9 +155,26 @@ func TestProxyHost(t *testing.T) {
}

proxyHost := proxy.URL[len("http://"):]

// test that for 'host=dst' the Host header is set to the hostname of the
// target, in this case 'a.com'
t.Run("host eq dst", func(t *testing.T) { check(t, "/hostdst", "a.com") })
t.Run("host is custom", func(t *testing.T) { check(t, "/hostcustom", "foo.com") })

// test that without a 'host' option no Host header is set
t.Run("no host", func(t *testing.T) { check(t, "/", proxyHost) })

// 1. Test that a host header is set when the 'host' option is used.
//
// 2. Test that the host header is set per target, i.e. that different
// targets can have different 'host' options.
//
// The proxy is configured to use "rr" (round-robin) distribution
// for the requests. Therefore, requests to '/hostcustom' will be
// sent to the two different targets in alternating order.
t.Run("host is custom", func(t *testing.T) {
check(t, "/hostcustom", "foo.com")
check(t, "/hostcustom", "bar.com")
})
}

func TestProxyLogOutput(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions route/picker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ func mustParse(rawurl string) *url.URL {

func TestRndPicker(t *testing.T) {
r := &Route{Host: "www.bar.com", Path: "/foo"}
r.addTarget("svc", fooDotCom, 0, nil)
r.addTarget("svc", barDotCom, 0, nil)
r.addTarget("svc", fooDotCom, 0, nil, nil)
r.addTarget("svc", barDotCom, 0, nil, nil)

tests := []struct {
rnd int
Expand All @@ -45,8 +45,8 @@ func TestRndPicker(t *testing.T) {

func TestRRPicker(t *testing.T) {
r := &Route{Host: "www.bar.com", Path: "/foo"}
r.addTarget("svc", fooDotCom, 0, nil)
r.addTarget("svc", barDotCom, 0, nil)
r.addTarget("svc", fooDotCom, 0, nil, nil)
r.addTarget("svc", barDotCom, 0, nil, nil)

tests := []*url.URL{fooDotCom, barDotCom, fooDotCom, barDotCom, fooDotCom, barDotCom}

Expand Down
20 changes: 9 additions & 11 deletions route/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ type Route struct {
// Path is the path prefix from a request uri
Path string

// Opts is the raw route options
Opts map[string]string

// Targets contains the list of URLs
Targets []*Target

Expand All @@ -40,7 +37,7 @@ type Route struct {
total uint64
}

func (r *Route) addTarget(service string, targetURL *url.URL, fixedWeight float64, tags []string) {
func (r *Route) addTarget(service string, targetURL *url.URL, fixedWeight float64, tags []string, opts map[string]string) {
if fixedWeight < 0 {
fixedWeight = 0
}
Expand All @@ -61,15 +58,16 @@ func (r *Route) addTarget(service string, targetURL *url.URL, fixedWeight float6
t := &Target{
Service: service,
Tags: tags,
Opts: opts,
URL: targetURL,
FixedWeight: fixedWeight,
Timer: ServiceRegistry.GetTimer(name),
TimerName: name,
}
if r.Opts != nil {
t.StripPath = r.Opts["strip"]
t.TLSSkipVerify = r.Opts["tlsskipverify"] == "true"
t.Host = r.Opts["host"]
if opts != nil {
t.StripPath = opts["strip"]
t.TLSSkipVerify = opts["tlsskipverify"] == "true"
t.Host = opts["host"]
}

r.Targets = append(r.Targets, t)
Expand Down Expand Up @@ -145,16 +143,16 @@ func (r *Route) TargetConfig(t *Target, addWeight bool) string {
if len(t.Tags) > 0 {
s += fmt.Sprintf(" tags %q", strings.Join(t.Tags, ","))
}
if len(r.Opts) > 0 {
if len(t.Opts) > 0 {
var keys []string
for k := range r.Opts {
for k := range t.Opts {
keys = append(keys, k)
}
sort.Strings(keys)

var vals []string
for _, k := range keys {
vals = append(vals, k+"="+r.Opts[k])
vals = append(vals, k+"="+t.Opts[k])
}
s += fmt.Sprintf(" opts \"%s\"", strings.Join(vals, " "))
}
Expand Down
10 changes: 5 additions & 5 deletions route/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,20 +153,20 @@ func (t Table) addRoute(d *RouteDef) error {
switch {
// add new host
case t[host] == nil:
r := &Route{Host: host, Path: path, Opts: d.Opts}
r.addTarget(d.Service, targetURL, d.Weight, d.Tags)
r := &Route{Host: host, Path: path}
r.addTarget(d.Service, targetURL, d.Weight, d.Tags, d.Opts)
t[host] = Routes{r}

// add new route to existing host
case t[host].find(path) == nil:
r := &Route{Host: host, Path: path, Opts: d.Opts}
r.addTarget(d.Service, targetURL, d.Weight, d.Tags)
r := &Route{Host: host, Path: path}
r.addTarget(d.Service, targetURL, d.Weight, d.Tags, d.Opts)
t[host] = append(t[host], r)
sort.Sort(t[host])

// add new target to existing route
default:
t[host].find(path).addTarget(d.Service, targetURL, d.Weight, d.Tags)
t[host].find(path).addTarget(d.Service, targetURL, d.Weight, d.Tags, d.Opts)
}

return nil
Expand Down
22 changes: 22 additions & 0 deletions route/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,28 @@ func TestTableParse(t *testing.T) {
},
},

{"1 service, 1 prefix with option",
[]string{
`route add svc-a / http://aaa.com/ opts "strip=/foo"`,
`route add svc-b / http://bbb.com/ opts "strip=/bar"`,
},
[]string{
`route add svc-a / http://aaa.com/ weight 0.5000 opts "strip=/foo"`,
`route add svc-b / http://bbb.com/ weight 0.5000 opts "strip=/bar"`,
},
},

{"1 service, 1 prefix, 2 instances with different options",
[]string{
`route add svc-a / http://aaa.com/ opts "strip=/foo"`,
`route add svc-b / http://bbb.com/ opts "strip=/bar"`,
},
[]string{
`route add svc-a / http://aaa.com/ weight 0.5000 opts "strip=/foo"`,
`route add svc-b / http://bbb.com/ weight 0.5000 opts "strip=/bar"`,
},
},

{"2 service, 1 prefix",
[]string{
`route add svc-a / http://aaa.com/`,
Expand Down
3 changes: 3 additions & 0 deletions route/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ type Target struct {
// Tags are the list of tags for this target
Tags []string

// Opts is the raw options for the target.
Opts map[string]string

// StripPath will be removed from the front of the outgoing
// request path
StripPath string
Expand Down

0 comments on commit c8518c0

Please sign in to comment.