fixed panic issue around deletes

Author: jhorwit2

pkg/oci/load_balancer.go

@@ -251,9 +251,18 @@ func (cp *CloudProvider) updateLoadBalancer(lb *baremetal.LoadBalancer, spec LBS
 				return fmt.Errorf("error updating BackendSet: %v", err)
 			}
 		case *ListenerAction:
-			err := cp.updateListener(lbOCID, spec, a, lbSubnets, nodeSubnets, sslConfigMap, sourceCIDRs)
+			backendSet := spec.GetBackendSets()[a.Listener.DefaultBackendSetName]
+			if a.Type() == Delete {
+				// If we need to delete the backendset then it'll no longer be present
+				// in the spec since that's what is desired, so we need to fetch it
+				// from the load balancer object.
+				backendSet = lb.BackendSets[a.Listener.DefaultBackendSetName]
+			}
+
+			backendPort := uint64(getBackendPort(backendSet.Backends))
+			err := cp.updateListener(lbOCID, a, backendPort, lbSubnets, nodeSubnets, sslConfigMap, sourceCIDRs)
 			if err != nil {
-				return fmt.Errorf("error updating BackendSet: %v", err)
+				return fmt.Errorf("error updating Listener: %v", err)
 			}
 		}
 	}
@@ -268,7 +277,7 @@ func (cp *CloudProvider) updateBackendSet(lbOCID string, action *BackendSetActio
 	var err error
 
 	be := action.BackendSet
-	glog.V(2).Infof("Applying `%s` action on backend set `%s` for lb `%s`", action.Type, be.Name, lbOCID)
+	glog.V(2).Infof("Applying %q action on backend set `%s` for lb `%s`", action.Type(), be.Name, lbOCID)
 
 	backendPort := uint64(getBackendPort(be.Backends))
 
@@ -322,8 +331,8 @@ func (cp *CloudProvider) updateBackendSet(lbOCID string, action *BackendSetActio
 }
 
 func (cp *CloudProvider) updateListener(lbOCID string,
-	spec LBSpec,
 	action *ListenerAction,
+	backendPort uint64,
 	lbSubnets []*baremetal.Subnet,
 	nodeSubnets []*baremetal.Subnet,
 	sslConfigMap map[int]*baremetal.SSLConfiguration,
@@ -334,10 +343,7 @@ func (cp *CloudProvider) updateListener(lbOCID string,
 	l := action.Listener
 	listenerPort := uint64(l.Port)
 
-	backends := spec.GetBackendSets()[l.DefaultBackendSetName].Backends
-	backendPort := uint64(getBackendPort(backends))
-
-	glog.V(2).Infof("Applying `%s` action on listener `%s` for lb `%s`", action.Type, l.Name, lbOCID)
+	glog.V(2).Infof("Applying %q action on listener `%s` for lb `%s`", action.Type(), l.Name, lbOCID)
 
 	switch action.Type() {
 	case Create:

pkg/oci/load_balancer_security_lists.go

@@ -200,11 +200,6 @@ func (s *securityListManagerImpl) updateSecurityListRules(securityListID string,
 	return err
 }
 
-func getBackendPort(backends []baremetal.Backend) uint64 {
-	// TODO: what happens if this is 0? e.g. we scale the pods to 0 for a deployment
-	return uint64(backends[0].Port)
-}
-
 func getNodeIngressRules(securityList *baremetal.SecurityList, lbSubnets []*baremetal.Subnet, port uint64) []baremetal.IngressSecurityRule {
 	desired := sets.NewString()
 	for _, lbSubnet := range lbSubnets {

pkg/oci/load_balancer_util.go

@@ -310,3 +310,8 @@ func sortAndCombineActions(backendSetActions []Action, listenerActions []Action)
 	})
 	return actions
 }
+
+func getBackendPort(backends []baremetal.Backend) uint64 {
+	// TODO: what happens if this is 0? e.g. we scale the pods to 0 for a deployment
+	return uint64(backends[0].Port)
+}

test/integration/loadbalancer/loadbalancer_test.go

@@ -125,6 +125,8 @@ func testLoadBalancer(t *testing.T, internal bool) {
 		nodes = append(nodes, node)
 	}
 
+	glog.Info("Stating test on creating initial load balancer")
+
 	status, err := loadbalancers.EnsureLoadBalancer("foo", service, nodes)
 	if err != nil {
 		t.Fatalf("Unable to ensure the load balancer: %v", err)
@@ -137,6 +139,8 @@ func testLoadBalancer(t *testing.T, internal bool) {
 		t.Fatalf("validation error: %v", err)
 	}
 
+	glog.Info("Stating test on decreasing node count to 1")
+
 	// Decrease the number of backends to 1
 	lessNodes := []*api.Node{nodes[0]}
 	status, err = loadbalancers.EnsureLoadBalancer("foo", service, lessNodes)
@@ -149,6 +153,8 @@ func testLoadBalancer(t *testing.T, internal bool) {
 		t.Fatalf("validation error: %v", err)
 	}
 
+	glog.Info("Stating test on increasing node count back to 2")
+
 	// Go back to 2 nodes
 	status, err = loadbalancers.EnsureLoadBalancer("foo", service, nodes)
 	if err != nil {
@@ -159,6 +165,33 @@ func testLoadBalancer(t *testing.T, internal bool) {
 	if err != nil {
 		t.Fatalf("validation error: %v", err)
 	}
+
+	glog.Info("Stating test on changing service port")
+
+	// Validate changing the service port.
+	service.Spec.Ports[0].Port = 81
+	status, err = loadbalancers.EnsureLoadBalancer("foo", service, nodes)
+	if err != nil {
+		t.Fatalf("Unable to ensure the load balancer: %v", err)
+	}
+
+	err = validateLoadBalancer(fw.Client, service, nodes)
+	if err != nil {
+		t.Fatalf("validation error: %v", err)
+	}
+
+	glog.Info("Stating test on changing node port")
+	// Validate changing the node port.
+	service.Spec.Ports[0].NodePort = 8081
+	status, err = loadbalancers.EnsureLoadBalancer("foo", service, nodes)
+	if err != nil {
+		t.Fatalf("Unable to ensure the load balancer: %v", err)
+	}
+
+	err = validateLoadBalancer(fw.Client, service, nodes)
+	if err != nil {
+		t.Fatalf("validation error: %v", err)
+	}
 }
 
 func validateLoadBalancer(client client.Interface, service *api.Service, nodes []*api.Node) error {
@@ -171,20 +204,27 @@ func validateLoadBalancer(client client.Interface, service *api.Service, nodes [
 	}
 
 	if len(lb.Listeners) != 1 {
-		return fmt.Errorf("Expected 1 Listener but got %d", len(lb.Listeners))
+		return fmt.Errorf("expected 1 Listener but got %d", len(lb.Listeners))
 	}
 
 	if len(lb.BackendSets) != 1 {
-		return fmt.Errorf("Expected 1 BackendSet but got %d", len(lb.BackendSets))
+		return fmt.Errorf("expected 1 BackendSet but got %d", len(lb.BackendSets))
 	}
 
-	backendSet, ok := lb.BackendSets["TCP-80"]
+	name := fmt.Sprintf("TCP-%d", service.Spec.Ports[0].Port)
+	backendSet, ok := lb.BackendSets[name]
 	if !ok {
-		return fmt.Errorf("Expected BackendSet with name `TCP-80` to exist but it doesn't")
+		return fmt.Errorf("expected BackendSet with name %q to exist but it doesn't", name)
 	}
 
 	if len(backendSet.Backends) != len(nodes) {
-		return fmt.Errorf("Expected %d backends but got %d", len(nodes), len(backendSet.Backends))
+		return fmt.Errorf("expected %d backends but got %d", len(nodes), len(backendSet.Backends))
+	}
+
+	expectedBackendPort := service.Spec.Ports[0].NodePort
+	actualBackendPort := backendSet.Backends[0].Port
+	if int(expectedBackendPort) != int(actualBackendPort) {
+		return fmt.Errorf("expected backend port %d but got %d", expectedBackendPort, actualBackendPort)
 	}
 
 	return nil