Skip to content

Make pod informer transform idempotent so WatchList streaming sync does not fall back to full LIST - #4881

Open
yash97 wants to merge 1 commit into
kubernetes-sigs:mainfrom
yash97:fix-podinfo-transform-watchlist-idempotency
Open

Make pod informer transform idempotent so WatchList streaming sync does not fall back to full LIST#4881
yash97 wants to merge 1 commit into
kubernetes-sigs:mainfrom
yash97:fix-podinfo-transform-watchlist-idempotency

Conversation

@yash97

@yash97 yash97 commented Aug 16, 2026

Copy link
Copy Markdown

Issue

N/A. Happy to file one if maintainers prefer tracking this as an issue first.

Description

The pod informer transform (podInfoConverter) only accepts *corev1.Pod and returns an error for anything else. That breaks streaming initial sync.

Since client-go v0.35 the WatchListClient feature is enabled by default, and this repo pins v0.36.2, so the controller attempts a WatchList streaming sync (KEP-3157) for the pod informer on every start. During the stream, the reflector applies the informer transform to objects in its temporary store, then the destination FIFO applies the same transform again on Replace. The second application hands the converter its own output, a *PodInfo. The converter returns "expect pod object", the streaming sync aborts, and the reflector silently falls back to a conventional LIST.

The fallback is expensive: it decodes every pod in the cluster into memory at once before the transform can slim anything down. On small clusters nobody notices. On a cluster with roughly 380k pods (kwok scale testing), the controller could not finish initial sync inside a 20Gi memory limit and crash looped on OOM, logging problem wait for podInfo repo sync on every attempt.

The fix is a two-line early return: when the transform receives a *PodInfo, hand it back unchanged. With an idempotent transform the streaming sync completes and peak memory during sync stays close to one decoded pod at a time, instead of the whole cluster.

Testing was done in three layers:

  1. The included unit test applies the transform twice and asserts the second application returns the same *PodInfo instead of an error.
  2. Wire-level verification against a real kube-apiserver (v1.34, envtest, WatchList=true feature gate), with a recording http.RoundTripper on the pods endpoint. Before the fix the request log shows the streaming attempt followed by the fallback:
    req[0]: allowWatchBookmarks=true&resourceVersionMatch=NotOlderThan&sendInitialEvents=true&watch=true
    req[1]: limit=500&resourceVersion=0        <- fallback LIST
    
    After the fix, req[0] is the only sync request and the informer syncs from the stream.
  3. Peak-heap comparison at 20,000 seeded pods (each a few KB, with realistic labels, annotations, env vars), sampling HeapInuse every 20ms through initial sync:
    before: peak 483MiB   (fallback full LIST)
    after:  peak  36MiB   (streaming sync)
    

The envtest harness for layers 2 and 3 is not part of this PR to keep it focused, since it needs kubebuilder assets in CI. I can contribute it in a follow-up if there is interest.

On API servers without WatchList support, client-go falls back to a plain LIST exactly as it does today, so behavior there is unchanged.

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the docs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes

…nc from falling back to full LIST

With client-go >= 0.35 the WatchListClient feature is on by default, so the
pod informer attempts a streaming initial sync (KEP-3157) on every start.
During the stream the reflector transforms objects in its temporary store,
and the destination FIFO applies the transform again on Replace.
podInfoConverter rejected its own output (*PodInfo), which aborted the
streaming sync and made the reflector fall back to a conventional LIST that
decodes every pod in the cluster into memory at once.

Return an existing *PodInfo unchanged so the streaming sync completes. On a
20k-pod test cluster this reduced peak heap during initial sync from 483MiB
to 36MiB.
@kubernetes-prow kubernetes-prow Bot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 16, 2026
@kubernetes-prow

Copy link
Copy Markdown
Contributor

Hi @yash97. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

@kubernetes-prow kubernetes-prow Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 16, 2026
@kubernetes-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yash97
Once this PR has been reviewed and has the lgtm label, please assign shraddhabang 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:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant