Skip to content

Commit

Permalink
Refactor findAndFilterMigrationEndpoints from syncer to dualstack pac…
Browse files Browse the repository at this point in the history 
…kage
  • Loading branch information
@gauravkghildiyal
gauravkghildiyal committed Apr 17, 2023
1 parent bca6ee4 commit 80420cf
Show file tree
Hide file tree
Showing 4 changed files with 312 additions and 320 deletions.
71 changes: 71 additions & 0 deletions pkg/neg/syncers/dualstack/migrator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,74 @@ func (d *Migrator) Continue(err error) {
}
}

// findAndFilterMigrationEndpoints will filter out the migration endpoints from
// the `addEndpoints` and `removeEndpoints` sets. The passed sets will get
// modified. The returned value will be two endpoints sets which will contain
// the values that were filtered out from the `addEndpoints` and
// `removeEndpoints` sets respectively.
func findAndFilterMigrationEndpoints(addEndpoints, removeEndpoints map[string]types.NetworkEndpointSet) (map[string]types.NetworkEndpointSet, map[string]types.NetworkEndpointSet) {
allEndpoints := make(map[string]types.NetworkEndpointSet)
for zone, endpointSet := range addEndpoints {
allEndpoints[zone] = allEndpoints[zone].Union(endpointSet)
}
for zone, endpointSet := range removeEndpoints {
allEndpoints[zone] = allEndpoints[zone].Union(endpointSet)
}

migrationEndpointsInAddSet := make(map[string]types.NetworkEndpointSet)
migrationEndpointsInRemoveSet := make(map[string]types.NetworkEndpointSet)
for zone, endpointSet := range allEndpoints {
for endpoint := range endpointSet {
if endpoint.IP == "" || endpoint.IPv6 == "" {
// Endpoint is not dual-stack so continue.
continue
}

// At this point, we know that `endpoint` is a dual-stack endpoint. An
// endpoint can be identified as migrating if the IPs from the dual-stack
// endpoint exist as individual single-stack endpoint inside
// `addEndpoints` or `removeEndpoints`.

// Construct single-stack endpoint corresponding to the dual-stack
// endpoint. Their existence will determine if an endpoint is migrating.
ipv4Only := endpoint
ipv4Only.IPv6 = ""
ipv6Only := endpoint
ipv6Only.IP = ""

isMigrating := false
// Check if endpoint is migrating from dual-stack to single-stack.
isMigrating = isMigrating || moveEndpoint(ipv4Only, addEndpoints, migrationEndpointsInAddSet, zone)
isMigrating = isMigrating || moveEndpoint(ipv6Only, addEndpoints, migrationEndpointsInAddSet, zone)
// Check if endpoint is migrating from single-stack to dual-stack.
isMigrating = isMigrating || moveEndpoint(ipv4Only, removeEndpoints, migrationEndpointsInRemoveSet, zone)
isMigrating = isMigrating || moveEndpoint(ipv6Only, removeEndpoints, migrationEndpointsInRemoveSet, zone)

if isMigrating {
moveEndpoint(endpoint, addEndpoints, migrationEndpointsInAddSet, zone)
moveEndpoint(endpoint, removeEndpoints, migrationEndpointsInRemoveSet, zone)
}
}
}

return migrationEndpointsInAddSet, migrationEndpointsInRemoveSet
}

// moveEndpoint deletes endpoint `e` from `source[zone]` and adds it to
// `dest[zone]`. If the move was successful, `true` is returned. A return value
// of `false` denotes that nothing was moved and no input variable were
// modified.
func moveEndpoint(e types.NetworkEndpoint, source, dest map[string]types.NetworkEndpointSet, zone string) bool {
if source == nil || dest == nil {
return false
}
if source[zone].Has(e) {
source[zone].Delete(e)
if dest[zone] == nil {
dest[zone] = types.NewNetworkEndpointSet()
}
dest[zone].Insert(e)
return true
}
return false
}
241 changes: 241 additions & 0 deletions pkg/neg/syncers/dualstack/migrator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,247 @@ func TestFilter(t *testing.T) {
}
}

func TestFindAndFilterMigrationEndpoints(t *testing.T) {
testCases := []struct {
name string
addEndpoints map[string]types.NetworkEndpointSet
removeEndpoints map[string]types.NetworkEndpointSet
wantMigrationEndpointsInAddSet map[string]types.NetworkEndpointSet
wantMigrationEndpointsInRemoveSet map[string]types.NetworkEndpointSet
}{
{
name: "detect multiple migrating endpoints",
addEndpoints: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a", IPv6: "A"}, // migrating
{IP: "b"},
{IP: "c", IPv6: "C"},
{IP: "d"}, // migrating
}...),
"zone2": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "e", IPv6: "E"}, // migrating
}...),
},
removeEndpoints: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a"}, // migrating
{IP: "f", IPv6: "F"},
{IP: "d", IPv6: "D"}, // migrating
}...),
"zone2": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IPv6: "E"}, // migrating
}...),
},
wantMigrationEndpointsInAddSet: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a", IPv6: "A"},
{IP: "d"},
}...),
"zone2": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "e", IPv6: "E"},
}...),
},
wantMigrationEndpointsInRemoveSet: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a"},
{IP: "d", IPv6: "D"},
}...),
"zone2": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IPv6: "E"},
}...),
},
},
{
name: "partial IP change without stack change is not considered migrating",
addEndpoints: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a", IPv6: "A"},
}...),
},
removeEndpoints: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a", Port: "B"},
}...),
},
wantMigrationEndpointsInAddSet: map[string]types.NetworkEndpointSet{},
wantMigrationEndpointsInRemoveSet: map[string]types.NetworkEndpointSet{},
},
{
name: "difference in port or node is not considered migrating",
addEndpoints: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a", IPv6: "A", Port: "80"},
{IP: "b", Node: "node2"},
}...),
},
removeEndpoints: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a", Port: "81"},
{IP: "b", IPv6: "B", Node: "node1"},
}...),
},
wantMigrationEndpointsInAddSet: map[string]types.NetworkEndpointSet{},
wantMigrationEndpointsInRemoveSet: map[string]types.NetworkEndpointSet{},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
gotMigrationEndpointsInAddSet, gotMigrationEndpointsInRemoveSet := findAndFilterMigrationEndpoints(tc.addEndpoints, tc.removeEndpoints)

if diff := cmp.Diff(tc.wantMigrationEndpointsInAddSet, gotMigrationEndpointsInAddSet); diff != "" {
t.Errorf("findAndFilterMigrationEndpoints(tc.addEndpoints, tc.removeEndpoints) returned unexpected diff for migrationEndpointsInAddSet (-want +got):\n%s", diff)
}
if diff := cmp.Diff(tc.wantMigrationEndpointsInRemoveSet, gotMigrationEndpointsInRemoveSet); diff != "" {
t.Errorf("findAndFilterMigrationEndpoints(tc.addEndpoints, tc.removeEndpoints) returned unexpected diff for migrationEndpointsInRemoveSet (-want +got):\n%s", diff)
}
})
}
}

func TestMoveEndpoint(t *testing.T) {
testCases := []struct {
name string
endpoint types.NetworkEndpoint
inputSource map[string]types.NetworkEndpointSet
inputDest map[string]types.NetworkEndpointSet
wantSource map[string]types.NetworkEndpointSet
wantDest map[string]types.NetworkEndpointSet
zone string
wantSuccess bool
}{
{
name: "completely valid input, shoud successfully move",
endpoint: types.NetworkEndpoint{IP: "a", IPv6: "A"},
inputSource: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a", IPv6: "A"},
{IP: "b", IPv6: "B"},
}...),
},
inputDest: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "c", IPv6: "C"},
}...),
},
wantSource: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "b", IPv6: "B"},
}...),
},
wantDest: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a", IPv6: "A"},
{IP: "c", IPv6: "C"},
}...),
},
zone: "zone1",
wantSuccess: true,
},
{
name: "zone does not exist in source",
endpoint: types.NetworkEndpoint{IP: "a", IPv6: "A"},
inputSource: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a", IPv6: "A"},
}...),
},
inputDest: map[string]types.NetworkEndpointSet{
"zone3": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "c", IPv6: "C"},
}...),
},
wantSource: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a", IPv6: "A"},
}...),
},
wantDest: map[string]types.NetworkEndpointSet{
"zone3": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "c", IPv6: "C"},
}...),
},
zone: "zone3",
},
{
name: "zone does not exist in destination, shoud successfully move",
endpoint: types.NetworkEndpoint{IP: "a", IPv6: "A"},
inputSource: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a", IPv6: "A"},
{IP: "b", IPv6: "B"},
}...),
},
inputDest: map[string]types.NetworkEndpointSet{
"zone2": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "c", IPv6: "C"},
}...),
},
wantSource: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "b", IPv6: "B"},
}...),
},
wantDest: map[string]types.NetworkEndpointSet{
"zone1": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "a", IPv6: "A"},
}...),
"zone2": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "c", IPv6: "C"},
}...),
},
zone: "zone1",
wantSuccess: true,
},
{
name: "source is nil",
endpoint: types.NetworkEndpoint{IP: "a", IPv6: "A"},
inputDest: map[string]types.NetworkEndpointSet{
"zone3": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "c", IPv6: "C"},
}...),
},
wantDest: map[string]types.NetworkEndpointSet{
"zone3": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "c", IPv6: "C"},
}...),
},
zone: "zone3",
},
{
name: "destination is nil",
endpoint: types.NetworkEndpoint{IP: "a", IPv6: "A"},
inputSource: map[string]types.NetworkEndpointSet{
"zone3": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "c", IPv6: "C"},
}...),
},
wantSource: map[string]types.NetworkEndpointSet{
"zone3": types.NewNetworkEndpointSet([]types.NetworkEndpoint{
{IP: "c", IPv6: "C"},
}...),
},
zone: "zone3",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
gotSuccess := moveEndpoint(tc.endpoint, tc.inputSource, tc.inputDest, tc.zone)

if gotSuccess != tc.wantSuccess {
t.Errorf("moveEndpoint(%v, ...) = %v, want = %v", tc.endpoint, gotSuccess, tc.wantSuccess)
}
if diff := cmp.Diff(tc.wantSource, tc.inputSource); diff != "" {
t.Errorf("moveEndpoint(%v, ...) returned unexpected diff for source (-want +got):\n%s", tc.endpoint, diff)
}
if diff := cmp.Diff(tc.wantDest, tc.inputDest); diff != "" {
t.Errorf("moveEndpoint(%v, ...) returned unexpected diff for destination (-want +got):\n%s", tc.endpoint, diff)
}
})
}
}

func cloneZoneNetworkEndpointsMap(m map[string]types.NetworkEndpointSet) map[string]types.NetworkEndpointSet {
clone := make(map[string]types.NetworkEndpointSet)
for zone, endpointSet := range m {
Expand Down
Loading

0 comments on commit 80420cf

Please sign in to comment.