Rebase to Kubernetes 1.35.1 by xmudrii · Pull Request #3842 · kcp-dev/kcp

xmudrii · 2026-02-18T15:47:23Z

Summary

This PR rebases kcp to Kubernetes 1.35.1. The kcp-dev/kubernetes fork has been already updated to 1.35.1. Go has been updated to 1.25 with this rebase.

The rebase applied mostly cleanly. Some tests that were very close to the timeout started hitting that timeout with this rebase and Go update, so I had to increase timeouts for those tests. TestAPIExportAPIBindingsAccess had to be disabled because it became very flaky with this PR, however, it has been flaky before too (#3844). This test will be handled as a follow up.

#3897 should be merged either before or after this PR, it includes some additional fixes.

What Type of PR Is This?

/kind feature

Related Issue(s)

xref #3813

Release Notes

- Update kcp to Kubernetes 1.35.1
- Update Go to 1.25.7

xmudrii · 2026-02-19T14:35:04Z

/test pull-kcp-verify

xmudrii · 2026-02-19T14:37:15Z

/retest

xmudrii · 2026-03-04T17:07:34Z

/retest

xmudrii · 2026-03-04T17:28:11Z

/test pull-kcp-test-e2e-sharded

xmudrii · 2026-03-04T18:00:51Z

/test pull-kcp-test-e2e-sharded

xmudrii · 2026-03-04T18:00:55Z

/test pull-kcp-test-e2e-multiple-runs

xmudrii · 2026-03-04T18:22:26Z

/test pull-kcp-test-e2e-sharded

xmudrii · 2026-03-04T19:12:33Z

/test pull-kcp-test-e2e-sharded

xmudrii · 2026-03-06T18:08:26Z

/test pull-kcp-test-e2e

xmudrii · 2026-03-09T09:37:19Z

/retest

xmudrii · 2026-03-09T09:46:21Z

/test pull-kcp-test-integration

xmudrii · 2026-03-09T10:22:11Z

/test pull-kcp-test-integration

Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>

mjudeikis-bot

Review: Kubernetes 1.35.1 Rebase

Overall the rebase looks clean. Most changes are mechanical API adaptations. A few things worth calling out:

🔴 `controller.go` — Silent error swallowing in `processLoop`

The old code:

utilruntime.HandleErrorWithContext(ctx, err, "Failed to process object from queue")

is removed. Non-ErrFIFOClosed errors are now silently swallowed. If the queue returns a processing error (e.g. a user-provided ProcessFunc errors out), it will be completely invisible.

The ShouldResync branch had no actual behavior (just a comment), so losing that is fine. But losing the error log is a regression — at minimum errors should still be logged.

Also: the obj == nil exit condition from the old Pop return is gone. Need to confirm upstream guarantees the new c.config.Pop path never needs the nil check (likely fine if API changed, but worth a comment).

🟡 `nodedeclaredfeatures` admission plugin — new default-on

nodedeclaredfeatures.PluginName added to defaultOnKubePluginsInKube. This validates node-declared feature gates. In kcp, nodes are virtual — need to confirm this plugin is a no-op when no Node objects exist, otherwise it could fail admission for node-touching requests. If kcp does not expose Node resources in virtual workspaces, this is probably fine.

🟡 `processDeltasInBatch` — partial state on `TransactionError`

err := txnStore.Transaction(txns...)
if err != nil {
    for _, i := range err.SuccessfulIndices {
        callbacks[i]()  // only fires for successful txns
    }
    return fmt.Errorf(...)
}

If a batch partially fails, the store has partial state but only the successful callbacks fire. Event handlers will see some adds/updates but not others. Depending on whether TransactionStore is truly atomic or partially applied, this could lead to inconsistency between store state and what handlers observed. Worth a comment or test.

✅ Things that look good

etcd separate context (etcdCtx in both cmd/kcp/kcp.go and cmd/cache-server/main.go): Correct fix — etcd now outlives the apiserver during graceful shutdown, preventing CRD watch cache lazy init from blocking on a dead etcd connection.
NewRealFIFOWithOptions: Mechanical API change, correct.
WatchList semantics check: Good defensive check — disables WatchList if the ListerWatcher does not support the semantics rather than silently misbehaving.
responsewriters.Forbidden ctx removal: Mechanical upstream API change, applied consistently in all call sites.
MustBaseEnvSet boolean param removal: Upstream API change, correct.
min-compatibility-version flag allowlisted: New upstream flag, benign.
NewServiceAccountsController logger param: Upstream API change, correct.

Questions

The processLoop now uses a non-blocking select { case <-ctx.Done(): return; default: ... } — context cancellation is only checked between pops, not mid-block. Is this intentional upstream behavior, or should the queue implement context-aware blocking?
Is nodedeclaredfeatures confirmed to be a no-op in kcp's virtual workspace model before merging?

/cc @mjudeikis

mjudeikis-bot

Inline review comments on key changes

mjudeikis-bot · 2026-03-12T11:47:38Z

staging/src/github.com/kcp-dev/apimachinery/third_party/reflector/controller.go

+					return
+				}
 			}
-			utilruntime.HandleErrorWithContext(ctx, err, "Failed to process object from queue")


🔴 Silent error swallowing regression

The previous code called utilruntime.HandleErrorWithContext(ctx, err, "Failed to process object from queue") here for any non-ErrFIFOClosed error. That line has been removed.

Now all non-FIFO errors (e.g. a ProcessFunc returning an error) are silently dropped with no log, no metric, no panic. This hides real problems and makes debugging very difficult.

Suggestion: add the error logging back:

if err != nil { if errors.Is(err, cache.ErrFIFOClosed) { return } utilruntime.HandleErrorWithContext(ctx, err, "Failed to process object from queue") }

This code has been updated to match the upstream code: https://github.com/kubernetes/kubernetes/blob/9cfdbc0d6ecfaaab403c8ce34d2967751519af12/staging/src/k8s.io/client-go/tools/cache/controller.go#L236-L261

Thanks for the link! Confirmed — aligning with upstream here is the right call. The error handling in upstream controller.go matches this pattern, so the concern is resolved. ✅

mjudeikis-bot · 2026-03-12T11:47:38Z

staging/src/github.com/kcp-dev/apimachinery/third_party/reflector/controller.go

-		if err != nil {
-			if err == cache.ErrFIFOClosed {
-				return
+		select {


🟡 Context cancellation is non-blocking between pops only

The select { case <-ctx.Done(): return; default: ... } check is non-blocking — it only fires at the top of the loop, not while blocked inside Pop/PopBatch. If the queue is empty and Pop is blocking, a ctx cancellation will not be noticed until the next item is popped.

This changes the shutdown behaviour compared to the old code where cancellation could theoretically be handled differently. Worth a comment explaining this is intentional and the queue is expected to unblock on close.

This is copy code, we need to check how its aligned with upstream

Again, this is copied code, I wouldn't touch it as of now.

Fair enough — if it is a verbatim copy of upstream code, keeping divergence low is the right trade-off. Worth opening a tracking issue upstream if the non-blocking ctx cancellation is ever a problem in practice. ✅

pkg/admission/plugins.go

mjudeikis-bot · 2026-03-12T11:47:38Z

staging/src/github.com/kcp-dev/apimachinery/third_party/informers/shared_informer.go

+				})
+			} else {
+				txn := cache.Transaction{
+					Type:   cache.TransactionTypeAdd,


🟡 Partial state on TransactionError — potential store/handler inconsistency

When txnStore.Transaction(txns...) returns a TransactionError, only callbacks for SuccessfulIndices are fired. This is correct if the TransactionStore is truly atomic (all-or-nothing). But if it's partially applied (some items written, some not), event handlers will be notified about some items but the store has a different set — leading to inconsistency between store state and handler view.

Please add a comment clarifying the atomicity contract expected of TransactionStore.Transaction(), or add a test covering the partial-failure path.

need to verify with upstream lib if this same issue in there

This code has been completely copied from the upstream.

Acknowledged — if this is a straight upstream copy, then the partial-state concern applies equally to upstream. Worth verifying via upstream issue tracker rather than diverging here. ✅

mjudeikis-bot · 2026-03-12T11:47:39Z

cmd/kcp/kcp.go

+			// This prevents CRD watch cache lazy initialization from blocking on a dead etcd.
+			etcdCtx, etcdCancel := context.WithCancel(context.Background())
+			defer etcdCancel()
 			if completedConfig.EmbeddedEtcd.Config != nil {


✅ Good fix — etcd now outlives the apiserver during graceful shutdown via a separate context.Background()-rooted context. This prevents CRD watch cache lazy initialization from blocking on a dead etcd connection during shutdown. The defer etcdCancel() correctly cleans up when the command handler returns.

Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>

mjudeikis · 2026-03-12T14:09:40Z

/lgtm
/approve

lets see if it now blow into our faces :D :D thanks @xmudrii

kcp-ci-bot · 2026-03-12T14:09:46Z

LGTM label has been added.

Details

Git tree hash: 23e6892c6df728d5c3cb8418eacdfacf66c699d1

kcp-ci-bot · 2026-03-12T14:09:49Z

[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:

~~OWNERS~~ [mjudeikis]
~~pkg/admission/OWNERS~~ [mjudeikis]
~~staging/src/github.com/kcp-dev/sdk/apis/OWNERS~~ [mjudeikis]

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

kcp-dev deleted a comment from kcp-ci-bot Feb 19, 2026

xmudrii force-pushed the 1.35.1-prep branch 2 times, most recently from a7e6911 to bdee9e5 Compare February 23, 2026 13:58

kcp-ci-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 27, 2026

xmudrii force-pushed the 1.35.1-prep branch from 362a95c to 58f35c9 Compare March 4, 2026 12:10

kcp-ci-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 4, 2026

xmudrii force-pushed the 1.35.1-prep branch 2 times, most recently from 032aad7 to e40997d Compare March 4, 2026 15:13

xmudrii force-pushed the 1.35.1-prep branch 3 times, most recently from 427cde2 to 4c4f54b Compare March 6, 2026 13:52

xmudrii force-pushed the 1.35.1-prep branch from 4c4f54b to c1b9d63 Compare March 9, 2026 09:05

xmudrii added 3 commits March 11, 2026 08:45

Update kcp-dev/kcp to Kubernetes 1.35.1

317163d

Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>

Update third_party code in apimachinery and client-go

93798e7

Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>

Update codegen

3a54dd1

Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>

xmudrii marked this pull request as draft March 11, 2026 08:02

xmudrii force-pushed the 1.35.1-prep branch from 123e5d1 to a9536f3 Compare March 11, 2026 08:04

kcp-ci-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 11, 2026

xmudrii added 4 commits March 12, 2026 12:08

Increase timeouts

2a161d8

Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>

Use different context for embedded etcd

cbd59c6

Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>

Update kcp-dev/kubernetes to kcp-1.35.1

5f16dfd

Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>

Disable TestAPIExportAPIBindingsAccess test

a40fa80

Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>

xmudrii force-pushed the 1.35.1-prep branch from a9536f3 to a40fa80 Compare March 12, 2026 11:23

xmudrii changed the title ~~[WIP] Rebase to Kubernetes 1.35.1~~ Rebase to Kubernetes 1.35.1 Mar 12, 2026

xmudrii marked this pull request as ready for review March 12, 2026 11:28

xmudrii mentioned this pull request Mar 12, 2026

flaky TestAPIExportAPIBindingsAccess #3844

Open

mjudeikis-bot reviewed Mar 12, 2026

View reviewed changes

kcp-ci-bot requested a review from mjudeikis March 12, 2026 11:44

mjudeikis-bot reviewed Mar 12, 2026

View reviewed changes

mjudeikis requested a review from mjudeikis-bot March 12, 2026 11:50

Fix TestAuthorizer flake

e19b6e1

Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>

kcp-ci-bot assigned mjudeikis Mar 12, 2026

kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 12, 2026

kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 12, 2026

kcp-ci-bot merged commit 758c435 into kcp-dev:main Mar 12, 2026
15 checks passed

xmudrii deleted the 1.35.1-prep branch March 12, 2026 14:10

Conversation

xmudrii commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What Type of PR Is This?

Related Issue(s)

Release Notes

Uh oh!

xmudrii commented Feb 19, 2026

Uh oh!

xmudrii commented Feb 19, 2026

Uh oh!

xmudrii commented Mar 4, 2026

Uh oh!

xmudrii commented Mar 4, 2026

Uh oh!

xmudrii commented Mar 4, 2026

Uh oh!

xmudrii commented Mar 4, 2026

Uh oh!

xmudrii commented Mar 4, 2026

Uh oh!

xmudrii commented Mar 4, 2026

Uh oh!

xmudrii commented Mar 6, 2026

Uh oh!

xmudrii commented Mar 9, 2026

Uh oh!

xmudrii commented Mar 9, 2026

Uh oh!

xmudrii commented Mar 9, 2026

Uh oh!

mjudeikis-bot left a comment

Choose a reason for hiding this comment

Review: Kubernetes 1.35.1 Rebase

🔴 controller.go — Silent error swallowing in processLoop

🟡 nodedeclaredfeatures admission plugin — new default-on

🟡 processDeltasInBatch — partial state on TransactionError

✅ Things that look good

Questions

Uh oh!

mjudeikis-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjudeikis commented Mar 12, 2026

Uh oh!

kcp-ci-bot commented Mar 12, 2026

Uh oh!

kcp-ci-bot commented Mar 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

xmudrii commented Feb 18, 2026 •

edited

Loading

🔴 `controller.go` — Silent error swallowing in `processLoop`

🟡 `nodedeclaredfeatures` admission plugin — new default-on

🟡 `processDeltasInBatch` — partial state on `TransactionError`