Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

further fully-automatic-egress-IP cleanup #20718

Closed
wants to merge 2 commits into from
+23 −23
Closed

further fully-automatic-egress-IP cleanup #20718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2bd41c8
Change ReallocateEgressIPs() to return the full allocation, not just …
danwinship Jul 27, 2018
0bb0602
Test deleting all EgressIPs from auto-assigned NetNamespace
danwinship Aug 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 2 additions & 14 deletions pkg/network/common/egressip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,22 +506,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
Expand All @@ -536,8 +534,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
Expand All @@ -553,7 +549,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
}
}
Expand All @@ -565,15 +560,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
}
21 changes: 14 additions & 7 deletions pkg/network/common/egressip_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -947,31 +944,41 @@ func TestEgressCIDRAllocation(t *testing.T) {
t.Fatalf("%v", err)
}

// Changing the EgressIPs of a namespace should drop the old allocation and create a new one
// Changing/Removing the EgressIPs of a namespace should drop the old allocation and create a new one
updateNetNamespaceEgress(eit, &networkapi.NetNamespace{
NetID: 46,
EgressIPs: []string{"172.17.0.202"}, // was 172.17.0.200
})
updateNetNamespaceEgress(eit, &networkapi.NetNamespace{
NetID: 44,
EgressIPs: []string{}, // was 172.17.1.1
})
err = w.assertChanges(
"release 172.17.0.200 on 172.17.0.4",
"namespace 46 dropped",
"update egress CIDRs",
"release 172.17.1.1 on 172.17.0.3",
"namespace 44 normal",
"update egress CIDRs",
)
if err != nil {
t.Fatalf("%v", err)
}

allocation = eit.ReallocateEgressIPs()
for _, ip := range allocation["node-4"] {
if ip == "172.17.0.200" {
t.Fatalf("reallocation failed to drop unused egress IP 172.17.0.200: %#v", allocation)
for _, nodeAllocation := range allocation {
for _, ip := range nodeAllocation {
if ip == "172.17.1.1" || ip == "172.17.0.200" {
t.Fatalf("reallocation failed to drop unused egress IP %s: %#v", ip, allocation)
}
}
}
updateAllocations(eit, allocation)
err = w.assertChanges(
"claim 172.17.0.202 on 172.17.0.4 for namespace 46",
"namespace 46 via 172.17.0.202 on 172.17.0.4",
"update egress CIDRs",
"update egress CIDRs",
)
if err != nil {
t.Fatalf("%v", err)
Expand Down
9 changes: 7 additions & 2 deletions pkg/network/master/egressip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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 {
Expand Down