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

Jump to bottom

Abort Kubernetes Ingress update if Kubernetes API call fails #1295

Merged
merged 10 commits into from
Mar 17, 2017
Merged

Abort Kubernetes Ingress update if Kubernetes API call fails #1295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fab3cb3
Abort Kubernetes Ingress update if Kubernetes API call fails
Mar 14, 2017
e85abc3
Merge branch 'v1.2' into 1240-skip-kubernetes-update-if-api-fails
regner Mar 16, 2017
e806930
Specifically capturing the tc range as documented here: https://blog.…
Mar 16, 2017
1f3e5b5
Updating service names in the mock data to be more clear
Mar 16, 2017
e06235b
Updated expected data to match what currently happens in the loadIngress
Mar 16, 2017
4814013
Adding a blank Servers to the expected output so we compare against t…
Mar 16, 2017
2b2674a
Replacing the JSON test output with spew for the TestMissingResources…
Mar 16, 2017
23b8568
Adding a temporary fix to the GetEndoints mocked function so we can o…
Mar 16, 2017
69442cb
Adding quick TODO line about removing the properExists property
Mar 16, 2017
f57e7a5
Link to issue 1307 re: properExists flag.
timoreimann Mar 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 16 additions & 4 deletions provider/kubernetes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,13 @@ func (provider *Kubernetes) loadIngresses(k8sClient k8s.Client) (*types.Configur
}
}
service, exists, err := k8sClient.GetService(i.ObjectMeta.Namespace, pa.Backend.ServiceName)
if err != nil || !exists {
log.Warnf("Error retrieving service %s/%s: %v", i.ObjectMeta.Namespace, pa.Backend.ServiceName, err)
if err != nil {
log.Errorf("Error while retrieving service information from k8s API %s/%s: %v", service.ObjectMeta.Namespace, pa.Backend.ServiceName, err)
return nil, err
}

if !exists {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a test for this case. I suppose it's already covered by a pre-existing test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean the does not exist part? No that was on my todo list, it is actually why the ingress part of the new test has sample data. Was thinking of moving it to a separate test though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate test sounds good. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added tests for this and the !exists endpoints

log.Errorf("Service not found for %s/%s", service.ObjectMeta.Namespace, pa.Backend.ServiceName)
delete(templateObjects.Frontends, r.Host+pa.Path)
continue
}
Expand All @@ -184,13 +189,20 @@ func (provider *Kubernetes) loadIngresses(k8sClient k8s.Client) (*types.Configur
if port.Port == 443 {
protocol = "https"
}

endpoints, exists, err := k8sClient.GetEndpoints(service.ObjectMeta.Namespace, service.ObjectMeta.Name)
if err != nil || !exists {
if err != nil {
log.Errorf("Error retrieving endpoints %s/%s: %v", service.ObjectMeta.Namespace, service.ObjectMeta.Name, err)
return nil, err
}

if !exists {
log.Errorf("Endpoints not found for %s/%s", service.ObjectMeta.Namespace, service.ObjectMeta.Name)
continue
}

if len(endpoints.Subsets) == 0 {
log.Warnf("Endpoints not found for %s/%s, falling back to Service ClusterIP", service.ObjectMeta.Namespace, service.ObjectMeta.Name)
log.Warnf("Service endpoints not found for %s/%s, falling back to Service ClusterIP", service.ObjectMeta.Namespace, service.ObjectMeta.Name)
templateObjects.Backends[r.Host+pa.Path].Servers[string(service.UID)] = types.Server{
URL: protocol + "://" + service.Spec.ClusterIP + ":" + strconv.Itoa(int(port.Port)),
Weight: 1,
Expand Down
292 changes: 291 additions & 1 deletion provider/kubernetes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ package provider

import (
"encoding/json"
"errors"
"reflect"
"testing"

"github.com/containous/traefik/provider/k8s"
"github.com/containous/traefik/types"
"github.com/davecgh/go-spew/spew"
"k8s.io/client-go/1.5/pkg/api/v1"
"k8s.io/client-go/1.5/pkg/apis/extensions/v1beta1"
"k8s.io/client-go/1.5/pkg/util/intstr"
Expand Down Expand Up @@ -1524,11 +1526,286 @@ func TestServiceAnnotations(t *testing.T) {
}
}

func TestKubeAPIErrors(t *testing.T) {
ingresses := []*v1beta1.Ingress{{
ObjectMeta: v1.ObjectMeta{
Namespace: "testing",
},
Spec: v1beta1.IngressSpec{
Rules: []v1beta1.IngressRule{
{
Host: "foo",
IngressRuleValue: v1beta1.IngressRuleValue{
HTTP: &v1beta1.HTTPIngressRuleValue{
Paths: []v1beta1.HTTPIngressPath{
{
Path: "/bar",
Backend: v1beta1.IngressBackend{
ServiceName: "service1",
ServicePort: intstr.FromInt(80),
},
},
},
},
},
},
},
},
}}

services := []*v1.Service{{
ObjectMeta: v1.ObjectMeta{
Name: "service1",
UID: "1",
Namespace: "testing",
},
Spec: v1.ServiceSpec{
ClusterIP: "10.0.0.1",
Ports: []v1.ServicePort{
{
Port: 80,
},
},
},
}}

endpoints := []*v1.Endpoints{}
watchChan := make(chan interface{})
apiErr := errors.New("failed kube api call")

testCases := []struct {
desc string
apiServiceErr error
apiEndpointsErr error
}{
{
desc: "failed service call",
apiServiceErr: apiErr,
},
{
desc: "failed endpoints call",
apiEndpointsErr: apiErr,
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.desc, func(t *testing.T) {
t.Parallel()

client := clientMock{
ingresses: ingresses,
services: services,
endpoints: endpoints,
watchChan: watchChan,
apiServiceError: tc.apiServiceErr,
apiEndpointsError: tc.apiEndpointsErr,
}

provider := Kubernetes{}
if _, err := provider.loadIngresses(client); err != apiErr {
t.Errorf("Got error %v, wanted error %v", err, apiErr)
}
})
}
}

func TestMissingResources(t *testing.T) {
ingresses := []*v1beta1.Ingress{{
ObjectMeta: v1.ObjectMeta{
Namespace: "testing",
},
Spec: v1beta1.IngressSpec{
Rules: []v1beta1.IngressRule{
{
Host: "fully_working",
IngressRuleValue: v1beta1.IngressRuleValue{
HTTP: &v1beta1.HTTPIngressRuleValue{
Paths: []v1beta1.HTTPIngressPath{
{
Backend: v1beta1.IngressBackend{
ServiceName: "fully_working_service",
ServicePort: intstr.FromInt(80),
},
},
},
},
},
},
{
Host: "missing_service",
IngressRuleValue: v1beta1.IngressRuleValue{
HTTP: &v1beta1.HTTPIngressRuleValue{
Paths: []v1beta1.HTTPIngressPath{
{
Backend: v1beta1.IngressBackend{
ServiceName: "missing_service_service",
ServicePort: intstr.FromInt(80),
},
},
},
},
},
},
{
Host: "missing_endpoints",
IngressRuleValue: v1beta1.IngressRuleValue{
HTTP: &v1beta1.HTTPIngressRuleValue{
Paths: []v1beta1.HTTPIngressPath{
{
Backend: v1beta1.IngressBackend{
ServiceName: "missing_endpoints_service",
ServicePort: intstr.FromInt(80),
},
},
},
},
},
},
},
},
}}
services := []*v1.Service{
{
ObjectMeta: v1.ObjectMeta{
Name: "fully_working_service",
UID: "1",
Namespace: "testing",
},
Spec: v1.ServiceSpec{
ClusterIP: "10.0.0.1",
Ports: []v1.ServicePort{
{
Port: 80,
},
},
},
},
{
ObjectMeta: v1.ObjectMeta{
Name: "missing_endpoints_service",
UID: "3",
Namespace: "testing",
},
Spec: v1.ServiceSpec{
ClusterIP: "10.0.0.3",
Ports: []v1.ServicePort{
{
Port: 80,
},
},
},
},
}
endpoints := []*v1.Endpoints{
{
ObjectMeta: v1.ObjectMeta{
Name: "fully_working_service",
UID: "1",
Namespace: "testing",
},
Subsets: []v1.EndpointSubset{
{
Addresses: []v1.EndpointAddress{
{
IP: "10.10.0.1",
},
},
Ports: []v1.EndpointPort{
{
Port: 8080,
},
},
},
},
},
}

watchChan := make(chan interface{})
client := clientMock{
ingresses: ingresses,
services: services,
endpoints: endpoints,
watchChan: watchChan,

// TODO: Update all tests to cope with "properExists == true" correctly and remove flag.
// See https://github.com/containous/traefik/issues/1307
properExists: true,
}
provider := Kubernetes{}
actual, err := provider.loadIngresses(client)
if err != nil {
t.Fatalf("error %+v", err)
}

expected := &types.Configuration{
Backends: map[string]*types.Backend{
"fully_working": {
Servers: map[string]types.Server{
"http://10.10.0.1:8080": {
URL: "http://10.10.0.1:8080",
Weight: 1,
},
},
CircuitBreaker: nil,
LoadBalancer: &types.LoadBalancer{
Method: "wrr",
Sticky: false,
},
},
"missing_service": {
Servers: map[string]types.Server{},
LoadBalancer: &types.LoadBalancer{
Method: "wrr",
Sticky: false,
},
},
"missing_endpoints": {
Servers: map[string]types.Server{},
CircuitBreaker: nil,
LoadBalancer: &types.LoadBalancer{
Method: "wrr",
Sticky: false,
},
},
},
Frontends: map[string]*types.Frontend{
"fully_working": {
Backend: "fully_working",
PassHostHeader: true,
Routes: map[string]types.Route{
"fully_working": {
Rule: "Host:fully_working",
},
},
},
"missing_endpoints": {
Backend: "missing_endpoints",
PassHostHeader: true,
Routes: map[string]types.Route{
"missing_endpoints": {
Rule: "Host:missing_endpoints",
},
},
},
},
}

if !reflect.DeepEqual(actual, expected) {
t.Fatalf("expected\n%v\ngot\n\n%v", spew.Sdump(expected), spew.Sdump(actual))
}
}

type clientMock struct {
ingresses []*v1beta1.Ingress
services []*v1.Service
endpoints []*v1.Endpoints
watchChan chan interface{}

apiServiceError error
apiEndpointsError error

properExists bool
}

func (c clientMock) GetIngresses(namespaces k8s.Namespaces) []*v1beta1.Ingress {
Expand All @@ -1543,20 +1820,33 @@ func (c clientMock) GetIngresses(namespaces k8s.Namespaces) []*v1beta1.Ingress {
}

func (c clientMock) GetService(namespace, name string) (*v1.Service, bool, error) {
if c.apiServiceError != nil {
return &v1.Service{}, false, c.apiServiceError
}

for _, service := range c.services {
if service.Namespace == namespace && service.Name == name {
return service, true, nil
}
}
return &v1.Service{}, true, nil
return &v1.Service{}, false, nil
}

func (c clientMock) GetEndpoints(namespace, name string) (*v1.Endpoints, bool, error) {
if c.apiEndpointsError != nil {
return &v1.Endpoints{}, false, c.apiEndpointsError
}

for _, endpoints := range c.endpoints {
if endpoints.Namespace == namespace && endpoints.Name == name {
return endpoints, true, nil
}
}

if c.properExists {
return &v1.Endpoints{}, false, nil
}

return &v1.Endpoints{}, true, nil
}

Expand Down