diff --git a/pkg/oci/load_balancer.go b/pkg/oci/load_balancer.go index 022b440e0b..eb0cdf638a 100644 --- a/pkg/oci/load_balancer.go +++ b/pkg/oci/load_balancer.go @@ -204,14 +204,9 @@ func (cp *CloudProvider) EnsureLoadBalancer(clusterName string, service *api.Ser return nil, err } - err = cp.updateBackendSets(lb, spec) + err = cp.updateLoadBalancer(lb, spec, sslConfigMap, sourceCIDRs) if err != nil { - return nil, fmt.Errorf("update backendsets: %v", err) - } - - err = cp.updateListeners(lb, spec, sslConfigMap, sourceCIDRs) - if err != nil { - return nil, fmt.Errorf("udpate listeners: %v", err) + return nil, err } status, err := loadBalancerToStatus(lb) @@ -223,15 +218,24 @@ func (cp *CloudProvider) EnsureLoadBalancer(clusterName string, service *api.Ser return status, nil } -func (cp *CloudProvider) updateBackendSets(lb *baremetal.LoadBalancer, spec LBSpec) error { +func (cp *CloudProvider) updateLoadBalancer( + lb *baremetal.LoadBalancer, + spec LBSpec, + sslConfigMap map[int]*baremetal.SSLConfiguration, + sourceCIDRs []string) error { lbOCID := lb.ID - actual := lb.BackendSets - desired := spec.GetBackendSets() + actualBackendSets := lb.BackendSets + desiredBackendSets := spec.GetBackendSets() - actions := getBackendSetChanges(actual, desired) - if len(actions) == 0 { - return nil + backendSetActions := getBackendSetChanges(actualBackendSets, desiredBackendSets) + + actualListeners := lb.Listeners + desiredListeners := spec.GetListeners(sslConfigMap) + listenerActions := getListenerChanges(actualListeners, desiredListeners) + + if len(backendSetActions) == 0 && len(listenerActions) == 0 { + return nil // Nothing to do. } lbSubnets, err := cp.client.GetSubnets(spec.Subnets) @@ -244,143 +248,153 @@ func (cp *CloudProvider) updateBackendSets(lb *baremetal.LoadBalancer, spec LBSp return fmt.Errorf("get subnets for nodes: %v", err) } - sourceCIDRs := []string{} - listenerPort := uint64(0) - + actions := sortAndCombineActions(backendSetActions, listenerActions) for _, action := range actions { - var workRequestID string - 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) - - backendPort := uint64(getBackendPort(be.Backends)) - - switch action.Type { - case Create: - err = cp.securityListManager.Update(lbSubnets, nodeSubnets, sourceCIDRs, listenerPort, backendPort) + switch a := action.(type) { + case *BackendSetAction: + err := cp.updateBackendSet(lbOCID, a, lbSubnets, nodeSubnets) if err != nil { - return err + return fmt.Errorf("error updating BackendSet: %v", err) } - - workRequestID, err = cp.client.CreateBackendSet( - lbOCID, - be.Name, - be.Policy, - be.Backends, - be.HealthChecker, - nil, // ssl config - nil, // session persistence - nil, // create opts - ) - case Update: - err = cp.securityListManager.Update(lbSubnets, nodeSubnets, sourceCIDRs, listenerPort, backendPort) - if err != nil { - return err + case *ListenerAction: + 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] } - workRequestID, err = cp.client.UpdateBackendSet(lbOCID, be.Name, &baremetal.UpdateLoadBalancerBackendSetOptions{ - Policy: be.Policy, - HealthChecker: be.HealthChecker, - Backends: be.Backends, - }) - case Delete: - err = cp.securityListManager.Delete(lbSubnets, nodeSubnets, listenerPort, backendPort) + backendPort := uint64(getBackendPort(backendSet.Backends)) + err := cp.updateListener(lbOCID, a, backendPort, lbSubnets, nodeSubnets, sslConfigMap, sourceCIDRs) if err != nil { - return err + return fmt.Errorf("error updating Listener: %v", err) } - - workRequestID, err = cp.client.DeleteBackendSet(lbOCID, be.Name, nil) } + } + return nil +} + +func (cp *CloudProvider) updateBackendSet(lbOCID string, action *BackendSetAction, lbSubnets, nodeSubnets []*baremetal.Subnet) error { + sourceCIDRs := []string{} + listenerPort := uint64(0) + + var workRequestID string + var err error + + be := action.BackendSet + glog.V(2).Infof("Applying %q action on backend set `%s` for lb `%s`", action.Type(), be.Name, lbOCID) + backendPort := uint64(getBackendPort(be.Backends)) + + switch action.Type() { + case Create: + err = cp.securityListManager.Update(lbSubnets, nodeSubnets, sourceCIDRs, listenerPort, backendPort) if err != nil { return err } - _, err = cp.client.AwaitWorkRequest(workRequestID) + workRequestID, err = cp.client.CreateBackendSet( + lbOCID, + be.Name, + be.Policy, + be.Backends, + be.HealthChecker, + nil, // ssl config + nil, // session persistence + nil, // create opts + ) + case Update: + err = cp.securityListManager.Update(lbSubnets, nodeSubnets, sourceCIDRs, listenerPort, backendPort) if err != nil { return err } - } - - return nil -} -func (cp *CloudProvider) updateListeners(lb *baremetal.LoadBalancer, spec LBSpec, sslConfigMap map[int]*baremetal.SSLConfiguration, sourceCIDRs []string) error { - lbOCID := lb.ID + workRequestID, err = cp.client.UpdateBackendSet(lbOCID, be.Name, &baremetal.UpdateLoadBalancerBackendSetOptions{ + Policy: be.Policy, + HealthChecker: be.HealthChecker, + Backends: be.Backends, + }) + case Delete: + err = cp.securityListManager.Delete(lbSubnets, nodeSubnets, listenerPort, backendPort) + if err != nil { + return err + } - desired := spec.GetListeners(sslConfigMap) - actions := getListenerChanges(lb.Listeners, desired) - if len(actions) == 0 { - return nil + workRequestID, err = cp.client.DeleteBackendSet(lbOCID, be.Name, nil) } - lbSubnets, err := cp.client.GetSubnets(spec.Subnets) if err != nil { - return fmt.Errorf("get subnets for lbs: %v", err) + return err } - nodeSubnets, err := cp.client.GetSubnetsForNodes(spec.Nodes) + _, err = cp.client.AwaitWorkRequest(workRequestID) if err != nil { - return fmt.Errorf("get subnets for nodes: %v", err) + return err } - for _, action := range actions { - var workRequestID string - var err error - l := action.Listener - listenerPort := uint64(l.Port) + return nil +} - backends := spec.GetBackendSets()[l.DefaultBackendSetName].Backends - backendPort := uint64(getBackendPort(backends)) +func (cp *CloudProvider) updateListener(lbOCID string, + action *ListenerAction, + backendPort uint64, + lbSubnets []*baremetal.Subnet, + nodeSubnets []*baremetal.Subnet, + sslConfigMap map[int]*baremetal.SSLConfiguration, + sourceCIDRs []string) error { - glog.V(2).Infof("Applying `%s` action on listener `%s` for lb `%s`", action.Type, l.Name, lbOCID) + var workRequestID string + var err error + l := action.Listener + listenerPort := uint64(l.Port) - switch action.Type { - case Create: - err = cp.securityListManager.Update(lbSubnets, nodeSubnets, sourceCIDRs, listenerPort, backendPort) - if err != nil { - return err - } + glog.V(2).Infof("Applying %q action on listener `%s` for lb `%s`", action.Type(), l.Name, lbOCID) - workRequestID, err = cp.client.CreateListener( - lbOCID, - l.Name, - l.DefaultBackendSetName, - l.Protocol, - l.Port, - l.SSLConfig, - nil, // create opts - ) - case Update: - err = cp.securityListManager.Update(lbSubnets, nodeSubnets, sourceCIDRs, listenerPort, backendPort) - if err != nil { - return err - } - - workRequestID, err = cp.client.UpdateListener(lbOCID, l.Name, &baremetal.UpdateLoadBalancerListenerOptions{ - DefaultBackendSetName: l.DefaultBackendSetName, - Port: l.Port, - Protocol: l.Protocol, - SSLConfig: l.SSLConfig, - }) - case Delete: - err = cp.securityListManager.Delete(lbSubnets, nodeSubnets, listenerPort, backendPort) - if err != nil { - return err - } - - workRequestID, err = cp.client.DeleteListener(lbOCID, l.Name, nil) + switch action.Type() { + case Create: + err = cp.securityListManager.Update(lbSubnets, nodeSubnets, sourceCIDRs, listenerPort, backendPort) + if err != nil { + return err } + workRequestID, err = cp.client.CreateListener( + lbOCID, + l.Name, + l.DefaultBackendSetName, + l.Protocol, + l.Port, + l.SSLConfig, + nil, // create opts + ) + case Update: + err = cp.securityListManager.Update(lbSubnets, nodeSubnets, sourceCIDRs, listenerPort, backendPort) if err != nil { return err } - _, err = cp.client.AwaitWorkRequest(workRequestID) + workRequestID, err = cp.client.UpdateListener(lbOCID, l.Name, &baremetal.UpdateLoadBalancerListenerOptions{ + DefaultBackendSetName: l.DefaultBackendSetName, + Port: l.Port, + Protocol: l.Protocol, + SSLConfig: l.SSLConfig, + }) + case Delete: + err = cp.securityListManager.Delete(lbSubnets, nodeSubnets, listenerPort, backendPort) if err != nil { return err } + + workRequestID, err = cp.client.DeleteListener(lbOCID, l.Name, nil) + } + + if err != nil { + return err + } + + _, err = cp.client.AwaitWorkRequest(workRequestID) + if err != nil { + return err } return nil diff --git a/pkg/oci/load_balancer_security_lists.go b/pkg/oci/load_balancer_security_lists.go index 3c1a8acb96..c89428fb74 100644 --- a/pkg/oci/load_balancer_security_lists.go +++ b/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 { diff --git a/pkg/oci/load_balancer_util.go b/pkg/oci/load_balancer_util.go index 75228dc091..19c139da06 100644 --- a/pkg/oci/load_balancer_util.go +++ b/pkg/oci/load_balancer_util.go @@ -18,9 +18,11 @@ import ( "fmt" "os" "reflect" + "sort" "strconv" "strings" + "github.com/golang/glog" baremetal "github.com/oracle/bmcs-go-sdk" api "k8s.io/api/core/v1" @@ -46,19 +48,59 @@ const ( Delete = "delete" ) -// BackendSetAction denotes the action that should be taken on the given backend set. +// Action that should take place on the resource. +type Action interface { + Type() ActionType + Name() string +} + +// BackendSetAction denotes the action that should be taken on the given +// BackendSet. type BackendSetAction struct { - Type ActionType + Action + + actionType ActionType BackendSet baremetal.BackendSet } -// ListenerAction denotes the action that should be taken on the given listener. +// Type of the Action. +func (b *BackendSetAction) Type() ActionType { + return b.actionType +} + +// Name of the action's object. +func (b *BackendSetAction) Name() string { + return b.BackendSet.Name +} + +func (b *BackendSetAction) String() string { + return fmt.Sprintf("BackendSetAction:{Name: %s, Type: %v }", b.Name(), b.actionType) +} + +// ListenerAction denotes the action that should be taken on the given Listener. type ListenerAction struct { - Type ActionType - Listener baremetal.Listener + Action + + actionType ActionType + Listener baremetal.Listener +} + +// Type of the Action. +func (l *ListenerAction) Type() ActionType { + return l.actionType +} + +// Name of the action's object. +func (l *ListenerAction) Name() string { + return l.Listener.Name } -// TODO(horwitz): this doesn't check weight which we may want in the future to evenly distribute Local traffic policy load. +func (l *ListenerAction) String() string { + return fmt.Sprintf("ListenerAction:{Name: %s, Type: %v }", l.Name(), l.actionType) +} + +// TODO(horwitz): this doesn't check weight which we may want in the future to +// evenly distribute Local traffic policy load. func hasBackendSetChanged(actual, desired baremetal.BackendSet) bool { if !reflect.DeepEqual(actual.HealthChecker, desired.HealthChecker) { return true @@ -74,8 +116,8 @@ func hasBackendSetChanged(actual, desired baremetal.BackendSet) bool { nameFormat := "%s:%d" - // Since the lengths are equal that means the membership must be the same else - // there has been change. + // Since the lengths are equal that means the membership must be the same + // else there has been change. desiredSet := sets.NewString() for _, backend := range desired.Backends { name := fmt.Sprintf(nameFormat, backend.IPAddress, backend.Port) @@ -92,24 +134,24 @@ func hasBackendSetChanged(actual, desired baremetal.BackendSet) bool { return false } -func getBackendSetChanges(actual, desired map[string]baremetal.BackendSet) []BackendSetAction { - var backendSetActions []BackendSetAction +func getBackendSetChanges(actual, desired map[string]baremetal.BackendSet) []Action { + var backendSetActions []Action // First check to see if any backendsets need to be deleted or updated. for name, actualBackendSet := range actual { desiredBackendSet, ok := desired[name] if !ok { - // no longer exists - backendSetActions = append(backendSetActions, BackendSetAction{ + // No longer exists. + backendSetActions = append(backendSetActions, &BackendSetAction{ BackendSet: actualBackendSet, - Type: Delete, + actionType: Delete, }) continue } if hasBackendSetChanged(actualBackendSet, desiredBackendSet) { - backendSetActions = append(backendSetActions, BackendSetAction{ + backendSetActions = append(backendSetActions, &BackendSetAction{ BackendSet: desiredBackendSet, - Type: Update, + actionType: Update, }) } } @@ -117,10 +159,10 @@ func getBackendSetChanges(actual, desired map[string]baremetal.BackendSet) []Bac // Now check if any need to be created. for name, desiredBackendSet := range desired { if _, ok := actual[name]; !ok { - // doesn't exist so lets create it - backendSetActions = append(backendSetActions, BackendSetAction{ + // Doesn't exist so lets create it. + backendSetActions = append(backendSetActions, &BackendSetAction{ BackendSet: desiredBackendSet, - Type: Create, + actionType: Create, }) } } @@ -132,24 +174,24 @@ func hasListenerChanged(actual, desired baremetal.Listener) bool { return !reflect.DeepEqual(actual, desired) } -func getListenerChanges(actual, desired map[string]baremetal.Listener) []ListenerAction { - var listenerActions []ListenerAction +func getListenerChanges(actual, desired map[string]baremetal.Listener) []Action { + var listenerActions []Action // First check to see if any listeners need to be deleted or updated. for name, actualListener := range actual { desiredListener, ok := desired[name] if !ok { // no longer exists - listenerActions = append(listenerActions, ListenerAction{ - Listener: actualListener, - Type: Delete, + listenerActions = append(listenerActions, &ListenerAction{ + Listener: actualListener, + actionType: Delete, }) continue } if hasListenerChanged(actualListener, desiredListener) { - listenerActions = append(listenerActions, ListenerAction{ - Listener: desiredListener, - Type: Update, + listenerActions = append(listenerActions, &ListenerAction{ + Listener: desiredListener, + actionType: Update, }) } } @@ -158,9 +200,9 @@ func getListenerChanges(actual, desired map[string]baremetal.Listener) []Listene for name, desiredListener := range desired { if _, ok := actual[name]; !ok { // doesn't exist so lets create it - listenerActions = append(listenerActions, ListenerAction{ - Listener: desiredListener, - Type: Create, + listenerActions = append(listenerActions, &ListenerAction{ + Listener: desiredListener, + actionType: Create, }) } } @@ -236,3 +278,48 @@ func parseSecretString(secretString string) (string, string) { } return "", secretString } + +// sortAndCombineActions combines two slices of Actions and then sorts them to +// ensure that BackendSets are created prior to their associated Listeners but +// deleted after their associated Listeners. +func sortAndCombineActions(backendSetActions []Action, listenerActions []Action) []Action { + actions := append(backendSetActions, listenerActions...) + sort.Slice(actions, func(i, j int) bool { + a1 := actions[i] + a2 := actions[j] + + // Sort by the name until we get to the point a1 and a2 are Actions upon + // an associated Listener and BackendSet (which share the same name). + if a1.Name() != a2.Name() { + return a1.Name() < a2.Name() + } + + // For Create and Delete (which is what we really care about) the + // ActionType will always be the same so we can get away with just + // checking the type of the first action. + switch a1.Type() { + case Create: + // Create the BackendSet then Listener. + _, ok := a1.(*BackendSetAction) + return ok + case Update: + // Doesn't matter. + return true + case Delete: + // Delete the Listener then BackendSet. + _, ok := a2.(*BackendSetAction) + return ok + default: + // Should never be reachable. + glog.Errorf("Unknown action type received: %+v", a1) + return true + } + }) + 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) +} diff --git a/pkg/oci/load_balancer_util_test.go b/pkg/oci/load_balancer_util_test.go index 0279657648..c687d2f81e 100644 --- a/pkg/oci/load_balancer_util_test.go +++ b/pkg/oci/load_balancer_util_test.go @@ -24,12 +24,150 @@ import ( baremetal "github.com/oracle/bmcs-go-sdk" ) +func TestSortAndCombineActions(t *testing.T) { + testCases := map[string]struct { + backendSetActions []Action + listenerActions []Action + expected []Action + }{ + "create": { + backendSetActions: []Action{ + &BackendSetAction{ + actionType: Create, + BackendSet: baremetal.BackendSet{Name: "TCP-80"}, + }, + &BackendSetAction{ + actionType: Create, + BackendSet: baremetal.BackendSet{Name: "TCP-443"}, + }, + }, + listenerActions: []Action{ + &ListenerAction{ + actionType: Create, + Listener: baremetal.Listener{Name: "TCP-443"}, + }, + &ListenerAction{ + actionType: Create, + Listener: baremetal.Listener{Name: "TCP-80"}, + }, + }, + expected: []Action{ + &BackendSetAction{ + actionType: Create, + BackendSet: baremetal.BackendSet{Name: "TCP-443"}, + }, + &ListenerAction{ + actionType: Create, + Listener: baremetal.Listener{Name: "TCP-443"}, + }, + &BackendSetAction{ + actionType: Create, + BackendSet: baremetal.BackendSet{Name: "TCP-80"}, + }, + &ListenerAction{ + actionType: Create, + Listener: baremetal.Listener{Name: "TCP-80"}, + }, + }, + }, + "update": { + backendSetActions: []Action{ + &BackendSetAction{ + actionType: Update, + BackendSet: baremetal.BackendSet{Name: "TCP-80"}, + }, + &BackendSetAction{ + actionType: Update, + BackendSet: baremetal.BackendSet{Name: "TCP-443"}, + }, + }, + listenerActions: []Action{ + &ListenerAction{ + actionType: Update, + Listener: baremetal.Listener{Name: "TCP-443"}, + }, + &ListenerAction{ + actionType: Update, + Listener: baremetal.Listener{Name: "TCP-80"}, + }, + }, + expected: []Action{ + &ListenerAction{ + actionType: Update, + Listener: baremetal.Listener{Name: "TCP-443"}, + }, + &BackendSetAction{ + actionType: Update, + BackendSet: baremetal.BackendSet{Name: "TCP-443"}, + }, + &ListenerAction{ + actionType: Update, + Listener: baremetal.Listener{Name: "TCP-80"}, + }, + &BackendSetAction{ + actionType: Update, + BackendSet: baremetal.BackendSet{Name: "TCP-80"}, + }, + }, + }, + "delete": { + backendSetActions: []Action{ + &BackendSetAction{ + actionType: Delete, + BackendSet: baremetal.BackendSet{Name: "TCP-80"}, + }, + &BackendSetAction{ + actionType: Delete, + BackendSet: baremetal.BackendSet{Name: "TCP-443"}, + }, + }, + listenerActions: []Action{ + &ListenerAction{ + actionType: Delete, + Listener: baremetal.Listener{Name: "TCP-443"}, + }, + &ListenerAction{ + actionType: Delete, + Listener: baremetal.Listener{Name: "TCP-80"}, + }, + }, + expected: []Action{ + &ListenerAction{ + actionType: Delete, + Listener: baremetal.Listener{Name: "TCP-443"}, + }, + &BackendSetAction{ + actionType: Delete, + BackendSet: baremetal.BackendSet{Name: "TCP-443"}, + }, + &ListenerAction{ + actionType: Delete, + Listener: baremetal.Listener{Name: "TCP-80"}, + }, + &BackendSetAction{ + actionType: Delete, + BackendSet: baremetal.BackendSet{Name: "TCP-80"}, + }, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + result := sortAndCombineActions(tc.backendSetActions, tc.listenerActions) + if !reflect.DeepEqual(result, tc.expected) { + t.Errorf("expected\n%+v\nbut got\n%+v", tc.expected, result) + } + }) + } +} + func TestGetBackendSetChanges(t *testing.T) { var testCases = []struct { name string desired map[string]baremetal.BackendSet actual map[string]baremetal.BackendSet - expected []BackendSetAction + expected []Action }{ { name: "create backendset", @@ -54,9 +192,9 @@ func TestGetBackendSetChanges(t *testing.T) { }, }, }, - expected: []BackendSetAction{ - { - Type: Create, + expected: []Action{ + &BackendSetAction{ + actionType: Create, BackendSet: baremetal.BackendSet{ Name: "two", Backends: []baremetal.Backend{ @@ -84,9 +222,9 @@ func TestGetBackendSetChanges(t *testing.T) { }, }, }, - expected: []BackendSetAction{ - { - Type: Update, + expected: []Action{ + &BackendSetAction{ + actionType: Update, BackendSet: baremetal.BackendSet{ Backends: []baremetal.Backend{ {IPAddress: "0.0.0.0", Port: 80}, @@ -113,9 +251,9 @@ func TestGetBackendSetChanges(t *testing.T) { }, }, }, - expected: []BackendSetAction{ - { - Type: Update, + expected: []Action{ + &BackendSetAction{ + actionType: Update, BackendSet: baremetal.BackendSet{ Backends: []baremetal.Backend{ {IPAddress: "0.0.0.0", Port: 80}, @@ -134,9 +272,9 @@ func TestGetBackendSetChanges(t *testing.T) { }, }, }, - expected: []BackendSetAction{ - { - Type: Delete, + expected: []Action{ + &BackendSetAction{ + actionType: Delete, BackendSet: baremetal.BackendSet{ Backends: []baremetal.Backend{ {IPAddress: "0.0.0.0", Port: 80}, @@ -161,7 +299,7 @@ func TestGetBackendSetChanges(t *testing.T) { }, }, }, - expected: []BackendSetAction{}, + expected: []Action{}, }, } @@ -183,7 +321,7 @@ func TestGetListenerChanges(t *testing.T) { name string desired map[string]baremetal.Listener actual map[string]baremetal.Listener - expected []ListenerAction + expected []Action }{ { name: "create listener", @@ -194,9 +332,9 @@ func TestGetListenerChanges(t *testing.T) { Port: 443, }}, actual: map[string]baremetal.Listener{}, - expected: []ListenerAction{ - { - Type: Create, + expected: []Action{ + &ListenerAction{ + actionType: Create, Listener: baremetal.Listener{ Name: "TCP-443", DefaultBackendSetName: "TCP-443", @@ -230,9 +368,9 @@ func TestGetListenerChanges(t *testing.T) { Port: 80, }, }, - expected: []ListenerAction{ - { - Type: Create, + expected: []Action{ + &ListenerAction{ + actionType: Create, Listener: baremetal.Listener{ Name: "TCP-443", DefaultBackendSetName: "TCP-443", @@ -266,9 +404,9 @@ func TestGetListenerChanges(t *testing.T) { Port: 80, }, }, - expected: []ListenerAction{ - { - Type: Delete, + expected: []Action{ + &ListenerAction{ + actionType: Delete, Listener: baremetal.Listener{ Name: "TCP-443", DefaultBackendSetName: "TCP-443", @@ -296,7 +434,7 @@ func TestGetListenerChanges(t *testing.T) { Port: 80, }, }, - expected: []ListenerAction{}, + expected: []Action{}, }, { name: "ssl config change", @@ -322,9 +460,9 @@ func TestGetListenerChanges(t *testing.T) { }, }, }, - expected: []ListenerAction{ - { - Type: Update, + expected: []Action{ + &ListenerAction{ + actionType: Update, Listener: baremetal.Listener{ Name: "TCP-80", DefaultBackendSetName: "TCP-80", diff --git a/test/integration/loadbalancer/loadbalancer_test.go b/test/integration/loadbalancer/loadbalancer_test.go index 9c483f24d1..3c5a4924dd 100644 --- a/test/integration/loadbalancer/loadbalancer_test.go +++ b/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