Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the "supress health checks when only one backing service" logic #17077

Merged
merged 3 commits into from
Oct 31, 2017
Merged

Fix the "supress health checks when only one backing service" logic #17077

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d0ad23
Revert "Made the router skip health checks when there is one endpoint"
knobunc Oct 26, 2017
712a22a
Fixed a TODO comment by reviewing the code
knobunc Oct 30, 2017
9835dca
Skip health checks when there is one server that backs the route
knobunc Oct 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions pkg/router/template/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,6 @@ func createRouterEndpoints(endpoints *kapi.Endpoints, excludeUDP bool, lookupSvc
wasIdled = true
}

// Take a reasonable guess at the number of endpoints to avoid reallocating the out array when we append
out := make([]Endpoint, 0, len(endpoints.Subsets)*4)

// Now build the actual endpoints we pass to the template
Expand Down Expand Up @@ -307,11 +306,5 @@ func createRouterEndpoints(endpoints *kapi.Endpoints, excludeUDP bool, lookupSvc
}
}

// We want to disable endpoint checks if there is only one endpoint
// We skip the case where it is idled, since we set NoHealthCheck above
if wasIdled == false && len(out) == 1 {
out[0].NoHealthCheck = true
}

return out
}
153 changes: 9 additions & 144 deletions pkg/router/template/plugin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,16 +275,15 @@ func TestHandleEndpoints(t *testing.T) {
Name: "foo/test", //service name from kapi.endpoints object
EndpointTable: []Endpoint{
{
ID: "ept:test:1.1.1.1:345",
IP: "1.1.1.1",
Port: "345",
NoHealthCheck: true,
ID: "ept:test:1.1.1.1:345",
IP: "1.1.1.1",
Port: "345",
},
},
},
},
{
name: "Endpoint mod (one ep, one address)",
name: "Endpoint mod",
eventType: watch.Modified,
endpoints: &kapi.Endpoints{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -300,143 +299,9 @@ func TestHandleEndpoints(t *testing.T) {
Name: "foo/test",
EndpointTable: []Endpoint{
{
ID: "pod:pod-1:test:2.2.2.2:8080",
IP: "2.2.2.2",
Port: "8080",
NoHealthCheck: true,
},
},
},
},
{
name: "Endpoint mod (second ep, one address each)",
eventType: watch.Modified,
endpoints: &kapi.Endpoints{
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "test",
},
Subsets: []kapi.EndpointSubset{
{
Addresses: []kapi.EndpointAddress{{IP: "2.2.2.2", TargetRef: &kapi.ObjectReference{Kind: "Pod", Name: "pod-1"}}},
Ports: []kapi.EndpointPort{{Port: 8080}},
},
{
Addresses: []kapi.EndpointAddress{{IP: "3.3.3.3", TargetRef: &kapi.ObjectReference{Kind: "Pod", Name: "pod-2"}}},
Ports: []kapi.EndpointPort{{Port: 8081}},
},
},
},
expectedServiceUnit: &ServiceUnit{
Name: "foo/test",
EndpointTable: []Endpoint{
{
ID: "pod:pod-1:test:2.2.2.2:8080",
IP: "2.2.2.2",
Port: "8080",
NoHealthCheck: false,
},
{
ID: "pod:pod-2:test:3.3.3.3:8081",
IP: "3.3.3.3",
Port: "8081",
NoHealthCheck: false,
},
},
},
},
{
name: "Endpoint mod (one ep, two addresses)",
eventType: watch.Modified,
endpoints: &kapi.Endpoints{
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "test",
},
Subsets: []kapi.EndpointSubset{
{
Addresses: []kapi.EndpointAddress{
{IP: "3.3.3.3", TargetRef: &kapi.ObjectReference{Kind: "Pod", Name: "pod-2"}},
{IP: "4.4.4.4", TargetRef: &kapi.ObjectReference{Kind: "Pod", Name: "pod-3"}},
},
Ports: []kapi.EndpointPort{{Port: 8080}},
},
},
},
expectedServiceUnit: &ServiceUnit{
Name: "foo/test",
EndpointTable: []Endpoint{
{
ID: "pod:pod-2:test:3.3.3.3:8080",
IP: "3.3.3.3",
Port: "8080",
NoHealthCheck: false,
},
{
ID: "pod:pod-3:test:4.4.4.4:8080",
IP: "4.4.4.4",
Port: "8080",
NoHealthCheck: false,
},
},
},
},
{
name: "Endpoint mod (one ep, two ports)",
eventType: watch.Modified,
endpoints: &kapi.Endpoints{
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "test",
},
Subsets: []kapi.EndpointSubset{
{
Addresses: []kapi.EndpointAddress{
{IP: "3.3.3.3", TargetRef: &kapi.ObjectReference{Kind: "Pod", Name: "pod-2"}},
},
Ports: []kapi.EndpointPort{{Port: 8080}, {Port: 8081}},
},
},
},
expectedServiceUnit: &ServiceUnit{
Name: "foo/test",
EndpointTable: []Endpoint{
{
ID: "pod:pod-2:test:3.3.3.3:8080",
IP: "3.3.3.3",
Port: "8080",
NoHealthCheck: false,
},
{
ID: "pod:pod-2:test:3.3.3.3:8081",
IP: "3.3.3.3",
Port: "8081",
NoHealthCheck: false,
},
},
},
},
{
name: "Endpoint mod (back to one ep)",
eventType: watch.Modified,
endpoints: &kapi.Endpoints{
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "test",
},
Subsets: []kapi.EndpointSubset{{
Addresses: []kapi.EndpointAddress{{IP: "3.3.3.3", TargetRef: &kapi.ObjectReference{Kind: "Pod", Name: "pod-1"}}},
Ports: []kapi.EndpointPort{{Port: 8080}},
}},
},
expectedServiceUnit: &ServiceUnit{
Name: "foo/test",
EndpointTable: []Endpoint{
{
ID: "pod:pod-1:test:3.3.3.3:8080",
IP: "3.3.3.3",
Port: "8080",
NoHealthCheck: true,
ID: "pod:pod-1:test:2.2.2.2:8080",
IP: "2.2.2.2",
Port: "8080",
},
},
},
Expand All @@ -450,8 +315,8 @@ func TestHandleEndpoints(t *testing.T) {
Name: "test",
},
Subsets: []kapi.EndpointSubset{{
Addresses: []kapi.EndpointAddress{{IP: "3.3.3.3", TargetRef: &kapi.ObjectReference{Kind: "Pod", Name: "pod-1"}}},
Ports: []kapi.EndpointPort{{Port: 8080}},
Addresses: []kapi.EndpointAddress{{IP: "3.3.3.3"}},
Ports: []kapi.EndpointPort{{Port: 0}},
}},
},
expectedServiceUnit: &ServiceUnit{
Expand Down
20 changes: 16 additions & 4 deletions pkg/router/template/template_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,16 +157,28 @@ func processEndpointsForAlias(alias ServiceAliasConfig, svc ServiceUnit, action
}

func endpointsForAlias(alias ServiceAliasConfig, svc ServiceUnit) []Endpoint {
if len(alias.PreferPort) == 0 {
if len(alias.PreferPort) == 0 && len(svc.EndpointTable) != 1 {
// We can skip the work. They are selecting everything, and since they don't have one endpoint,
// we can't disable the health checks. So we can just use their list.
return svc.EndpointTable
}

endpoints := make([]Endpoint, 0, len(svc.EndpointTable))
for i := range svc.EndpointTable {
endpoint := svc.EndpointTable[i]
if endpoint.PortName == alias.PreferPort || endpoint.Port == alias.PreferPort {
for _, endpoint := range svc.EndpointTable {
// Filter the list:
// - If PreferPort length is 0, match everything
// - Otherwise, if the PortName or Port matches PreferPort, then we have a match
if len(alias.PreferPort) == 0 || endpoint.PortName == alias.PreferPort || endpoint.Port == alias.PreferPort {
endpoints = append(endpoints, endpoint)
}
}

// We want to disable endpoint checks if there is only one endpoint since there's no point in
// testing and removing it from the set ahead of time since there are no other eps to replace it.
if len(endpoints) == 1 {
endpoints[0].NoHealthCheck = true
}

return endpoints
}

Expand Down
Loading