Merge pull request #33260 from svanharmelen/b-cloudstack-loadbalancer

Automatic merge from submit-queue

cloudprovider/cloudstack: Fix a bug where we assume IP addresses instead of a hostnames

Because of how our test environment was setup, we didn’t notice that we were assuming the load balancer hosts list to always be IP addresses, while they actually are hostnames.

So without this PR, the load balancer code will not work as expected as it will not be able to find the nodes that need to be load balanced.

Also updated some comments and added a check to prevent trying to release a public IP if we don’t have one.
This commit is contained in:
Kubernetes Submit Queue 2016-11-08 21:36:16 -08:00 committed by GitHub
commit c640eeb841

View File

@ -227,7 +227,7 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(clusterName string, service *api.Se
}
}
if lb.ipAddr != service.Spec.LoadBalancerIP {
if lb.ipAddr != "" && lb.ipAddr != service.Spec.LoadBalancerIP {
glog.V(4).Infof("Releasing load balancer IP: %v", lb.ipAddr)
if err := lb.releaseLoadBalancerIP(); err != nil {
return err
@ -275,11 +275,11 @@ func (cs *CSCloud) getLoadBalancer(service *api.Service) (*loadBalancer, error)
return lb, nil
}
// verifyHosts verifies if all hosts belong to the same network, and returns the network and host ID's.
// verifyHosts verifies if all hosts belong to the same network, and returns the host ID's and network ID.
func (cs *CSCloud) verifyHosts(hosts []string) ([]string, string, error) {
ipAddrs := map[string]bool{}
hostNames := map[string]bool{}
for _, host := range hosts {
ipAddrs[host] = true
hostNames[host] = true
}
p := cs.client.VirtualMachine.NewListVirtualMachinesParams()
@ -291,16 +291,15 @@ func (cs *CSCloud) verifyHosts(hosts []string) ([]string, string, error) {
l, err := cs.client.VirtualMachine.ListVirtualMachines(p)
if err != nil {
return nil, "", fmt.Errorf("error retrieving a list of hosts: %v", err)
return nil, "", fmt.Errorf("error retrieving list of hosts: %v", err)
}
var hostIDs []string
var networkID string
// Check if the address belongs to the hosts slice, then add the corresponding vm ID.
// Check if the virtual machine is in the hosts slice, then add the corresponding ID.
for _, vm := range l.VirtualMachines {
// We only check the primary NIC.
if ipAddrs[vm.Nic[0].Ipaddress] {
if hostNames[vm.Name] {
if networkID != "" && networkID != vm.Nic[0].Networkid {
return nil, "", fmt.Errorf("found hosts that belong to different networks")
}
@ -313,12 +312,12 @@ func (cs *CSCloud) verifyHosts(hosts []string) ([]string, string, error) {
return hostIDs, networkID, nil
}
// getLoadBalancerIP retieves an existing IP or associates a new IP and returns the address and it's ID.
// hasLoadBalancerIP returns true if we have a load balancer address and ID.
func (lb *loadBalancer) hasLoadBalancerIP() bool {
return lb.ipAddr != "" && lb.ipAddrID != ""
}
// getLoadBalancerIP retieves an existing IP or associates a new IP and returns the address and it's ID.
// getLoadBalancerIP retieves an existing IP or associates a new IP.
func (lb *loadBalancer) getLoadBalancerIP(loadBalancerIP string) error {
if loadBalancerIP != "" {
return lb.getPublicIPAddress(loadBalancerIP)
@ -327,7 +326,7 @@ func (lb *loadBalancer) getLoadBalancerIP(loadBalancerIP string) error {
return lb.associatePublicIPAddress()
}
// getPublicIPAddressID retrieves the ID of the given IP, and returns the address and it's ID.
// getPublicIPAddressID retrieves the ID of the given IP, and sets the address and it's ID.
func (lb *loadBalancer) getPublicIPAddress(loadBalancerIP string) error {
glog.V(4).Infof("Retrieve load balancer IP details: %v", loadBalancerIP)
@ -341,7 +340,7 @@ func (lb *loadBalancer) getPublicIPAddress(loadBalancerIP string) error {
l, err := lb.Address.ListPublicIpAddresses(p)
if err != nil {
return fmt.Errorf("error retrieving the IP address: %v", err)
return fmt.Errorf("error retrieving IP address: %v", err)
}
if l.Count != 1 {
@ -354,7 +353,7 @@ func (lb *loadBalancer) getPublicIPAddress(loadBalancerIP string) error {
return nil
}
// associatePublicIPAddress associates a new IP and returns the address and it's ID.
// associatePublicIPAddress associates a new IP and sets the address and it's ID.
func (lb *loadBalancer) associatePublicIPAddress() error {
glog.V(4).Infof("Allocate new IP for load balancer: %v", lb.name)
// If a network belongs to a VPC, the IP address needs to be associated with
@ -382,7 +381,7 @@ func (lb *loadBalancer) associatePublicIPAddress() error {
// Associate a new IP address
r, err := lb.Address.AssociateIpAddress(p)
if err != nil {
return fmt.Errorf("error associating a new IP address: %v", err)
return fmt.Errorf("error associating new IP address: %v", err)
}
lb.ipAddr = r.Ipaddress
@ -461,7 +460,7 @@ func (lb *loadBalancer) createLoadBalancerRule(lbRuleName string, port api.Servi
// Create a new load balancer rule.
r, err := lb.LoadBalancer.CreateLoadBalancerRule(p)
if err != nil {
return nil, fmt.Errorf("error creating the load balancer rule %v: %v", lbRuleName, err)
return nil, fmt.Errorf("error creating load balancer rule %v: %v", lbRuleName, err)
}
lbRule := &cloudstack.LoadBalancerRule{