Repository: kubernetes-sigs/aws-alb-ingress-controller
Pull request #1520 (2020-10-19 16:10:31)

improve performance
performance improvements for TargetGroupBinding by:
1. use hash of IPPermission to diff permissions, the old o(n^2) way with go-cmp is way too slow.
2. use strategyMergePatch for pod conditions, since each TGB owns it's own condition, we don't need to check for conflicts.

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/aws-alb-ingress-controller/pull/1520#" title="Author self-approved">M00nF1sh</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign  after the PR has been reviewed.
You can assign the PR to them by writing `/assign ` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Faws-alb-ingress-controller).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-sigs/aws-alb-ingress-controller/blob/v2_ga/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/aws-alb-ingress-controller/pull/1520#pullrequestreview-512065895" title="Approved">kishorj</a>*, *<a href="https://github.com/kubernetes-sigs/aws-alb-ingress-controller/pull/1520#" title="Author self-approved">M00nF1sh</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign  after the PR has been reviewed.
You can assign the PR to them by writing `/assign ` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Faws-alb-ingress-controller).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-sigs/aws-alb-ingress-controller/blob/v2_ga/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/pkg/equality/ec2/compare_option_for_ip_permission.go b/pkg/equality/ec2/compare_option_for_ip_permission.go
deleted file mode 100644
index 173ae18b4..000000000
--- a/pkg/equality/ec2/compare_option_for_ip_permission.go
+++ /dev/null
@@ -1,80 +0,0 @@
-package ec2
-
-import (
-	"github.com/aws/aws-sdk-go/aws"
-	ec2sdk "github.com/aws/aws-sdk-go/service/ec2"
-	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
-	"sigs.k8s.io/aws-load-balancer-controller/pkg/equality"
-)
-
-// CompareOptionForIPRange returns the compare option for ec2 IPRange
-func CompareOptionForIPRange() cmp.Option {
-	return equality.IgnoreOtherFields(ec2sdk.IpRange{}, "CidrIp")
-}
-
-// CompareOptionForIPRanges returns the compare option for ec2 IPRange slice
-func CompareOptionForIPRanges() cmp.Options {
-	return cmp.Options{
-		cmpopts.SortSlices(func(lhs *ec2sdk.IpRange, rhs *ec2sdk.IpRange) bool {
-			return aws.StringValue(lhs.CidrIp) < aws.StringValue(rhs.CidrIp)
-		}),
-		CompareOptionForIPRange(),
-	}
-}
-
-// CompareOptionForIPv6Range returns the compare option for ec2 IPv6Range
-func CompareOptionForIPv6Range() cmp.Option {
-	return equality.IgnoreOtherFields(ec2sdk.Ipv6Range{}, "CidrIpv6")
-}
-
-// CompareOptionForIPV6Ranges returns the compare option for ec2 IPv6Range slice
-func CompareOptionForIPv6Ranges() cmp.Options {
-	return cmp.Options{
-		cmpopts.SortSlices(func(lhs *ec2sdk.Ipv6Range, rhs *ec2sdk.Ipv6Range) bool {
-			return aws.StringValue(lhs.CidrIpv6) < aws.StringValue(rhs.CidrIpv6)
-		}),
-		CompareOptionForIPv6Range(),
-	}
-}
-
-// CompareOptionForIPV6Ranges returns the compare option for ec2 UserIDGroupPair
-func CompareOptionForUserIDGroupPair() cmp.Option {
-	return equality.IgnoreOtherFields(ec2sdk.UserIdGroupPair{}, "GroupId")
-}
-
-// CompareOptionForIPV6Ranges returns the compare option for ec2 UserIDGroupPair slice
-func CompareOptionForUserIDGroupPairs() cmp.Option {
-	return cmp.Options{
-		cmpopts.SortSlices(func(lhs *ec2sdk.UserIdGroupPair, rhs *ec2sdk.UserIdGroupPair) bool {
-			return aws.StringValue(lhs.GroupId) < aws.StringValue(rhs.GroupId)
-		}),
-		CompareOptionForUserIDGroupPair(),
-	}
-}
-
-// CompareOptionForIPV6Ranges returns the compare option for ec2 prefixListId
-func CompareOptionForPrefixListId() cmp.Option {
-	return equality.IgnoreOtherFields(ec2sdk.PrefixListId{}, "PrefixListId")
-}
-
-// CompareOptionForIPV6Ranges returns the compare option for ec2 prefixListId slice
-func CompareOptionForPrefixListIds() cmp.Option {
-	return cmp.Options{
-		cmpopts.SortSlices(func(lhs *ec2sdk.PrefixListId, rhs *ec2sdk.PrefixListId) bool {
-			return aws.StringValue(lhs.PrefixListId) < aws.StringValue(rhs.PrefixListId)
-		}),
-		CompareOptionForPrefixListId(),
-	}
-}
-
-// CompareOptionForIPPermission returns the compare option for ec2 IPPermission object.
-func CompareOptionForIPPermission() cmp.Option {
-	return cmp.Options{
-		cmpopts.EquateEmpty(),
-		CompareOptionForIPRanges(),
-		CompareOptionForIPv6Ranges(),
-		CompareOptionForUserIDGroupPairs(),
-		CompareOptionForPrefixListIds(),
-	}
-}
diff --git a/pkg/networking/security_group_info.go b/pkg/networking/security_group_info.go
index 94964b7a7..20879cfd3 100644
--- a/pkg/networking/security_group_info.go
+++ b/pkg/networking/security_group_info.go
@@ -1,6 +1,7 @@
 package networking
 
 import (
+	"encoding/json"
 	"fmt"
 	awssdk "github.com/aws/aws-sdk-go/aws"
 	ec2sdk "github.com/aws/aws-sdk-go/service/ec2"
@@ -35,6 +36,35 @@ type IPPermissionInfo struct {
 	Labels map[string]string
 }
 
+// HashCode returns the hashcode for the IPPermissionInfo.
+// The hashCode should only include the actual permission but not labels/descriptions.
+func (perm *IPPermissionInfo) HashCode() string {
+	protocol := awssdk.StringValue(perm.Permission.IpProtocol)
+	fromPort := awssdk.Int64Value(perm.Permission.FromPort)
+	toPort := awssdk.Int64Value(perm.Permission.ToPort)
+	base := fmt.Sprintf("IpProtocol: %v, FromPort: %v, ToPort: %v", protocol, fromPort, toPort)
+	if len(perm.Permission.IpRanges) == 1 {
+		cidrIP := awssdk.StringValue(perm.Permission.IpRanges[0].CidrIp)
+		return fmt.Sprintf("%v, IpRange: %v", base, cidrIP)
+	}
+	if len(perm.Permission.Ipv6Ranges) == 1 {
+		cidrIPv6 := awssdk.StringValue(perm.Permission.Ipv6Ranges[0].CidrIpv6)
+		return fmt.Sprintf("%v, Ipv6Range: %v", base, cidrIPv6)
+	}
+	if len(perm.Permission.PrefixListIds) == 1 {
+		prefixListID := awssdk.StringValue(perm.Permission.PrefixListIds[0].PrefixListId)
+		return fmt.Sprintf("%v, PrefixListId: %v", base, prefixListID)
+	}
+	if len(perm.Permission.UserIdGroupPairs) == 1 {
+		groupID := awssdk.StringValue(perm.Permission.UserIdGroupPairs[0].GroupId)
+		return fmt.Sprintf("%v, UserIdGroupPair: %v", base, groupID)
+	}
+
+	// we are facing some unknown permission, let's include all fields.
+	payload, _ := json.Marshal(perm.Permission)
+	return string(payload)
+}
+
 // NewRawSecurityGroupInfo constructs new SecurityGroupInfo with raw ec2SDK's SecurityGroup object.
 func NewRawSecurityGroupInfo(sdkSG *ec2sdk.SecurityGroup) SecurityGroupInfo {
 	sgID := awssdk.StringValue(sdkSG.GroupId)
diff --git a/pkg/networking/security_group_reconciler.go b/pkg/networking/security_group_reconciler.go
index dffd4ed59..ea1ea104b 100644
--- a/pkg/networking/security_group_reconciler.go
+++ b/pkg/networking/security_group_reconciler.go
@@ -4,11 +4,9 @@ import (
 	"context"
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/go-logr/logr"
-	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/labels"
-	ec2equality "sigs.k8s.io/aws-load-balancer-controller/pkg/equality/ec2"
+	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 // configuration options for SecurityGroup Reconcile options.
@@ -128,22 +126,20 @@ func (r *defaultSecurityGroupReconciler) shouldRetryWithoutCache(err error) bool
 
 // diffIPPermissionInfos calculates set_difference as source - target
 func diffIPPermissionInfos(source []IPPermissionInfo, target []IPPermissionInfo) []IPPermissionInfo {
-	opts := cmp.Options{
-		ec2equality.CompareOptionForIPPermission(),
-		cmpopts.IgnoreFields(IPPermissionInfo{}, "Labels"),
+	sourceByHashCode := make(map[string]IPPermissionInfo, len(source))
+	for _, perm := range source {
+		sourceByHashCode[perm.HashCode()] = perm
 	}
+	targetByHashCode := make(map[string]IPPermissionInfo, len(target))
+	for _, perm := range target {
+		targetByHashCode[perm.HashCode()] = perm
+	}
+	sourceHashCodeSet := sets.StringKeySet(sourceByHashCode)
+	targetHashCodeSet := sets.StringKeySet(targetByHashCode)
+
 	var diffs []IPPermissionInfo
-	for _, sPermission := range source {
-		containsInTarget := false
-		for _, tPermission := range target {
-			if cmp.Equal(sPermission, tPermission, opts) {
-				containsInTarget = true
-				break
-			}
-		}
-		if !containsInTarget {
-			diffs = append(diffs, sPermission)
-		}
+	for _, hashCode := range sourceHashCodeSet.Difference(targetHashCodeSet).List() {
+		diffs = append(diffs, sourceByHashCode[hashCode])
 	}
 	return diffs
 }
diff --git a/pkg/targetgroupbinding/networking_manager.go b/pkg/targetgroupbinding/networking_manager.go
index 4c4f07317..2e0420012 100644
--- a/pkg/targetgroupbinding/networking_manager.go
+++ b/pkg/targetgroupbinding/networking_manager.go
@@ -7,8 +7,6 @@ import (
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	ec2sdk "github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/go-logr/logr"
-	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -18,7 +16,6 @@ import (
 	"net"
 	elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/backend"
-	ec2equality "sigs.k8s.io/aws-load-balancer-controller/pkg/equality/ec2"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/networking"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -229,29 +226,26 @@ func (m *defaultNetworkingManager) consolidateIngressPermissionsPerSGByTGB(_ con
 
 // computeAggregatedIngressPermissionsPerSG will aggregate ingress permissions by SG across all TGBs.
 func (m *defaultNetworkingManager) computeAggregatedIngressPermissionsPerSG(_ context.Context) map[string][]networking.IPPermissionInfo {
-	opts := cmp.Options{
-		ec2equality.CompareOptionForIPPermission(),
-		cmpopts.IgnoreFields(networking.IPPermissionInfo{}, "Labels"),
-	}
-
-	aggregatedIngressPermissionsPerSG := make(map[string][]networking.IPPermissionInfo)
+	permByHashCodePerSG := make(map[string]map[string]networking.IPPermissionInfo)
 	for _, ingressPermissionsPerSG := range m.ingressPermissionsPerSGByTGB {
 		for sgID, permissions := range ingressPermissionsPerSG {
+			if _, ok := permByHashCodePerSG[sgID]; !ok {
+				permByHashCodePerSG[sgID] = make(map[string]networking.IPPermissionInfo)
+			}
 			for _, permission := range permissions {
-				containsPermission := false
-				for _, existingPermission := range aggregatedIngressPermissionsPerSG[sgID] {
-					if cmp.Equal(permission, existingPermission, opts) {
-						containsPermission = true
-						break
-					}
-				}
-				if !containsPermission {
-					aggregatedIngressPermissionsPerSG[sgID] = append(aggregatedIngressPermissionsPerSG[sgID], permission)
-				}
+				permByHashCodePerSG[sgID][permission.HashCode()] = permission
 			}
 		}
 	}
-	return aggregatedIngressPermissionsPerSG
+	aggregatedPermsPerSG := make(map[string][]networking.IPPermissionInfo)
+	for sgID, permByHashCode := range permByHashCodePerSG {
+		aggregatedPerms := make([]networking.IPPermissionInfo, 0, len(permByHashCode))
+		for _, hashCode := range sets.StringKeySet(permByHashCode).List() {
+			aggregatedPerms = append(aggregatedPerms, permByHashCode[hashCode])
+		}
+		aggregatedPermsPerSG[sgID] = aggregatedPerms
+	}
+	return aggregatedPermsPerSG
 }
 
 // computeIngressPermissionsForTGBNetworking computes the needed Inbound IPPermissions for specified TargetGroupBinding.
diff --git a/pkg/targetgroupbinding/resource_manager.go b/pkg/targetgroupbinding/resource_manager.go
index 096a62a23..dd7c97bfa 100644
--- a/pkg/targetgroupbinding/resource_manager.go
+++ b/pkg/targetgroupbinding/resource_manager.go
@@ -2,6 +2,7 @@ package targetgroupbinding
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	awssdk "github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
@@ -9,9 +10,11 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
-	"k8s.io/client-go/util/retry"
+	"k8s.io/apimachinery/pkg/util/strategicpatch"
 	elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/backend"
@@ -212,57 +215,54 @@ func (m *defaultResourceManager) updateTargetHealthPodCondition(ctx context.Cont
 func (m *defaultResourceManager) updateTargetHealthPodConditionForPod(ctx context.Context, pod *corev1.Pod,
 	targetHealth *elbv2sdk.TargetHealth, targetHealthCondType corev1.PodConditionType) (bool, error) {
 
-	needFurtherProbe := false
-	if err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
-		if err := m.k8sClient.Get(ctx, k8s.NamespacedName(pod), pod); err != nil {
-			return err
-		}
-		if !k8s.IsPodHasReadinessGate(pod, targetHealthCondType) {
-			needFurtherProbe = false
-			return nil
-		}
-		targetHealthCondStatus := corev1.ConditionUnknown
-		var reason, message string
-		if targetHealth != nil {
-			if awssdk.StringValue(targetHealth.State) == elbv2sdk.TargetHealthStateEnumHealthy {
-				targetHealthCondStatus = corev1.ConditionTrue
-			} else {
-				targetHealthCondStatus = corev1.ConditionFalse
-			}
-
-			reason = awssdk.StringValue(targetHealth.Reason)
-			message = awssdk.StringValue(targetHealth.Description)
+	if !k8s.IsPodHasReadinessGate(pod, targetHealthCondType) {
+		return false, nil
+	}
+	targetHealthCondStatus := corev1.ConditionUnknown
+	var reason, message string
+	if targetHealth != nil {
+		if awssdk.StringValue(targetHealth.State) == elbv2sdk.TargetHealthStateEnumHealthy {
+			targetHealthCondStatus = corev1.ConditionTrue
+		} else {
+			targetHealthCondStatus = corev1.ConditionFalse
 		}
 
-		existingTargetHealthCond := k8s.GetPodCondition(pod, targetHealthCondType)
-		// we skip patch pod if it's already true, and match current computed status/reason/message.
-		if existingTargetHealthCond != nil &&
-			existingTargetHealthCond.Status == corev1.ConditionTrue &&
-			existingTargetHealthCond.Status == targetHealthCondStatus &&
-			existingTargetHealthCond.Reason == reason &&
-			existingTargetHealthCond.Message == message {
-			needFurtherProbe = false
-			return nil
-		}
+		reason = awssdk.StringValue(targetHealth.Reason)
+		message = awssdk.StringValue(targetHealth.Description)
+	}
+	needFurtherProbe := targetHealthCondStatus != corev1.ConditionTrue
+	existingTargetHealthCond := k8s.GetPodCondition(pod, targetHealthCondType)
+	// we skip patch pod if it matches current computed status/reason/message.
+	if existingTargetHealthCond != nil &&
+		existingTargetHealthCond.Status == targetHealthCondStatus &&
+		existingTargetHealthCond.Reason == reason &&
+		existingTargetHealthCond.Message == message {
+		return needFurtherProbe, nil
+	}
 
-		newTargetHealthCond := corev1.PodCondition{
-			Type:          targetHealthCondType,
-			Status:        targetHealthCondStatus,
-			LastProbeTime: metav1.Now(),
-			Reason:        reason,
-			Message:       message,
-		}
-		if existingTargetHealthCond == nil || existingTargetHealthCond.Status != targetHealthCondStatus {
-			newTargetHealthCond.LastTransitionTime = metav1.Now()
-		}
+	newTargetHealthCond := corev1.PodCondition{
+		Type:    targetHealthCondType,
+		Status:  targetHealthCondStatus,
+		Reason:  reason,
+		Message: message,
+	}
+	if existingTargetHealthCond == nil || existingTargetHealthCond.Status != targetHealthCondStatus {
+		newTargetHealthCond.LastTransitionTime = metav1.Now()
+	}
 
-		oldPod := pod.DeepCopy()
-		k8s.UpdatePodCondition(pod, newTargetHealthCond)
-		needFurtherProbe = targetHealthCondStatus != corev1.ConditionTrue
-		return m.k8sClient.Status().Patch(ctx, pod, client.MergeFromWithOptions(oldPod, client.MergeFromWithOptimisticLock{}))
-	}); err != nil {
+	oldPod := pod.DeepCopy()
+	k8s.UpdatePodCondition(pod, newTargetHealthCond)
+	patch, err := buildPodStrategicMergePatch(oldPod, pod)
+	if err != nil {
 		return false, err
 	}
+	if err := m.k8sClient.Status().Patch(ctx, pod, patch); err != nil {
+		if apierrors.IsNotFound(err) {
+			return false, nil
+		}
+		return false, err
+	}
+
 	return needFurtherProbe, nil
 }
 
@@ -396,6 +396,23 @@ func matchNodePortEndpointWithTargets(endpoints []backend.NodePortEndpoint, targ
 	return matchedEndpointAndTargets, unmatchedEndpoints, unmatchedTargets
 }
 
+func buildPodStrategicMergePatch(oldPod *corev1.Pod, newPod *corev1.Pod) (client.Patch, error) {
+	oldData, err := json.Marshal(oldPod)
+	if err != nil {
+		return nil, err
+	}
+
+	newData, err := json.Marshal(newPod)
+	if err != nil {
+		return nil, err
+	}
+	patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, corev1.Pod{})
+	if err != nil {
+		return nil, err
+	}
+	return client.RawPatch(types.StrategicMergePatchType, patchBytes), nil
+}
+
 func isELBV2TargetGroupNotFoundError(err error) bool {
 	var awsErr awserr.Error
 	if errors.As(err, &awsErr) {


Test patch:
diff --git a/pkg/networking/security_group_info_test.go b/pkg/networking/security_group_info_test.go
new file mode 100644
index 000000000..8cfbbc7af
--- /dev/null
+++ b/pkg/networking/security_group_info_test.go
@@ -0,0 +1,95 @@
+package networking
+
+import (
+	awssdk "github.com/aws/aws-sdk-go/aws"
+	ec2sdk "github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestIPPermissionInfo_HashCode(t *testing.T) {
+	type fields struct {
+		Permission ec2sdk.IpPermission
+		Labels     map[string]string
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		want   string
+	}{
+		{
+			name: "IpRange permission",
+			fields: fields{
+				Permission: ec2sdk.IpPermission{
+					IpProtocol: awssdk.String("tcp"),
+					FromPort:   awssdk.Int64(80),
+					ToPort:     awssdk.Int64(8080),
+					IpRanges: []*ec2sdk.IpRange{
+						{
+							CidrIp: awssdk.String("192.168.0.0/16"),
+						},
+					},
+				},
+			},
+			want: "IpProtocol: tcp, FromPort: 80, ToPort: 8080, IpRange: 192.168.0.0/16",
+		},
+		{
+			name: "Ipv6Range permission",
+			fields: fields{
+				Permission: ec2sdk.IpPermission{
+					IpProtocol: awssdk.String("tcp"),
+					FromPort:   awssdk.Int64(80),
+					ToPort:     awssdk.Int64(8080),
+					Ipv6Ranges: []*ec2sdk.Ipv6Range{
+						{
+							CidrIpv6: awssdk.String("::/0"),
+						},
+					},
+				},
+			},
+			want: "IpProtocol: tcp, FromPort: 80, ToPort: 8080, Ipv6Range: ::/0",
+		},
+		{
+			name: "PrefixListId permission",
+			fields: fields{
+				Permission: ec2sdk.IpPermission{
+					IpProtocol: awssdk.String("tcp"),
+					FromPort:   awssdk.Int64(80),
+					ToPort:     awssdk.Int64(8080),
+					PrefixListIds: []*ec2sdk.PrefixListId{
+						{
+							PrefixListId: awssdk.String("pl-123456abcde123456"),
+						},
+					},
+				},
+			},
+			want: "IpProtocol: tcp, FromPort: 80, ToPort: 8080, PrefixListId: pl-123456abcde123456",
+		},
+		{
+			name: "UserIdGroupPair permission",
+			fields: fields{
+				Permission: ec2sdk.IpPermission{
+					IpProtocol: awssdk.String("tcp"),
+					FromPort:   awssdk.Int64(80),
+					ToPort:     awssdk.Int64(8080),
+					UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{
+						{
+							GroupId: awssdk.String("sg-xxxx"),
+						},
+					},
+				},
+			},
+			want: "IpProtocol: tcp, FromPort: 80, ToPort: 8080, UserIdGroupPair: sg-xxxx",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			perm := &IPPermissionInfo{
+				Permission: tt.fields.Permission,
+				Labels:     tt.fields.Labels,
+			}
+			got := perm.HashCode()
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
diff --git a/pkg/networking/security_group_reconciler_test.go b/pkg/networking/security_group_reconciler_test.go
index b5eea9fe8..0a2271309 100644
--- a/pkg/networking/security_group_reconciler_test.go
+++ b/pkg/networking/security_group_reconciler_test.go
@@ -1,7 +1,9 @@
 package networking
 
 import (
+	awssdk "github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
+	ec2sdk "github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/stretchr/testify/assert"
 	"testing"
 )
@@ -45,3 +47,171 @@ func Test_defaultSecurityGroupReconciler_shouldRetryWithoutCache(t *testing.T) {
 		})
 	}
 }
+
+func Test_diffIPPermissionInfos(t *testing.T) {
+	type args struct {
+		source []IPPermissionInfo
+		target []IPPermissionInfo
+	}
+	tests := []struct {
+		name string
+		args args
+		want []IPPermissionInfo
+	}{
+		{
+			name: "source contains more than target",
+			args: args{
+				source: []IPPermissionInfo{
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.168.0.0/16"),
+								},
+							},
+						},
+					},
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.171.0.0/16"),
+								},
+							},
+						},
+					},
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.170.0.0/16"),
+								},
+							},
+						},
+					},
+				},
+				target: []IPPermissionInfo{
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.168.0.0/16"),
+								},
+							},
+						},
+					},
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.170.0.0/16"),
+								},
+							},
+						},
+					},
+				},
+			},
+			want: []IPPermissionInfo{
+				{
+					Permission: ec2sdk.IpPermission{
+						IpProtocol: awssdk.String("tcp"),
+						FromPort:   awssdk.Int64(80),
+						ToPort:     awssdk.Int64(8080),
+						IpRanges: []*ec2sdk.IpRange{
+							{
+								CidrIp: awssdk.String("192.171.0.0/16"),
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "source equals to target",
+			args: args{
+				source: []IPPermissionInfo{
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.168.0.0/16"),
+								},
+							},
+						},
+					},
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.170.0.0/16"),
+								},
+							},
+						},
+					},
+				},
+				target: []IPPermissionInfo{
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.168.0.0/16"),
+								},
+							},
+						},
+					},
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.170.0.0/16"),
+								},
+							},
+						},
+					},
+				},
+			},
+			want: nil,
+		},
+		{
+			name: "both source & target is nil",
+			args: args{
+				source: nil,
+				target: nil,
+			},
+			want: nil,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := diffIPPermissionInfos(tt.args.source, tt.args.target)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
diff --git a/pkg/targetgroupbinding/networking_manager_test.go b/pkg/targetgroupbinding/networking_manager_test.go
index 1e68aaabe..6c895052e 100644
--- a/pkg/targetgroupbinding/networking_manager_test.go
+++ b/pkg/targetgroupbinding/networking_manager_test.go
@@ -7,6 +7,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/networking"
@@ -609,3 +610,281 @@ func Test_defaultNetworkingManager_computeNumericalPorts(t *testing.T) {
 		})
 	}
 }
+
+func Test_defaultNetworkingManager_computeAggregatedIngressPermissionsPerSG(t *testing.T) {
+	type fields struct {
+		ingressPermissionsPerSGByTGB map[types.NamespacedName]map[string][]networking.IPPermissionInfo
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		want   map[string][]networking.IPPermissionInfo
+	}{
+		{
+			name: "single tgb",
+			fields: fields{
+				ingressPermissionsPerSGByTGB: map[types.NamespacedName]map[string][]networking.IPPermissionInfo{
+					types.NamespacedName{Namespace: "ns-1", Name: "tgb-1"}: {
+						"sg-a": {
+							{
+								Permission: ec2sdk.IpPermission{
+									IpProtocol: awssdk.String("tcp"),
+									FromPort:   awssdk.Int64(80),
+									ToPort:     awssdk.Int64(8080),
+									IpRanges: []*ec2sdk.IpRange{
+										{
+											CidrIp: awssdk.String("192.168.0.0/16"),
+										},
+									},
+								},
+							},
+							{
+								Permission: ec2sdk.IpPermission{
+									IpProtocol: awssdk.String("tcp"),
+									FromPort:   awssdk.Int64(80),
+									ToPort:     awssdk.Int64(8080),
+									IpRanges: []*ec2sdk.IpRange{
+										{
+											CidrIp: awssdk.String("192.169.0.0/16"),
+										},
+									},
+								},
+							},
+						},
+						"sg-b": {
+							{
+								Permission: ec2sdk.IpPermission{
+									IpProtocol: awssdk.String("tcp"),
+									FromPort:   awssdk.Int64(80),
+									ToPort:     awssdk.Int64(8080),
+									IpRanges: []*ec2sdk.IpRange{
+										{
+											CidrIp: awssdk.String("192.168.0.0/16"),
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			want: map[string][]networking.IPPermissionInfo{
+				"sg-a": {
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.168.0.0/16"),
+								},
+							},
+						},
+					},
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.169.0.0/16"),
+								},
+							},
+						},
+					},
+				},
+				"sg-b": {
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.168.0.0/16"),
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "multiple tgb",
+			fields: fields{
+				ingressPermissionsPerSGByTGB: map[types.NamespacedName]map[string][]networking.IPPermissionInfo{
+					types.NamespacedName{Namespace: "ns-1", Name: "tgb-1"}: {
+						"sg-a": {
+							{
+								Permission: ec2sdk.IpPermission{
+									IpProtocol: awssdk.String("tcp"),
+									FromPort:   awssdk.Int64(80),
+									ToPort:     awssdk.Int64(8080),
+									IpRanges: []*ec2sdk.IpRange{
+										{
+											CidrIp: awssdk.String("192.168.0.0/16"),
+										},
+									},
+								},
+							},
+							{
+								Permission: ec2sdk.IpPermission{
+									IpProtocol: awssdk.String("tcp"),
+									FromPort:   awssdk.Int64(80),
+									ToPort:     awssdk.Int64(8080),
+									IpRanges: []*ec2sdk.IpRange{
+										{
+											CidrIp: awssdk.String("192.169.0.0/16"),
+										},
+									},
+								},
+							},
+						},
+						"sg-b": {
+							{
+								Permission: ec2sdk.IpPermission{
+									IpProtocol: awssdk.String("tcp"),
+									FromPort:   awssdk.Int64(80),
+									ToPort:     awssdk.Int64(8080),
+									IpRanges: []*ec2sdk.IpRange{
+										{
+											CidrIp: awssdk.String("192.168.0.0/16"),
+										},
+									},
+								},
+							},
+						},
+					},
+					types.NamespacedName{Namespace: "ns-1", Name: "tgb-2"}: {
+						"sg-a": {
+							{
+								Permission: ec2sdk.IpPermission{
+									IpProtocol: awssdk.String("tcp"),
+									FromPort:   awssdk.Int64(80),
+									ToPort:     awssdk.Int64(8080),
+									IpRanges: []*ec2sdk.IpRange{
+										{
+											CidrIp: awssdk.String("192.168.0.0/16"),
+										},
+									},
+								},
+							},
+							{
+								Permission: ec2sdk.IpPermission{
+									IpProtocol: awssdk.String("tcp"),
+									FromPort:   awssdk.Int64(80),
+									ToPort:     awssdk.Int64(8080),
+									IpRanges: []*ec2sdk.IpRange{
+										{
+											CidrIp: awssdk.String("192.170.0.0/16"),
+										},
+									},
+								},
+							},
+						},
+						"sg-c": {
+							{
+								Permission: ec2sdk.IpPermission{
+									IpProtocol: awssdk.String("tcp"),
+									FromPort:   awssdk.Int64(80),
+									ToPort:     awssdk.Int64(8080),
+									IpRanges: []*ec2sdk.IpRange{
+										{
+											CidrIp: awssdk.String("192.168.0.0/16"),
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			want: map[string][]networking.IPPermissionInfo{
+				"sg-a": {
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.168.0.0/16"),
+								},
+							},
+						},
+					},
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.169.0.0/16"),
+								},
+							},
+						},
+					},
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.170.0.0/16"),
+								},
+							},
+						},
+					},
+				},
+				"sg-b": {
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.168.0.0/16"),
+								},
+							},
+						},
+					},
+				},
+				"sg-c": {
+					{
+						Permission: ec2sdk.IpPermission{
+							IpProtocol: awssdk.String("tcp"),
+							FromPort:   awssdk.Int64(80),
+							ToPort:     awssdk.Int64(8080),
+							IpRanges: []*ec2sdk.IpRange{
+								{
+									CidrIp: awssdk.String("192.168.0.0/16"),
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "no tgb",
+			fields: fields{
+				ingressPermissionsPerSGByTGB: nil,
+			},
+			want: map[string][]networking.IPPermissionInfo{},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			m := &defaultNetworkingManager{
+				ingressPermissionsPerSGByTGB: tt.fields.ingressPermissionsPerSGByTGB,
+			}
+			got := m.computeAggregatedIngressPermissionsPerSG(context.Background())
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}