From a3b43a36fdf80feed8a457b34b74aa3c863020d4 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Fri, 12 Jun 2015 12:27:10 -0400 Subject: [PATCH 1/3] Refactor cloud route interface, to avoid assumption that routes are named --- pkg/cloudprovider/cloud.go | 18 +++-- pkg/cloudprovider/fake/fake.go | 32 +++++--- pkg/cloudprovider/gce/gce.go | 42 ++++++++--- .../routecontroller/routecontroller.go | 39 +++------- .../routecontroller/routecontroller_test.go | 75 +++++++++---------- 5 files changed, 108 insertions(+), 98 deletions(-) diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index 3e0d50086ab..6f109aa710d 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -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") diff --git a/pkg/cloudprovider/fake/fake.go b/pkg/cloudprovider/fake/fake.go index 96f39e3abde..29393b34670 100644 --- a/pkg/cloudprovider/fake/fake.go +++ b/pkg/cloudprovider/fake/fake.go @@ -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 diff --git a/pkg/cloudprovider/gce/gce.go b/pkg/cloudprovider/gce/gce.go index 76f3b10b6e6..e18b4ca5902 100644 --- a/pkg/cloudprovider/gce/gce.go +++ b/pkg/cloudprovider/gce/gce.go @@ -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 + 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 } diff --git a/pkg/cloudprovider/routecontroller/routecontroller.go b/pkg/cloudprovider/routecontroller/routecontroller.go index 5cbe5859764..2f461e9c8c5 100644 --- a/pkg/cloudprovider/routecontroller/routecontroller.go +++ b/pkg/cloudprovider/routecontroller/routecontroller.go @@ -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 - 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 index 3d256b4b91b..d5a550262e9 100644 --- a/pkg/cloudprovider/routecontroller/routecontroller_test.go +++ b/pkg/cloudprovider/routecontroller/routecontroller_test.go @@ -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: From a4e15cdf3e16c688569d766c38cbc635030835b0 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Fri, 12 Jun 2015 12:33:17 -0400 Subject: [PATCH 2/3] AWS: Configure minion routes dynamically We need to implement the Routes interface, and then enable the functionality in the cluster scripts. --- cluster/aws/config-default.sh | 2 +- cluster/aws/config-test.sh | 2 +- cluster/aws/coreos/util.sh | 3 +- .../templates/create-dynamic-salt-files.sh | 2 + cluster/aws/templates/salt-minion.sh | 2 +- cluster/aws/ubuntu/common.sh | 1 - cluster/aws/util.sh | 8 +- pkg/cloudprovider/aws/aws.go | 23 +++- pkg/cloudprovider/aws/aws_routes.go | 114 ++++++++++++++++++ pkg/cloudprovider/aws/aws_test.go | 12 ++ 10 files changed, 160 insertions(+), 9 deletions(-) create mode 100644 pkg/cloudprovider/aws/aws_routes.go diff --git a/cluster/aws/config-default.sh b/cluster/aws/config-default.sh index 8ed226a48c1..dd961a6903c 100644 --- a/cluster/aws/config-default.sh +++ b/cluster/aws/config-default.sh @@ -41,10 +41,10 @@ MASTER_NAME="${INSTANCE_PREFIX}-master" MINION_NAMES=($(eval echo ${INSTANCE_PREFIX}-minion-{1..${NUM_MINIONS}})) MASTER_TAG="${INSTANCE_PREFIX}-master" MINION_TAG="${INSTANCE_PREFIX}-minion" -MINION_IP_RANGES=($(eval echo "10.244.{1..${NUM_MINIONS}}.0/24")) MINION_SCOPES="" POLL_SLEEP_INTERVAL=3 SERVICE_CLUSTER_IP_RANGE="10.0.0.0/16" # formerly PORTAL_NET +CLUSTER_IP_RANGE="${CLUSTER_IP_RANGE:-10.244.0.0/16}" MASTER_IP_RANGE="${MASTER_IP_RANGE:-10.246.0.0/24}" # If set to Elastic IP, master instance will be associated with this IP. # If set to auto, a new Elastic IP will be aquired diff --git a/cluster/aws/config-test.sh b/cluster/aws/config-test.sh index 87cdf8c29cf..5c51fd380fe 100755 --- a/cluster/aws/config-test.sh +++ b/cluster/aws/config-test.sh @@ -37,10 +37,10 @@ MASTER_NAME="${INSTANCE_PREFIX}-master" MINION_NAMES=($(eval echo ${INSTANCE_PREFIX}-minion-{1..${NUM_MINIONS}})) MASTER_TAG="${INSTANCE_PREFIX}-master" MINION_TAG="${INSTANCE_PREFIX}-minion" -MINION_IP_RANGES=($(eval echo "10.244.{1..${NUM_MINIONS}}.0/24")) MINION_SCOPES="" POLL_SLEEP_INTERVAL=3 SERVICE_CLUSTER_IP_RANGE="10.0.0.0/16" # formerly PORTAL_NET +CLUSTER_IP_RANGE="${CLUSTER_IP_RANGE:-10.245.0.0/16}" MASTER_IP_RANGE="${MASTER_IP_RANGE:-10.246.0.0/24}" # If set to Elastic IP, master instance will be associated with this IP. # If set to auto, a new Elastic IP will be aquired diff --git a/cluster/aws/coreos/util.sh b/cluster/aws/coreos/util.sh index 9b8557dafea..77ea538df5f 100644 --- a/cluster/aws/coreos/util.sh +++ b/cluster/aws/coreos/util.sh @@ -30,8 +30,8 @@ function detect-minion-image (){ function generate-minion-user-data() { i=$1 + # TODO(bakins): Is this actually used? MINION_PRIVATE_IP=$INTERNAL_IP_BASE.1${i} - MINION_IP_RANGE=${MINION_IP_RANGES[$i]} # this is a bit of a hack. We make all of our "variables" in # our cloud config controlled by env vars from this script @@ -44,7 +44,6 @@ function generate-minion-user-data() { DNS_SERVER_IP=$(yaml-quote ${DNS_SERVER_IP:-}) DNS_DOMAIN=$(yaml-quote ${DNS_DOMAIN:-}) MASTER_IP=$(yaml-quote ${MASTER_INTERNAL_IP}) - MINION_IP_RANGE=$(yaml-quote ${MINION_IP_RANGE}) MINION_IP=$(yaml-quote ${MINION_PRIVATE_IP}) KUBELET_TOKEN=$(yaml-quote ${KUBELET_TOKEN:-}) KUBE_PROXY_TOKEN=$(yaml-quote ${KUBE_PROXY_TOKEN:-}) diff --git a/cluster/aws/templates/create-dynamic-salt-files.sh b/cluster/aws/templates/create-dynamic-salt-files.sh index afff0d38d2d..428659ca2b3 100644 --- a/cluster/aws/templates/create-dynamic-salt-files.sh +++ b/cluster/aws/templates/create-dynamic-salt-files.sh @@ -22,6 +22,8 @@ mkdir -p /srv/salt-overlay/pillar cat </srv/salt-overlay/pillar/cluster-params.sls instance_prefix: '$(echo "$INSTANCE_PREFIX" | sed -e "s/'/''/g")' node_instance_prefix: '$(echo "$NODE_INSTANCE_PREFIX" | sed -e "s/'/''/g")' +cluster_cidr: '$(echo "$CLUSTER_IP_RANGE" | sed -e "s/'/''/g")' +allocate_node_cidrs: '$(echo "$ALLOCATE_NODE_CIDRS" | sed -e "s/'/''/g")' service_cluster_ip_range: '$(echo "$SERVICE_CLUSTER_IP_RANGE" | sed -e "s/'/''/g")' enable_cluster_monitoring: '$(echo "$ENABLE_CLUSTER_MONITORING" | sed -e "s/'/''/g")' enable_node_monitoring: '$(echo "$ENABLE_NODE_MONITORING" | sed -e "s/'/''/g")' diff --git a/cluster/aws/templates/salt-minion.sh b/cluster/aws/templates/salt-minion.sh index 0462ab9ad72..e5e25722c1b 100755 --- a/cluster/aws/templates/salt-minion.sh +++ b/cluster/aws/templates/salt-minion.sh @@ -26,7 +26,7 @@ cat </etc/salt/minion.d/grains.conf grains: roles: - kubernetes-pool - cbr-cidr: $MINION_IP_RANGE + cbr-cidr: 10.123.45.0/30 cloud: aws EOF diff --git a/cluster/aws/ubuntu/common.sh b/cluster/aws/ubuntu/common.sh index cb8282a6fc8..126c3166713 100644 --- a/cluster/aws/ubuntu/common.sh +++ b/cluster/aws/ubuntu/common.sh @@ -29,7 +29,6 @@ function generate-minion-user-data { # We pipe this to the ami as a startup script in the user-data field. Requires a compatible ami echo "#! /bin/bash" echo "SALT_MASTER='${MASTER_INTERNAL_IP}'" - echo "MINION_IP_RANGE='${MINION_IP_RANGES[$i]}'" echo "DOCKER_OPTS='${EXTRA_DOCKER_OPTS:-}'" echo "readonly DOCKER_STORAGE='${DOCKER_STORAGE:-}'" grep -v "^#" "${KUBE_ROOT}/cluster/aws/templates/common.sh" diff --git a/cluster/aws/util.sh b/cluster/aws/util.sh index d2c5c32d2d8..02da313c4c2 100644 --- a/cluster/aws/util.sh +++ b/cluster/aws/util.sh @@ -22,6 +22,8 @@ KUBE_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${KUBE_ROOT}/cluster/aws/${KUBE_CONFIG_FILE-"config-default.sh"}" source "${KUBE_ROOT}/cluster/common.sh" +ALLOCATE_NODE_CIDRS=true + case "${KUBE_OS_DISTRIBUTION}" in ubuntu|wheezy|coreos) source "${KUBE_ROOT}/cluster/aws/${KUBE_OS_DISTRIBUTION}/util.sh" @@ -695,6 +697,8 @@ function kube-up { echo "readonly SALT_MASTER='${MASTER_INTERNAL_IP}'" echo "readonly INSTANCE_PREFIX='${INSTANCE_PREFIX}'" echo "readonly NODE_INSTANCE_PREFIX='${INSTANCE_PREFIX}-minion'" + echo "readonly CLUSTER_IP_RANGE='${CLUSTER_IP_RANGE}'" + echo "readonly ALLOCATE_NODE_CIDRS='${ALLOCATE_NODE_CIDRS}'" echo "readonly SERVER_BINARY_TAR_URL='${SERVER_BINARY_TAR_URL}'" echo "readonly SALT_TAR_URL='${SALT_TAR_URL}'" echo "readonly ZONE='${ZONE}'" @@ -854,7 +858,8 @@ function kube-up { MINION_IDS[$i]=$minion_id done - # Add routes to minions + # Configure minion networking + # TODO(justinsb): Check if we can change source-dest-check before instance fully running for (( i=0; i<${#MINION_NAMES[@]}; i++)); do # We are not able to add a route to the instance until that instance is in "running" state. # This is quite an ugly solution to this problem. In Bash 4 we could use assoc. arrays to do this for @@ -864,7 +869,6 @@ function kube-up { echo "Minion ${MINION_NAMES[$i]} running" sleep 10 $AWS_CMD modify-instance-attribute --instance-id $minion_id --source-dest-check '{"Value": false}' > $LOG - $AWS_CMD create-route --route-table-id $ROUTE_TABLE_ID --destination-cidr-block ${MINION_IP_RANGES[$i]} --instance-id $minion_id > $LOG done FAIL=0 diff --git a/pkg/cloudprovider/aws/aws.go b/pkg/cloudprovider/aws/aws.go index 54d3b7dbd35..4aa8e7e564e 100644 --- a/pkg/cloudprovider/aws/aws.go +++ b/pkg/cloudprovider/aws/aws.go @@ -86,6 +86,10 @@ type EC2 interface { DescribeSubnets(*ec2.DescribeSubnetsInput) ([]*ec2.Subnet, error) CreateTags(*ec2.CreateTagsInput) (*ec2.CreateTagsOutput, error) + + DescribeRouteTables(request *ec2.DescribeRouteTablesInput) ([]*ec2.RouteTable, error) + CreateRoute(request *ec2.CreateRouteInput) (*ec2.CreateRouteOutput, error) + DeleteRoute(request *ec2.DeleteRouteInput) (*ec2.DeleteRouteOutput, error) } // This is a simple pass-through of the ELB client interface, which allows for testing @@ -393,6 +397,23 @@ func (s *awsSdkEC2) CreateTags(request *ec2.CreateTagsInput) (*ec2.CreateTagsOut return s.ec2.CreateTags(request) } +func (s *awsSdkEC2) DescribeRouteTables(request *ec2.DescribeRouteTablesInput) ([]*ec2.RouteTable, error) { + // Not paged + response, err := s.ec2.DescribeRouteTables(request) + if err != nil { + return nil, fmt.Errorf("error listing AWS route tables: %v", err) + } + return response.RouteTables, nil +} + +func (s *awsSdkEC2) CreateRoute(request *ec2.CreateRouteInput) (*ec2.CreateRouteOutput, error) { + return s.ec2.CreateRoute(request) +} + +func (s *awsSdkEC2) DeleteRoute(request *ec2.DeleteRouteInput) (*ec2.DeleteRouteOutput, error) { + return s.ec2.DeleteRoute(request) +} + func init() { cloudprovider.RegisterCloudProvider(ProviderName, func(config io.Reader) (cloudprovider.Interface, error) { creds := credentials.NewChainCredentials( @@ -550,7 +571,7 @@ func (aws *AWSCloud) Zones() (cloudprovider.Zones, bool) { // Routes returns an implementation of Routes for Amazon Web Services. func (aws *AWSCloud) Routes() (cloudprovider.Routes, bool) { - return nil, false + return aws, true } // NodeAddresses is an implementation of Instances.NodeAddresses. diff --git a/pkg/cloudprovider/aws/aws_routes.go b/pkg/cloudprovider/aws/aws_routes.go new file mode 100644 index 00000000000..ba9b4d9da17 --- /dev/null +++ b/pkg/cloudprovider/aws/aws_routes.go @@ -0,0 +1,114 @@ +/* +Copyright 2014 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 aws_cloud + +import ( + "fmt" + "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" +) + +func (s *AWSCloud) findRouteTable(clusterName string) (*ec2.RouteTable, error) { + request := &ec2.DescribeRouteTablesInput{} + filters := []*ec2.Filter{} + // This should be unnecessary (we already filter on TagNameKubernetesCluster, + // and something is broken if cluster name doesn't match, but anyway... + // TODO: All clouds should be cluster-aware by default + filters = append(filters, newEc2Filter("tag:"+TagNameKubernetesCluster, clusterName)) + request.Filters = s.addFilters(filters) + + tables, err := s.ec2.DescribeRouteTables(request) + if err != nil { + return nil, err + } + + if len(tables) == 0 { + return nil, fmt.Errorf("unable to find route table for AWS cluster: %s", clusterName) + } + + if len(tables) != 1 { + return nil, fmt.Errorf("found multiple matching AWS route tables for AWS cluster: %s", clusterName) + } + return tables[0], nil +} + +// ListRoutes implements Routes.ListRoutes +// List all routes that match the filter +func (s *AWSCloud) ListRoutes(clusterName string) ([]*cloudprovider.Route, error) { + table, err := s.findRouteTable(clusterName) + if err != nil { + return nil, err + } + + var routes []*cloudprovider.Route + for _, r := range table.Routes { + instanceID := orEmpty(r.InstanceID) + destinationCIDR := orEmpty(r.DestinationCIDRBlock) + + if instanceID == "" || destinationCIDR == "" { + continue + } + + routeName := clusterName + "-" + destinationCIDR + routes = append(routes, &cloudprovider.Route{routeName, instanceID, destinationCIDR}) + } + + return routes, nil +} + +// CreateRoute implements Routes.CreateRoute +// Create the described route +func (s *AWSCloud) CreateRoute(clusterName string, nameHint string, route *cloudprovider.Route) error { + table, err := s.findRouteTable(clusterName) + if err != nil { + return err + } + + request := &ec2.CreateRouteInput{} + // TODO: use ClientToken for idempotency? + request.DestinationCIDRBlock = aws.String(route.DestinationCIDR) + request.InstanceID = aws.String(route.TargetInstance) + request.RouteTableID = table.RouteTableID + + _, err = s.ec2.CreateRoute(request) + if err != nil { + return fmt.Errorf("error creating AWS route (%s): %v", route.DestinationCIDR, err) + } + + return nil +} + +// DeleteRoute implements Routes.DeleteRoute +// Delete the specified route +func (s *AWSCloud) DeleteRoute(clusterName string, route *cloudprovider.Route) error { + table, err := s.findRouteTable(clusterName) + if err != nil { + return err + } + + request := &ec2.DeleteRouteInput{} + request.DestinationCIDRBlock = aws.String(route.DestinationCIDR) + request.RouteTableID = table.RouteTableID + + _, err = s.ec2.DeleteRoute(request) + if err != nil { + return fmt.Errorf("error deleting AWS route (%s): %v", route.DestinationCIDR, err) + } + + return nil +} diff --git a/pkg/cloudprovider/aws/aws_test.go b/pkg/cloudprovider/aws/aws_test.go index 60462a74155..f45537abd81 100644 --- a/pkg/cloudprovider/aws/aws_test.go +++ b/pkg/cloudprovider/aws/aws_test.go @@ -360,6 +360,18 @@ func (ec2 *FakeEC2) CreateTags(*ec2.CreateTagsInput) (*ec2.CreateTagsOutput, err panic("Not implemented") } +func (s *FakeEC2) DescribeRouteTables(request *ec2.DescribeRouteTablesInput) ([]*ec2.RouteTable, error) { + panic("Not implemented") +} + +func (s *FakeEC2) CreateRoute(request *ec2.CreateRouteInput) (*ec2.CreateRouteOutput, error) { + panic("Not implemented") +} + +func (s *FakeEC2) DeleteRoute(request *ec2.DeleteRouteInput) (*ec2.DeleteRouteOutput, error) { + panic("Not implemented") +} + type FakeELB struct { aws *FakeAWSServices } From 0ad16a187d7f5ffecda5e8b049a1da0e267a8a70 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Thu, 18 Jun 2015 17:08:32 -0700 Subject: [PATCH 3/3] Refactor findRouteTable to be less verbose Thanks for the suggestion @cjcullen --- pkg/cloudprovider/aws/aws_routes.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/cloudprovider/aws/aws_routes.go b/pkg/cloudprovider/aws/aws_routes.go index ba9b4d9da17..a87f61da613 100644 --- a/pkg/cloudprovider/aws/aws_routes.go +++ b/pkg/cloudprovider/aws/aws_routes.go @@ -24,13 +24,11 @@ import ( ) func (s *AWSCloud) findRouteTable(clusterName string) (*ec2.RouteTable, error) { - request := &ec2.DescribeRouteTablesInput{} - filters := []*ec2.Filter{} // This should be unnecessary (we already filter on TagNameKubernetesCluster, // and something is broken if cluster name doesn't match, but anyway... // TODO: All clouds should be cluster-aware by default - filters = append(filters, newEc2Filter("tag:"+TagNameKubernetesCluster, clusterName)) - request.Filters = s.addFilters(filters) + filters := []*ec2.Filter{newEc2Filter("tag:"+TagNameKubernetesCluster, clusterName)} + request := &ec2.DescribeRouteTablesInput{Filters: s.addFilters(filters)} tables, err := s.ec2.DescribeRouteTables(request) if err != nil {