feat: add annotations to skip target and weight reconciliation for externally-managed resources by mfanjie · Pull Request #85 · aws-controllers-k8s/elbv2-controller

mfanjie · 2026-06-06T14:11:19Z

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

Add the 'elbv2.services.k8s.aws/target-management: ignore' annotation that tells the controller to leave targets alone — no DescribeTargetHealth, no RegisterTargets, no DeregisterTargets. This allows an external controller to own target registration while ACK still manages the target group itself (health checks, attributes, etc.). Three guard points: - sdkFind: skip describeTargets so externally-registered targets are never read from AWS (no drift detected) - sdkUpdate: guard the register/deregister block with the annotation check - sdkCreate: skip the post-create requeue for target registration Templates updated so future code regeneration preserves the behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…for listeners Add the 'elbv2.services.k8s.aws/weight-management: ignore' annotation that tells the controller to not reconcile forward action target group weights. This allows external blue/green deployment tools to manage traffic shifting independently while ACK still manages the rest of the listener configuration. Uses a delta_pre_compare hook (customPreCompare) that copies AWS-side weights into the desired spec before DeepEqual comparison, so weight differences never generate deltas. Since the desired spec is mutated with the latest weights, any ModifyListener calls triggered by other field changes also preserve the AWS-side weights. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

ack-prow · 2026-06-06T14:11:25Z

[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 · 2026-06-06T14:11:31Z

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.

- Add nil guard for action pointer in mergeLatestWeights (both listener and rule) since Actions/DefaultActions is []*Action, individual elements can be nil - Add weight-management annotation support to rule resources (isWeightManagementIgnored + mergeLatestWeights + customPreCompare hook) - Add tests for rule weight-management: annotation check, weight merge, and delta suppression with annotation - Fix: use string literal for annotation key in rule package to avoid duplicating listener's exported AnnotationWeightManagement constant Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

a-hilaly · 2026-06-11T18:07:08Z

/ok-to-test

a-hilaly · 2026-06-11T18:12:25Z

+	// action weights for target groups. When set to "ignore", the controller
+	// will not reconcile weight differences — allowing an external controller
+	// or deployment tool to manage blue/green traffic shifting independently.
+	AnnotationWeightManagement = "elbv2.services.k8s.aws/weight-management"


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 · 2026-06-12T03:05:18Z

/retest

…ecksum The generator.yaml was modified but the metadata file wasn't regenerated, causing the CI verify-code-gen check to fail on file_checksum mismatch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

mfanjie · 2026-06-16T05:21:43Z

@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 · 2026-06-17T22:08:33Z

@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 · 2026-07-09T16:54:40Z

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 · 2026-07-09T17:32:36Z

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 · 2026-07-29T16:37:44Z

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 · 2026-07-29T23:02:07Z

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 · 2026-07-29T22:42:06Z

@@ -1,4 +1,4 @@
-	if delta.DifferentAt("Spec.Targets") {
+	if delta.DifferentAt("Spec.Targets") && !isTargetManagementIgnored(desired) {


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.

@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.

Add end-to-end coverage for the services.k8s.aws/ignore-field-drift annotation (aws-controllers-k8s/runtime#256) on an ELBv2 Listener's forward-action target-group weights, mirroring "Scenario 2" of aws-controllers-k8s#85: a blue/green deploy tool shifts traffic weights across target groups on the live listener, and without ignore-field-drift the controller reconciles the weights back to the declared spec, breaking the deployment. TestListenerIgnoreFieldDrift.test_weight_drift_ignored declares a weighted forward action (90/10) annotated to ignore spec.defaultActions, shifts the weights out-of-band to 50/50 via ModifyListener, and asserts that the external distribution survives, the resource stays Synced, and a subsequent spec edit is retained in the CR but not pushed to AWS. The IgnoreFieldDrift feature gate is Alpha and disabled by default, so the test enables it on the deployed controller for the module and restores the prior value afterwards (mirrors the ec2-controller VPC coverage for the same runtime feature). The gate is only present once the controller's runtime dependency includes runtime#256, so this coverage runs green after the corresponding runtime bump. Two ip-type target groups back the weighted action so the fixture does not depend on registered targets. Signed-off-by: Hao Wang <rhaowang@amazon.com>

…90) ## Description Adds end-to-end coverage for the `services.k8s.aws/ignore-field-drift` annotation ([aws-controllers-k8s/runtime#256](aws-controllers-k8s/runtime#256)) on an ELBv2 `Listener`'s forward-action target-group weights. This mirrors **"Scenario 2"** of #85: a blue/green deploy tool shifts traffic weights across target groups on the live listener, and without ignore-field-drift the controller reconciles the weights back to the declared spec, breaking the deployment. The generic runtime feature lets the resource opt `spec.defaultActions` out of drift reconciliation via the annotation — no controller-specific code required. Test-only change: `test/e2e/tests/test_listener.py`, `test/e2e/resources/listener_ignore_field_drift.yaml`, `test/e2e/resources/target_group_ip.yaml`. ## What the test does `TestListenerIgnoreFieldDrift.test_weight_drift_ignored`: 1. Creates a `Listener` with a weighted forward action across two target groups (declared 90/10), annotated `services.k8s.aws/ignore-field-drift: "spec.defaultActions"`. 2. Asserts the declared weights were applied at create and the resource is `Synced`. 3. Shifts the weights out-of-band to 50/50 via `ModifyListener` (simulating the deploy tool). 4. Asserts the external distribution **survives** (controller does not revert), the resource **stays `Synced`**, and a subsequent spec edit (→70/30) is **retained in the CR but not pushed** to AWS. Two **ip-type** target groups back the weighted action so the fixture does not depend on registered targets. ## Feature gate handling The `IgnoreFieldDrift` gate is Alpha and disabled by default, so the test enables it on the deployed controller by patching `FEATURE_GATES` on the Deployment. The gate landed in **runtime v0.62.0**; `main` is now on v0.63.0, so it is available. The `ignore_field_drift_enabled` fixture is **session-scoped, check-then-set, and never restores** — this matters and is not just style. pytest-xdist spreads individual tests across 16 worker *processes* under `LoadScheduling`, so a fixture at *any* scope is instantiated once in every worker that picks up a test from this file. An enable/restore pair therefore rolls the shared controller Deployment twice per such worker, and each restart stops reconciliation cluster-wide long enough to fail an unrelated listener / load balancer / target group test waiting on `ACK.ResourceSynced`. This is the same fix as [ec2-controller#361](aws-controllers-k8s/ec2-controller#361) `b2cb92a`, where the module-scoped version cost three unrelated tests in one run. Check-then-set needs no cross-process lock: concurrent workers that both observe the gate off compute the same `FEATURE_GATES` string from the same starting value, so the second patch leaves the pod template byte-identical and does not bump the Deployment generation. Leaving the gate on is safe — it is inert unless a resource carries the annotation, and only this file's drift resources do; the kind cluster is torn down at the end of the run. Follow-up worth doing: set the gate at controller setup time (`FEATURE_GATES` in test-infra's `controller-setup.sh`, as `IAMRoleSelector` already does), after which this fixture degrades to a no-op check and the mid-run rollout disappears entirely. ## Testing Verified locally via a full `kind` e2e run with the gate enabled: `1 passed`. The external 50/50 shift survived, the resource stayed `Synced`, and the 70/30 spec edit was retained but not pushed. Note for anyone hitting this on an older base: against a runtime without the gate, the controller treats the unknown gate as fatal and exits, which surfaces only as `controller deployment ack-elbv2-controller did not roll out within 120s`. The crashlooping pod's logs are *not* in the prow `controller_logs` artifact (it captures the surviving pod), so that artifact looks misleadingly clean. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

jessemeng and others added 2 commits June 6, 2026 21:58

ack-prow Bot requested review from gustavodiaz7722 and sapphirew June 6, 2026 14:11

ack-prow Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 6, 2026

ack-prow Bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 11, 2026

a-hilaly reviewed Jun 11, 2026

View reviewed changes

fix: regenerate ack-generate-metadata.yaml to match generator.yaml ch…

5bfc39c

…ecksum The generator.yaml was modified but the metadata file wasn't regenerated, causing the CI verify-code-gen check to fail on file_checksum mismatch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

knottnt reviewed Jul 29, 2026

View reviewed changes

sapphirew mentioned this pull request Jul 30, 2026

test(listener): add ignore-field-drift e2e coverage for weight drift #90

Merged

Navigation Menu

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

feat: add annotations to skip target and weight reconciliation for externally-managed resources#85
mfanjie wants to merge 4 commits into
aws-controllers-k8s:mainfrom
mfanjie:preserve-target

mfanjie commented Jun 6, 2026

Uh oh!

ack-prow Bot commented Jun 6, 2026

Uh oh!

ack-prow Bot commented Jun 6, 2026

Uh oh!

a-hilaly commented Jun 11, 2026

Uh oh!

a-hilaly Jun 11, 2026

Uh oh!

mfanjie commented Jun 12, 2026

Uh oh!

mfanjie commented Jun 16, 2026

Uh oh!

knottnt commented Jun 17, 2026

Uh oh!

sapphirew commented Jul 9, 2026

Uh oh!

blackdog0403 commented Jul 9, 2026

Uh oh!

blackdog0403 commented Jul 29, 2026

Uh oh!

knottnt commented Jul 29, 2026

Uh oh!

knottnt Jul 29, 2026

Uh oh!

blackdog0403 Jul 31, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		@@ -1,4 +1,4 @@
		if delta.DifferentAt("Spec.Targets") {
		if delta.DifferentAt("Spec.Targets") && !isTargetManagementIgnored(desired) {

Conversation

mfanjie commented Jun 6, 2026

Problem

Scenario 1 — Externally-registered targets

Scenario 2 — Blue/green weight shifting

Solution

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

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

Usage

Files changed

Target group (target-management annotation)

Listener (weight-management annotation)

Uh oh!

ack-prow Bot commented Jun 6, 2026

Uh oh!

ack-prow Bot commented Jun 6, 2026

Uh oh!

a-hilaly commented Jun 11, 2026

Uh oh!

a-hilaly Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

mfanjie commented Jun 12, 2026

Uh oh!

mfanjie commented Jun 16, 2026

Uh oh!

knottnt commented Jun 17, 2026

Uh oh!

sapphirew commented Jul 9, 2026

Uh oh!

blackdog0403 commented Jul 9, 2026

Uh oh!

blackdog0403 commented Jul 29, 2026

Uh oh!

knottnt commented Jul 29, 2026

Uh oh!

knottnt Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

blackdog0403 Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

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

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

blackdog0403 Jul 31, 2026 •

edited

Loading