feat: add annotations to skip target and weight reconciliation for externally-managed resources

[mfanjie]

Problem

The ACK ELBv2 controller treats the Kubernetes resource spec as the complete source of truth. During reconciliation, it reads the current state from AWS and forces it back to match the spec. This breaks setups where targets or traffic weights are managed externally:

Scenario 1 — Externally-registered targets

A separate controller (not ACK) registers targets with a target group that ACK manages. When the ACK TargetGroup spec has targets: [] (empty), the controller sees drift between spec (0 targets) and AWS (N targets) and deregisters all externally-registered targets.

Scenario 2 — Blue/green weight shifting

A deployment tool shifts traffic weights between target groups (e.g., blue 100→0, green 0→100). On the next reconciliation, ACK sees the weight drift on the Listener's DefaultActions.ForwardConfig and resets weights back to the spec values, breaking the active deployment.

Solution

Two new annotations allow opting out of specific reconciliation behaviors:

elbv2.services.k8s.aws/target-management: ignore (TargetGroup)

When set, the controller will not read, register, or deregister targets. Three guard points:

Guard	What it prevents
sdkFind — skips DescribeTargetHealth	Externally-registered targets are never read → no drift detected
sdkUpdate — skips register/deregister block	Even if a delta existed, targets aren't touched
sdkCreate — skips post-create requeue	No target registration requeue on initial creation

elbv2.services.k8s.aws/weight-management: ignore (Listener)

When set, the controller will not reconcile ForwardConfig.TargetGroups[].Weight. Uses a delta_pre_compare hook that copies AWS-side weights into the desired spec before the DeepEqual comparison runs:

• Weight-only changes: no delta → no ModifyListener call → external weights preserved
• Other field changes (port, certificates, etc.): ModifyListener IS called, but with AWS weights merged in → external weights preserved
• Adding/removing a target group: delta IS created → new TG added, existing weights preserved

Usage

# TargetGroup — let external controller manage targets
apiVersion: elbv2.services.k8s.aws/v1alpha1
kind: TargetGroup
metadata:
  annotations:
    elbv2.services.k8s.aws/target-management: ignore
spec:
  name: my-target-group
  port: 80
  protocol: HTTP
  vpcID: vpc-xxx
  # targets can be omitted

---
# Listener — let external tool manage traffic weights
apiVersion: elbv2.services.k8s.aws/v1alpha1
kind: Listener
metadata:
  annotations:
    elbv2.services.k8s.aws/weight-management: ignore
spec:
  defaultActions:
  - type: forward
    forwardConfig:
      targetGroups:
      - targetGroupARN: arn:aws:...:targetgroup/blue
        weight: 100
      - targetGroupARN: arn:aws:...:targetgroup/green
        weight: 0

Files changed

Target group (target-management annotation)

• pkg/resource/target_group/hooks.go — added AnnotationTargetManagement constant and isTargetManagementIgnored() helper
• pkg/resource/target_group/sdk.go — guards in sdkFind, sdkCreate, sdkUpdate
• pkg/resource/target_group/hooks_test.go — 12 new test cases
• templates/hooks/target_group/*.tpl — template files updated for future codegen

Listener (weight-management annotation)

• pkg/resource/listener/hooks.go — added customPreCompare, mergeLatestWeights, isWeightManagementIgnored
• pkg/resource/listener/delta.go — added customPreCompare call in newResourceDelta
• pkg/resource/listener/hooks_test.go — 11 new test cases (new file)
• generator.yaml — added delta_pre_compare hook for Listener
• apis/v1alpha1/generator.yaml — same

🤖 Generated with Claude Code

[ack-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mfanjie
Once this PR has been reviewed and has the lgtm label, please assign michaelhtm for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ack-prow]

Hi @mfanjie. Thanks for your PR.

I'm waiting for a aws-controllers-k8s member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[a-hilaly]

/ok-to-test

[a-hilaly]

Feels like this is a use case off ignore-deltas for specific fields? cc @michaelhtm @knottnt - if yes, i think we need a generic solution for all controllers

[mfanjie]

/retest

[mfanjie]

@a-hilaly @sapphirew can you please check if it is possible to merge this pr before we have the generic solution being defined, we really need the feature before we use ack in our production env

[knottnt]

@mfanjie We've seen a few similar feature requests for other controllers. Given this the ACK team is currently looking into adding a feature for read-only/ignored fields similar to what is discussed in this aws-controllers-k8s/community#2367. The ACK team is hoping that this will allow us to provide a general solution across all of our controllers and avoid adding adhoc custom behavior to individual controllers.

[sapphirew]

Hi @mfanjie, thanks for the ping and for your patience.

Quick update: the generic feature @knottnt mentioned is now in flight. It's an opt-in services.k8s.aws/ignore-field-drift annotation (gated by a new SelectiveReconciliation feature gate, Alpha/off by default) that tells the controller to stop reconciling drift on named spec fields while still creating/adopting/deleting the resource.

• Runtime: feat(runtime): add ignore-field-drift annotation for selective field reconciliation runtime#256
• Code-generator: feat: filter ignore-field-drift differences from generated resource delta code-generator#714

How it maps to your two scenarios:

Scenario 2 (blue/green weights) — covered by ignore-field-drift, but only at whole-field granularity today. ignore-field-drift: "spec.defaultActions" stops reverting external weight changes and re-sends the observed values on unrelated updates, which matches what your weight-management hook does. The gap: it ignores the entire defaultActions list, so it can't yet express "keep managing the target set but ignore only the weight sub-field." Sub-field ignore inside a list entry is a tracked follow-up.

Scenario 1 (externally-registered targets) — this one is better solved by remodeling rather than by an ignore annotation. ignore-field-drift still reads AWS state to compute the diff, so it isn't the right tool for "don't manage the target membership at all." The cleaner fix is to promote target registration to its own resource — a TargetGroupAttachment CRD that references its parent TargetGroup (the same pattern Terraform uses with aws_lb_target_group_attachment). ACK then owns the target group's lifecycle, each target attachment is an independently-reconciled object, and external tooling owns the targets it manages simply by not having ACK CRs for them. "Manage a subset" falls out naturally, with no drift and no target-management annotation needed. This would be a per-controller modeling change (a new CRD + cross-resource reference in this controller); we see it as the recommended long-term direction for this case.

So the generic feature addresses your weight case at a coarse grain now, and the target case is best handled by the sub-resource approach rather than being folded into the annotation.

Since the generic feature is still pre-release and doesn't fully subsume this PR in v1, I'd defer the merge decision to the maintainers. If you need the fine-grained weight behavior and the target opt-out in production now, this controller-specific PR may still be the right near-term path; we'll keep the long-term direction (sub-field ignore + the TargetGroupAttachment sub-resource) tracked so it can converge later.

[blackdog0403]

Hi @michaelhtm (#85),

Wanted to check in on the path forward for this PR. For context, I'm the AWS SA working with the team that submitted this — they're actively migrating ELBv2 infrastructure to ACK and hitting both scenarios (target deregistration + weight reset) in production today.

I see the generic ignore-field-drift feature is progressing (runtime#256 + code-generator#714) — that's great. However, looking at the coverage:

Scenario 2 (weights): covered by ignore-field-drift ✅
Scenario 1 (targets): not fully covered — the generic feature still calls DescribeTargetHealth, which means externally-registered targets are still read and can cause drift noise. This PR's target-management: ignore annotation skips the API call entirely, which is what's needed for their setup.
Given that the generic feature is still pre-release and doesn't fully subsume Scenario 1, would you be open to merging this as an interim bridge? The team can rebase to resolve conflicts if that helps move things forward.

Happy to discuss — just want to make sure we have a clear path so the customer isn't blocked longer than necessary. What would you need to feel comfortable with an approve here?

[blackdog0403]

Hi @knottnt @michaelhtm @mfanjie — following up on this. runtime#256 is now in final review stages and addresses Scenario 2 (weight drift) well. However, after detailed analysis, we've confirmed that Scenario 1 (externally-registered targets) is architecturally distinct and cannot be solved by ignore-field-drift:

• ignore-field-drift suppresses the delta but still calls DescribeTargetHealth → externally-registered targets are still read from AWS
• This PR's target-management: ignore skips the API call entirely in sdkFind, which is the correct behavior for "ACK should not manage target membership at all"

The TargetGroupAttachment CRD (long-term direction) doesn't have a timeline yet, and the customer is actively blocked in production today.

Request: Could we proceed with merging the target-management annotation portion of this PR as an interim solution for Scenario 1? The weight-management portion can be dropped in favor of the generic ignore-field-drift feature.

Happy to help the contributor (@mfanjie) split the PR if that makes review easier. This is the customer's highest priority request.

[knottnt]

Hi @knottnt @michaelhtm @mfanjie — following up on this. runtime#256 is now in final review stages and addresses Scenario 2 (weight drift) well. However, after detailed analysis, we've confirmed that Scenario 1 (externally-registered targets) is architecturally distinct and cannot be solved by ignore-field-drift:

* `ignore-field-drift` suppresses the _delta_ but still calls `DescribeTargetHealth` → externally-registered targets are still read from AWS

* This PR's `target-management: ignore` skips the API call entirely in `sdkFind`, which is the correct behavior for "ACK should not manage target membership at all"

The TargetGroupAttachment CRD (long-term direction) doesn't have a timeline yet, and the customer is actively blocked in production today.

Request: Could we proceed with merging the target-management annotation portion of this PR as an interim solution for Scenario 1? The weight-management portion can be dropped in favor of the generic ignore-field-drift feature.

Happy to help the contributor (@mfanjie) split the PR if that makes review easier. This is the customer's highest priority request.

@blackdog0403 The proposed ignored-field-drift should still prevent the ACK controller from attempting to Add/Remove targets despite still calling DescribeTargetHealth. If the read call also needs to be prevented we could potentially add a check for the Spec.Targets field in the ignore-field-drift annotation before calling rm.describeTargets.

That said we can take a look at splitting out the target-management annotation into a separate PR to meet the customer's request. While we're hoping to merge the ignore-drift-field feature soon it is possible the feature could be further delayed.

[knottnt]

Q: Any reason to not filter the Spec.Targets from the Delta entirely? If it is the only field that has drifted there no need to enter sdkUpdate at all.

[blackdog0403]

@knottnt @mfanjie This is comment from the customer. - "It's better not reading the endpoint back as we want to match the spec, but it's should be fine for now even it it exposed. can we release what we have now?"

Please progress with existing PR #256 and split out target-management annotation to separate PR.