From 0d12a15971cd289ef4482f6743ea5637d188e3b4 Mon Sep 17 00:00:00 2001 From: CJ Cullen Date: Fri, 15 May 2015 14:49:26 -0700 Subject: [PATCH] Route creation reconciler loop. --- cluster/gce/util.sh | 2 +- .../app/controllermanager.go | 10 + pkg/cloudprovider/aws/aws.go | 13 +- pkg/cloudprovider/cloud.go | 23 +- pkg/cloudprovider/fake/fake.go | 49 ++++- pkg/cloudprovider/gce/gce.go | 47 ++-- pkg/cloudprovider/mesos/mesos.go | 17 +- .../nodecontroller/nodecontroller.go | 27 --- pkg/cloudprovider/openstack/openstack.go | 12 +- pkg/cloudprovider/ovirt/ovirt.go | 13 +- pkg/cloudprovider/rackspace/rackspace.go | 13 +- pkg/cloudprovider/routecontroller/doc.go | 19 ++ .../routecontroller/routecontroller.go | 145 +++++++++++++ .../routecontroller/routecontroller_test.go | 200 ++++++++++++++++++ pkg/cloudprovider/vagrant/vagrant.go | 13 +- 15 files changed, 498 insertions(+), 105 deletions(-) create mode 100644 pkg/cloudprovider/routecontroller/doc.go create mode 100644 pkg/cloudprovider/routecontroller/routecontroller.go create mode 100644 pkg/cloudprovider/routecontroller/routecontroller_test.go diff --git a/cluster/gce/util.sh b/cluster/gce/util.sh index 3d3b513e19..efe5c10fc9 100755 --- a/cluster/gce/util.sh +++ b/cluster/gce/util.sh @@ -772,7 +772,7 @@ function kube-down { # Delete routes. local -a routes routes=( $(gcloud compute routes list --project "${PROJECT}" \ - --regexp "${NODE_INSTANCE_PREFIX}-.+" | awk 'NR >= 2 { print $1 }') ) + --regexp "${INSTANCE_PREFIX}-.{8}-.{4}-.{4}-.{4}-.{12}" | awk 'NR >= 2 { print $1 }') ) routes+=("${MASTER_NAME}") while (( "${#routes[@]}" > 0 )); do echo Deleting routes "${routes[*]::10}" diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index 11278bd084..4b404af064 100644 --- a/cmd/kube-controller-manager/app/controllermanager.go +++ b/cmd/kube-controller-manager/app/controllermanager.go @@ -33,6 +33,7 @@ import ( clientcmdapi "github.com/GoogleCloudPlatform/kubernetes/pkg/client/clientcmd/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider/nodecontroller" + "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider/routecontroller" "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider/servicecontroller" replicationControllerPkg "github.com/GoogleCloudPlatform/kubernetes/pkg/controller" "github.com/GoogleCloudPlatform/kubernetes/pkg/healthz" @@ -238,6 +239,15 @@ func (s *CMServer) Run(_ []string) error { glog.Errorf("Failed to start service controller: %v", err) } + if s.AllocateNodeCIDRs { + routes, ok := cloud.Routes() + if !ok { + glog.Fatal("Cloud provider must support routes if allocate-node-cidrs is set") + } + routeController := routecontroller.New(routes, kubeClient, s.ClusterName, (*net.IPNet)(&s.ClusterCIDR)) + routeController.Run(s.NodeSyncPeriod) + } + resourceQuotaManager := resourcequota.NewResourceQuotaManager(kubeClient) resourceQuotaManager.Run(s.ResourceQuotaSyncPeriod) diff --git a/pkg/cloudprovider/aws/aws.go b/pkg/cloudprovider/aws/aws.go index d369634381..0b91b3caa1 100644 --- a/pkg/cloudprovider/aws/aws.go +++ b/pkg/cloudprovider/aws/aws.go @@ -273,6 +273,11 @@ func (aws *AWSCloud) Zones() (cloudprovider.Zones, bool) { return aws, true } +// Routes returns an implementation of Routes for Amazon Web Services. +func (aws *AWSCloud) Routes() (cloudprovider.Routes, bool) { + return nil, false +} + // NodeAddresses is an implementation of Instances.NodeAddresses. func (aws *AWSCloud) NodeAddresses(name string) ([]api.NodeAddress, error) { instance, err := aws.getInstancesByDnsName(name) @@ -973,11 +978,3 @@ func (aws *AWSCloud) DeleteVolume(volumeName string) error { } return awsDisk.delete() } - -func (v *AWSCloud) Configure(name string, spec *api.NodeSpec) error { - return nil -} - -func (v *AWSCloud) Release(name string) error { - return nil -} diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index f62b31ff6c..bc487d7ec4 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -34,6 +34,8 @@ type Interface interface { Zones() (Zones, bool) // Clusters returns a clusters interface. Also returns true if the interface is supported, false otherwise. Clusters() (Clusters, bool) + // Routes returns a routes interface along with whether the interface is supported. + Routes() (Routes, bool) } // Clusters is an abstract, pluggable interface for clusters of containers. @@ -81,10 +83,23 @@ type Instances interface { List(filter string) ([]string, error) // GetNodeResources gets the resources for a particular node GetNodeResources(name string) (*api.NodeResources, error) - // Configure the specified instance using the spec - Configure(name string, spec *api.NodeSpec) error - // Delete all the configuration related to the instance, including other cloud resources - Release(name string) error +} + +// Route is a representation of an advanced routing rule. +type Route struct { + Name string + TargetInstance string + DestinationCIDR string + Description string +} + +// Routes is an abstract, pluggable interface for advanced routing rules. +type Routes interface { + ListRoutes(filter string) ([]*Route, error) + // Create the described route + CreateRoute(route *Route) error + // Delete the specified route + DeleteRoute(name string) error } var InstanceNotFound = errors.New("instance not found") diff --git a/pkg/cloudprovider/fake/fake.go b/pkg/cloudprovider/fake/fake.go index c0daf8832c..9871130ce2 100644 --- a/pkg/cloudprovider/fake/fake.go +++ b/pkg/cloudprovider/fake/fake.go @@ -17,8 +17,10 @@ limitations under the License. package fake_cloud import ( + "fmt" "net" "regexp" + "sync" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" @@ -39,7 +41,7 @@ type FakeUpdateBalancerCall struct { Hosts []string } -// FakeCloud is a test-double implementation of Interface, TCPLoadBalancer and Instances. It is useful for testing. +// FakeCloud is a test-double implementation of Interface, TCPLoadBalancer, Instances, and Routes. It is useful for testing. type FakeCloud struct { Exists bool Err error @@ -53,6 +55,8 @@ type FakeCloud struct { ExternalIP net.IP Balancers []FakeBalancer UpdateCalls []FakeUpdateBalancerCall + RouteMap map[string]*cloudprovider.Route + Lock sync.Mutex cloudprovider.Zone } @@ -94,6 +98,10 @@ func (f *FakeCloud) Zones() (cloudprovider.Zones, bool) { return f, true } +func (f *FakeCloud) Routes() (cloudprovider.Routes, bool) { + return f, true +} + // GetTCPLoadBalancer is a stub implementation of TCPLoadBalancer.GetTCPLoadBalancer. func (f *FakeCloud) GetTCPLoadBalancer(name, region string) (endpoint string, exists bool, err error) { return f.ExternalIP.String(), f.Exists, f.Err @@ -160,12 +168,39 @@ func (f *FakeCloud) GetNodeResources(name string) (*api.NodeResources, error) { return f.NodeResources, f.Err } -func (f *FakeCloud) Configure(name string, spec *api.NodeSpec) error { - f.addCall("configure") - return f.Err +func (f *FakeCloud) ListRoutes(filter 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) + } + } + return routes, f.Err } -func (f *FakeCloud) Release(name string) error { - f.addCall("release") - return f.Err +func (f *FakeCloud) CreateRoute(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") + return f.Err + } + f.RouteMap[route.Name] = route + return nil +} + +func (f *FakeCloud) DeleteRoute(name string) error { + f.Lock.Lock() + defer f.Lock.Unlock() + f.addCall("delete-route") + if _, exists := f.RouteMap[name]; !exists { + f.Err = fmt.Errorf("no route found with name %q", name) + return f.Err + } + delete(f.RouteMap, name) + return nil } diff --git a/pkg/cloudprovider/gce/gce.go b/pkg/cloudprovider/gce/gce.go index 08f7a090ab..4fcc639bfe 100644 --- a/pkg/cloudprovider/gce/gce.go +++ b/pkg/cloudprovider/gce/gce.go @@ -195,6 +195,11 @@ func (gce *GCECloud) Zones() (cloudprovider.Zones, bool) { return gce, true } +// Routes returns an implementation of Routes for Google Compute Engine. +func (gce *GCECloud) Routes() (cloudprovider.Routes, bool) { + return gce, true +} + func makeHostLink(projectID, zone, host string) string { host = canonicalizeInstanceName(host) return fmt.Sprintf("https://www.googleapis.com/compute/v1/projects/%s/zones/%s/instances/%s", @@ -558,31 +563,45 @@ func getMetadataValue(metadata *compute.Metadata, key string) (string, bool) { return "", false } -func (gce *GCECloud) Configure(name string, spec *api.NodeSpec) error { - instanceName := canonicalizeInstanceName(name) +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) + } + res, err := listCall.Do() + if err != nil { + return nil, err + } + var routes []*cloudprovider.Route + for _, r := range res.Items { + if path.Base(r.Network) != gce.networkName { + continue + } + target := path.Base(r.NextHopInstance) + routes = append(routes, &cloudprovider.Route{r.Name, target, r.DestRange, r.Description}) + } + return routes, nil +} + +func (gce *GCECloud) CreateRoute(route *cloudprovider.Route) error { + instanceName := canonicalizeInstanceName(route.TargetInstance) insertOp, err := gce.service.Routes.Insert(gce.projectID, &compute.Route{ - Name: instanceName, - DestRange: spec.PodCIDR, + Name: route.Name, + 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, }).Do() if err != nil { return err } - if err := gce.waitForGlobalOp(insertOp); err != nil { - if gapiErr, ok := err.(*googleapi.Error); ok && gapiErr.Code == http.StatusConflict { - // TODO (cjcullen): Make this actually check the route is correct. - return nil - } - } - return err + return gce.waitForGlobalOp(insertOp) } -func (gce *GCECloud) Release(name string) error { +func (gce *GCECloud) DeleteRoute(name string) error { instanceName := canonicalizeInstanceName(name) - deleteCall := gce.service.Routes.Delete(gce.projectID, instanceName) - deleteOp, err := deleteCall.Do() + deleteOp, err := gce.service.Routes.Delete(gce.projectID, instanceName).Do() if err != nil { return err } diff --git a/pkg/cloudprovider/mesos/mesos.go b/pkg/cloudprovider/mesos/mesos.go index 7334f8560a..97cd5407d1 100644 --- a/pkg/cloudprovider/mesos/mesos.go +++ b/pkg/cloudprovider/mesos/mesos.go @@ -105,6 +105,11 @@ func (c *MesosCloud) Clusters() (cloudprovider.Clusters, bool) { return c, true } +// Routes always returns nil, false in this implementation. +func (c *MesosCloud) Routes() (cloudprovider.Routes, bool) { + return nil, false +} + // ListClusters lists the names of the available Mesos clusters. func (c *MesosCloud) ListClusters() ([]string, error) { // Always returns a single cluster (this one!) @@ -224,15 +229,3 @@ func (c *MesosCloud) NodeAddresses(name string) ([]api.NodeAddress, error) { } return []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: ip.String()}}, nil } - -// Configure the specified instance using the spec. -// Ths implementation is a noop. -func (c *MesosCloud) Configure(name string, spec *api.NodeSpec) error { - return nil -} - -// Release deletes all the configuration related to the instance, including other cloud resources. -// Ths implementation is a noop. -func (c *MesosCloud) Release(name string) error { - return nil -} diff --git a/pkg/cloudprovider/nodecontroller/nodecontroller.go b/pkg/cloudprovider/nodecontroller/nodecontroller.go index e9aa54e0f8..166522f201 100644 --- a/pkg/cloudprovider/nodecontroller/nodecontroller.go +++ b/pkg/cloudprovider/nodecontroller/nodecontroller.go @@ -170,11 +170,6 @@ func (nc *NodeController) reconcilePodCIDRs(nodes *api.NodeList) { } glog.V(4).Infof("Assigning node %s CIDR %s", node.Name, podCIDR) node.Spec.PodCIDR = podCIDR - if err := nc.configureNodeCIDR(&node); err != nil { - glog.Errorf("Error configuring node %s: %s", node.Name, err) - // The newly assigned CIDR was not properly configured, so don't save it in the API server. - continue - } if _, err := nc.kubeClient.Nodes().Update(&node); err != nil { glog.Errorf("Unable to assign node %s CIDR %s: %v", node.Name, podCIDR, err) } @@ -182,25 +177,6 @@ func (nc *NodeController) reconcilePodCIDRs(nodes *api.NodeList) { } } -func (nc *NodeController) configureNodeCIDR(node *api.Node) error { - instances, ok := nc.cloud.Instances() - if !ok { - return fmt.Errorf("error configuring node %s: CloudProvider does not support Instances()", node.Name) - } - return instances.Configure(node.Name, &node.Spec) -} - -func (nc *NodeController) unassignNodeCIDR(nodeName string) { - instances, ok := nc.cloud.Instances() - if !ok { - glog.Errorf("Error deconfiguring node %s: CloudProvider does not support Instances()", nodeName) - return - } - if err := instances.Release(nodeName); err != nil { - glog.Errorf("Error deconfiguring node %s: %s", nodeName, err) - } -} - // Run starts an asynchronous loop that monitors the status of cluster nodes. func (nc *NodeController) Run(period time.Duration, syncNodeList bool) { // Incorporate the results of node status pushed from kubelet to master. @@ -435,9 +411,6 @@ func (nc *NodeController) monitorNodeStatus() error { continue } if _, err := instances.ExternalID(node.Name); err != nil && err == cloudprovider.InstanceNotFound { - if nc.allocateNodeCIDRs { - nc.unassignNodeCIDR(node.Name) - } if err := nc.kubeClient.Nodes().Delete(node.Name); err != nil { glog.Errorf("Unable to delete node %s: %v", node.Name, err) continue diff --git a/pkg/cloudprovider/openstack/openstack.go b/pkg/cloudprovider/openstack/openstack.go index 5f80d97b31..357a1be538 100644 --- a/pkg/cloudprovider/openstack/openstack.go +++ b/pkg/cloudprovider/openstack/openstack.go @@ -389,14 +389,6 @@ func (i *Instances) GetNodeResources(name string) (*api.NodeResources, error) { return rsrc, nil } -func (i *Instances) Configure(name string, spec *api.NodeSpec) error { - return nil -} - -func (i *Instances) Release(name string) error { - return nil -} - func (os *OpenStack) Clusters() (cloudprovider.Clusters, bool) { return nil, false } @@ -669,3 +661,7 @@ func (os *OpenStack) GetZone() (cloudprovider.Zone, error) { return cloudprovider.Zone{Region: os.region}, nil } + +func (os *OpenStack) Routes() (cloudprovider.Routes, bool) { + return nil, false +} diff --git a/pkg/cloudprovider/ovirt/ovirt.go b/pkg/cloudprovider/ovirt/ovirt.go index 68156c5252..f4325a4f7d 100644 --- a/pkg/cloudprovider/ovirt/ovirt.go +++ b/pkg/cloudprovider/ovirt/ovirt.go @@ -130,6 +130,11 @@ func (v *OVirtCloud) Zones() (cloudprovider.Zones, bool) { return nil, false } +// Routes returns an implementation of Routes for oVirt cloud +func (v *OVirtCloud) Routes() (cloudprovider.Routes, bool) { + return nil, false +} + // NodeAddresses returns the NodeAddresses of a particular machine instance func (v *OVirtCloud) NodeAddresses(name string) ([]api.NodeAddress, error) { instance, err := v.fetchInstance(name) @@ -250,11 +255,3 @@ func (v *OVirtCloud) List(filter string) ([]string, error) { func (v *OVirtCloud) GetNodeResources(name string) (*api.NodeResources, error) { return nil, nil } - -func (v *OVirtCloud) Configure(name string, spec *api.NodeSpec) error { - return nil -} - -func (v *OVirtCloud) Release(name string) error { - return nil -} diff --git a/pkg/cloudprovider/rackspace/rackspace.go b/pkg/cloudprovider/rackspace/rackspace.go index 8bc8f5c633..72fce7bcc0 100644 --- a/pkg/cloudprovider/rackspace/rackspace.go +++ b/pkg/cloudprovider/rackspace/rackspace.go @@ -395,14 +395,6 @@ func (i *Instances) GetNodeResources(name string) (*api.NodeResources, error) { return rsrc, nil } -func (i *Instances) Configure(name string, spec *api.NodeSpec) error { - return nil -} - -func (i *Instances) Release(name string) error { - return nil -} - func (os *Rackspace) Clusters() (cloudprovider.Clusters, bool) { return nil, false } @@ -416,6 +408,11 @@ func (os *Rackspace) Zones() (cloudprovider.Zones, bool) { return os, true } + +func (os *Rackspace) Routes() (cloudprovider.Routes, bool) { + return nil, false +} + func (os *Rackspace) GetZone() (cloudprovider.Zone, error) { glog.V(1).Infof("Current zone is %v", os.region) diff --git a/pkg/cloudprovider/routecontroller/doc.go b/pkg/cloudprovider/routecontroller/doc.go new file mode 100644 index 0000000000..8ad76248a5 --- /dev/null +++ b/pkg/cloudprovider/routecontroller/doc.go @@ -0,0 +1,19 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +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 routecontroller contains code for syncing cloud routing rules with +// the list of registered nodes. +package routecontroller diff --git a/pkg/cloudprovider/routecontroller/routecontroller.go b/pkg/cloudprovider/routecontroller/routecontroller.go new file mode 100644 index 0000000000..1ba08a7e63 --- /dev/null +++ b/pkg/cloudprovider/routecontroller/routecontroller.go @@ -0,0 +1,145 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +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 routecontroller + +import ( + "fmt" + "net" + "strings" + "time" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/client" + "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" + "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/golang/glog" +) + +type RouteController struct { + routes cloudprovider.Routes + kubeClient client.Interface + clusterName string + 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, + kubeClient: kubeClient, + clusterName: clusterName, + clusterCIDR: clusterCIDR, + } +} + +func (rc *RouteController) Run(syncPeriod time.Duration) { + go util.Forever(func() { + if err := rc.reconcileNodeRoutes(); err != nil { + glog.Errorf("Couldn't reconcile node routes: %v", err) + } + }, syncPeriod) +} + +func (rc *RouteController) reconcileNodeRoutes() error { + routeList, err := rc.routes.ListRoutes(rc.truncatedClusterName() + "-.*") + if err != nil { + return fmt.Errorf("error listing routes: %v", err) + } + nodeList, err := rc.kubeClient.Nodes().List(labels.Everything(), fields.Everything()) + if err != nil { + return fmt.Errorf("error listing nodes: %v", err) + } + return rc.reconcile(nodeList.Items, routeList) +} + +func (rc *RouteController) reconcile(nodes []api.Node, routes []*cloudprovider.Route) error { + // nodeCIDRs maps nodeName->nodeCIDR + nodeCIDRs := make(map[string]string) + // routeMap maps routeTargetInstance->route + routeMap := make(map[string]*cloudprovider.Route) + for _, route := range routes { + routeMap[route.TargetInstance] = route + } + for _, node := range nodes { + // Check if we have a route for this node w/ the correct CIDR. + r := routeMap[node.Name] + 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) + } + }(route) + } + nodeCIDRs[node.Name] = node.Spec.PodCIDR + } + for _, route := range routes { + if rc.isResponsibleForRoute(route) { + // 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) + } + }(route.Name) + } + } + } + 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 { + glog.Errorf("Ignoring route %s, unparsable CIDR: %v", route.Name, err) + return false + } + // Not responsible if this route's CIDR is not within our clusterCIDR + lastIP := make([]byte, len(cidr.IP)) + for i := range lastIP { + lastIP[i] = cidr.IP[i] | ^cidr.Mask[i] + } + if !rc.clusterCIDR.Contains(cidr.IP) || !rc.clusterCIDR.Contains(lastIP) { + return false + } + // Not responsible if route name doesn't start with + 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 +} diff --git a/pkg/cloudprovider/routecontroller/routecontroller_test.go b/pkg/cloudprovider/routecontroller/routecontroller_test.go new file mode 100644 index 0000000000..baf68e2eeb --- /dev/null +++ b/pkg/cloudprovider/routecontroller/routecontroller_test.go @@ -0,0 +1,200 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +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 routecontroller + +import ( + "net" + "testing" + "time" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" + "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider/fake" +) + +func TestIsResponsibleForRoute(t *testing.T) { + myClusterName := "my-awesome-cluster" + myClusterRoute := "my-awesome-cluster-12345678-90ab-cdef-1234-567890abcdef" + testCases := []struct { + 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}, + // 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}, + } + for i, testCase := range testCases { + _, cidr, err := net.ParseCIDR(testCase.clusterCIDR) + if err != nil { + t.Errorf("%d. Error in test case: unparsable cidr %q", i, testCase.clusterCIDR) + } + rc := New(nil, nil, myClusterName, cidr) + route := &cloudprovider.Route{ + 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) + } + } +} + +func TestReconcile(t *testing.T) { + cluster := "my-k8s" + testCases := []struct { + nodes []api.Node + initialRoutes []*cloudprovider.Route + expectedRoutes []*cloudprovider.Route + }{ + // 2 nodes, routes already there + { + nodes: []api.Node{ + {ObjectMeta: api.ObjectMeta{Name: "node-1", UID: "01"}, Spec: api.NodeSpec{PodCIDR: "10.120.0.0/24"}}, + {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"}, + }, + 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"}, + }, + }, + // 2 nodes, one route already there + { + nodes: []api.Node{ + {ObjectMeta: api.ObjectMeta{Name: "node-1", UID: "01"}, Spec: api.NodeSpec{PodCIDR: "10.120.0.0/24"}}, + {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"}, + }, + 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"}, + }, + }, + // 2 nodes, no routes yet + { + nodes: []api.Node{ + {ObjectMeta: api.ObjectMeta{Name: "node-1", UID: "01"}, Spec: api.NodeSpec{PodCIDR: "10.120.0.0/24"}}, + {ObjectMeta: api.ObjectMeta{Name: "node-2", UID: "02"}, Spec: api.NodeSpec{PodCIDR: "10.120.1.0/24"}}, + }, + 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"}, + }, + }, + // 2 nodes, a few too many routes + { + nodes: []api.Node{ + {ObjectMeta: api.ObjectMeta{Name: "node-1", UID: "01"}, Spec: api.NodeSpec{PodCIDR: "10.120.0.0/24"}}, + {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"}, + }, + 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"}, + }, + }, + // 2 nodes, 2 routes, but only 1 is right + { + nodes: []api.Node{ + {ObjectMeta: api.ObjectMeta{Name: "node-1", UID: "01"}, Spec: api.NodeSpec{PodCIDR: "10.120.0.0/24"}}, + {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"}, + }, + 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"}, + }, + }, + } + for i, testCase := range testCases { + cloud := &fake_cloud.FakeCloud{RouteMap: make(map[string]*cloudprovider.Route)} + for _, route := range testCase.initialRoutes { + cloud.RouteMap[route.Name] = route + } + routes, ok := cloud.Routes() + if !ok { + t.Error("Error in test: fake_cloud doesn't support Routes()") + } + _, cidr, _ := net.ParseCIDR("10.120.0.0/16") + rc := New(routes, nil, cluster, cidr) + if err := rc.reconcile(testCase.nodes, testCase.initialRoutes); err != nil { + t.Errorf("%d. Error from rc.reconcile(): %v", i, err) + } + time.Sleep(10 * time.Millisecond) + if finalRoutes, err := routes.ListRoutes(""); err != nil || !routeListEqual(finalRoutes, testCase.expectedRoutes) { + t.Errorf("%d. rc.reconcile() = %v, routes:\n%v\nexpected: nil, routes:\n%v\n", i, err, flatten(finalRoutes), flatten(testCase.expectedRoutes)) + } + } +} + +func routeListEqual(list1, list2 []*cloudprovider.Route) bool { + if len(list1) != len(list2) { + return false + } + routeMap1 := make(map[string]*cloudprovider.Route) + for _, route1 := range list1 { + routeMap1[route1.Name] = route1 + } + for _, route2 := range list2 { + if route1, exists := routeMap1[route2.Name]; !exists || *route1 != *route2 { + return false + } + } + return true +} + +func flatten(list []*cloudprovider.Route) []cloudprovider.Route { + var structList []cloudprovider.Route + for _, route := range list { + structList = append(structList, *route) + } + return structList +} diff --git a/pkg/cloudprovider/vagrant/vagrant.go b/pkg/cloudprovider/vagrant/vagrant.go index fa45180ac9..825802d150 100644 --- a/pkg/cloudprovider/vagrant/vagrant.go +++ b/pkg/cloudprovider/vagrant/vagrant.go @@ -99,6 +99,11 @@ func (v *VagrantCloud) Zones() (cloudprovider.Zones, bool) { return nil, false } +// Routes returns an implementation of Routes for Vagrant cloud. +func (v *VagrantCloud) Routes() (cloudprovider.Routes, bool) { + return nil, false +} + // getInstanceByAddress retuns func (v *VagrantCloud) getInstanceByAddress(address string) (*SaltMinion, error) { token, err := v.saltLogin() @@ -239,11 +244,3 @@ func (v *VagrantCloud) List(filter string) ([]string, error) { func (v *VagrantCloud) GetNodeResources(name string) (*api.NodeResources, error) { return nil, nil } - -func (v *VagrantCloud) Configure(name string, spec *api.NodeSpec) error { - return nil -} - -func (v *VagrantCloud) Release(name string) error { - return nil -}