[mjudeikis]

Summary

What Type of PR Is This?

Must merge after #4494
Deprecated--root-shard-kubeconfig-file from all components
Move to use cached-backed APIIdentities when identities are used.

Related Issue(s)

Fixes #

Release Notes

Non-root shards and standalone virtual workspace servers no longer connect to the root shard. They read the identities of the core APIExports from the cache server, so they can start and restart while the root shard is down. The `--root-shard-kubeconfig-file` flag of `kcp start` is deprecated and ignored, and can be dropped from shard deployments. `--cache-kubeconfig` remains required on non-root shards.

[mjudeikis]

/build-image

[mjudeikis]

/test all

[mjudeikis]

@mjudeikis-bot review this. Key points are upgradability (assume kcp operator helm-charts will have the right changes), hot paths, do we have new hot paths, and inefficiencies, does this PR fully reprecated root-shard kubeconfig flag in ALL codebase?

[mjudeikis-bot]

No Critical or Warning findings. Two Notes. Answers to the questions asked:

Upgradability. Safe, assuming the operator charts catch up.

• --root-shard-kubeconfig-file is still registered, so existing manifests and charts that pass it keep starting. It is marked deprecated, prints a warning and is ignored (pkg/server/options/options.go:189-192).
• Nothing is lost by dropping the ConfigMap fallback. The old non-root path read identitycache ConfigMap -> root shard. The new one reads core-export identities from the APIExports in the cache server. Replication copies whole objects, including status.identityHash (pkg/reconciler/cache/replication/replication_controller.go:251), and the cache already holds them on running installs, so a rolling upgrade finds them.
• A non-root shard restarted while the root shard is down now starts, which it did not before without the ConfigMap. It does need the cache server up. That is not a new dependency: the same hook already blocks on CacheKcpSharedInformerFactory...APIExports().HasSynced() (pkg/server/server.go ~L540).

Hot paths / inefficiencies. No new hot path. The identity handler runs on APIExport add/update events in two informers (local and cache). Its first check is IsCoreExport (cluster == root and a set lookup), and the lock is only taken for core exports whose hash actually changes. On non-root shards the local informer never sees root-cluster exports, so that handler is a no-op there. This removes a per-startup remote GET loop on the root shard, plus the identitycache controller with its extra ConfigMap informer and client per non-root shard. Rotation handling (ids.set -> expireWatches) is unchanged.

Is --root-shard-kubeconfig-file gone from the codebase? Functionally yes. A repo-wide case-insensitive grep for root-shard-kubeconfig / RootShardKubeconfig / RootShardKcpClusterClient finds only:

• the deprecated, ignored flag and its struct field in pkg/server/options/options.go:61,189, kept on purpose for compatibility;
• the sharded-test-server and the migration e2e, which no longer pass it.

Nothing in contrib/, test/load, the docs or the e2e framework passes it. The RootShardSystemMasterBaseConfig hits in the e2e tests are an unrelated test helper. The front-proxy's --root-kubeconfig was already deprecated in the same way (pkg/proxy/options/options.go:67). If you want the flag removed outright, do it one release after the charts stop passing it.

Not anchored

Note: ConfigMap left behind on upgraded non-root shards. The removed identitycache controller created a ConfigMap (identitycache.ConfigMapName) in default of the system:shard cluster of every non-root shard. Nothing deletes it after this PR, so it stays there forever and goes stale. It is harmless, but the PR could delete it once at startup, or the release notes could say it is safe to remove.

Note: roots-shard identity sources can disagree transiently. On the shard hosting root, the same handler is now registered on both the local and the cache APIExport informer (pkg/server/server.go ~L611). If the cache copy lags the local one (for example the cache was restored from a backup), startup sets the identity from whichever list arrives last, and the later event corrects it. Informers are not started until coreIdentitiesReady succeeds, so no request is sent with a stale identity at startup. After startup the two sources only agree or advance, so I don't see a bad interleaving. Mentioning it because the dual registration is subtle and the comment doesn't say so.

Checked

• Read beyond the diff: pkg/server/bootstrap/identity.go in full (identities.set, expireWatches, injectKcpIdentities, decorateWildcardPathsWithResourceIdentities), the whole kcp-start-informers hook in pkg/server/server.go, cmd/virtual-workspaces/command/cmd.go Run, pkg/proxy identity wiring, the replication controller registration for APIExports, and pkg/server/options / pkg/proxy/options.
• Correctness: the informers are started with go ...Informer().Run before the handlers are added and the poll runs, and AddEventHandler on a running informer replays the existing items, so no event is missed. The scoped-informer fallback to the root cluster is kept. The new poll exits on context cancel, same as before. No ignored errors.
• Concurrency: the identities locking is unchanged. Two informers calling set concurrently is safe, and the set is idempotent.
• kcp specifics: cluster scoping is checked via IsCoreExport(root, name) on both object types. Shards, the cache server and the VW wiring were traced. I found no authorizer or workspace-isolation change.
• API/storage: no API types change. The only compatibility surface is the flag, which is handled (see above). No codegen impact.
• Security: no new request parsing, and the root-shard credentials are no longer loaded by shards.
• Tests: pkg/server/bootstrap/identity_test.go is reworked to cover the combined APIExport/APIIdentity handler. The removed identitycache tests go with the removed controller. Coverage of the new shard startup path comes from the sharded e2e and migration e2e.
• Commands: go build ./... ok; go vet ./pkg/server/... ./cmd/virtual-workspaces/... ./cmd/sharded-test-server/... clean; go test ./pkg/server/bootstrap/... ./pkg/server/options/... pass. CI on this head: lint, verify, codegen, unit, integration, upgrade, sharded, sharded-embedded-vw and sharded-controller-runtime pass. The pull-kcp-test-e2e, -multiple-runs and -serial jobs were still pending when I looked (the earlier failure was on the previous commit 9b2c13d). I did not run the e2e locally: the sharded suites take longer than 10 minutes.

[mjudeikis-bot]

Note: doc contradicts the code

This says a starting shard reads the identities from the APIIdentity registry. Shards and virtual workspaces don't read that view. They feed coreIdentityHandler from the replicated APIExport objects' status.identityHash (pkg/server/server.go, kcp-start-informers hook; cmd/virtual-workspaces/command/cmd.go). Only the front-proxy reads APIIdentity. An operator debugging a stuck shard would look in the wrong place.

Suggested wording:

Their identities are read from the `APIExport` objects replicated to the cache
server (`status.identityHash`), which is where a starting shard gets them. The
root shard is not contacted. The front-proxy reads the same identities through
the `APIIdentity` view of the Admin workspace (see
[the Admin workspace](#the-admin-workspace-admin)). Once they are known the
informers can be started.

[ntnn]

/lgtm
/approve

[kcp-ci-bot]

LGTM label has been added.

Details

Git tree hash: 1225401ce279aacfaa8c42e7bd408228128f8f89

[kcp-ci-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ntnn

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 [ntnn]

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