Change ReallocateEgressIPs() to return the full allocation, not just changes

Author: danwinship

pkg/network/common/egressip.go

@@ -498,22 +498,20 @@ func (eit *EgressIPTracker) findEgressIPAllocation(ip net.IP, allocation map[str
 	return bestNode, otherNodes
 }
 
-// ReallocateEgressIPs returns a map from Node name to array-of-Egress-IP. Unchanged nodes are not included.
+// ReallocateEgressIPs returns a map from Node name to array-of-Egress-IP for all auto-allocated egress IPs
 func (eit *EgressIPTracker) ReallocateEgressIPs() map[string][]string {
 	eit.Lock()
 	defer eit.Unlock()
 
 	allocation := make(map[string][]string)
-	changed := make(map[string]bool)
 	alreadyAllocated := make(map[string]bool)
 	for _, node := range eit.nodes {
 		if len(node.parsedCIDRs) > 0 {
 			allocation[node.nodeName] = make([]string, 0, node.requestedIPs.Len())
 		}
 	}
 	// For each active egress IP, if it still fits within some egress CIDR on its node,
-	// add it to that node's allocation. (Otherwise add the node to the "changed" map,
-	// since we'll be removing this egress IP from it.)
+	// add it to that node's allocation.
 	for egressIP, eip := range eit.egressIPs {
 		if eip.assignedNodeIP == "" {
 			continue
@@ -528,8 +526,6 @@ func (eit *EgressIPTracker) ReallocateEgressIPs() map[string][]string {
 		}
 		if found {
 			allocation[node.nodeName] = append(allocation[node.nodeName], egressIP)
-		} else {
-			changed[node.nodeName] = true
 		}
 		// (We set alreadyAllocated even if the egressIP will be removed from
 		// its current node; we can't assign it to a new node until the next
@@ -545,7 +541,6 @@ func (eit *EgressIPTracker) ReallocateEgressIPs() map[string][]string {
 		nodeName, otherNodes := eit.findEgressIPAllocation(eip.parsed, allocation)
 		if nodeName != "" && !otherNodes {
 			allocation[nodeName] = append(allocation[nodeName], egressIP)
-			changed[nodeName] = true
 			alreadyAllocated[egressIP] = true
 		}
 	}
@@ -557,15 +552,8 @@ func (eit *EgressIPTracker) ReallocateEgressIPs() map[string][]string {
 		nodeName, _ := eit.findEgressIPAllocation(eip.parsed, allocation)
 		if nodeName != "" {
 			allocation[nodeName] = append(allocation[nodeName], egressIP)
-			changed[nodeName] = true
 		}
 	}
 
-	// Remove unchanged nodes from the return value
-	for _, node := range eit.nodes {
-		if !changed[node.nodeName] {
-			delete(allocation, node.nodeName)
-		}
-	}
 	return allocation
 }

pkg/network/common/egressip_test.go

@@ -864,9 +864,6 @@ func TestEgressCIDRAllocation(t *testing.T) {
 		t.Fatalf("%v", err)
 	}
 	allocation = eit.ReallocateEgressIPs()
-	if len(allocation) != 0 {
-		t.Fatalf("Unexpected allocation: %#v", allocation)
-	}
 	updateAllocations(eit, allocation)
 	err = w.assertNoChanges()
 	if err != nil {

pkg/network/master/egressip.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 	utilwait "k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/util/retry"
 
@@ -82,8 +83,12 @@ func (eim *egressIPManager) maybeDoUpdateEgressCIDRs() (bool, error) {
 				return err
 			}
 
-			hs.EgressIPs = egressIPs
-			_, err = eim.networkClient.Network().HostSubnets().Update(hs)
+			oldIPs := sets.NewString(hs.EgressIPs...)
+			newIPs := sets.NewString(egressIPs...)
+			if !oldIPs.Equal(newIPs) {
+				hs.EgressIPs = egressIPs
+				_, err = eim.networkClient.Network().HostSubnets().Update(hs)
+			}
 			return err
 		})
 		if resultErr != nil {