Merge pull request #195 from davecheney/backport/187

Backport #187
projectcontour · Jan 28, 2018 · 5bd3bb8 · 5bd3bb8
2 parents e3826fb + aaa94c2
commit 5bd3bb8
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 3 deletions.
diff --git a/internal/contour/translator.go b/internal/contour/translator.go
@@ -31,6 +31,7 @@ import (
 	"github.com/heptio/contour/internal/log"
 	"k8s.io/api/core/v1"
 	"k8s.io/api/extensions/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/cache"
 )
 
@@ -41,7 +42,6 @@ type metadata struct {
 // Translator receives notifications from the Kubernetes API and translates those
 // objects into additions and removals entries of Envoy gRPC objects from a cache.
 type Translator struct {
-
 	// The logger for this Translator. There is no valid default, this value
 	// must be supplied by the caller.
 	log.Logger
@@ -116,7 +116,7 @@ func (t *Translator) addService(svc *v1.Service) {
 		case "TCP":
 			config := &v2.Cluster_EdsClusterConfig{
 				EdsConfig:   apiconfigsource("xds_cluster"), // hard coded by initconfig
-				ServiceName: svc.ObjectMeta.Namespace + "/" + svc.ObjectMeta.Name + "/" + p.TargetPort.String(),
+				ServiceName: servicename(svc.ObjectMeta, p.TargetPort.String()),
 			}
 			if p.Name != "" {
 				// service port is named, so we must generate both a cluster for the port name
@@ -178,7 +178,8 @@ func (t *Translator) addEndpoints(e *v1.Endpoints) {
 
 		for _, p := range s.Ports {
 			cla := v2.ClusterLoadAssignment{
-				ClusterName: hashname(60, e.ObjectMeta.Namespace, e.ObjectMeta.Name, strconv.Itoa(int(p.Port))),
+				// ClusterName must match Cluster.ServiceName
+				ClusterName: servicename(e.ObjectMeta, strconv.Itoa(int(p.Port))),
 				Endpoints: []*v2.LocalityLbEndpoints{{
 					Locality: &v2.Locality{
 						Region:  "ap-southeast-2",
@@ -350,6 +351,10 @@ func (t *Translator) writeCerts(s *v1.Secret) {
 	}
 }
 
+func servicename(meta metav1.ObjectMeta, port string) string {
+	return meta.Namespace + "/" + meta.Name + "/" + port
+}
+
 // TODO(dfc) need tests
 func appendIfMissing(haystack []*v1beta1.Ingress, needle *v1beta1.Ingress) []*v1beta1.Ingress {
 	for i := range haystack {

diff --git a/internal/contour/translator_test.go b/internal/contour/translator_test.go
@@ -945,6 +945,56 @@ func TestTranslatorRemoveIngress(t *testing.T) {
 	}
 }
 
+func TestClusterEDSConfigServiceNameMatchesLoadAssignmentClusterName(t *testing.T) {
+	tests := []struct {
+		name string
+		svc  *v1.Service
+		ep   *v1.Endpoints
+	}{
+		{
+			name: "simple service and endpoint",
+			svc: service("default", "simple", v1.ServicePort{
+				Protocol:   "TCP",
+				Port:       80,
+				TargetPort: intstr.FromInt(8080),
+			}),
+			ep: endpoints("default", "simple", v1.EndpointSubset{
+				Addresses: addresses("192.168.183.24"),
+				Ports:     ports(8080),
+			}),
+		},
+		{
+			name: "long namespace and service name",
+			svc: service("beurocratic-company-test-domain-1", "tiny-cog-department-test-instance", v1.ServicePort{
+				Protocol:   "TCP",
+				Port:       80,
+				TargetPort: intstr.FromInt(8080),
+			}),
+			ep: endpoints("beurocratic-company-test-domain-1", "tiny-cog-department-test-instance", v1.EndpointSubset{
+				Addresses: addresses("192.168.183.24"),
+				Ports:     ports(8080),
+			}),
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			const NOFLAGS = 1 << 16
+			tr := &Translator{
+				Logger: stdlog.New(ioutil.Discard, ioutil.Discard, NOFLAGS),
+			}
+			tr.addService(tc.svc)
+			tr.addEndpoints(tc.ep)
+			c := tr.ClusterCache.Values()
+			cla := tr.ClusterLoadAssignmentCache.Values()
+
+			if c[0].EdsClusterConfig.GetServiceName() != cla[0].ClusterName {
+				t.Errorf("cluster EDS config service name: %q does not match clusterLoadAssignment cluster name: %q", c[0].EdsClusterConfig.GetServiceName(), cla[0].ClusterName)
+			}
+		})
+	}
+}
+
 func TestHashname(t *testing.T) {
 	tests := []struct {
 		name string