Skip to content

fix: use v1alpha2 APIBindings informer in DynamicRESTMapper HasSynced check#3870

Merged
kcp-ci-bot merged 1 commit intokcp-dev:mainfrom
maxpain:fix/dynamic-rest-mapper-apibinding-version
Mar 4, 2026
Merged

fix: use v1alpha2 APIBindings informer in DynamicRESTMapper HasSynced check#3870
kcp-ci-bot merged 1 commit intokcp-dev:mainfrom
maxpain:fix/dynamic-rest-mapper-apibinding-version

Conversation

@maxpain
Copy link
Member

@maxpain maxpain commented Mar 2, 2026

Problem

The DynamicTypesController is instantiated with a v1alpha2 APIBindings informer:

dynamicTypesController, err := dynamicrestmapper.NewDynamicTypesController(ctx, s.completedConfig.DynamicRESTMapper,
    s.ApiExtensionsSharedInformerFactory.Apiextensions().V1().CustomResourceDefinitions(),
    s.KcpSharedInformerFactory.Apis().V1alpha2().APIBindings(),  // ← v1alpha2
    ...
)

But the HasSynced wait function checks the v1alpha1 informer:

return s.KcpSharedInformerFactory.Apis().V1alpha1().APIBindings().Informer().HasSynced() && // ← v1alpha1

Since the SharedInformerFactory creates separate informers per API version (keyed by reflect.Type), and factory.Start() only starts previously registered informers, the sequence is:

  1. Controller registration calls V1alpha2().APIBindings() → registers v1alpha2 informer in factory
  2. factory.Start() starts all registered informers (only v1alpha2)
  3. HasSynced calls V1alpha1().APIBindings().Informer() → lazily creates a new v1alpha1 informer, but it is never started
  4. An unstarted informer has controller == nil, so HasSynced() returns false indefinitely
  5. The DynamicRESTMapper controller never starts — blocked forever in PollUntilContextCancel

Fix

Change the HasSynced check to use V1alpha2().APIBindings(), matching the informer version passed to the controller constructor.

Impact

Without this fix, the DynamicRESTMapper controller never starts, which prevents dynamic API discovery from working correctly for resources introduced via APIBindings.

NONE

@kcp-ci-bot kcp-ci-bot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. dco-signoff: no Indicates the PR's author has not signed the DCO. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 2, 2026
@kcp-ci-bot
Copy link
Contributor

Hi @maxpain. Thanks for your PR.

I'm waiting for a kcp-dev 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.

@kcp-ci-bot kcp-ci-bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 2, 2026
… check

The DynamicTypesController is instantiated with a v1alpha2 APIBindings
informer, but the HasSynced wait function checks v1alpha1 APIBindings
informer sync status. Since the SharedInformerFactory creates separate
informers per API version (keyed by reflect.Type), the v1alpha1
informer is never started by factory.Start() (which only starts
previously registered informers). An unstarted informer has a nil
controller, causing HasSynced() to return false indefinitely, which
prevents the DynamicRESTMapper controller from ever starting.

Signed-off-by: Max Makarov <maxpain@linux.com>
@maxpain maxpain force-pushed the fix/dynamic-rest-mapper-apibinding-version branch from fcd6c35 to 25e5532 Compare March 2, 2026 10:19
@kcp-ci-bot kcp-ci-bot added dco-signoff: yes Indicates the PR's author has signed the DCO. and removed dco-signoff: no Indicates the PR's author has not signed the DCO. labels Mar 2, 2026
@cnvergence
Copy link
Member

/ok-to-test

@kcp-ci-bot kcp-ci-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 Mar 2, 2026
@gman0
Copy link
Contributor

gman0 commented Mar 2, 2026

/kind bug

@kcp-ci-bot kcp-ci-bot added kind/bug Categorizes issue or PR as related to a bug. and removed do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Mar 2, 2026
@mjudeikis
Copy link
Contributor

/retest

Copy link
Contributor

@mjudeikis mjudeikis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 3, 2026
@kcp-ci-bot
Copy link
Contributor

LGTM label has been added.

DetailsGit tree hash: a0c5264a1572838953b588644dccc21f79b11b11

@kcp-ci-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mjudeikis

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

The pull request process is described 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

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 3, 2026
@mjudeikis
Copy link
Contributor

/ok-to-test

@gman0
Copy link
Contributor

gman0 commented Mar 3, 2026

/release-note

@kcp-ci-bot
Copy link
Contributor

@gman0: the /release-note and /release-note-action-required commands have been deprecated.
Please edit the release-note block in the PR body text to include the release note. If the release note requires additional action include the string action required in the release note. For example:

```release-note
Some release note with action required.
```
Details

In response to this:

/release-note

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.

@gman0
Copy link
Contributor

gman0 commented Mar 3, 2026

/release-note Fixed informer sync wait

@gman0
Copy link
Contributor

gman0 commented Mar 3, 2026

@maxpain please update your release notes in the PR description -- it may be left empty. Like so:

```release-note
NONE
```

@kcp-ci-bot kcp-ci-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Mar 3, 2026
@maxpain
Copy link
Member Author

maxpain commented Mar 3, 2026

@maxpain please update your release notes in the PR description -- it may be left empty. Like so:

```release-note
NONE

Done

@mjudeikis
Copy link
Contributor

/retest

@kcp-ci-bot kcp-ci-bot merged commit 12d7897 into kcp-dev:main Mar 4, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has signed the DCO. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants