After upgrade ack-ec2-controller caused production impact from SG reconciliation behaviour

[Preranahp]

Describe the bug

Hi team,

After upgrading the ACK EC2 controller from v1.3.2 to v1.6.2, we observed that the controller revoked and immediately re-authorized Security Group ingress rules across multiple Security Groups. During this reconciliation, it also logged EC2 retry quota exceeded errors, which impacted application health probes in our production environment.
This pattern is not ideal for production workloads requiring continuous network connectivity, it resulted in approximately 2-3 mins of application downtime.

Errors occued during upgrading ack-ec2-controller:

operation error EC2: RevokeSecurityGroupIngress, failed to get rate limit token, retry quota exceeded, 1 available, 5 requested
AuthorizeSecurityGroupIngress: failed to get rate limit token, retry quota exceeded, 0 available, 5 requested

We couldn't find any release notes mentioning a change in SecurityGroup reconciliation behavior.

AWS Support informed us that the probe failures were caused by the Security Group reconciliation pattern, where ingress rules are deleted and then recreated.

Could you help us understand:

• Is this revoke-then-authorize reconciliation expected behavior in v1.6.2?
• Was there any change in the SecurityGroup reconciler or ACK runtime between v1.3.2 and v1.6.2 that would explain this?
• If so, which release or commit introduced it?
• Is there a recommended way to avoid this behavior for production workloads where temporary loss of Security Group rules can impact connectivity?

Thanks in advance!

Environment

• Kubernetes version: 1.33
• Using EKS (yes/no), if so version? yes,1.33
• AWS service targeted (S3, RDS, etc.) : EC2 (Security group)

[github-actions]

Hello @Preranahp 👋 Thank you for opening an issue in ACK! A maintainer will triage this issue soon.

We encourage community contributions, so if you're interested in tackling this yourself or suggesting a solution, please check out our Contribution and Code of Conduct guidelines.

You can find more information about ACK on our website.

[Preranahp]

Can we prioritize this? This incident is impacting our production.

[gustavodiaz7722]

@Preranahp Hello, I am working fix for this issue. I have reproduced the issue successfully and identified approximate root cause. Testing of fix is still in progress. I will link PR once fix is ready.

[Preranahp]

@gustavodiaz7722, thank you!
Can you also mention the root cause for this behavioural change please?

And also, Is the fix using "diff and patch" instead of "replace" reconciliation strategy?

[gustavodiaz7722]

1. Is revoke-then-authorize reconciliation expected in v1.6.2?

Yes, and it is not new behavior. EC2 has no in-place "modify rule" call in ACK's
model, so to change any ingress/egress rule the controller must revoke the old rule
and authorize the new one. This logic lives in syncSGRules in
pkg/resource/security_group/hooks.go:

// Delete before create for the following reasons:
// - Updating a rule requires that it be removed before the updated version be added.
// - If there is an error with adding new rules, it occurs after deletion of old ones;
//   This is safer and closer to achieving desired resource state.
if err = rm.deleteSecurityGroupRules(ctx, latest, toDeleteIngress, toDeleteEgress); err != nil {
    return err
}
if err = rm.createSecurityGroupRules(ctx, desired, toAddIngress, toAddEgress); err != nil {
    return err
}

A diff of v1.3.2..v1.6.2 for the security_group package shows the rule-diffing code
(syncSGRules, containsRule, compareIPPermission, and the IngressRules /
EgressRules delta logic) is functionally identical — only SDK v1→v2 type changes
and a tags-comparison refactor. The delete/recreate behavior was the same in v1.3.2.

2. Was there a change in the SecurityGroup reconciler or ACK runtime between v1.3.2 and v1.6.2?

The significant change is the SDK v2 migration (#237). go.mod confirms:

Version	AWS SDK
v1.3.2	github.com/aws/aws-sdk-go v1.49.0 only
v1.6.2	adds aws-sdk-go-v2 v1.35.0 + service/ec2 v1.202.1

The exact errors reported —
failed to get rate limit token, retry quota exceeded, 1 available, 5 requested —
are emitted by aws-sdk-go-v2's standard retryer token-bucket rate limiter
(ratelimit). aws-sdk-go v1 has no such "retry quota" concept or message.

The ACK runtime builds the client with config.LoadDefaultConfig(...) and no custom
retryer (runtime/pkg/runtime/config.go), so the SDK v2 default standard retryer
applies out of the box (max 3 attempts, 500-token bucket, 5 tokens per retry). Under a
burst of throttled calls the bucket drains and subsequent retries fail with
retry quota exceeded.

The SG rule-diffing logic itself did not change — so the reconciler did not become
more aggressive; the SDK's retry/rate-limiting behavior changed.

3. Which release or commit introduced it?

• New retry-quota behavior: PR #237
  (42b398d, "Generate with SDK-V2 model"), first shipped in v1.3.3.
• Revoke/authorize pattern: traces to PR checking in SNS service controller #227 and earlier — well before v1.3.2,
  so it is not a regression introduced in this upgrade range.

Key conclusions

1. The revoke-then-authorize of SG-reference rules is a pre-existing bug, NOT a v1.6.2
   regression. Both v1.3.2 and v1.6.2 revoke+re-authorize such rules on every reconcile,
   because the AWS-populated userID field never round-trips into the CR spec, so the
   desired-vs-observed comparison always differs. (This is exactly the "nil field in spec
   triggers a delta" mechanism — the field is userID/userIDGroupPairs, populated by AWS
   on read but absent from the spec.)

2. Plain CIDR rules are safe — only a cosmetic ordering delta, no actual revoke.

3. Why the upgrade caused a visible outage: steady-state, each SG re-reconciles roughly
   once per resync interval (default 10h), so the sub-second revoke→authorize gap goes
   unnoticed. On upgrade/restart the controller reconciles all SGs at once, revoking +
   re-authorizing every SG-reference rule simultaneously. Under v1.6.2's aws-sdk-go-v2
   standard retryer (new via RDS service controller #237), the burst hit EC2 throttling and drained the retry token
   bucket → retry quota exceeded → Authorize calls failed and were deferred to later
   reconciles, stretching the revoke→authorize gap into the reported 2–3 min connectivity loss.

[gustavodiaz7722]

@Preranahp

The fix for this issue requires further investigation. Here is a short term solution we can try to mitigate impact in your production environment.

Immediate action: backfill userID on existing CRs

Adding the account ID into spec.ingressRules[].userIDGroupPairs[].userID
(and the same under egressRules) makes desired == observed, so the rule stops
being flagged as a delta and is no longer revoked/re-authorized on reconcile.

Example patch (replace <ACCOUNT_ID> with your AWS account ID):

spec:
  ingressRules:
    - fromPort: 5432
      toPort: 5432
      ipProtocol: tcp
      userIDGroupPairs:
        - groupID: sg-0d67xxxxxxxx
          userID: "<ACCOUNT_ID>"   # <-- backfilled to match what AWS returns

Apply to every existing SecurityGroup CR that has userIDGroupPairs rules
(ingress and egress). Do this before the next controller restart/upgrade so
the reconcile is a no-op instead of a revoke/authorize storm.

Also check controller logs for other spurious-delta fields

userID is the confirmed culprit, but any other field that AWS populates on
read and is absent from your spec can trigger the same revoke/authorize churn.
Before upgrading, scan the controller logs for delta entries on SG rules and
backfill any other mismatched fields the same way.

kubectl -n <ack-namespace> logs deploy/ack-ec2-controller \
  | grep -iE "syncSGRules|IngressRules|EgressRules|delta"

Look for delta lines showing a field present in the observed (AWS) side but
missing/different on the desired (spec) side — those are your candidates to
backfill so the upgrade reconcile stays a no-op.