Skip to content

k8s: minikube-like runtime parity across EKS/GKE/AKS + real kubectl support - #299

Merged
NitinKumar004 merged 21 commits into
developmentfrom
feat/k8s-runtime-parity
Jul 30, 2026
Merged

k8s: minikube-like runtime parity across EKS/GKE/AKS + real kubectl support#299
NitinKumar004 merged 21 commits into
developmentfrom
feat/k8s-runtime-parity

Conversation

@NitinKumar004

Copy link
Copy Markdown
Collaborator

Summary

Turns the cloudemu Kubernetes data plane into a minikube-like, always-converged cluster with full parity across EKS/GKE/AKS, and makes real kubectl work end-to-end (not just client-go). One PR, creation → running → scale → rolling update → teardown, across every common resource.

The data plane is a single shared in-memory API server registered by every provider's cluster; a synchronous reconcile engine (no goroutines — deterministic) drives objects to their healthy terminal state on every write.

What's included

Connect parity (real TLS). All three control planes advertise a shared CA (internal/k8spki) that certifies the serving cert, so client-go/kubectl validate the connection normally. Removed insecure-skip-tls-verify from rendered kubeconfigs.

Reconcile engine. Deployment / ReplicaSet / StatefulSet / DaemonSet materialize Running Pods (owned via ownerReferences); Services get populated Endpoints; PVCs bind; Jobs complete. Rolling updates replace Pods on a pod-template change (pod-template-hash). Cascade delete + endpoint draining via ownerReferences GC. /scale and /status subresources; label/field selectors.

Full resource surface, registry-driven so discovery can never advertise a kind that 404s: core (incl. PV/Node/Event/ResourceQuota/LimitRange), apps, batch, networking.k8s.io, rbac.authorization.k8s.io, storage.k8s.io, autoscaling/v2, discovery.k8s.io, policy.

Real kubectl parity. Decodes the protobuf request bodies kubectl sends on writes (kubectl does not retry as JSON on 415); serves OpenAPI v3 (per-group docs carrying each GVK) + a protobuf v2 fallback so kubectl apply validation passes; supports strategic-merge and JSONPatch in addition to merge-patch; advertises kubectl short names (pvc, hpa, sts, …).

Testing

  • client-go E2E through each provider's connect path (EKS deep matrix; GKE/AKS full-stack).
  • Real kubectl v1.36 against a standalone cloudemu serve: a 63-check lifecycle (apply → scale → rolling update → StatefulSet+PVCs → DaemonSet → Job → CronJob → Ingress → HPA → PV/PVC/StorageClass → RBAC → NetworkPolicy → Node → all three patch types → cascade teardown to zero) — all green across EKS, GKE, AKS.
  • Full repo suite green with -race; deterministic across repeated runs.

Review notes

An adversarial self-review (reconcile / registry / PKI) ran during development; the confirmed findings are fixed here, most notably a kubectl scale merge-patch that silently scaled workloads to zero (float64 vs int64 decode) — now fixed at the root with a regression test.

Not in scope (deliberate emulation boundaries)

No kubelet-backed Pod subresources (/log, /exec, /attach, /portforward); no real scheduling (single synthetic node); no quota/RBAC/NetworkPolicy/PDB enforcement; HPA/CronJob don't actuate; rollouts converge instantly (no surge pacing / revision history); server-side-apply is treated as a merge; lists are unpaginated.

The data-plane serving cert and every provider's advertised CA must be the
same authority or client-go's TLS handshake fails. Extract the CA into a new
internal/k8spki package used by both the serving TLS config and all three
control planes:

- EKS: tls.go now delegates to k8spki (serve + EKS call sites unchanged).
- GKE: advertise the real CA in masterAuth.clusterCaCertificate; drop the
  unparseable dummy blob that broke the handshake outright.
- AKS: embed the real CA in the rendered kubeconfig and drop
  insecure-skip-tls-verify — parity with EKS/GKE.

Tests: AKS data-plane test now serves with the k8spki cert and validates
end-to-end (no skip-verify); new GKE real-TLS connect-parity test proves the
advertised CA certifies the endpoint (create cluster -> validate CA ->
client-go ConfigMap round-trip). RenderKubeconfig test updated to the real-CA
behavior.

First phase of the k8s runtime/parity work; registry refactor + reconcile
engine + workload kinds follow on this branch.
…ion)

Add Route.Subresource and parse the /{name}/{subresource} tail for both
cluster-scoped (/api/v1/nodes/n/status) and namespaced
(/apis/apps/v1/namespaces/ns/deployments/d/scale) shapes. ServeHTTP routes
subresource requests to a dedicated dispatcher (stubbed to 404 until the
reconcile phase wires /status and /scale) so a subresource path is never
mis-parsed as a write against the parent object. Updated the parseRoute unit
test to the new (correct) cluster-subresource semantics.
Turn the k8s data plane from a CRUD store into a minikube-like runtime.

Registry (registry.go, registry_ops.go, registry_defs.go): a generic
unstructured-backed store + one handler serving CRUD, list (label & field
selectors), watch, patch, delete (ownerReference garbage collection), and the
/status + /scale subresources for any registered kind. New kinds are a
registration + optional reconcile hook. Registers apps/v1 ReplicaSet,
StatefulSet, DaemonSet and core/v1 PersistentVolumeClaim; discovery is derived
from the registry so it can't drift.

Reconcile engine (reconcile.go), run synchronously on every write (no
controller goroutines, so it stays deterministic):
- Pods are driven Running with a synthetic Pod IP and ready containers.
- Deployment materializes its Pods and reports real status; ReplicaSet and
  DaemonSet do likewise; StatefulSet creates stable-ordinal Pods (name-0..N-1)
  plus a Bound PVC per volumeClaimTemplate.
- The endpoints controller fills a Service's Endpoints from the Running Pods
  matching its selector, and drains them when Pods are deleted/GC'd.
- Deleting a controller cascades to its Pods; scaling (spec.replicas or the
  /scale subresource) adjusts the Pod count.

Typed handlers: Deployment now reconciles + serves /scale and /status; direct
Pod creates come up Running; Pod list honors label/field selectors; Service
create populates endpoints.

Tests: new client-go WorkloadRuntime E2E (Deployment+Service -> Running pods +
endpoints -> scale to 4 -> StatefulSet with 3 stable pods + 3 Bound PVCs ->
DaemonSet -> cascade teardown). Existing pod/cascade/provider tests updated to
the new Running/materialized behavior. Deployment /scale + /status advertised
in discovery.

Deferred to later phases: the intermediate ReplicaSet object for Deployments
(pods are owned by the Deployment directly); Job/CronJob, Ingress, RBAC, HPA,
Node, Event, NetworkPolicy, EndpointSlice; strategic-merge / server-side-apply.
…ore supporting kinds; registry-driven discovery

Adds registry entries for Job/CronJob, Ingress/IngressClass/NetworkPolicy,
RBAC (Role/RoleBinding/ClusterRole/ClusterRoleBinding), StorageClass,
HorizontalPodAutoscaler, EndpointSlice, and core PVC/PV/Node/Event/
ResourceQuota/LimitRange. Reconcile hooks drive Job pods to Succeeded,
Ingress to a load-balancer IP, and PV to Available/Bound.

serveDiscovery now derives the /apis group list and every
/apis/<group>/<version> resource list from registeredResources() (seeded
with the typed apps/policy groups) instead of a hardcoded switch, so new
groups and their subresources are discoverable by kubectl and client-go
without drifting from what the server serves.
Drives Jobs (complete to Succeeded with materialized pods), Ingresses
(get a load-balancer IP), PVCs (bind), StorageClass/RBAC/HPA/Node
round-trips, and asserts the new API groups are discoverable — the
negotiation kubectl and client-go do before any typed request.
…ndpoints

- Controllers stamp a pod-template-hash label on Pods; reconcile treats a
  changed template hash as a rolling update and replaces stale-hash Pods
  (Deployment/ReplicaSet via syncScaledPods, StatefulSet via syncStablePods).
  Convergence is instant — no surge/unavailable pacing.
- buildControllerPod now copies the template label map before stamping, so it
  can't mutate the controller's shared template.
- Discovery advertises core/v1 endpoints (get/list/watch only, matching the
  read-only handler) so kubectl/client-go can resolve them.

E2E: runtime test now exercises a rolling update (image change replaces all
Pods, endpoints re-point); supporting-kinds test asserts endpoints discovery.
Update services.md §18, sdk-server.md, and the package doc to reflect the
reconcile engine (Running Pods, Endpoints, binding PVCs, completing Jobs),
validated TLS via the shared CA, the full multi-group resource surface with
/scale and /status subresources, rolling updates, and the deliberate emulation
boundaries (no exec/logs/portforward, no scheduling, no quota/RBAC/policy
enforcement, no HPA/CronJob actuation).
applyUnstructuredPatch decoded the merged JSON with plain json.Unmarshal
into map[string]any, which turns whole-number JSON into float64.
unstructured.NestedInt64 accepts only int64, so spec.replicas read back as
0 — a 'kubectl scale --replicas=N' (a merge-patch to the /scale subresource,
or any merge-patch touching replicas) silently scaled the workload to zero.

Decode via unstructured.Unstructured.UnmarshalJSON instead, which preserves
integers as int64. This fixes every merge-patch path (object and /scale) at
the root. Regression test drives a merge-patch scale-up and asserts both the
returned Scale and the stored object carry replicas=4.
- Endpoints: only bump ResourceVersion / publish MODIFIED when the address set
  actually changes. resyncEndpointsForNamespaceLocked runs for every Service on
  any Pod change, so an unchanged Service was emitting a spurious watch event
  (with a climbing RV) on unrelated Pod churn. Regression test added.
- Pod field selector: support spec.nodeName. Every materialized Pod is scheduled
  to the synthetic node, so 'kubectl get pods --field-selector spec.nodeName=...'
  (node-drain/kubelet tooling) previously returned an empty list. E2E covers it.
- Garbage collection: walk the owned set breadth-first and collect UIDs before
  deleting, instead of mutating each store's map while ranging it and recursing.
  Pods owned by an intermediate controller (not just the root) are now reaped.
- Scale subresource: only bump generation when spec.replicas actually changes,
  matching registryUpdate/registryPatch (no spurious generation != observed).
- Job: ignore a non-positive spec.completions (default to 1) so a Job can't
  report Complete having run zero Pods.
- StatefulSet PVCs: deep-copy the volumeClaimTemplate spec per ordinal instead
  of aliasing one map across every PVC.
- PKI: give each serving leaf a random 128-bit serial (was fixed '2') and assert
  BasicConstraintsValid (cA=FALSE) so strict non-Go verifiers accept the leaf.
- docs: correct the data-plane list to say it is unpaginated (limit/continue are
  not honored) and scope field-selector support accurately.
…egic & JSON patch

Driving a running cloudemu server with real kubectl (cluster created via the
EKS/GKE/AKS SDK, then kubectl against the advertised endpoint) surfaced gaps
the JSON-forcing client-go tests masked. kubectl now works end-to-end.

- Protobuf request bodies: kubectl sends built-in kinds as protobuf on writes
  and does NOT retry as JSON on 415, so every 'kubectl create/apply/scale'
  write failed. Decode protobuf via the client-go scheme's recognizing
  deserializer (typed handlers decode in place; registry handlers convert to
  unstructured). Responses stay JSON — clients' Accept allows it.
- OpenAPI: serve a v3 discovery root + per-group docs that carry each served
  GVK (with a permissive schema) so kubectl resolves the kind and stays on the
  JSON v3 path; and serve the legacy v2 doc as protobuf bytes (mime-safe
  application/octet-stream content type) for the fallback. Without this
  'kubectl apply' died at 'failed to download openapi'. Served
  cluster-independently in APIServer.ServeHTTP so the prefix-less v3
  serverRelativeURL follow-ups resolve.
- Patch types: typed handlers now accept strategic-merge-patch (kubectl's
  default for set/edit/label — real strategic merge, so the container list
  merges by name) and JSONPatch (RFC 6902), in addition to merge-patch;
  registry handlers gain JSONPatch too.
- Discovery: advertise kubectl short names (pvc, hpa, sts, ds, rs, ing, sc, …)
  for registry kinds so 'kubectl get pvc' resolves.

Verified with real kubectl v1.36 against a standalone server: full lifecycle
(apply → scale → rolling update → statefulset/PVCs → daemonset → job → cronjob
→ ingress → hpa → pv/pvc/storageclass → rbac → networkpolicy → node → all three
patch types → cascade teardown) across EKS, GKE, and AKS connect paths.
Comment thread services/kubernetes/reconcile.go Fixed

@NitinKumar004 NitinKumar004 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Deep review — k8s runtime parity

Reviewed end-to-end against the cloudemu architecture: data-plane design, reconcile engine, discovery/registry, patch/protobuf/OpenAPI parity, watch semantics, PKI, and the EKS/GKE/AKS connect paths. Ran build / vet / test ./services/kubernetes/... + eks/gke/aks — all green, including -race -count=2.

Architecture fit: strong. The registry-driven design (discovery derived from registeredResources() so it can't advertise a kind that 404s), the single-mutex ClusterState with reconcile hooks running under the same lock as the typed maps, and the subscribe-before-snapshot watch ordering are all sound and idiomatic for this codebase. The shared-CA k8spki approach is the right call and is wired symmetrically across all three providers. The float64→int64 scale fix + regression test is a genuinely good catch.

Below are the things worth addressing, roughly by severity.


Blockers

1. golangci-lint run is not clean on non-test files (documented gate).
CLAUDE.md requires 0 lint issues; the repo .golangci.yml enables these. Under golangci-lint v2.11.4 the new files report (excluding likely version-drift wsl):

  • gocyclo > 10: state.go:131 ServeHTTP (15), registry.go:87 serveRegistry (14), reconcile.go:237 reconcileServiceEndpointsLocked (11)
  • gochecknoglobals: discovery.go:251 registryShortNames, wire.go:89 protobufMagic — need a //nolint:gochecknoglobals // reason or a refactor into a func
  • nolintlint: pod.go:18 unused //nolint:dupl; reconcile.go:337 //nolint:gosec missing an explanation
  • lll > 140: 11 lines, mostly the one-liner resourceDef entries in registry_defs.go (up to 193 chars)
  • plus goconst/gocritic/prealloc

govet also flags a shadow at registry.go:394 (merged shadows line 391) — harmless because that branch returns immediately, but it should be cleaned to satisfy the shadow gate. Some wsl findings may be linter-version drift (a stable package showed 2), so please confirm CI lint is green and, if so, note the golangci-lint version.

2. Watch streams ignore labelSelector / fieldSelector.
This is the headline use case of the feature (client-go Reflector list+watch), so it matters most here.

  • Typed watch applies no selector at all — neither initial nor stream: pod.go:76 watchPods uses collectPodsLocked (unfiltered), whereas listPods (pod.go:143) wraps in filterPods. Same for watchDeployments (deployment.go:76).
  • Registry watch filters only the initial snapshot (registry.go:154 snapshotLocked), but streamed events from the broadcaster (watch.go publish/streamWatch) are not filtered.

Effect: kubectl get pods -l app=x -w, or any informer/controller-runtime cache built with a label/field selector, receives non-matching objects → polluted caches and spurious reconciles. The new reconcile engine amplifies this because creating one Deployment now emits many Pod events. Fix: apply the request's selector inside streamWatch (pass it through) and use filterPods/snapshotLocked for the typed watch initial snapshot too.


Correctness / robustness gaps

3. Unbounded replicas / completions materialize Pods synchronously under the global lock.
reconcileStatefulSet does make([]string, desired) (reconcile.go:364); syncScaledPods and reconcileJob (reconcile.go:428) create desired/completions Pods one-by-one while holding s.mu.Lock. A single manifest with a large value (a copied prod manifest, a typo, a fuzz test) will allocate/hang the entire cluster's API and can OOM — a real apiserver just stores the integer. reconcileJob is additionally O(n²) (re-scans all Pods each iteration). Suggest clamping to a sane cap (a few hundred) with a log()ged note, since this is a test emulator.

4. EndpointSlice is advertised with full CRUD but never populated.
registry_defs.go:73 registers discovery.k8s.io/v1 EndpointSlice with rwVerbs, but no reconcile hook mirrors Service endpoints into slices — only the typed Endpoints object is filled (reconcileServiceEndpointsLocked). So a selector Service has populated Endpoints but empty EndpointSlices. Modern consumers (kube-proxy EndpointSlice mode, Gateway API, controller-runtime) read slices and would see no backends. This isn't in the "Not in scope" list — either mirror a slice in the endpoints reconcile or add it to the documented boundaries.

5. No Node object for cloudemu-node-0.
Every reconciler-materialized Pod sets spec.nodeName=cloudemu-node-0 and endpoints reference it (reconcile.go:28, :256), but newClusterState (state.go:91) bootstraps only namespaces + default ServiceAccounts — no Node. So kubectl get nodes is empty on a fresh cluster and pods/DaemonSets reference a node object that doesn't exist. Bootstrapping the synthetic Node alongside the namespaces would close the gap and make DaemonSet scheduling consistent.

6. Watch load-shedding silently drops events with no relist signal.
broadcaster.publish (watch.go:101) drops on a full 64-buffer channel. A real apiserver disconnects the slow watcher, forcing a relist; here the client silently misses events and never re-syncs → permanently stale cache. Low probability in deterministic tests, but it's a correctness cliff under load and worth a comment/doc at minimum.


Minor / consistency / follow-ups

  • Disconnected watchers linger until the next publish on that broadcaster (pruned only in the publish pass, watch.go:82). On a broadcaster with no further writes, closed watches accumulate — a small leak.
  • Typed Deployment never bumps .metadata.generation on spec change (stamp() doesn't set it; updateDeployment/patchDeployment don't increment it), whereas the registry path does (registry.go:287). observedGeneration is force-set equal each reconcile so rollout status still passes, but generation semantics differ between Deployment and RS/STS.
  • deploymentScale PATCH uses flat RFC-7396 mergePatch regardless of content-type (subresource.go:151), while registry /scale handles merge/strategic/JSONPatch. kubectl scale uses merge-patch so it works in practice, but the two scale paths are inconsistent.
  • Unknown field selectors match nothing (registry.go:501). Events are registry-backed and commonly filtered by involvedObject.name/reason (e.g. kubectl describe), which would then return no events. Consider honoring the common Event field selectors or noting the limitation.
  • parseRoute mis-parses /api/v1/namespaces/{name}/status as a namespaced collection (Namespace={name}, Resource="status") — route.go:96. Rare (kubectl doesn't hit namespace /status//finalize), but worth a guard.
  • time.Now() used throughout the reconciler/registry instead of the project's config.FakeClock determinism convention. Timestamps aren't asserted so tests stay deterministic, but it deviates from the documented pattern.
  • Provider fallback asymmetry (misconfig window only): when the APIServer is wired but BaseURL()=="", EKS (eks.go:301) and GKE (gke.go:160) advertise the real CA against the *-NOT-IMPLEMENTED sentinel endpoint (CA/endpoint mismatch), while AKS (aks.go:725) omits the CA entirely. Normal startup calls SetBaseURL before any CreateCluster, so this is a window, not a hot-path break — but the three should agree (suppress the CA when pointing at the sentinel, or keep it everywhere).
  • Comment cleanup: AKS Kubeconfig doc says "Phase 3 onward" (aks.go:711) where the rest of the tree says "Wave 2"; and the three provider package headers still describe the data plane as unimplemented/future even though it's now wired.

Nice work

Discovery-can't-lie invariant, the protobuf write-path decode (kubectl doesn't retry as JSON on 415), OpenAPI v3-per-group + v2 protobuf fallback, the subscribe-before-snapshot race handling, the shared-CA design, and the honest non-goals section are all well done. The bulk of this is solid; the watch-selector and lint items are the two I'd want fixed before merge.

@NitinKumar004 NitinKumar004 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Inline comments for the findings from my summary review above.

A few items can't anchor to this PR's diff, so noting them here:

  • Watch load-shedding (watch.go:101) drops events on a full 64-buffer channel with no relist signal — unlike a real apiserver (which disconnects → client relists), a client here silently misses events and never re-syncs. Also, disconnected watchers are pruned only on the next publish, a small leak on write-idle broadcasters.
  • Typed Deployment never bumps .metadata.generation on spec change (stamp() doesn't set it; update/patch don't increment), whereas the registry path does. observedGeneration is force-set equal each reconcile so rollout status passes, but semantics differ from RS/STS.
  • parseRoute mis-parses /api/v1/namespaces/{name}/status (route.go:96) as a namespaced collection. Rare, but worth a guard.
  • Provider fallback asymmetry: when BaseURL()=="", EKS/GKE advertise the real CA against the *-NOT-IMPLEMENTED sentinel endpoint, while AKS omits the CA. Normal startup calls SetBaseURL first, so it's a misconfig window, not a hot path — but the three should agree. Also aks.go:711 says "Phase 3" where the rest of the tree says "Wave 2", and the provider package headers still describe the data plane as unimplemented.

func objKey(namespace, name string) string { return namespace + "/" + name }

// serveRegistry is the generic handler entry point for a registry-backed kind.
func (s *ClusterState) serveRegistry(w http.ResponseWriter, r *http.Request, route *Route, st *registryStore) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Lint gate (blocker). golangci-lint run (repo .golangci.yml, v2.11.4) is not clean on the new non-test files — CLAUDE.md requires 0 issues. Substantive ones:

  • gocyclo>10: serveRegistry here (14), ServeHTTP (state.go:131, 15), reconcileServiceEndpointsLocked (reconcile.go:237, 11)
  • gochecknoglobals: registryShortNames (discovery.go:251), protobufMagic (wire.go:89) — add //nolint:gochecknoglobals // reason or refactor
  • nolintlint: unused //nolint:dupl (pod.go:18); //nolint:gosec missing explanation (reconcile.go:337)
  • lll>140: 11 lines, mostly the resourceDef one-liners in registry_defs.go
  • govet shadow: merged at registry.go:394 (harmless — returns immediately — but flagged)

Some wsl may be linter-version drift (a stable pkg showed 2). Please confirm CI lint is green and note the golangci-lint version.

s.mu.RLock()
defer s.mu.RUnlock()

items := st.snapshotLocked(namespace, r)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Watch ignores label/field selectors (blocker). snapshotLocked filters the initial watch snapshot, but streamed events from the broadcaster (watch.go publish/streamWatch) are not filtered. The typed watch is worse — watchPods uses collectPodsLocked (unfiltered) for both initial and stream, unlike listPods which uses filterPods; same for watchDeployments.

Effect: kubectl get pods -l app=x -w, and any informer/controller-runtime cache built with a selector, receives non-matching objects → polluted caches and spurious reconciles. The reconcile engine amplifies this (one Deployment emits many Pod events). Fix: thread the selector into streamWatch and filter the typed watch initial snapshot too.

func reconcileStatefulSet(s *ClusterState, obj *unstructured.Unstructured) {
desired := replicasOf(obj)

names := make([]string, desired)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Unbounded replicas/completions under the global lock. make([]string, desired) here (and the top-up loops in syncScaledPods / reconcileJob at line 428) materialize desired/completions Pods synchronously while holding s.mu.Lock. A single manifest with a large value (copied prod manifest, typo, fuzz) allocates/hangs the whole cluster API and can OOM — a real apiserver just stores the int. reconcileJob is additionally O(n²) (re-scans all Pods each iteration). Suggest clamping to a sane cap with a logged note, since this is a test emulator.

Comment thread services/kubernetes/registry_defs.go Outdated

func discoveryRegistryDefs() []*resourceDef {
return []*resourceDef{
{group: "discovery.k8s.io", version: "v1", kind: "EndpointSlice", listKind: "EndpointSliceList", plural: "endpointslices", namespaced: true},

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

EndpointSlice advertised but never populated. Registered with full rwVerbs, but no reconcile hook mirrors Service endpoints into slices — only the typed Endpoints object is filled (reconcileServiceEndpointsLocked). So a selector Service has populated Endpoints but empty EndpointSlices; modern consumers (kube-proxy EndpointSlice mode, Gateway API, controller-runtime) would see no backends. Not in the documented non-goals — either mirror a slice in the endpoints reconcile or add it to the boundaries list.

Comment thread services/kubernetes/registry_defs.go Outdated
return []*resourceDef{
{group: "", version: "v1", kind: "PersistentVolumeClaim", listKind: "PersistentVolumeClaimList", plural: "persistentvolumeclaims", namespaced: true, hasStatus: true, reconcile: reconcilePVC},
{group: "", version: "v1", kind: "PersistentVolume", listKind: "PersistentVolumeList", plural: "persistentvolumes", namespaced: false, hasStatus: true, reconcile: reconcilePV},
{group: "", version: "v1", kind: "Node", listKind: "NodeList", plural: "nodes", namespaced: false, hasStatus: true},

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No Node object for cloudemu-node-0. nodes is registered (empty store), but every reconciler Pod sets spec.nodeName=cloudemu-node-0 and endpoints reference it, while newClusterState (state.go:91) bootstraps only namespaces + default SAs. So kubectl get nodes is empty on a fresh cluster and Pods/DaemonSets reference a node object that doesn't exist. Consider bootstrapping the synthetic Node alongside the namespaces.

return false
}
default:
// Unknown field selector: match nothing rather than silently

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Unknown field selectors match nothing. Reasonable default, but Event is registry-backed and commonly filtered by involvedObject.name/reason (e.g. kubectl describe), which would then return no events. Consider honoring the common Event field selectors, or documenting the limitation.

Comment thread services/kubernetes/subresource.go Outdated
return 0, true
}

merged, err := mergePatch(cur, body)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Scale PATCH content-type inconsistency (minor). deploymentScale uses flat RFC-7396 mergePatch regardless of Content-Type, whereas registry /scale (registry_ops.go) handles merge/strategic/JSONPatch. kubectl scale uses merge-patch so it works in practice, but the two scale paths diverge for strategic/JSON patches.

return
}

now := metav1.NewTime(time.Now())

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

time.Now() vs FakeClock (minor). The reconciler/registry use time.Now() directly rather than the project's config.FakeClock determinism convention (CLAUDE.md). Timestamps aren't asserted so tests stay deterministic, but it deviates from the documented pattern.

Watch streams ignored labelSelector/fieldSelector: typed watches (watchPods,
watchDeployments, …) filtered neither initial nor streamed events, and the
registry watch filtered only the initial snapshot. A selective watch
('kubectl get pods -l app=x -w', or any informer/controller-runtime cache
built with a selector) therefore received non-matching objects — polluting
reflector caches and firing spurious reconciles, which the reconcile engine
amplifies (one Deployment emits many Pod events).

streamWatch now takes a keep(T) predicate applied to both the initial snapshot
and every streamed event; each watch handler builds it from the request's
selectors (parseListSelectors + metaFieldsMatch/podMatchesFields/matchesFields).
Also extracts field-selector-name constants (fixes goconst) and indexes
filterPods to avoid per-item Pod copies.
…sync endpoints on pod update/patch

Correctness gaps from the PR review:
- Unbounded replicas/completions: the reconciler runs synchronously under the
  cluster lock, so a huge spec value would allocate/hang the whole API. Clamp
  the materialized Pod count to maxReconciledPods (500) for Deployment,
  ReplicaSet/StatefulSet (via replicasOf), and Job. reconcileJob's top-up is
  also made O(n) instead of O(n²).
- Synthetic Node: bootstrap cloudemu-node-0 (Ready, InternalIP) in
  newClusterState so 'kubectl get nodes' is non-empty and the node every Pod is
  scheduled onto actually exists.
- EndpointSlices: mirror each Service's endpoints into a discovery.k8s.io
  EndpointSlice (labelled kubernetes.io/service-name) so EndpointSlice-mode
  consumers (kube-proxy, Gateway API) see the same backends as Endpoints.
- Pod update/patch now resync endpoints (a label change matching a Service
  selector was invisible until unrelated churn) and re-drive a spec-only PUT
  back to Running so it isn't dropped out of the endpoint set.

reconcileServiceEndpointsLocked is split into matchingEndpointAddressesLocked /
writeEndpointsLocked / syncEndpointSliceLocked (also lowers its complexity).
E2E asserts the synthetic Node and populated EndpointSlices; a new watch test
asserts label-selector stream filtering.
…cker)

- All 8 typed watch handlers now share a generic serveWatch[T] helper (removes
  the dupl the near-identical subscribe/snapshot/stream blocks triggered, and
  addresses the reuse the review flagged).
- golangci-lint (repo .golangci.yml, v2.11.4) is clean on the new non-test
  files: extracted goconst constants (api/apis path segments, status/scale
  subresources, group names), named crypto/mnd magic numbers in k8spki, gave
  parseListSelectors named results, indexed the container-status range,
  lowered ServeHTTP/serveRegistry complexity via dispatchResource /
  serveRegistryItem, fixed the govet shadow, and added reasoned //nolint for
  the legitimately-global lookup tables and hugeParam k8s structs.

No behavior change; build, vet, tests, and -race remain green.
- Typed Deployment now sets metadata.generation=1 on create and advances it
  only on a spec change (update/patch), matching apiserver semantics and the
  registry path so observedGeneration comparisons are meaningful.
- deploymentScale PATCH routes through the shared applyPatchBytes dispatcher,
  so the typed /scale honors merge / strategic-merge / JSONPatch like the
  registry /scale (the two paths no longer diverge).
- Tidy a stale AKS kubeconfig comment (Phase 3 -> the normal path).

@thzgajendra thzgajendra left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review — k8s minikube-like runtime parity (EKS/GKE/AKS + real kubectl)

Deep 8-lens pass (reconcile, registry/discovery, TLS/PKI, protobuf/OpenAPI/patch, watch/selectors, concurrency/GC, tests, conventions/docs), verified against source and by running the suite. This is a large, well-engineered PR, and the adversarial self-review's fixes genuinely landed — I verified each rather than taking the description's word. Confirmed sound:

  • Reconcile engine — Pods converge to Running with ownerRefs; rolling updates copy the template label map before stamping (no shared-map aliasing); StatefulSet PVC specs are deep-copied per ordinal; scale up and down work; Job completions<=0 defaults to 1; deterministic (no goroutines).
  • The pod-cap fix resolves the CodeQL allocation-overflow — every user-sized allocation (reconcileStatefulSet make([]string, desired), syncScaledPods, reconcileJob) is clamped to 500 before allocating; no bypass.
  • The float64→int64 scale-to-zero fix is complete — traced every merge-patch integer path (registry object patch, registry /scale, typed /scale, typed object/status); all now decode via Unstructured.UnmarshalJSON or into typed structs. No remaining path can silently scale to zero. Typed and registry /scale no longer diverge.
  • Watch selector filtering (the self-review blocker) is genuinely fixed — the keep predicate is applied to both the initial snapshot and every streamed event, for typed (serveWatch[T]) and registry watches, honoring label and field selectors. No unfiltered watch path remains.
  • Discovery-can't-lie invariant holds — groups/resources/OpenAPI GVKs all derive from registeredResources(); every advertised kind is served and vice versa. EndpointSlice mirror and synthetic Node bootstrap both landed correctly.
  • TLS/PKI — one shared CA certifies the serving leaf; SANs match the dialed host; insecure-skip-tls-verify removed; leaf has a random serial, cA=FALSE, serverAuth — validates for strict verifiers.
  • Protobuf writes / OpenAPI v2+v3 / strategic-merge (real, merges container lists by name) / JSONPatch all correct.
  • Concurrency — no deadlock (the *Locked discipline is honored; s.mu → b.mu ordering is clean), GC reaps intermediate-owned Pods and is cycle-safe, decode failures surface as 400 (no silent wrong-state-200). go test ./services/kubernetes/... + eks/gke/aks and -race -count=2 pass, deterministic.

Findings (none block correctness; #1#3 worth addressing before merge)

1. [Medium] golangci-lint is not actually zero on the new files. I ran it (v2.4.0) on ./services/kubernetes/... ./internal/k8spki/...5 issues: two unused //nolint:prealloc directives at discovery.go:202,286 (nolintlint — self-inflicted: the decomposition moved the code and left the directives dangling), plus three prealloc suggestions (discovery.go:225, openapi.go:188, reconcile.go:279). This contradicts the "resolved to zero" claim and the CLAUDE.md 0-issue gate. Caveat: you cited v2.11.4 and I have v2.4.0 — prealloc placement is version-sensitive, so the three suggestions may differ on CI. But the two unused-nolint findings are version-robust (dangling directives) and trivially removable. Please confirm against CI's pinned version and drop the two stale directives regardless.

2. [Medium] docs/sdk-server.md overstates k8s pagination (advertised-but-not-simulated). Line 322 lists "pagination" among the k8s data-plane capabilities and line 359 says pagination tokens are honored — but docs/services.md:1303 correctly states k8s lists are unpaginated (limit/continue ignored, full set every time), and the code doesn't paginate. Remove "pagination" from the k8s capability sentence so the two docs and the code agree — it's exactly the kind of claim a caller would trust.

3. [Medium — documented cliff] Watch load-shedding silently drops events with no relist. broadcaster.publish drops on a full 64-buffer channel with no 410 Gone/relist signal — a slow watcher (e.g. an informer during a 500-pod reconcile burst) silently misses events and its cache diverges permanently, where a real apiserver forces a relist. It's in the comments and in your own self-review (#6), not fixed. Acceptable at emulator scale, but it's the one genuine correctness cliff — worth either signalling a relist (close the channel / 410) or promoting it to an explicit documented non-goal + tracking issue. (The disconnected-watcher prune-only-on-publish leak is a related minor.)

4. [Low] GC comment is factually wrong. registry_ops.go garbageCollectLocked says "we never mutate a store's map while ranging it," but the code does delete(st.items, key) while ranging st.items. It's safe (deleting the current key during a Go range is legal) and the real improvement (BFS instead of recursion + map-order independence) is genuine — but correct the comment so a future reader doesn't trust a false invariant.

5. [Low] Pod-cap clamp is silent. Clamping replicas/completions 5000→500 emits no log/status/annotation; the object keeps spec.replicas: 5000 while only 500 Pods and status.replicas: 500 appear, with no signal. Consistent with the package having no logger, but a status condition/annotation noting the clamp would avoid confusion.

6. [Low] reconcileJob doesn't reconcile a shrunk completions / changed template. It only tops up (have < completions); reducing completions or changing the pod template leaves stale Pods and can inflate status.succeeded. Jobs are effectively immutable in real k8s, so low blast radius — a guard or note would close it.

7. [Low] Automated coverage gap on the headline "real kubectl" paths. The 63-check kubectl v1.36 lifecycle is a manual/out-of-band run, not in the repo; the automated client-go E2E forces application/json, so the protobuf-read / OpenAPI-negotiated / kubectl-specific surface has no automated regression guard. The write-side fixes (scale-to-zero, watch selector, endpoints resync, patch types, protobuf-write) do have genuine regression tests. Consider committing the lifecycle script or a lightweight kubectl smoke. (TypedConfigMap protobuf test only asserts 200 — shallow; pod-cap and load-shedding have no test.)

8. [Low] Provider CA fallback asymmetry + stale comments. When BaseURL()=="" (misconfig window before SetBaseURL), EKS/GKE advertise the real CA against the *-NOT-IMPLEMENTED sentinel while AKS omits the CA — make the three consistent. Plus aks.go says "Phase 3" where the tree says "Wave 2", and the provider package headers still describe the data plane as unimplemented.

Bottom line: strong, correct, and tested — the reconcile/registry/TLS/patch/watch mechanisms all hold up under scrutiny and the self-review fixes are verified. I'd resolve #1 (lint, at least the two stale nolints) and #2 (docs pagination) before merge, and either close or explicitly track #3 (watch load-shedding). The rest are minor/follow-ups. Deferring the merge decision to you.

…ocs)

From thzgajendra's review of PR #299:
- Lint (#1): replace the version-sensitive //nolint:prealloc directives with
  real preallocation (coreResources/appsResources/registryAPIResources,
  openapi kinds, endpoint-address slice), so golangci-lint is clean regardless
  of prealloc's version-dependent placement — no dangling directives.
- Docs (#2): drop the 'pagination' claim from the k8s data-plane sentence in
  sdk-server.md; data-plane lists are unpaginated, matching services.md.
- Watch load-shedding (#3): a slow watcher that overflows its buffer now
  receives a 410 Gone (ERROR) event and the stream ends, so client-go relists
  instead of running with a permanently-divergent cache. Regression test added.
- GC comment (#4): correct the 'never mutate while ranging' wording — deleting
  the current key mid-range is legal; the BFS is what makes the cascade
  order-independent.
- Provider package headers (#8): describe the now-wired data plane instead of
  'out of scope / Wave 2'.

Deferred as documented follow-ups (all Low): clamp/silent-signal, Job shrink
reconcile, committed kubectl smoke test, provider CA-vs-sentinel misconfig
window.

@thzgajendra thzgajendra left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-review of a3aa5a485 — verified against source, suite green

All three Medium findings plus #4/#8 addressed correctly; deferrals are reasonable. Verified each:

  • #1 Lint — ✅ genuinely zero now. Ran golangci-lint run ./services/kubernetes/... ./internal/k8spki/...0 issues. Good call replacing the version-sensitive //nolint:prealloc directives with real preallocation (coreResources/appsResources/registryAPIResources, openapi kinds, endpoint-address slice) — no dangling directives to drift, so it's robust across golangci-lint versions rather than passing only on the CI-pinned one.
  • #2 Docs — ✅ fixed. sdk-server.md drops the "pagination" claim and now states "Data-plane lists are unpaginated (limit/continue are ignored)", matching services.md and the code. No more advertised-but-not-simulated surface.
  • #3 Watch load-shedding — ✅ properly fixed, not just documented. A per-subscriber overflow channel is signalled (non-blocking, once) when the buffer fills; streamWatch turns it into a 410 Gone / StatusReasonExpired ERROR event and ends the stream, exactly what a client-go reflector needs to relist instead of running a permanently-divergent cache. TestWatch_OverflowEmits410Gone drives buffer+5 undrained events and asserts the "type":"ERROR" + 410 — a real regression guard. This closes the one genuine correctness cliff.
  • #4 GC comment — ✅ accurate now. Correctly states mid-range delete is legal and that the BFS queue is what makes the cascade order-independent (vs the old false "never mutate while ranging").
  • #8 Provider headers — ✅ updated to describe the wired data plane.
  • #5 (silent clamp signal), #6 (Job shrink reconcile), #7 (committed kubectl smoke), CA-vs-sentinel window — deferred as documented Low follow-ups. All reasonable; #7 (an automated kubectl-path smoke) is the one I'd most encourage landing eventually, since the protobuf/OpenAPI read surface is still only covered by the out-of-repo manual run.

go test ./services/kubernetes/... + eks/gke/aks and -race -count=2 pass. The PR is correct, tested, lint-clean, and honestly documented — LGTM from my side (deferring the merge decision to you).

@NitinKumar004
NitinKumar004 merged commit a3aefa8 into development Jul 30, 2026
11 checks passed
@NitinKumar004
NitinKumar004 deleted the feat/k8s-runtime-parity branch July 30, 2026 17:39
@thzgajendra thzgajendra mentioned this pull request Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants