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.

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

Merge rules per ingress by the same host, pathType and backend service #2986

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
107 changes: 85 additions & 22 deletions pkg/ingress/model_build_listener_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
"sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use goimports sorting style

"sort"
"strconv"
"strings"

"github.com/pkg/errors"
Expand Down Expand Up @@ -83,7 +84,7 @@ func (t *defaultModelBuildTask) buildListenerRules(ctx context.Context, lsARN co
}
var rules []Rule
for _, ing := range ingList {
mergedRules, err := t.mergeRulesPerIngress(ing.Ing.Spec.Rules)
mergedRules, err := t.buildMergedRules(ing.Ing.Spec.Rules)
if err != nil {
return err
}
Expand Down Expand Up @@ -149,16 +150,42 @@ func (t *defaultModelBuildTask) buildListenerRules(ctx context.Context, lsARN co
})
priority += 1
}

return nil
}

func (t *defaultModelBuildTask) mergeRulesPerIngress(rules []networking.IngressRule) ([]networking.IngressRule, error) {
// use a map to help merge rules by host, pathType and service
// getRulesToMerge classifies rules into two categories - rules with unique host, rules with replicate hosts
// only rules with replicate hosts need merge
func (t *defaultModelBuildTask) getRulesToMerge(rules []networking.IngressRule) ([]networking.IngressRule, []networking.IngressRule, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps name the return values?

var rulesWithReplicateHosts []networking.IngressRule
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rulesWithReplicatedHosts

var rulesWithUniqueHost []networking.IngressRule
hostToRulesMap := make(map[string][]networking.IngressRule)
for _, rule := range rules {
host := rule.Host
_, exists := hostToRulesMap[host]
if exists {
hostToRulesMap[host] = append(hostToRulesMap[host], rule)
} else {
hostToRulesMap[host] = []networking.IngressRule{rule}
}
Comment on lines +75 to +80
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hostToRulesMap[host] = append(hostToRulesMap[host], rule)

}
for host, rules := range hostToRulesMap {
if len(rules) == 1 {
rulesWithUniqueHost = append(rulesWithUniqueHost, rules...)
} else if len(rules) > 1 {
rulesWithReplicateHosts = append(rulesWithReplicateHosts, rules...)
} else {
return nil, nil, errors.Errorf("no rules for Host %s", host)
}
}
return rulesWithReplicateHosts, rulesWithUniqueHost, nil
}

func (t *defaultModelBuildTask) getMergeRuleRefMaps(rules []networking.IngressRule) (map[[4]string][]networking.HTTPIngressPath, map[networking.HTTPIngressPath]int) {
// use a map to help merge paths of rules by host, pathType and service
// e.g. {(hostA, pathTypeA, serviceNameA, PortNumberA): [path1, path2,...],
// (hostB, pathTypeB, serviceNameB, PortNumberB): [path3, path4,...],
// ...}
mergeRefMap := make(map[[4]string][]networking.HTTPIngressPath)
mergePathsRefMap := make(map[[4]string][]networking.HTTPIngressPath)
// pathToRuleMap stores {path: ruleIdx} relationship
pathToRuleMap := make(map[networking.HTTPIngressPath]int)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pathToRuleIndexMap?


Expand All @@ -176,29 +203,17 @@ func (t *defaultModelBuildTask) mergeRulesPerIngress(rules []networking.IngressR
pathToRuleMap[path] = idx
pathType := string(*path.PathType)
serviceName := path.Backend.Service.Name
portNumber := string(path.Backend.Service.Port.Number)
_, exist := mergeRefMap[[4]string{host, pathType, serviceName, portNumber}]
portNumber := strconv.Itoa(int(path.Backend.Service.Port.Number))
_, exist := mergePathsRefMap[[4]string{host, pathType, serviceName, portNumber}]
if !exist {
mergeRefMap[[4]string{host, pathType, serviceName, portNumber}] = []networking.HTTPIngressPath{path}
mergePathsRefMap[[4]string{host, pathType, serviceName, portNumber}] = []networking.HTTPIngressPath{path}
} else {
mergeRefMap[[4]string{host, pathType, serviceName, portNumber}] = append(mergeRefMap[[4]string{host, pathType, serviceName, portNumber}],
mergePathsRefMap[[4]string{host, pathType, serviceName, portNumber}] = append(mergePathsRefMap[[4]string{host, pathType, serviceName, portNumber}],
path)
}
}
}
// iterate the mergeRefMap and append all paths in a group to the rule of the min ruleIdx
var mergedRules []networking.IngressRule
for _, mergedPaths := range mergeRefMap {
minIdx := pathToRuleMap[mergedPaths[0]]
for _, path := range mergedPaths {
currIdx := pathToRuleMap[path]
minIdx = algorithm.GetMin(minIdx, currIdx)
}
mergedRule := rules[minIdx]
mergedRule.HTTP.Paths = append(mergedRule.HTTP.Paths, mergedPaths...)
mergedRules = append(mergedRules, mergedRule)
}
return mergedRules, nil
return mergePathsRefMap, pathToRuleMap
}

// sortIngressPaths will sort the paths following the strategy:
Expand Down Expand Up @@ -308,6 +323,54 @@ func (t *defaultModelBuildTask) classifyIngressPathsByType(paths []networking.HT
// }
// return conditions, nil
// }
func (t *defaultModelBuildTask) buildMergedRulesWithReplicateHosts(rules []networking.IngressRule) []networking.IngressRule {
// iterate the mergeRefMap and append all paths in a group to the rule of the min ruleIdx
mergePathsRefMap, pathToRuleMap := t.getMergeRuleRefMaps(rules)
var mergedRules []networking.IngressRule
// mergedRulesIdxMap store the {oldIdx: newIdx in mergedRules} reference once a rule gets merged into mergedRules
mergedRulesIdxMap := make(map[int]int)
for _, mergedPaths := range mergePathsRefMap {
minIdx := pathToRuleMap[mergedPaths[0]]
if len(mergedPaths) > 1 {
for _, path := range mergedPaths {
currIdx := pathToRuleMap[path]
minIdx = algorithm.GetMin(minIdx, currIdx)
}
}
// check if the rule has already been processed
var mergedRule *networking.IngressRule
_, exists := mergedRulesIdxMap[minIdx]
if !exists {
mergedRule = &networking.IngressRule{
Host: rules[minIdx].Host,
IngressRuleValue: networking.IngressRuleValue{
HTTP: &networking.HTTPIngressRuleValue{
Paths: []networking.HTTPIngressPath{},
},
},
}
mergedRulesIdxMap[minIdx] = len(mergedRules)
} else {
mergedRule = &mergedRules[mergedRulesIdxMap[minIdx]]
}
mergedRule.HTTP.Paths = append(mergedRule.HTTP.Paths, mergedPaths...)
if !exists {
mergedRules = append(mergedRules, *mergedRule)
}
}
return mergedRules
}

func (t *defaultModelBuildTask) buildMergedRules(rules []networking.IngressRule) ([]networking.IngressRule, error) {
rulesWithReplicateHosts, rulesWithUniqueHost, err := t.getRulesToMerge(rules)
if err != nil {
return nil, err
}
mergedRules := t.buildMergedRulesWithReplicateHosts(rulesWithReplicateHosts)
mergedRules = append(mergedRules, rulesWithUniqueHost...)
return mergedRules, nil
}

func (t *defaultModelBuildTask) buildRuleConditions(ctx context.Context, host string,
classfiedPaths []string, backend EnhancedBackend, pathType networking.PathType) ([]elbv2model.RuleCondition, error) {
var hosts []string
Expand Down
Loading