Skip to content

Commit

Permalink
Update rule management to avoid sporadic 503 errors
Browse files Browse the repository at this point in the history
  • Loading branch information
shraddhabang committed Feb 26, 2025
1 parent 16900de commit 42385aa
Show file tree
Hide file tree
Showing 9 changed files with 137 additions and 39 deletions.
6 changes: 3 additions & 3 deletions docs/install/iam_policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@
"elasticloadbalancing:DescribeTags",
"elasticloadbalancing:DescribeTrustStores",
"elasticloadbalancing:DescribeListenerAttributes",
"elasticloadbalancing:DescribeCapacityReservation",
"elasticloadbalancing:SetRulePriorities"
"elasticloadbalancing:DescribeCapacityReservation"
],
"Resource": "*"
},
Expand Down Expand Up @@ -240,7 +239,8 @@
"elasticloadbalancing:ModifyListener",
"elasticloadbalancing:AddListenerCertificates",
"elasticloadbalancing:RemoveListenerCertificates",
"elasticloadbalancing:ModifyRule"
"elasticloadbalancing:ModifyRule",
"elasticloadbalancing:SetRulePriorities"
],
"Resource": "*"
}
Expand Down
6 changes: 3 additions & 3 deletions docs/install/iam_policy_cn.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@
"elasticloadbalancing:DescribeTags",
"elasticloadbalancing:DescribeTrustStores",
"elasticloadbalancing:DescribeListenerAttributes",
"elasticloadbalancing:DescribeCapacityReservation",
"elasticloadbalancing:SetRulePriorities"
"elasticloadbalancing:DescribeCapacityReservation"
],
"Resource": "*"
},
Expand Down Expand Up @@ -240,7 +239,8 @@
"elasticloadbalancing:ModifyListener",
"elasticloadbalancing:AddListenerCertificates",
"elasticloadbalancing:RemoveListenerCertificates",
"elasticloadbalancing:ModifyRule"
"elasticloadbalancing:ModifyRule",
"elasticloadbalancing:SetRulePriorities"
],
"Resource": "*"
}
Expand Down
6 changes: 3 additions & 3 deletions docs/install/iam_policy_iso.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@
"elasticloadbalancing:DescribeTargetGroups",
"elasticloadbalancing:DescribeTargetGroupAttributes",
"elasticloadbalancing:DescribeTargetHealth",
"elasticloadbalancing:DescribeTags",
"elasticloadbalancing:SetRulePriorities"
"elasticloadbalancing:DescribeTags"
],
"Resource": "*"
},
Expand Down Expand Up @@ -235,7 +234,8 @@
"elasticloadbalancing:ModifyListener",
"elasticloadbalancing:AddListenerCertificates",
"elasticloadbalancing:RemoveListenerCertificates",
"elasticloadbalancing:ModifyRule"
"elasticloadbalancing:ModifyRule",
"elasticloadbalancing:SetRulePriorities"
],
"Resource": "*"
}
Expand Down
6 changes: 3 additions & 3 deletions docs/install/iam_policy_isob.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@
"elasticloadbalancing:DescribeTargetGroups",
"elasticloadbalancing:DescribeTargetGroupAttributes",
"elasticloadbalancing:DescribeTargetHealth",
"elasticloadbalancing:DescribeTags",
"elasticloadbalancing:SetRulePriorities"
"elasticloadbalancing:DescribeTags"
],
"Resource": "*"
},
Expand Down Expand Up @@ -235,7 +234,8 @@
"elasticloadbalancing:ModifyListener",
"elasticloadbalancing:AddListenerCertificates",
"elasticloadbalancing:RemoveListenerCertificates",
"elasticloadbalancing:ModifyRule"
"elasticloadbalancing:ModifyRule",
"elasticloadbalancing:SetRulePriorities"
],
"Resource": "*"
}
Expand Down
3 changes: 2 additions & 1 deletion docs/install/iam_policy_isoe.json
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@
"elasticloadbalancing:ModifyListener",
"elasticloadbalancing:AddListenerCertificates",
"elasticloadbalancing:RemoveListenerCertificates",
"elasticloadbalancing:ModifyRule"
"elasticloadbalancing:ModifyRule",
"elasticloadbalancing:SetRulePriorities"
],
"Resource": "*"
}
Expand Down
3 changes: 2 additions & 1 deletion docs/install/iam_policy_isof.json
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@
"elasticloadbalancing:ModifyListener",
"elasticloadbalancing:AddListenerCertificates",
"elasticloadbalancing:RemoveListenerCertificates",
"elasticloadbalancing:ModifyRule"
"elasticloadbalancing:ModifyRule",
"elasticloadbalancing:SetRulePriorities"
],
"Resource": "*"
}
Expand Down
3 changes: 2 additions & 1 deletion docs/install/iam_policy_us-gov.json
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,8 @@
"elasticloadbalancing:ModifyListener",
"elasticloadbalancing:AddListenerCertificates",
"elasticloadbalancing:RemoveListenerCertificates",
"elasticloadbalancing:ModifyRule"
"elasticloadbalancing:ModifyRule",
"elasticloadbalancing:SetRulePriorities"
],
"Resource": "*"
}
Expand Down
55 changes: 50 additions & 5 deletions pkg/deploy/elbv2/listener_rule_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/tracking"
elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
"sigs.k8s.io/aws-load-balancer-controller/pkg/runtime"
"slices"
"strconv"
"time"
)
Expand All @@ -20,11 +21,13 @@ import (
type ListenerRuleManager interface {
Create(ctx context.Context, resLR *elbv2model.ListenerRule, desiredActionsAndConditions *resLRDesiredActionsAndConditionsPair) (elbv2model.ListenerRuleStatus, error)

Update(ctx context.Context, resLR *elbv2model.ListenerRule, sdkLR ListenerRuleWithTags) (elbv2model.ListenerRuleStatus, error)
UpdateRules(ctx context.Context, resLR *elbv2model.ListenerRule, sdkLR ListenerRuleWithTags, desiredActionsAndConditions *resLRDesiredActionsAndConditionsPair) (elbv2model.ListenerRuleStatus, error)

UpdateRulesTags(ctx context.Context, resLR *elbv2model.ListenerRule, sdkLR ListenerRuleWithTags) (elbv2model.ListenerRuleStatus, error)

Delete(ctx context.Context, sdkLR ListenerRuleWithTags) error

SetRulePriorities(ctx context.Context, matchedResAndSDKLRsBySettings []resAndSDKListenerRulePair) error
SetRulePriorities(ctx context.Context, matchedResAndSDKLRsBySettings []resAndSDKListenerRulePair, unmatchedSDKLRs []ListenerRuleWithTags) error
}

// NewDefaultListenerRuleManager constructs new defaultListenerRuleManager.
Expand Down Expand Up @@ -91,7 +94,7 @@ func (m *defaultListenerRuleManager) Create(ctx context.Context, resLR *elbv2mod
return buildResListenerRuleStatus(sdkLR), nil
}

func (m *defaultListenerRuleManager) Update(ctx context.Context, resLR *elbv2model.ListenerRule, sdkLR ListenerRuleWithTags) (elbv2model.ListenerRuleStatus, error) {
func (m *defaultListenerRuleManager) UpdateRulesTags(ctx context.Context, resLR *elbv2model.ListenerRule, sdkLR ListenerRuleWithTags) (elbv2model.ListenerRuleStatus, error) {
if m.featureGates.Enabled(config.ListenerRulesTagging) {
if err := m.updateSDKListenerRuleWithTags(ctx, resLR, sdkLR); err != nil {
return elbv2model.ListenerRuleStatus{}, err
Expand All @@ -100,6 +103,13 @@ func (m *defaultListenerRuleManager) Update(ctx context.Context, resLR *elbv2mod
return buildResListenerRuleStatus(sdkLR), nil
}

func (m *defaultListenerRuleManager) UpdateRules(ctx context.Context, resLR *elbv2model.ListenerRule, sdkLR ListenerRuleWithTags, desiredActionsAndConditions *resLRDesiredActionsAndConditionsPair) (elbv2model.ListenerRuleStatus, error) {
if err := m.updateSDKListenerRuleWithSettings(ctx, resLR, sdkLR, desiredActionsAndConditions); err != nil {
return elbv2model.ListenerRuleStatus{}, err
}
return buildResListenerRuleStatus(sdkLR), nil
}

func (m *defaultListenerRuleManager) Delete(ctx context.Context, sdkLR ListenerRuleWithTags) error {
req := &elbv2sdk.DeleteRuleInput{
RuleArn: sdkLR.ListenerRule.RuleArn,
Expand All @@ -114,10 +124,18 @@ func (m *defaultListenerRuleManager) Delete(ctx context.Context, sdkLR ListenerR
return nil
}

func (m *defaultListenerRuleManager) SetRulePriorities(ctx context.Context, matchedResAndSDKLRsBySettings []resAndSDKListenerRulePair) error {
func (m *defaultListenerRuleManager) SetRulePriorities(ctx context.Context, matchedResAndSDKLRsBySettings []resAndSDKListenerRulePair, unmatchedSDKLRs []ListenerRuleWithTags) error {
var lastAvailablePriority int32 = 50000
var sdkLRs []ListenerRuleWithTags

// Push down all the unmatched existing SDK rules on load balancer so that updated rules can take their place
for _, sdkLR := range slices.Backward(unmatchedSDKLRs) {
sdkLR.ListenerRule.Priority = awssdk.String(strconv.Itoa(int(lastAvailablePriority)))
sdkLRs = append(sdkLRs, sdkLR)
lastAvailablePriority--
}
//Reprioratize matched rules by settings
for _, resAndSDKLR := range matchedResAndSDKLRsBySettings {
//Update rule priorities
resAndSDKLR.sdkLR.ListenerRule.Priority = awssdk.String(strconv.Itoa(int(resAndSDKLR.resLR.Spec.Priority)))
sdkLRs = append(sdkLRs, resAndSDKLR.sdkLR)
}
Expand All @@ -139,6 +157,26 @@ func (m *defaultListenerRuleManager) updateSDKListenerRuleWithTags(ctx context.C
WithIgnoredTagKeys(m.externalManagedTags))
}

func (m *defaultListenerRuleManager) updateSDKListenerRuleWithSettings(ctx context.Context, resLR *elbv2model.ListenerRule, sdkLR ListenerRuleWithTags, desiredActionsAndConditions *resLRDesiredActionsAndConditionsPair) error {
desiredActions := desiredActionsAndConditions.desiredActions
desiredConditions := desiredActionsAndConditions.desiredConditions

req := buildSDKModifyListenerRuleInput(resLR.Spec, desiredActions, desiredConditions)
req.RuleArn = sdkLR.ListenerRule.RuleArn
m.logger.Info("modifying listener rule",
"stackID", resLR.Stack().StackID(),
"resourceID", resLR.ID(),
"arn", awssdk.ToString(sdkLR.ListenerRule.RuleArn))
if _, err := m.elbv2Client.ModifyRuleWithContext(ctx, req); err != nil {
return err
}
m.logger.Info("modified listener rule",
"stackID", resLR.Stack().StackID(),
"resourceID", resLR.ID(),
"arn", awssdk.ToString(sdkLR.ListenerRule.RuleArn))
return nil
}

func buildSDKCreateListenerRuleInput(lrSpec elbv2model.ListenerRuleSpec, desiredActionsAndConditions *resLRDesiredActionsAndConditionsPair, featureGates config.FeatureGates) (*elbv2sdk.CreateRuleInput, error) {
ctx := context.Background()
lsARN, err := lrSpec.ListenerARN.Resolve(ctx)
Expand All @@ -165,6 +203,13 @@ func buildSDKCreateListenerRuleInput(lrSpec elbv2model.ListenerRuleSpec, desired
return sdkObj, nil
}

func buildSDKModifyListenerRuleInput(_ elbv2model.ListenerRuleSpec, desiredActions []elbv2types.Action, desiredConditions []elbv2types.RuleCondition) *elbv2sdk.ModifyRuleInput {
sdkObj := &elbv2sdk.ModifyRuleInput{}
sdkObj.Actions = desiredActions
sdkObj.Conditions = desiredConditions
return sdkObj
}

func buildSDKSetRulePrioritiesInput(sdkLRs []ListenerRuleWithTags) *elbv2sdk.SetRulePrioritiesInput {
var rulePriorities []elbv2types.RulePriorityPair
for _, sdkLR := range sdkLRs {
Expand Down
88 changes: 69 additions & 19 deletions pkg/deploy/elbv2/listener_rule_synthesizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
"github.com/go-logr/logr"
"github.com/google/go-cmp/cmp"
"k8s.io/apimachinery/pkg/util/sets"
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
"sigs.k8s.io/aws-load-balancer-controller/pkg/config"
elbv2equality "sigs.k8s.io/aws-load-balancer-controller/pkg/equality/elbv2"
Expand Down Expand Up @@ -66,6 +67,11 @@ func (s *listenerRuleSynthesizer) PostSynthesize(ctx context.Context) error {
}

func (s *listenerRuleSynthesizer) synthesizeListenerRulesOnListener(ctx context.Context, lsARN string, resLRs []*elbv2model.ListenerRule) error {
// Find existing listener rules on the load balancer
sdkLRs, err := s.findSDKListenersRulesOnLS(ctx, lsARN)
if err != nil {
return err
}
// Build desired actions and conditions pairs for resource listener rules.
resLRDesiredActionsAndConditionsPairs := make(map[*elbv2model.ListenerRule]*resLRDesiredActionsAndConditionsPair, len(resLRs))
for _, resLR := range resLRs {
Expand All @@ -75,29 +81,28 @@ func (s *listenerRuleSynthesizer) synthesizeListenerRulesOnListener(ctx context.
}
resLRDesiredActionsAndConditionsPairs[resLR] = resLRDesiredActionsAndConditionsPair
}
// Find existing listener rules on the load balancer
sdkLRs, err := s.findSDKListenersRulesOnLS(ctx, lsARN)
if err != nil {
return err
}
// matchedResAndSDKLRsBySettings : A slice of matched resLR and SDKLR rule pairs that have matching settings like actions and conditions
// unmatchedResLRs : A slice of resLR) that do not have a corresponding match in the sdkLRs. These rules need to be created on the load balancer.
// unmatchedSDKLRs : A slice of sdkLRs that do not have a corresponding match in the resLRs. These rules need to be deleted from the load balancer.
matchedResAndSDKLRsBySettings, unmatchedResLRs, unmatchedSDKLRs, err := s.matchResAndSDKListenerRules(resLRs, sdkLRs, resLRDesiredActionsAndConditionsPairs)
// matchedResAndSDKLRsBySettings : A slice of matched resLR and SDKLR rule pairs that have matching settings like actions and conditions. These needs to be only reprioratized to their corresponding priorities
// matchedResAndSDKLRsByPriority : A slice of matched resLR and SDKLR rule pairs that have matching priorities but not settings like actions and conditions. These needs to be modified in place to avoid any 503 errors
// unmatchedResLRs : A slice of resLR that do not have a corresponding match in the sdkLRs. These rules need to be created on the load balancer.
// unmatchedSDKLRs : A slice of sdkLRs that do not have a corresponding match in the resLRs. These rules need to be first pushed down in the priority so that the new rules are created/modified at higher priority first and then deleted from the load balancer to avoid any 503 errors.
matchedResAndSDKLRsBySettings, matchedResAndSDKLRsByPriority, unmatchedResLRs, unmatchedSDKLRs, err := s.matchResAndSDKListenerRules(resLRs, sdkLRs, resLRDesiredActionsAndConditionsPairs)
if err != nil {
return err
}
for _, sdkLR := range unmatchedSDKLRs {
if err := s.lrManager.Delete(ctx, sdkLR); err != nil {
// Re-prioritize matched listener rules.
if len(matchedResAndSDKLRsBySettings) > 0 {
err := s.lrManager.SetRulePriorities(ctx, matchedResAndSDKLRsBySettings, unmatchedSDKLRs)
if err != nil {
return err
}
}
// Re-prioritize matched listener rules.
if len(matchedResAndSDKLRsBySettings) > 0 {
err := s.lrManager.SetRulePriorities(ctx, matchedResAndSDKLRsBySettings)
// Modify rules in place which are matching priorities
for _, resAndSDKLR := range matchedResAndSDKLRsByPriority {
lsStatus, err := s.lrManager.UpdateRules(ctx, resAndSDKLR.resLR, resAndSDKLR.sdkLR, resLRDesiredActionsAndConditionsPairs[resAndSDKLR.resLR])
if err != nil {
return err
}
resAndSDKLR.resLR.SetStatus(lsStatus)
}
// Create all the new rules on the LB
for _, resLR := range unmatchedResLRs {
Expand All @@ -107,9 +112,15 @@ func (s *listenerRuleSynthesizer) synthesizeListenerRulesOnListener(ctx context.
}
resLR.SetStatus(lrStatus)
}
// Delete all unmatched sdk LRs which were pushed down as new rules are either modified or created at higher priority
for _, sdkLR := range unmatchedSDKLRs {
if err := s.lrManager.Delete(ctx, sdkLR); err != nil {
return err
}
}
// Update existing listener rules on the load balancer for their tags
for _, resAndSDKLR := range matchedResAndSDKLRsBySettings {
lsStatus, err := s.lrManager.Update(ctx, resAndSDKLR.resLR, resAndSDKLR.sdkLR)
lsStatus, err := s.lrManager.UpdateRulesTags(ctx, resAndSDKLR.resLR, resAndSDKLR.sdkLR)
if err != nil {
return err
}
Expand Down Expand Up @@ -144,11 +155,14 @@ type resLRDesiredActionsAndConditionsPair struct {
desiredConditions []types.RuleCondition
}

func (s *listenerRuleSynthesizer) matchResAndSDKListenerRules(unmatchedResLRs []*elbv2model.ListenerRule, unmatchedSDKLRs []ListenerRuleWithTags, resLRDesiredActionsAndConditionsPairs map[*elbv2model.ListenerRule]*resLRDesiredActionsAndConditionsPair) ([]resAndSDKListenerRulePair, []*elbv2model.ListenerRule, []ListenerRuleWithTags, error) {
func (s *listenerRuleSynthesizer) matchResAndSDKListenerRules(resLRs []*elbv2model.ListenerRule, unmatchedSDKLRs []ListenerRuleWithTags, resLRDesiredActionsAndConditionsPairs map[*elbv2model.ListenerRule]*resLRDesiredActionsAndConditionsPair) ([]resAndSDKListenerRulePair, []resAndSDKListenerRulePair, []*elbv2model.ListenerRule, []ListenerRuleWithTags, error) {
var matchedResAndSDKLRsBySettings []resAndSDKListenerRulePair
var matchedResAndSDKLRsByPriority []resAndSDKListenerRulePair
var unmatchedResLRs []*elbv2model.ListenerRule
var resLRsToCreate []*elbv2model.ListenerRule
var sdkLRsToDelete []ListenerRuleWithTags

for _, resLR := range unmatchedResLRs {
for _, resLR := range resLRs {
resLRDesiredActionsAndConditionsPair := resLRDesiredActionsAndConditionsPairs[resLR]
found := false
for i := 0; i < len(unmatchedSDKLRs); i++ {
Expand All @@ -169,10 +183,46 @@ func (s *listenerRuleSynthesizer) matchResAndSDKListenerRules(unmatchedResLRs []
}
}
if !found {
resLRsToCreate = append(resLRsToCreate, resLR)
unmatchedResLRs = append(unmatchedResLRs, resLR)
}
}
return matchedResAndSDKLRsBySettings, resLRsToCreate, unmatchedSDKLRs, nil

resLRByPriority := mapResListenerRuleByPriority(unmatchedResLRs)
sdkLRByPriority := mapSDKListenerRuleByPriority(unmatchedSDKLRs)
resLRPriorities := sets.Int32KeySet(resLRByPriority)
sdkLRPriorities := sets.Int32KeySet(sdkLRByPriority)
for _, priority := range resLRPriorities.Intersection(sdkLRPriorities).List() {
resLR := resLRByPriority[priority]
sdkLR := sdkLRByPriority[priority]
matchedResAndSDKLRsByPriority = append(matchedResAndSDKLRsByPriority, resAndSDKListenerRulePair{
resLR: resLR,
sdkLR: sdkLR,
})
}
for _, priority := range resLRPriorities.Difference(sdkLRPriorities).List() {
resLRsToCreate = append(resLRsToCreate, resLRByPriority[priority])
}
for _, priority := range sdkLRPriorities.Difference(resLRPriorities).List() {
sdkLRsToDelete = append(sdkLRsToDelete, sdkLRByPriority[priority])
}
return matchedResAndSDKLRsBySettings, matchedResAndSDKLRsByPriority, resLRsToCreate, sdkLRsToDelete, nil
}

func mapResListenerRuleByPriority(resLRs []*elbv2model.ListenerRule) map[int32]*elbv2model.ListenerRule {
resLRByPriority := make(map[int32]*elbv2model.ListenerRule, len(resLRs))
for _, resLR := range resLRs {
resLRByPriority[resLR.Spec.Priority] = resLR
}
return resLRByPriority
}

func mapSDKListenerRuleByPriority(sdkLRs []ListenerRuleWithTags) map[int32]ListenerRuleWithTags {
sdkLRByPriority := make(map[int32]ListenerRuleWithTags, len(sdkLRs))
for _, sdkLR := range sdkLRs {
priority, _ := strconv.ParseInt(awssdk.ToString(sdkLR.ListenerRule.Priority), 10, 64)
sdkLRByPriority[int32(priority)] = sdkLR
}
return sdkLRByPriority
}

func mapResListenerRuleByListenerARN(resLRs []*elbv2model.ListenerRule) (map[string][]*elbv2model.ListenerRule, error) {
Expand Down

0 comments on commit 42385aa

Please sign in to comment.