Skip to content

Commit aad81e1

Browse files
committed
Skip health checks when there is one server that backs the route
Previously we had attempted to suppress the health checks, but it was looking at the endpoints of the service, not the servers that back the route. The problem is that if we just look at endpoints, then if a route has two backing services, each with one endpoint, we would incorrectly suppress the health checks.
1 parent 1d0ad23 commit aad81e1

File tree

2 files changed

+178
-6
lines changed

2 files changed

+178
-6
lines changed

pkg/router/template/template_helper.go

+12-6
Original file line numberDiff line numberDiff line change
@@ -157,16 +157,22 @@ func processEndpointsForAlias(alias ServiceAliasConfig, svc ServiceUnit, action
157157
}
158158

159159
func endpointsForAlias(alias ServiceAliasConfig, svc ServiceUnit) []Endpoint {
160-
if len(alias.PreferPort) == 0 {
161-
return svc.EndpointTable
162-
}
163160
endpoints := make([]Endpoint, 0, len(svc.EndpointTable))
164-
for i := range svc.EndpointTable {
165-
endpoint := svc.EndpointTable[i]
166-
if endpoint.PortName == alias.PreferPort || endpoint.Port == alias.PreferPort {
161+
for _, endpoint := range svc.EndpointTable {
162+
// Filter the list:
163+
// - If PreferPort length is 0, match everything
164+
// - Otherwise, if the PortName or Port matches PreferPort, then we have a match
165+
if len(alias.PreferPort) == 0 || endpoint.PortName == alias.PreferPort || endpoint.Port == alias.PreferPort {
167166
endpoints = append(endpoints, endpoint)
168167
}
169168
}
169+
170+
// We want to disable endpoint checks if there is only one endpoint since there's no point in
171+
// testing and removing it from the set ahead of time since there are no other eps to replace it.
172+
if len(endpoints) == 1 {
173+
endpoints[0].NoHealthCheck = true
174+
}
175+
170176
return endpoints
171177
}
172178

pkg/router/template/template_helper_test.go

+166
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package templaterouter
22

33
import (
4+
"reflect"
45
"regexp"
56
"testing"
67
)
@@ -297,3 +298,168 @@ func TestMatchPattern(t *testing.T) {
297298
}
298299
}
299300
}
301+
302+
func TestEndpointsForAlias(t *testing.T) {
303+
testEndpoints := []struct {
304+
name string
305+
preferPort string
306+
eps []Endpoint
307+
expected []Endpoint
308+
}{
309+
{
310+
name: "Test no preference with a single endpoint",
311+
preferPort: "",
312+
eps: []Endpoint{
313+
{
314+
ID: "ep1",
315+
Port: "80",
316+
PortName: "http",
317+
},
318+
},
319+
expected: []Endpoint{
320+
{
321+
ID: "ep1",
322+
Port: "80",
323+
PortName: "http",
324+
NoHealthCheck: true,
325+
},
326+
},
327+
},
328+
{
329+
name: "Test no preference with multiple",
330+
preferPort: "",
331+
eps: []Endpoint{
332+
{
333+
ID: "ep1",
334+
Port: "80",
335+
PortName: "http",
336+
},
337+
{
338+
ID: "ep1",
339+
Port: "443",
340+
PortName: "https",
341+
},
342+
{
343+
ID: "ep2",
344+
Port: "80",
345+
PortName: "http",
346+
},
347+
{
348+
ID: "ep3",
349+
Port: "80",
350+
PortName: "http",
351+
},
352+
},
353+
expected: []Endpoint{
354+
{
355+
ID: "ep1",
356+
Port: "80",
357+
PortName: "http",
358+
},
359+
{
360+
ID: "ep1",
361+
Port: "443",
362+
PortName: "https",
363+
},
364+
{
365+
ID: "ep2",
366+
Port: "80",
367+
PortName: "http",
368+
},
369+
{
370+
ID: "ep3",
371+
Port: "80",
372+
PortName: "http",
373+
},
374+
},
375+
},
376+
{
377+
name: "Test a preference resulting in multiple",
378+
preferPort: "http",
379+
eps: []Endpoint{
380+
{
381+
ID: "ep1",
382+
Port: "80",
383+
PortName: "http",
384+
},
385+
{
386+
ID: "ep1",
387+
Port: "443",
388+
PortName: "https",
389+
},
390+
{
391+
ID: "ep2",
392+
Port: "80",
393+
PortName: "http",
394+
},
395+
{
396+
ID: "ep3",
397+
Port: "80",
398+
PortName: "http",
399+
},
400+
},
401+
expected: []Endpoint{
402+
{
403+
ID: "ep1",
404+
Port: "80",
405+
PortName: "http",
406+
},
407+
{
408+
ID: "ep2",
409+
Port: "80",
410+
PortName: "http",
411+
},
412+
{
413+
ID: "ep3",
414+
Port: "80",
415+
PortName: "http",
416+
},
417+
},
418+
},
419+
{
420+
name: "Test a preference resulting in one",
421+
preferPort: "https",
422+
eps: []Endpoint{
423+
{
424+
ID: "ep1",
425+
Port: "80",
426+
PortName: "http",
427+
},
428+
{
429+
ID: "ep1",
430+
Port: "443",
431+
PortName: "https",
432+
},
433+
{
434+
ID: "ep2",
435+
Port: "80",
436+
PortName: "http",
437+
},
438+
{
439+
ID: "ep3",
440+
Port: "80",
441+
PortName: "http",
442+
},
443+
},
444+
expected: []Endpoint{
445+
{
446+
ID: "ep1",
447+
Port: "443",
448+
PortName: "https",
449+
NoHealthCheck: true,
450+
},
451+
},
452+
},
453+
}
454+
455+
for _, te := range testEndpoints {
456+
eps := endpointsForAlias(
457+
ServiceAliasConfig{PreferPort: te.preferPort},
458+
ServiceUnit{EndpointTable: te.eps},
459+
)
460+
461+
if !reflect.DeepEqual(te.expected, eps) {
462+
t.Errorf("%s: expected result: %#v, got: %#v", te.name, te.expected, eps)
463+
}
464+
}
465+
}

0 commit comments

Comments
 (0)