scottk/kubernetes
1
0
Fork 0
mirror of https://github.com/optim-enterprises-bv/kubernetes.git synced 2025-11-03 19:58:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

Refactor cloud route interface, to avoid assumption that routes are named

Browse Source
This commit is contained in:
Justin Santa Barbara
2015-06-12 12:27:10 -04:00
parent 19e574c170
commit a3b43a36fd
5 changed files with 108 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
pkg/cloudprovider/cloud.go
View File

@@ -119,6 +119,7 @@ type Instances interface {
// Route is a representation of an advanced routing rule.
type Route struct {
// Name is the name of the routing rule in the cloud-provider.
// It will be ignored in a Create (although nameHint may influence it)
Name string
// TargetInstance is the name of the instance as specified in routing rules
// for the cloud-provider (in gce: the Instance Name).
@@ -126,18 +127,19 @@ type Route struct {
// Destination CIDR is the CIDR format IP range that this routing rule
// applies to.
DestinationCIDR string
// Description is a free-form string. It can be useful for tagging Routes.
Description string
}
// Routes is an abstract, pluggable interface for advanced routing rules.
type Routes interface {
// List all routes that match the filter
ListRoutes(filter string) ([]*Route, error)
// Create the described route
CreateRoute(route *Route) error
// Delete the specified route
DeleteRoute(name string) error
// List all managed routes that belong to the specified clusterName
ListRoutes(clusterName string) ([]*Route, error)
// Create the described managed route
// route.Name will be ignored, although the cloud-provider may use nameHint
// to create a more user-meaningful name.
CreateRoute(clusterName string, nameHint string, route *Route) error
// Delete the specified managed route
// Route should be as returned by ListRoutes
DeleteRoute(clusterName string, route *Route) error
}
var InstanceNotFound = errors.New("instance not found")

32
pkg/cloudprovider/fake/fake.go
View File

@@ -58,11 +58,16 @@ type FakeCloud struct {
ExternalIP net.IP
Balancers []FakeBalancer
UpdateCalls []FakeUpdateBalancerCall
RouteMap map[string]*cloudprovider.Route
RouteMap map[string]*FakeRoute
Lock sync.Mutex
cloudprovider.Zone
}
type FakeRoute struct {
ClusterName string
Route cloudprovider.Route
}
func (f *FakeCloud) addCall(desc string) {
f.Calls = append(f.Calls, desc)
}
@@ -198,35 +203,42 @@ func (f *FakeCloud) GetNodeResources(name string) (*api.NodeResources, error) {
return f.NodeResources, f.Err
}
func (f *FakeCloud) ListRoutes(filter string) ([]*cloudprovider.Route, error) {
func (f *FakeCloud) ListRoutes(clusterName string) ([]*cloudprovider.Route, error) {
f.Lock.Lock()
defer f.Lock.Unlock()
f.addCall("list-routes")
var routes []*cloudprovider.Route
for _, route := range f.RouteMap {
if match, _ := regexp.MatchString(filter, route.Name); match {
routes = append(routes, route)
for _, fakeRoute := range f.RouteMap {
if clusterName == fakeRoute.ClusterName {
routeCopy := fakeRoute.Route
routes = append(routes, &routeCopy)
}
}
return routes, f.Err
}
func (f *FakeCloud) CreateRoute(route *cloudprovider.Route) error {
func (f *FakeCloud) CreateRoute(clusterName string, nameHint string, route *cloudprovider.Route) error {
f.Lock.Lock()
defer f.Lock.Unlock()
f.addCall("create-route")
if _, exists := f.RouteMap[route.Name]; exists {
f.Err = fmt.Errorf("route with name %q already exists")
name := clusterName + "-" + nameHint
if _, exists := f.RouteMap[name]; exists {
f.Err = fmt.Errorf("route %q already exists", name)
return f.Err
}
f.RouteMap[route.Name] = route
fakeRoute := FakeRoute{}
fakeRoute.Route = *route
fakeRoute.Route.Name = name
fakeRoute.ClusterName = clusterName
f.RouteMap[name] = &fakeRoute
return nil
}
func (f *FakeCloud) DeleteRoute(name string) error {
func (f *FakeCloud) DeleteRoute(clusterName string, route *cloudprovider.Route) error {
f.Lock.Lock()
defer f.Lock.Unlock()
f.addCall("delete-route")
name := route.Name
if _, exists := f.RouteMap[name]; !exists {
f.Err = fmt.Errorf("no route found with name %q", name)
return f.Err

42
pkg/cloudprovider/gce/gce.go
View File

@@ -49,6 +49,8 @@ const (
INTERNAL_IP_METADATA_URL = "http://169.254.169.254/computeMetadata/v1/instance/network-interfaces/0/ip"
)
const k8sNodeRouteTag = "k8s-node-route"
// GCECloud is an implementation of Interface, TCPLoadBalancer and Instances for Google Compute Engine.
type GCECloud struct {
service *compute.Service
@@ -631,11 +633,19 @@ func getMetadataValue(metadata *compute.Metadata, key string) (string, bool) {
return "", false
}
func (gce *GCECloud) ListRoutes(filter string) ([]*cloudprovider.Route, error) {
listCall := gce.service.Routes.List(gce.projectID)
if len(filter) > 0 {
listCall = listCall.Filter("name eq " + filter)
func truncateClusterName(clusterName string) string {
if len(clusterName) > 26 {
return clusterName[:26]
}
return clusterName
}
func (gce *GCECloud) ListRoutes(clusterName string) ([]*cloudprovider.Route, error) {
listCall := gce.service.Routes.List(gce.projectID)
prefix := truncateClusterName(clusterName)
listCall = listCall.Filter("name eq " + prefix + "-.*")
res, err := listCall.Do()
if err != nil {
return nil, err
@@ -645,21 +655,32 @@ func (gce *GCECloud) ListRoutes(filter string) ([]*cloudprovider.Route, error) {
if path.Base(r.Network) != gce.networkName {
continue
}
// Not managed if route description != "k8s-node-route"
if r.Description != k8sNodeRouteTag {
continue
}
// Not managed if route name doesn't start with <clusterName>
if !strings.HasPrefix(r.Name, prefix) {
continue
}
target := path.Base(r.NextHopInstance)
routes = append(routes, &cloudprovider.Route{r.Name, target, r.DestRange, r.Description})
routes = append(routes, &cloudprovider.Route{r.Name, target, r.DestRange})
}
return routes, nil
}
func (gce *GCECloud) CreateRoute(route *cloudprovider.Route) error {
func (gce *GCECloud) CreateRoute(clusterName string, nameHint string, route *cloudprovider.Route) error {
routeName := truncateClusterName(clusterName) + "-" + nameHint
instanceName := canonicalizeInstanceName(route.TargetInstance)
insertOp, err := gce.service.Routes.Insert(gce.projectID, &compute.Route{
Name: route.Name,
Name: routeName,
DestRange: route.DestinationCIDR,
NextHopInstance: fmt.Sprintf("zones/%s/instances/%s", gce.zone, instanceName),
Network: fmt.Sprintf("global/networks/%s", gce.networkName),
Priority: 1000,
Description: route.Description,
Description: k8sNodeRouteTag,
}).Do()
if err != nil {
return err
@@ -667,9 +688,8 @@ func (gce *GCECloud) CreateRoute(route *cloudprovider.Route) error {
return gce.waitForGlobalOp(insertOp)
}
func (gce *GCECloud) DeleteRoute(name string) error {
instanceName := canonicalizeInstanceName(name)
deleteOp, err := gce.service.Routes.Delete(gce.projectID, instanceName).Do()
func (gce *GCECloud) DeleteRoute(clusterName string, route *cloudprovider.Route) error {
deleteOp, err := gce.service.Routes.Delete(gce.projectID, route.Name).Do()
if err != nil {
return err
}

39
pkg/cloudprovider/routecontroller/routecontroller.go
View File

@@ -19,7 +19,6 @@ package routecontroller
import (
"fmt"
"net"
"strings"
"time"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -38,8 +37,6 @@ type RouteController struct {
clusterCIDR *net.IPNet
}
const k8sNodeRouteTag = "k8s-node-route"
func New(routes cloudprovider.Routes, kubeClient client.Interface, clusterName string, clusterCIDR *net.IPNet) *RouteController {
return &RouteController{
routes: routes,
@@ -58,7 +55,7 @@ func (rc *RouteController) Run(syncPeriod time.Duration) {
}
func (rc *RouteController) reconcileNodeRoutes() error {
routeList, err := rc.routes.ListRoutes(rc.truncatedClusterName() + "-.*")
routeList, err := rc.routes.ListRoutes(rc.clusterName)
if err != nil {
return fmt.Errorf("error listing routes: %v", err)
}
@@ -85,16 +82,15 @@ func (rc *RouteController) reconcile(nodes []api.Node, routes []*cloudprovider.R
if r == nil || r.DestinationCIDR != node.Spec.PodCIDR {
// If not, create the route.
route := &cloudprovider.Route{
Name: rc.truncatedClusterName() + "-" + string(node.UID),
TargetInstance: node.Name,
DestinationCIDR: node.Spec.PodCIDR,
Description: k8sNodeRouteTag,
}
go func(route *cloudprovider.Route) {
if err := rc.routes.CreateRoute(route); err != nil {
glog.Errorf("Could not create route %s: %v", route.Name, err)
nameHint := string(node.UID)
go func(nameHint string, route *cloudprovider.Route) {
if err := rc.routes.CreateRoute(rc.clusterName, nameHint, route); err != nil {
glog.Errorf("Could not create route %s %s: %v", nameHint, route.DestinationCIDR, err)
}
}(route)
}(nameHint, route)
}
nodeCIDRs[node.Name] = node.Spec.PodCIDR
}
@@ -103,24 +99,17 @@ func (rc *RouteController) reconcile(nodes []api.Node, routes []*cloudprovider.R
// Check if this route applies to a node we know about & has correct CIDR.
if nodeCIDRs[route.TargetInstance] != route.DestinationCIDR {
// Delete the route.
go func(routeName string) {
if err := rc.routes.DeleteRoute(routeName); err != nil {
glog.Errorf("Could not delete route %s: %v", routeName, err)
go func(route *cloudprovider.Route) {
if err := rc.routes.DeleteRoute(rc.clusterName, route); err != nil {
glog.Errorf("Could not delete route %s %s: %v", route.Name, route.DestinationCIDR, err)
}
}(route.Name)
}(route)
}
}
}
return nil
}
func (rc *RouteController) truncatedClusterName() string {
if len(rc.clusterName) > 26 {
return rc.clusterName[:26]
}
return rc.clusterName
}
func (rc *RouteController) isResponsibleForRoute(route *cloudprovider.Route) bool {
_, cidr, err := net.ParseCIDR(route.DestinationCIDR)
if err != nil {
@@ -135,13 +124,5 @@ func (rc *RouteController) isResponsibleForRoute(route *cloudprovider.Route) boo
if !rc.clusterCIDR.Contains(cidr.IP) || !rc.clusterCIDR.Contains(lastIP) {
return false
}
// Not responsible if route name doesn't start with <clusterName>
if !strings.HasPrefix(route.Name, rc.clusterName) {
return false
}
// Not responsible if route description != "k8s-node-route"
if route.Description != k8sNodeRouteTag {
return false
}
return true
}

75
pkg/cloudprovider/routecontroller/routecontroller_test.go
View File

@@ -33,27 +33,20 @@ func TestIsResponsibleForRoute(t *testing.T) {
clusterCIDR string
routeName string
routeCIDR string
routeDescription string
expectedResponsible bool
}{
// Routes that belong to this cluster
{"10.244.0.0/16", myClusterRoute, "10.244.0.0/24", "k8s-node-route", true},
{"10.244.0.0/16", myClusterRoute, "10.244.10.0/24", "k8s-node-route", true},
{"10.244.0.0/16", myClusterRoute, "10.244.255.0/24", "k8s-node-route", true},
{"10.244.0.0/14", myClusterRoute, "10.244.0.0/24", "k8s-node-route", true},
{"10.244.0.0/14", myClusterRoute, "10.247.255.0/24", "k8s-node-route", true},
// Routes inside our cidr, but not named how we would have named them
{"10.244.0.0/16", "background-cluster-route", "10.244.0.0/16", "k8s-node-route", false},
{"10.244.0.0/16", "special-single-route", "10.244.12.34/32", "k8s-node-route", false},
// Routes inside our cidr, but not tagged how we would have tagged them in the description
{"10.244.0.0/16", "my-awesome-cluster-background", "10.244.0.0/16", "", false},
{"10.244.0.0/16", "my-awesome-cluster-single-route", "10.244.12.34/32", "this is a route", false},
{"10.244.0.0/16", myClusterRoute, "10.244.0.0/24", true},
{"10.244.0.0/16", myClusterRoute, "10.244.10.0/24", true},
{"10.244.0.0/16", myClusterRoute, "10.244.255.0/24", true},
{"10.244.0.0/14", myClusterRoute, "10.244.0.0/24", true},
{"10.244.0.0/14", myClusterRoute, "10.247.255.0/24", true},
// Routes that match our naming/tagging scheme, but are outside our cidr
{"10.244.0.0/16", myClusterRoute, "10.224.0.0/24", "k8s-node-route", false},
{"10.244.0.0/16", myClusterRoute, "10.0.10.0/24", "k8s-node-route", false},
{"10.244.0.0/16", myClusterRoute, "10.255.255.0/24", "k8s-node-route", false},
{"10.244.0.0/14", myClusterRoute, "10.248.0.0/24", "k8s-node-route", false},
{"10.244.0.0/14", myClusterRoute, "10.243.255.0/24", "k8s-node-route", false},
{"10.244.0.0/16", myClusterRoute, "10.224.0.0/24", false},
{"10.244.0.0/16", myClusterRoute, "10.0.10.0/24", false},
{"10.244.0.0/16", myClusterRoute, "10.255.255.0/24", false},
{"10.244.0.0/14", myClusterRoute, "10.248.0.0/24", false},
{"10.244.0.0/14", myClusterRoute, "10.243.255.0/24", false},
}
for i, testCase := range testCases {
_, cidr, err := net.ParseCIDR(testCase.clusterCIDR)
@@ -65,7 +58,6 @@ func TestIsResponsibleForRoute(t *testing.T) {
Name: testCase.routeName,
TargetInstance: "doesnt-matter-for-this-test",
DestinationCIDR: testCase.routeCIDR,
Description: testCase.routeDescription,
}
if resp := rc.isResponsibleForRoute(route); resp != testCase.expectedResponsible {
t.Errorf("%d. isResponsibleForRoute() = %t; want %t", i, resp, testCase.expectedResponsible)
@@ -87,12 +79,12 @@ func TestReconcile(t *testing.T) {
{ObjectMeta: api.ObjectMeta{Name: "node-2", UID: "02"}, Spec: api.NodeSpec{PodCIDR: "10.120.1.0/24"}},
},
initialRoutes: []*cloudprovider.Route{
{cluster + "-01", "node-1", "10.120.0.0/24", "k8s-node-route"},
{cluster + "-02", "node-2", "10.120.1.0/24", "k8s-node-route"},
{cluster + "-01", "node-1", "10.120.0.0/24"},
{cluster + "-02", "node-2", "10.120.1.0/24"},
},
expectedRoutes: []*cloudprovider.Route{
{cluster + "-01", "node-1", "10.120.0.0/24", "k8s-node-route"},
{cluster + "-02", "node-2", "10.120.1.0/24", "k8s-node-route"},
{cluster + "-01", "node-1", "10.120.0.0/24"},
{cluster + "-02", "node-2", "10.120.1.0/24"},
},
},
// 2 nodes, one route already there
@@ -102,11 +94,11 @@ func TestReconcile(t *testing.T) {
{ObjectMeta: api.ObjectMeta{Name: "node-2", UID: "02"}, Spec: api.NodeSpec{PodCIDR: "10.120.1.0/24"}},
},
initialRoutes: []*cloudprovider.Route{
{cluster + "-01", "node-1", "10.120.0.0/24", "k8s-node-route"},
{cluster + "-01", "node-1", "10.120.0.0/24"},
},
expectedRoutes: []*cloudprovider.Route{
{cluster + "-01", "node-1", "10.120.0.0/24", "k8s-node-route"},
{cluster + "-02", "node-2", "10.120.1.0/24", "k8s-node-route"},
{cluster + "-01", "node-1", "10.120.0.0/24"},
{cluster + "-02", "node-2", "10.120.1.0/24"},
},
},
// 2 nodes, no routes yet
@@ -117,8 +109,8 @@ func TestReconcile(t *testing.T) {
},
initialRoutes: []*cloudprovider.Route{},
expectedRoutes: []*cloudprovider.Route{
{cluster + "-01", "node-1", "10.120.0.0/24", "k8s-node-route"},
{cluster + "-02", "node-2", "10.120.1.0/24", "k8s-node-route"},
{cluster + "-01", "node-1", "10.120.0.0/24"},
{cluster + "-02", "node-2", "10.120.1.0/24"},
},
},
// 2 nodes, a few too many routes
@@ -128,14 +120,14 @@ func TestReconcile(t *testing.T) {
{ObjectMeta: api.ObjectMeta{Name: "node-2", UID: "02"}, Spec: api.NodeSpec{PodCIDR: "10.120.1.0/24"}},
},
initialRoutes: []*cloudprovider.Route{
{cluster + "-01", "node-1", "10.120.0.0/24", "k8s-node-route"},
{cluster + "-02", "node-2", "10.120.1.0/24", "k8s-node-route"},
{cluster + "-03", "node-3", "10.120.2.0/24", "k8s-node-route"},
{cluster + "-04", "node-4", "10.120.3.0/24", "k8s-node-route"},
{cluster + "-01", "node-1", "10.120.0.0/24"},
{cluster + "-02", "node-2", "10.120.1.0/24"},
{cluster + "-03", "node-3", "10.120.2.0/24"},
{cluster + "-04", "node-4", "10.120.3.0/24"},
},
expectedRoutes: []*cloudprovider.Route{
{cluster + "-01", "node-1", "10.120.0.0/24", "k8s-node-route"},
{cluster + "-02", "node-2", "10.120.1.0/24", "k8s-node-route"},
{cluster + "-01", "node-1", "10.120.0.0/24"},
{cluster + "-02", "node-2", "10.120.1.0/24"},
},
},
// 2 nodes, 2 routes, but only 1 is right
@@ -145,19 +137,22 @@ func TestReconcile(t *testing.T) {
{ObjectMeta: api.ObjectMeta{Name: "node-2", UID: "02"}, Spec: api.NodeSpec{PodCIDR: "10.120.1.0/24"}},
},
initialRoutes: []*cloudprovider.Route{
{cluster + "-01", "node-1", "10.120.0.0/24", "k8s-node-route"},
{cluster + "-03", "node-3", "10.120.2.0/24", "k8s-node-route"},
{cluster + "-01", "node-1", "10.120.0.0/24"},
{cluster + "-03", "node-3", "10.120.2.0/24"},
},
expectedRoutes: []*cloudprovider.Route{
{cluster + "-01", "node-1", "10.120.0.0/24", "k8s-node-route"},
{cluster + "-02", "node-2", "10.120.1.0/24", "k8s-node-route"},
{cluster + "-01", "node-1", "10.120.0.0/24"},
{cluster + "-02", "node-2", "10.120.1.0/24"},
},
},
}
for i, testCase := range testCases {
cloud := &fake_cloud.FakeCloud{RouteMap: make(map[string]*cloudprovider.Route)}
cloud := &fake_cloud.FakeCloud{RouteMap: make(map[string]*fake_cloud.FakeRoute)}
for _, route := range testCase.initialRoutes {
cloud.RouteMap[route.Name] = route
fakeRoute := &fake_cloud.FakeRoute{}
fakeRoute.ClusterName = cluster
fakeRoute.Route = *route
cloud.RouteMap[route.Name] = fakeRoute
}
routes, ok := cloud.Routes()
if !ok {
@@ -177,7 +172,7 @@ func TestReconcile(t *testing.T) {
for {
select {
case <-tick.C:
if finalRoutes, err = routes.ListRoutes(""); err == nil && routeListEqual(finalRoutes, testCase.expectedRoutes) {
if finalRoutes, err = routes.ListRoutes(cluster); err == nil && routeListEqual(finalRoutes, testCase.expectedRoutes) {
break poll
}
case <-timeoutChan: