diff --git a/core/pkg/ingress/annotations/service/service.go b/core/pkg/ingress/annotations/service/service.go deleted file mode 100644 index b268e63ed5..0000000000 --- a/core/pkg/ingress/annotations/service/service.go +++ /dev/null @@ -1,73 +0,0 @@ -/* -Copyright 2015 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package service - -import ( - "encoding/json" - "fmt" - "strconv" - - "github.com/golang/glog" - - api_v1 "k8s.io/client-go/pkg/api/v1" -) - -const ( - // NamedPortAnnotation annotation used to map named port in services - NamedPortAnnotation = "ingress.kubernetes.io/named-ports" -) - -type namedPortMapping map[string]string - -// getPort returns the port defined in a named port -func (npm namedPortMapping) getPort(name string) (string, bool) { - val, ok := npm.getPortMappings()[name] - return val, ok -} - -// getPortMappings returns a map containing the mapping of named ports names and number -func (npm namedPortMapping) getPortMappings() map[string]string { - data := npm[NamedPortAnnotation] - var mapping map[string]string - if data == "" { - return mapping - } - if err := json.Unmarshal([]byte(data), &mapping); err != nil { - glog.Errorf("unexpected error reading annotations: %v", err) - } - - return mapping -} - -// GetPortMapping returns the number of the named port or an error if is not valid -func GetPortMapping(name string, s *api_v1.Service) (int32, error) { - if s == nil { - return -1, fmt.Errorf("impossible to extract por mapping from %v (missing service)", name) - } - namedPorts := s.ObjectMeta.Annotations - val, ok := namedPortMapping(namedPorts).getPort(name) - if ok { - port, err := strconv.Atoi(val) - if err != nil { - return -1, fmt.Errorf("service %v contains an invalid port mapping for %v (%v), %v", s.Name, name, val, err) - } - - return int32(port), nil - } - - return -1, fmt.Errorf("there is no port with name %v", name) -} diff --git a/core/pkg/ingress/annotations/service/service_test.go b/core/pkg/ingress/annotations/service/service_test.go deleted file mode 100644 index 2eaf016aa3..0000000000 --- a/core/pkg/ingress/annotations/service/service_test.go +++ /dev/null @@ -1,100 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package service - -import ( - "encoding/json" - "testing" - - meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - api "k8s.io/client-go/pkg/api/v1" -) - -func fakeService(npa bool, ps bool, expectedP string) *api.Service { - // fake name for the map of ports - fakeNpa := NamedPortAnnotation - if !npa { - fakeNpa = "fake" + NamedPortAnnotation - } - - // fake ports - fakePs, _ := json.Marshal(map[string]string{ - "port1": expectedP, - "port2": "10211", - }) - if !ps { - fakePs, _ = json.Marshal(expectedP) - } - - // fake service - return &api.Service{ - TypeMeta: meta_v1.TypeMeta{ - Kind: "ingress", - APIVersion: "v1", - }, - ObjectMeta: meta_v1.ObjectMeta{ - Annotations: map[string]string{ - fakeNpa: string(fakePs), - }, - Namespace: api.NamespaceDefault, - Name: "named-port-test", - }, - } -} - -func TestGetPortMappingSuccess(t *testing.T) { - fakeS := fakeService(true, true, "10011") - port, err := GetPortMapping("port1", fakeS) - if err != nil { - t.Errorf("failed to get port with name %s, error: %v", "port1", err) - return - } - if port != 10011 { - t.Errorf("%s: expected %d but returned %d", "port1", 10011, port) - } -} - -func TestGetPortMappingFailedNamedPortMappingNotExist(t *testing.T) { - fakeS := fakeService(false, true, "10011") - _, err := GetPortMapping("port1", fakeS) - if err == nil { - t.Errorf("%s: expected error but returned nil", "port1") - } -} - -func TestGetPortMappingFailedPortNotExist(t *testing.T) { - fakeS := fakeService(true, true, "10011") - _, err := GetPortMapping("port3", fakeS) - if err == nil { - t.Errorf("%s: expected error but returned nil", "port3") - } -} - -func TestGetPortMappingFailedPortInvalid(t *testing.T) { - fakeS := fakeService(true, true, "s2017") - _, err := GetPortMapping("port1", fakeS) - if err == nil { - t.Errorf("%s: expected error but returned nil", "port1") - } -} - -func TestGetPortMappingFailedApiServiceIsNil(t *testing.T) { - _, err := GetPortMapping("port1", nil) - if err == nil { - t.Errorf("%s: expected error but returned nil", "port1") - } -} diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index c7d854dc54..096c01ddb4 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -45,7 +45,6 @@ import ( "k8s.io/ingress/core/pkg/ingress/annotations/healthcheck" "k8s.io/ingress/core/pkg/ingress/annotations/parser" "k8s.io/ingress/core/pkg/ingress/annotations/proxy" - "k8s.io/ingress/core/pkg/ingress/annotations/service" "k8s.io/ingress/core/pkg/ingress/defaults" "k8s.io/ingress/core/pkg/ingress/resolver" "k8s.io/ingress/core/pkg/ingress/status" @@ -1046,31 +1045,7 @@ func (ic *GenericController) getEndpoints( // ExternalName services if s.Spec.Type == api.ServiceTypeExternalName { - var targetPort int - - switch servicePort.Type { - case intstr.Int: - targetPort = servicePort.IntValue() - case intstr.String: - port, err := service.GetPortMapping(servicePort.StrVal, s) - if err == nil { - targetPort = int(port) - break - } - - glog.Warningf("error mapping service port: %v", err) - err = ic.checkSvcForUpdate(s) - if err != nil { - glog.Warningf("error mapping service ports: %v", err) - return upsServers - } - - port, err = service.GetPortMapping(servicePort.StrVal, s) - if err == nil { - targetPort = int(port) - } - } - + targetPort := servicePort.IntValue() // check for invalid port value if targetPort <= 0 { return upsServers @@ -1106,22 +1081,8 @@ func (ic *GenericController) getEndpoints( targetPort = epPort.Port } case intstr.String: - port, err := service.GetPortMapping(servicePort.StrVal, s) - if err == nil { - targetPort = port - break - } - - glog.Warningf("error mapping service port: %v", err) - err = ic.checkSvcForUpdate(s) - if err != nil { - glog.Warningf("error mapping service ports: %v", err) - continue - } - - port, err = service.GetPortMapping(servicePort.StrVal, s) - if err == nil { - targetPort = port + if epPort.Name == servicePort.StrVal { + targetPort = epPort.Port } } diff --git a/core/pkg/ingress/controller/named_port.go b/core/pkg/ingress/controller/named_port.go index a3a5551515..99e43c826f 100644 --- a/core/pkg/ingress/controller/named_port.go +++ b/core/pkg/ingress/controller/named_port.go @@ -17,94 +17,12 @@ limitations under the License. package controller import ( - "encoding/json" "fmt" - "reflect" - "strconv" - "github.com/golang/glog" - - meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/intstr" api_v1 "k8s.io/client-go/pkg/api/v1" - - "k8s.io/ingress/core/pkg/ingress/annotations/service" ) -// checkSvcForUpdate verifies if one of the running pods for a service contains -// named port. If the annotation in the service does not exist or is not equals -// to the port mapping obtained from the pod the service must be updated to reflect -// the current state -func (ic *GenericController) checkSvcForUpdate(svc *api_v1.Service) error { - // get the pods associated with the service - // TODO: switch this to a watch - pods, err := ic.cfg.Client.Core().Pods(svc.Namespace).List(meta_v1.ListOptions{ - LabelSelector: labels.Set(svc.Spec.Selector).AsSelector().String(), - }) - - if err != nil { - return fmt.Errorf("error searching service pods %v/%v: %v", svc.Namespace, svc.Name, err) - } - - if len(pods.Items) == 0 { - return nil - } - - namedPorts := map[string]string{} - - // we need to check only one pod searching for named ports - pod := &pods.Items[0] - glog.V(4).Infof("checking pod %v/%v for named port information", pod.Namespace, pod.Name) - for i := range svc.Spec.Ports { - servicePort := &svc.Spec.Ports[i] - - _, err := strconv.Atoi(servicePort.TargetPort.StrVal) - if err != nil { - portNum, err := findPort(pod, servicePort) - if err != nil { - glog.V(4).Infof("failed to find port for service %s/%s: %v", portNum, svc.Namespace, svc.Name, err) - continue - } - - if servicePort.TargetPort.StrVal == "" { - continue - } - - namedPorts[servicePort.TargetPort.StrVal] = fmt.Sprintf("%v", portNum) - } - } - - if svc.ObjectMeta.Annotations == nil { - svc.ObjectMeta.Annotations = map[string]string{} - } - - curNamedPort := svc.ObjectMeta.Annotations[service.NamedPortAnnotation] - if len(namedPorts) > 0 && !reflect.DeepEqual(curNamedPort, namedPorts) { - data, _ := json.Marshal(namedPorts) - - newSvc, err := ic.cfg.Client.Core().Services(svc.Namespace).Get(svc.Name, meta_v1.GetOptions{}) - if err != nil { - return fmt.Errorf("error getting service %v/%v: %v", svc.Namespace, svc.Name, err) - } - - if newSvc.ObjectMeta.Annotations == nil { - newSvc.ObjectMeta.Annotations = map[string]string{} - } - - newSvc.ObjectMeta.Annotations[service.NamedPortAnnotation] = string(data) - glog.Infof("updating service %v with new named port mappings", svc.Name) - _, err = ic.cfg.Client.Core().Services(svc.Namespace).Update(newSvc) - if err != nil { - return fmt.Errorf("error syncing service %v/%v: %v", svc.Namespace, svc.Name, err) - } - - return nil - } - - return nil -} - // FindPort locates the container port for the given pod and portName. If the // targetPort is a number, use that. If the targetPort is a string, look that // string up in all named ports in all containers in the target pod. If no diff --git a/core/pkg/ingress/controller/named_port_test.go b/core/pkg/ingress/controller/named_port_test.go index 0f87a4e0c4..09637cb254 100644 --- a/core/pkg/ingress/controller/named_port_test.go +++ b/core/pkg/ingress/controller/named_port_test.go @@ -17,15 +17,10 @@ limitations under the License. package controller import ( - "reflect" - "testing" - meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/pkg/api" api_v1 "k8s.io/client-go/pkg/api/v1" - "k8s.io/ingress/core/pkg/ingress/annotations/service" ) func buildSimpleClientSet() *fake.Clientset { @@ -96,99 +91,3 @@ func buildService() *api_v1.Service { }, } } - -func TestCheckSvcForUpdate(t *testing.T) { - foos := []struct { - n string - ns string - sps []api_v1.ServicePort - sl map[string]string - er string - }{ - { - "pods_have_not_been_found_in_this_namespace", - api.NamespaceSystem, - []api_v1.ServicePort{ - {Name: "foo_port_1", Port: 8080, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromString("foo1_named_port_c1")}, - {Name: "foo_port_2", Port: 8181, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromInt(81)}, - {Name: "foo_port_3", Port: 8282, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromString("")}, - }, - map[string]string{ - "lable_sig": "foo_pod", - }, - "", - }, - { - "ports_have_not_been_found_in_this_pod", - api.NamespaceDefault, - []api_v1.ServicePort{ - {Name: "foo_port_1", Port: 8080, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromString("foo1_named_port_cXX")}, - {Name: "foo_port_2", Port: 8181, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromInt(81)}, - {Name: "foo_port_3", Port: 8282, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromString("")}, - }, - map[string]string{ - "lable_sig": "foo_pod", - }, - "", - }, - - { - "ports_fixed", - api.NamespaceDefault, - []api_v1.ServicePort{ - {Name: "foo_port_1", Port: 8080, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromInt(80)}, - {Name: "foo_port_2", Port: 8181, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromInt(81)}, - {Name: "foo_port_3", Port: 8282, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromString("")}, - }, - map[string]string{ - "lable_sig": "foo_pod", - }, - "", - }, - { - "nil_selector", - api.NamespaceDefault, - []api_v1.ServicePort{ - {Name: "foo_port_1", Port: 8080, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromString("foo1_named_port_c1")}, - {Name: "foo_port_2", Port: 8181, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromInt(81)}, - {Name: "foo_port_3", Port: 8282, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromString("")}, - }, - nil, - "{\"foo1_named_port_c1\":\"80\"}", - }, - { - "normal_update", - api.NamespaceDefault, - []api_v1.ServicePort{ - {Name: "foo_port_1", Port: 8080, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromString("foo1_named_port_c1")}, - {Name: "foo_port_2", Port: 8181, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromInt(81)}, - {Name: "foo_port_3", Port: 8282, Protocol: api_v1.ProtocolTCP, TargetPort: intstr.FromString("")}, - }, - map[string]string{ - "lable_sig": "foo_pod", - }, - "{\"foo1_named_port_c1\":\"80\"}", - }, - } - - for _, foo := range foos { - t.Run(foo.n, func(t *testing.T) { - gc := buildGenericController() - s := buildService() - s.SetNamespace(foo.ns) - s.Spec.Ports = foo.sps - s.Spec.Selector = foo.sl - - err := gc.checkSvcForUpdate(s) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - rs, _ := gc.cfg.Client.Core().Services(api.NamespaceDefault).Get("named_port_test_service", meta_v1.GetOptions{}) - rr := rs.ObjectMeta.Annotations[service.NamedPortAnnotation] - if !reflect.DeepEqual(rr, foo.er) { - t.Errorf("Returned %s, but expected %s for %s", rr, foo.er, foo.n) - } - }) - } -}