Merge pull request #72 from bowei/named-port

Move GetNamedPort to Namer
kubernetes · Nov 7, 2017 · 0d15eed · 0d15eed
2 parents 74bbb01 + 756d2c1
commit 0d15eed
Show file tree

Hide file tree

Showing 11 changed files with 109 additions and 79 deletions.
diff --git a/pkg/backends/backends.go b/pkg/backends/backends.go
@@ -265,7 +265,10 @@ func (b *Backends) ensureBackendService(p ServicePort, igs []*compute.InstanceGr
 	beName := b.namer.Backend(p.Port)
 	be, _ = b.Get(p.Port)
 	if be == nil {
-		namedPort := utils.GetNamedPort(p.Port)
+		namedPort := &compute.NamedPort{
+			Name: b.namer.NamedPort(p.Port),
+			Port: p.Port,
+		}
 		glog.V(2).Infof("Creating backend service for port %v named port %v", p.Port, namedPort)
 		be, err = b.create(namedPort, hcLink, p, beName)
 		if err != nil {

diff --git a/pkg/backends/backends_test.go b/pkg/backends/backends_test.go
@@ -57,7 +57,7 @@ var existingProbe = &api_v1.Probe{
 func newTestJig(f BackendServices, fakeIGs instances.InstanceGroups, syncWithCloud bool) (*Backends, healthchecks.HealthCheckProvider) {
 	namer := &utils.Namer{}
 	negGetter := networkendpointgroup.NewFakeNetworkEndpointGroupCloud("test-subnetwork", "test-network")
-	nodePool := instances.NewNodePool(fakeIGs)
+	nodePool := instances.NewNodePool(fakeIGs, namer)
 	nodePool.Init(&instances.FakeZoneLister{Zones: []string{defaultZone}})
 	healthCheckProvider := healthchecks.NewFakeHealthCheckProvider()
 	healthChecks := healthchecks.NewHealthChecker(healthCheckProvider, "/", namer)
@@ -338,7 +338,7 @@ func TestBackendPoolDeleteLegacyHealthChecks(t *testing.T) {
 	f := NewFakeBackendServices(noOpErrFunc)
 	fakeIGs := instances.NewFakeInstanceGroups(sets.NewString())
 	negGetter := networkendpointgroup.NewFakeNetworkEndpointGroupCloud("test-subnetwork", "test-network")
-	nodePool := instances.NewNodePool(fakeIGs)
+	nodePool := instances.NewNodePool(fakeIGs, namer)
 	nodePool.Init(&instances.FakeZoneLister{Zones: []string{defaultZone}})
 	hcp := healthchecks.NewFakeHealthCheckProvider()
 	healthChecks := healthchecks.NewHealthChecker(hcp, "/", namer)
@@ -520,7 +520,7 @@ func TestLinkBackendServiceToNEG(t *testing.T) {
 	f := NewFakeBackendServices(noOpErrFunc)
 	fakeIGs := instances.NewFakeInstanceGroups(sets.NewString())
 	fakeNEG := networkendpointgroup.NewFakeNetworkEndpointGroupCloud("test-subnetwork", "test-network")
-	nodePool := instances.NewNodePool(fakeIGs)
+	nodePool := instances.NewNodePool(fakeIGs, namer)
 	nodePool.Init(&instances.FakeZoneLister{Zones: []string{defaultZone}})
 	hcp := healthchecks.NewFakeHealthCheckProvider()
 	healthChecks := healthchecks.NewHealthChecker(hcp, "/", namer)

diff --git a/pkg/controller/cluster_manager.go b/pkg/controller/cluster_manager.go
@@ -220,7 +220,7 @@ func NewClusterManager(
 	cluster := ClusterManager{ClusterNamer: namer}
 
 	// NodePool stores GCE vms that are in this Kubernetes cluster.
-	cluster.instancePool = instances.NewNodePool(cloud)
+	cluster.instancePool = instances.NewNodePool(cloud, namer)
 
 	// BackendPool creates GCE BackendServices and associated health checks.
 	healthChecker := healthchecks.NewHealthChecker(cloud, defaultHealthCheckPath, cluster.ClusterNamer)

diff --git a/pkg/controller/fakes.go b/pkg/controller/fakes.go
@@ -52,7 +52,7 @@ func NewFakeClusterManager(clusterName, firewallName string) *fakeClusterManager
 	fakeNEG := networkendpointgroup.NewFakeNetworkEndpointGroupCloud("test-subnet", "test-network")
 	namer := utils.NewNamer(clusterName, firewallName)
 
-	nodePool := instances.NewNodePool(fakeIGs)
+	nodePool := instances.NewNodePool(fakeIGs, namer)
 	nodePool.Init(&instances.FakeZoneLister{Zones: []string{"zone-a"}})
 
 	healthChecker := healthchecks.NewHealthChecker(fakeHCP, "/", namer)

diff --git a/pkg/instances/instances.go b/pkg/instances/instances.go
@@ -42,15 +42,17 @@ type Instances struct {
 	// TODO: we can figure this out.
 	snapshotter storage.Snapshotter
 	zoneLister
+	namer *utils.Namer
 }
 
 // NewNodePool creates a new node pool.
 // - cloud: implements InstanceGroups, used to sync Kubernetes nodes with
 //   members of the cloud InstanceGroup.
-func NewNodePool(cloud InstanceGroups) NodePool {
+func NewNodePool(cloud InstanceGroups, namer *utils.Namer) NodePool {
 	return &Instances{
 		cloud:       cloud,
 		snapshotter: storage.NewInMemoryPool(),
+		namer:       namer,
 	}
 }
 
@@ -128,8 +130,8 @@ func (i *Instances) ensureInstanceGroupAndPorts(name, zone string, ports []int64
 
 	// Build slice of NamedPorts for adding
 	var newNamedPorts []*compute.NamedPort
-	for _, v := range newPorts {
-		newNamedPorts = append(newNamedPorts, utils.GetNamedPort(v))
+	for _, port := range newPorts {
+		newNamedPorts = append(newNamedPorts, &compute.NamedPort{Name: i.namer.NamedPort(port), Port: port})
 	}
 
 	if len(newNamedPorts) > 0 {

diff --git a/pkg/instances/instances_test.go b/pkg/instances/instances_test.go
@@ -20,12 +20,13 @@ import (
 	"testing"
 
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/ingress-gce/pkg/utils"
 )
 
 const defaultZone = "default-zone"
 
 func newNodePool(f *FakeInstanceGroups, zone string) NodePool {
-	pool := NewNodePool(f)
+	pool := NewNodePool(f, utils.NewNamer("cluster-uid", "cluster-fw"))
 	pool.Init(&FakeZoneLister{[]string{zone}})
 	return pool
 }

diff --git a/pkg/loadbalancers/loadbalancers_test.go b/pkg/loadbalancers/loadbalancers_test.go
@@ -45,7 +45,7 @@ func newFakeLoadBalancerPool(f LoadBalancers, t *testing.T) LoadBalancerPool {
 	fakeNEG := networkendpointgroup.NewFakeNetworkEndpointGroupCloud("test-subnet", "test-network")
 	namer := &utils.Namer{}
 	healthChecker := healthchecks.NewHealthChecker(fakeHCP, "/", namer)
-	nodePool := instances.NewNodePool(fakeIGs)
+	nodePool := instances.NewNodePool(fakeIGs, namer)
 	nodePool.Init(&instances.FakeZoneLister{Zones: []string{defaultZone}})
 	backendPool := backends.NewBackendPool(
 		fakeBackends, fakeNEG, healthChecker, nodePool, namer, []int64{}, false)

diff --git a/pkg/utils/namer.go b/pkg/utils/namer.go
@@ -311,6 +311,11 @@ func (n *Namer) UrlMap(lbName string) string {
 	return truncate(fmt.Sprintf("%v-%v", urlMapPrefix, lbName))
 }
 
+// NamedPort returns the name for a named port.
+func (n *Namer) NamedPort(port int64) string {
+	return fmt.Sprintf("port%v", port)
+}
+
 // NEG returns the gce neg name based on the service namespace,
 // name and target port.  NEG naming convention:
 // k8s-{clusterid}-{namespace}-{name}-{target port}-{hash} Output name

diff --git a/pkg/utils/namer_test.go b/pkg/utils/namer_test.go
@@ -0,0 +1,84 @@
+/*
+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 utils
+
+import (
+	"testing"
+)
+
+const (
+	clusterId = "0123456789abcdef"
+)
+
+func TestNamerNEG(t *testing.T) {
+	longstring := "01234567890123456789012345678901234567890123456789"
+	testCases := []struct {
+		desc      string
+		namespace string
+		name      string
+		port      string
+		expect    string
+	}{
+		{
+			"simple case",
+			"namespace",
+			"name",
+			"80",
+			"k8s1-0123456789abcdef-namespace-name-80-1e047e33",
+		},
+		{
+			"63 characters",
+			longstring[:10],
+			longstring[:10],
+			longstring[:10],
+			"k8s1-0123456789abcdef-0123456789-0123456789-0123456789-4f7223eb",
+		},
+		{
+			"long namespace",
+			longstring,
+			"0",
+			"0",
+			"k8s1-0123456789abcdef-01234567890123456789012345678-0--44255b67",
+		},
+
+		{
+			"long name and namespace",
+			longstring,
+			longstring,
+			"0",
+			"k8s1-0123456789abcdef-012345678901234-012345678901234--525cce3d",
+		},
+		{
+			" long name, namespace and port",
+			longstring,
+			longstring[:40],
+			longstring[:30],
+			"k8s1-0123456789abcdef-0123456789012-0123456789-0123456-71877a60",
+		},
+	}
+
+	namer := NewNamer(clusterId, "")
+	for _, tc := range testCases {
+		res := namer.NEG(tc.namespace, tc.name, tc.port)
+		if len(res) > 63 {
+			t.Errorf("%s: got len(res) == %v, want <= 63", tc.desc, len(res))
+		}
+		if res != tc.expect {
+			t.Errorf("%s: got %q, want %q", tc.desc, res, tc.expect)
+		}
+	}
+}
diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
@@ -154,12 +154,6 @@ func CompareLinks(l1, l2 string) bool {
 // that share the same testing methods.
 type FakeIngressRuleValueMap map[string]string
 
-// GetNamedPort creates the NamedPort API object for the given port.
-func GetNamedPort(port int64) *compute.NamedPort {
-	// TODO: move port naming to namer
-	return &compute.NamedPort{Name: fmt.Sprintf("port%v", port), Port: port}
-}
-
 // trimFieldsEvenly trims the fields evenly and keeps the total length
 // <= max. Truncation is spread in ratio with their original length,
 // meaning smaller fields will be truncated less than longer ones.

diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go
@@ -20,9 +20,9 @@ import (
 	"testing"
 )
 
-const (
-	ClusterId = "0123456789abcdef"
-)
+func TestGCEURLMap(t *testing.T) {
+	// TODO
+}
 
 func TestTrimFieldsEvenly(t *testing.T) {
 	longString := "01234567890123456789012345678901234567890123456789"
@@ -95,62 +95,3 @@ func TestTrimFieldsEvenly(t *testing.T) {
 		}
 	}
 }
-
-func TestNEG(t *testing.T) {
-	longstring := "01234567890123456789012345678901234567890123456789"
-	testCases := []struct {
-		desc      string
-		namespace string
-		name      string
-		port      string
-		expect    string
-	}{
-		{
-			"simple case",
-			"namespace",
-			"name",
-			"80",
-			"k8s1-0123456789abcdef-namespace-name-80-1e047e33",
-		},
-		{
-			"63 characters",
-			longstring[:10],
-			longstring[:10],
-			longstring[:10],
-			"k8s1-0123456789abcdef-0123456789-0123456789-0123456789-4f7223eb",
-		},
-		{
-			"long namespace",
-			longstring,
-			"0",
-			"0",
-			"k8s1-0123456789abcdef-01234567890123456789012345678-0--44255b67",
-		},
-
-		{
-			"long name and namespace",
-			longstring,
-			longstring,
-			"0",
-			"k8s1-0123456789abcdef-012345678901234-012345678901234--525cce3d",
-		},
-		{
-			" long name, namespace and port",
-			longstring,
-			longstring[:40],
-			longstring[:30],
-			"k8s1-0123456789abcdef-0123456789012-0123456789-0123456-71877a60",
-		},
-	}
-
-	namer := NewNamer(ClusterId, "")
-	for _, tc := range testCases {
-		res := namer.NEG(tc.namespace, tc.name, tc.port)
-		if len(res) > 63 {
-			t.Errorf("%s: got len(res) == %v, want <= 63", tc.desc, len(res))
-		}
-		if res != tc.expect {
-			t.Errorf("%s: got %q, want %q", tc.desc, res, tc.expect)
-		}
-	}
-}