From 4ff292a65e422504c26a302147c5be4e17bc790f Mon Sep 17 00:00:00 2001 From: Angus Lees Date: Wed, 24 Dec 2014 10:35:59 +1100 Subject: [PATCH 1/6] OpenStack: Support loadbalancer client IP affinity --- pkg/cloudprovider/openstack/openstack.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/cloudprovider/openstack/openstack.go b/pkg/cloudprovider/openstack/openstack.go index fc9f677106f..ef46b7d689e 100644 --- a/pkg/cloudprovider/openstack/openstack.go +++ b/pkg/cloudprovider/openstack/openstack.go @@ -427,10 +427,18 @@ func (lb *LoadBalancer) TCPLoadBalancerExists(name, region string) (bool, error) // each region. func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP net.IP, port int, hosts []string, affinity api.AffinityType) (string, error) { - glog.V(2).Infof("CreateTCPLoadBalancer(%v, %v, %v, %v, %v)", name, region, externalIP, port, hosts) - if affinity != api.AffinityTypeNone { + glog.V(2).Infof("CreateTCPLoadBalancer(%v, %v, %v, %v, %v, %v)", name, region, externalIP, port, hosts, affinity) + + var persistence *vips.SessionPersistence + switch affinity { + case api.AffinityTypeNone: + persistence = nil + case api.AffinityTypeClientIP: + persistence = &vips.SessionPersistence{Type: "SOURCE_IP"} + default: return "", fmt.Errorf("unsupported load balancer affinity: %v", affinity) } + pool, err := pools.Create(lb.network, pools.CreateOpts{ Name: name, Protocol: pools.ProtocolTCP, @@ -485,6 +493,7 @@ func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP ne Protocol: "TCP", ProtocolPort: port, PoolID: pool.ID, + Persistence: persistence, }).Extract() if err != nil { if mon != nil { From 762225ed476249334385dbb1ee252ca7b3eb1ce4 Mon Sep 17 00:00:00 2001 From: Angus Lees Date: Wed, 24 Dec 2014 12:32:41 +1100 Subject: [PATCH 2/6] OpenStack: Align logging levels with devel/logging.md --- pkg/cloudprovider/openstack/openstack.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/pkg/cloudprovider/openstack/openstack.go b/pkg/cloudprovider/openstack/openstack.go index ef46b7d689e..ef3f4c60edf 100644 --- a/pkg/cloudprovider/openstack/openstack.go +++ b/pkg/cloudprovider/openstack/openstack.go @@ -148,7 +148,7 @@ type Instances struct { // Instances returns an implementation of Instances for OpenStack. func (os *OpenStack) Instances() (cloudprovider.Instances, bool) { - glog.V(2).Info("openstack.Instances() called") + glog.V(4).Info("openstack.Instances() called") compute, err := openstack.NewComputeV2(os.provider, gophercloud.EndpointOpts{ Region: os.region, @@ -185,14 +185,14 @@ func (os *OpenStack) Instances() (cloudprovider.Instances, bool) { return nil, false } - glog.V(2).Infof("Found %v compute flavors", len(flavor_to_resource)) + glog.V(3).Infof("Found %v compute flavors", len(flavor_to_resource)) glog.V(1).Info("Claiming to support Instances") return &Instances{compute, flavor_to_resource}, true } func (i *Instances) List(name_filter string) ([]string, error) { - glog.V(2).Infof("openstack List(%v) called", name_filter) + glog.V(4).Infof("openstack List(%v) called", name_filter) opts := servers.ListOpts{ Name: name_filter, @@ -215,7 +215,8 @@ func (i *Instances) List(name_filter string) ([]string, error) { return nil, err } - glog.V(2).Infof("Found %v entries: %v", len(ret), ret) + glog.V(3).Infof("Found %v instances matching %v: %v", + len(ret), name_filter, ret) return ret, nil } @@ -300,14 +301,14 @@ func getAddressByName(api *gophercloud.ServiceClient, name string) (string, erro } func (i *Instances) NodeAddresses(name string) ([]api.NodeAddress, error) { - glog.V(2).Infof("NodeAddresses(%v) called", name) + glog.V(4).Infof("NodeAddresses(%v) called", name) ip, err := getAddressByName(i.compute, name) if err != nil { return nil, err } - glog.V(2).Infof("NodeAddresses(%v) => %v", name, ip) + glog.V(4).Infof("NodeAddresses(%v) => %v", name, ip) // net.ParseIP().String() is to maintain compatibility with the old code return []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: net.ParseIP(ip).String()}}, nil @@ -323,7 +324,7 @@ func (i *Instances) ExternalID(name string) (string, error) { } func (i *Instances) GetNodeResources(name string) (*api.NodeResources, error) { - glog.V(2).Infof("GetNodeResources(%v) called", name) + glog.V(4).Infof("GetNodeResources(%v) called", name) srv, err := getServerByName(i.compute, name) if err != nil { @@ -343,7 +344,7 @@ func (i *Instances) GetNodeResources(name string) (*api.NodeResources, error) { return nil, ErrNotFound } - glog.V(2).Infof("GetNodeResources(%v) => %v", name, rsrc) + glog.V(4).Infof("GetNodeResources(%v) => %v", name, rsrc) return rsrc, nil } @@ -427,7 +428,7 @@ func (lb *LoadBalancer) TCPLoadBalancerExists(name, region string) (bool, error) // each region. func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP net.IP, port int, hosts []string, affinity api.AffinityType) (string, error) { - glog.V(2).Infof("CreateTCPLoadBalancer(%v, %v, %v, %v, %v, %v)", name, region, externalIP, port, hosts, affinity) + glog.V(4).Infof("CreateTCPLoadBalancer(%v, %v, %v, %v, %v, %v)", name, region, externalIP, port, hosts, affinity) var persistence *vips.SessionPersistence switch affinity { @@ -507,7 +508,7 @@ func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP ne } func (lb *LoadBalancer) UpdateTCPLoadBalancer(name, region string, hosts []string) error { - glog.V(2).Infof("UpdateTCPLoadBalancer(%v, %v, %v)", name, region, hosts) + glog.V(4).Infof("UpdateTCPLoadBalancer(%v, %v, %v)", name, region, hosts) vip, err := getVipByName(lb.network, name) if err != nil { @@ -568,7 +569,7 @@ func (lb *LoadBalancer) UpdateTCPLoadBalancer(name, region string, hosts []strin } func (lb *LoadBalancer) DeleteTCPLoadBalancer(name, region string) error { - glog.V(2).Infof("DeleteTCPLoadBalancer(%v, %v)", name, region) + glog.V(4).Infof("DeleteTCPLoadBalancer(%v, %v)", name, region) vip, err := getVipByName(lb.network, name) if err != nil { From e14b73bd308fd92871be305ec5c83eaf5fcb7484 Mon Sep 17 00:00:00 2001 From: Angus Lees Date: Tue, 30 Dec 2014 15:47:04 +1100 Subject: [PATCH 3/6] Reauthenticate to OpenStack periodically It appears that gophercloud's "AllowReauth" AuthOption doesn't actually do anything, and the keystone/auth token is never refreshed. Eventually it expires and all OpenStack calls receive HTTP 401 responses. This change reauthenticates every time the Instances() or TCPLoadBalancer() API object is requested. This is more frequently than required, but exposing token expiry information will require gophercloud surgery. --- pkg/cloudprovider/openstack/openstack.go | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/pkg/cloudprovider/openstack/openstack.go b/pkg/cloudprovider/openstack/openstack.go index ef3f4c60edf..1df479f48ef 100644 --- a/pkg/cloudprovider/openstack/openstack.go +++ b/pkg/cloudprovider/openstack/openstack.go @@ -71,6 +71,7 @@ type LoadBalancerOpts struct { // OpenStack is an implementation of cloud provider Interface for OpenStack. type OpenStack struct { provider *gophercloud.ProviderClient + authOpts gophercloud.AuthOptions region string lbOpts LoadBalancerOpts } @@ -111,7 +112,11 @@ func (cfg Config) toAuthOptions() gophercloud.AuthOptions { TenantID: cfg.Global.TenantId, TenantName: cfg.Global.TenantName, - // Persistent service, so we need to be able to renew tokens + // Persistent service, so we need to be able to renew + // tokens. + // (gophercloud doesn't appear to actually reauth yet, + // hence the explicit openstack.Authenticate() calls + // below) AllowReauth: true, } } @@ -128,13 +133,15 @@ func readConfig(config io.Reader) (Config, error) { } func newOpenStack(cfg Config) (*OpenStack, error) { - provider, err := openstack.AuthenticatedClient(cfg.toAuthOptions()) + authOpts := cfg.toAuthOptions() + provider, err := openstack.AuthenticatedClient(authOpts) if err != nil { return nil, err } os := OpenStack{ provider: provider, + authOpts: authOpts, region: cfg.Global.Region, lbOpts: cfg.LoadBalancer, } @@ -150,6 +157,11 @@ type Instances struct { func (os *OpenStack) Instances() (cloudprovider.Instances, bool) { glog.V(4).Info("openstack.Instances() called") + if err := openstack.Authenticate(os.provider, os.authOpts); err != nil { + glog.Warningf("Failed to reauthenticate: %v", err) + return nil, false + } + compute, err := openstack.NewComputeV2(os.provider, gophercloud.EndpointOpts{ Region: os.region, }) @@ -360,6 +372,13 @@ type LoadBalancer struct { } func (os *OpenStack) TCPLoadBalancer() (cloudprovider.TCPLoadBalancer, bool) { + glog.V(4).Info("openstack.TCPLoadBalancer() called") + + if err := openstack.Authenticate(os.provider, os.authOpts); err != nil { + glog.Warningf("Failed to reauthenticate: %v", err) + return nil, false + } + // TODO: Search for and support Rackspace loadbalancer API, and others. network, err := openstack.NewNetworkV2(os.provider, gophercloud.EndpointOpts{ Region: os.region, From 437853b8cd510342da720f5ff2672693dcc8ecba Mon Sep 17 00:00:00 2001 From: Angus Lees Date: Wed, 31 Dec 2014 15:07:43 +1100 Subject: [PATCH 4/6] Keep resources as numeric values Previously, this code converted to a string and reparsed back to a numeric Quantity. Keeping it numeric avoids the overhead, but also removes a (theoretical) run-time panic if the re-parse failed. In addition, since Quantity can now store quantities <1, convert openstack.org/rxTxFactor back to its original ratio, rather than arbitrarily scaling it up by 1000. --- pkg/cloudprovider/openstack/openstack.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pkg/cloudprovider/openstack/openstack.go b/pkg/cloudprovider/openstack/openstack.go index 1df479f48ef..77a877f7e6b 100644 --- a/pkg/cloudprovider/openstack/openstack.go +++ b/pkg/cloudprovider/openstack/openstack.go @@ -46,6 +46,11 @@ var ErrMultipleResults = errors.New("Multiple results where only one expected") var ErrNoAddressFound = errors.New("No address found for host") var ErrAttrNotFound = errors.New("Expected attribute not found") +const ( + MiB = 1024 * 1024 + GB = 1000 * 1000 * 1000 +) + // encoding.TextUnmarshaler interface for time.Duration type MyDuration struct { time.Duration @@ -181,11 +186,11 @@ func (os *OpenStack) Instances() (cloudprovider.Instances, bool) { for _, flavor := range flavorList { rsrc := api.NodeResources{ Capacity: api.ResourceList{ - api.ResourceCPU: *resource.NewMilliQuantity(int64(flavor.VCPUs*1000), resource.DecimalSI), - api.ResourceMemory: resource.MustParse(fmt.Sprintf("%dMi", flavor.RAM)), - "openstack.org/disk": resource.MustParse(fmt.Sprintf("%dG", flavor.Disk)), - "openstack.org/rxTxFactor": *resource.NewQuantity(int64(flavor.RxTxFactor*1000), resource.DecimalSI), - "openstack.org/swap": resource.MustParse(fmt.Sprintf("%dMi", flavor.Swap)), + api.ResourceCPU: *resource.NewQuantity(int64(flavor.VCPUs), resource.DecimalSI), + api.ResourceMemory: *resource.NewQuantity(int64(flavor.RAM)*MiB, resource.BinarySI), + "openstack.org/disk": *resource.NewQuantity(int64(flavor.Disk)*GB, resource.DecimalSI), + "openstack.org/rxTxFactor": *resource.NewMilliQuantity(int64(flavor.RxTxFactor)*1000, resource.DecimalSI), + "openstack.org/swap": *resource.NewQuantity(int64(flavor.Swap)*MiB, resource.BinarySI), }, } flavor_to_resource[flavor.ID] = &rsrc From 8f5064bfeabeab95e8fc627742229ef3db3239de Mon Sep 17 00:00:00 2001 From: Angus Lees Date: Fri, 2 Jan 2015 20:53:10 +1100 Subject: [PATCH 5/6] nodecontroller: Include error in error messages Makes debugging node creation failures far easier... --- pkg/cloudprovider/controller/nodecontroller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cloudprovider/controller/nodecontroller.go b/pkg/cloudprovider/controller/nodecontroller.go index 64aae14a919..696a1f22083 100644 --- a/pkg/cloudprovider/controller/nodecontroller.go +++ b/pkg/cloudprovider/controller/nodecontroller.go @@ -195,7 +195,7 @@ func (s *NodeController) SyncCloud() error { glog.Infof("Create node in registry: %s", node.Name) _, err = s.kubeClient.Nodes().Create(&node) if err != nil { - glog.Errorf("Create node error: %s", node.Name) + glog.Errorf("Create node %s error: %v", node.Name, err) } } delete(nodeMap, node.Name) @@ -206,7 +206,7 @@ func (s *NodeController) SyncCloud() error { glog.Infof("Delete node from registry: %s", nodeID) err = s.kubeClient.Nodes().Delete(nodeID) if err != nil { - glog.Errorf("Delete node error: %s", nodeID) + glog.Errorf("Delete node %s error: %v", nodeID, err) } s.deletePods(nodeID) } From 1437e6b7ca313cf7a7719b760004105572f494d9 Mon Sep 17 00:00:00 2001 From: Angus Lees Date: Mon, 16 Mar 2015 17:11:36 +1100 Subject: [PATCH 6/6] OpenStack: Return multiple private+public NodeAddresses Add better support for the new NodeAddresses call. Note the returned list may include IPv6 addresses (as strings) on many OpenStack clouds. --- pkg/cloudprovider/openstack/openstack.go | 74 +++++++++++++++++------- 1 file changed, 54 insertions(+), 20 deletions(-) diff --git a/pkg/cloudprovider/openstack/openstack.go b/pkg/cloudprovider/openstack/openstack.go index 77a877f7e6b..2d07106cc8d 100644 --- a/pkg/cloudprovider/openstack/openstack.go +++ b/pkg/cloudprovider/openstack/openstack.go @@ -271,25 +271,29 @@ func getServerByName(client *gophercloud.ServiceClient, name string) (*servers.S return &serverList[0], nil } -func firstAddr(netblob interface{}) string { +func findAddrs(netblob interface{}) []string { // Run-time types for the win :( + ret := []string{} list, ok := netblob.([]interface{}) - if !ok || len(list) < 1 { - return "" - } - props, ok := list[0].(map[string]interface{}) if !ok { - return "" + return ret } - tmp, ok := props["addr"] - if !ok { - return "" + for _, item := range list { + props, ok := item.(map[string]interface{}) + if !ok { + continue + } + tmp, ok := props["addr"] + if !ok { + continue + } + addr, ok := tmp.(string) + if !ok { + continue + } + ret = append(ret, addr) } - addr, ok := tmp.(string) - if !ok { - return "" - } - return addr + return ret } func getAddressByName(api *gophercloud.ServiceClient, name string) (string, error) { @@ -300,10 +304,14 @@ func getAddressByName(api *gophercloud.ServiceClient, name string) (string, erro var s string if s == "" { - s = firstAddr(srv.Addresses["private"]) + if tmp := findAddrs(srv.Addresses["private"]); len(tmp) >= 1 { + s = tmp[0] + } } if s == "" { - s = firstAddr(srv.Addresses["public"]) + if tmp := findAddrs(srv.Addresses["public"]); len(tmp) >= 1 { + s = tmp[0] + } } if s == "" { s = srv.AccessIPv4 @@ -320,15 +328,41 @@ func getAddressByName(api *gophercloud.ServiceClient, name string) (string, erro func (i *Instances) NodeAddresses(name string) ([]api.NodeAddress, error) { glog.V(4).Infof("NodeAddresses(%v) called", name) - ip, err := getAddressByName(i.compute, name) + srv, err := getServerByName(i.compute, name) if err != nil { return nil, err } - glog.V(4).Infof("NodeAddresses(%v) => %v", name, ip) + addrs := []api.NodeAddress{} - // net.ParseIP().String() is to maintain compatibility with the old code - return []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: net.ParseIP(ip).String()}}, nil + for _, addr := range findAddrs(srv.Addresses["private"]) { + addrs = append(addrs, api.NodeAddress{ + Type: api.NodeInternalIP, + Address: addr, + }) + } + + for _, addr := range findAddrs(srv.Addresses["public"]) { + addrs = append(addrs, api.NodeAddress{ + Type: api.NodeExternalIP, + Address: addr, + }) + } + + // AccessIPs are usually duplicates of "public" addresses. + api.AddToNodeAddresses(&addrs, + api.NodeAddress{ + Type: api.NodeExternalIP, + Address: srv.AccessIPv6, + }, + api.NodeAddress{ + Type: api.NodeExternalIP, + Address: srv.AccessIPv4, + }, + ) + + glog.V(4).Infof("NodeAddresses(%v) => %v", name, addrs) + return addrs, nil } // ExternalID returns the cloud provider ID of the specified instance.