feat(workergateway): cross-cluster InstanceTypeFlavor listing via a live-list proxy#30
Conversation
- add OpListClusterInstanceTypeFlavors: tag each cluster's flavors by cluster - add OpListAggregateInstanceTypeFlavors: dedup flavors by Spec, sort accelerated-first then name - wire GET /instancetypeflavors to both operators via IterateWorkers Task 1 of workergateway-instancetypeflavor-listing. Signed-off-by: thxCode <thxcode0824@gmail.com>
- table-driven cases for per-cluster tagging and empty result - aggregate dedup by Spec, differing specs preserved - sorted ordering: accelerated first then name, deterministic within groups - Next rejects non-flavor objects Task 2 of workergateway-instancetypeflavor-listing. Signed-off-by: thxCode <thxcode0824@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR completes the worker-gateway wiring for cross-cluster InstanceTypeFlavor listing by implementing the missing list operators and extending the manager’s IterateWorkers to fall back to a per-cluster live List when no informer exists for a requested GVK (notably InstanceTypeFlavor, which is list-only). It also adjusts worker subscription lifecycle behavior so a worker subscribed with no GVKs does not immediately churn.
Changes:
- Add
GET /instancetypeflavorsendpoint with both per-cluster and?aggregated=true(deduped + deterministic sort) responses. - Implement list operators for cluster-tagged and aggregated
InstanceTypeFlavoroutput, plus unit tests. - Update manager iteration to support informer-less GVKs via a live-list proxy, and fix the zero-informer worker subscribe lifecycle behavior (with a regression test).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workergateway/service/types.go | Adds response types for per-cluster and aggregated InstanceTypeFlavor listing. |
| pkg/workergateway/service/service.go | Wires /instancetypeflavors GET handler and documents empty-GVK subscribe behavior. |
| pkg/workergateway/service/helper.go | Implements list operators for cluster and aggregated InstanceTypeFlavor (dedupe + sorting). |
| pkg/workergateway/service/helper_test.go | Adds unit tests covering the new flavor list operators and sorting behavior. |
| pkg/workergateway/manager/manager.go | Adds live-list fallback in IterateWorkers, stores per-worker client, and adds lister factory table. |
| pkg/workergateway/manager/manager_test.go | Adds regression test to ensure zero-informer workers remain subscribed until cancel. |
| worker.SchemeGroupVersionKind("Instance"): func(ctx context.Context, cli kubernetes.Interface, ns string) ([]any, error) { | ||
| return listAll(ctx, cli.WorkerV1().Instances(ns)) | ||
| }, |
There was a problem hiding this comment.
False positive — no change. core.NamespaceAll is the empty string (k8s.io/api/core/v1: NamespaceAll string = ""), so passing an empty namespace here is byte-identical to what the informer factories already do (cli.WorkerV1().Instances(core.NamespaceAll)). The generated client is gentype-based, and its List only adds a namespace path segment when the namespace is non-empty (NamespaceIfScoped), so Instances("").List() issues a cluster-wide GET …/instances across all namespaces — not /namespaces//…. When IteratorOptions.Namespace is set the proxy filters server-side; when empty it lists all namespaces, matching the informer path.
| worker.SchemeGroupVersionKind("InstancePersistentVolume"): func(ctx context.Context, cli kubernetes.Interface, ns string) ([]any, error) { | ||
| return listAll(ctx, cli.WorkerV1().InstancePersistentVolumes(ns)) | ||
| }, |
There was a problem hiding this comment.
Same false positive as the Instance thread — core.NamespaceAll == "", so an empty namespace lists across all namespaces (identical to the informer factory's InstancePersistentVolumes(core.NamespaceAll)), not a /namespaces//… request. gentype's List omits the namespace segment when it's empty. No change.
| worker.SchemeGroupVersionKind("InstanceImagePullSecret"): func(ctx context.Context, cli kubernetes.Interface, ns string) ([]any, error) { | ||
| return listAll(ctx, cli.WorkerV1().InstanceImagePullSecrets(ns)) | ||
| }, |
There was a problem hiding this comment.
Same false positive as the Instance thread — core.NamespaceAll == "", so an empty namespace lists across all namespaces (identical to the informer factory's InstanceImagePullSecrets(core.NamespaceAll)), not a /namespaces//… request. No change.
| worker.SchemeGroupVersionKind("InstanceSSHPublicKey"): func(ctx context.Context, cli kubernetes.Interface, ns string) ([]any, error) { | ||
| return listAll(ctx, cli.WorkerV1().InstanceSSHPublicKeys(ns)) | ||
| }, |
There was a problem hiding this comment.
Same false positive as the Instance thread — core.NamespaceAll == "", so an empty namespace lists across all namespaces (identical to the informer factory's InstanceSSHPublicKeys(core.NamespaceAll)), not a /namespaces//… request. No change.
…list - retain the typed client on _Worker for on-demand listing - add defaultListerFactories covering all worker GVKs via a generic listAll + kmeta.EachListItem - IterateWorkers lists live when a GVK has no informer, else skips; a failed cluster list is skipped - drop defaultGVKs: subscribing without GVKs builds no informers (list still proxied, watch yields no events) - block Subscribe until cancel so a zero-informer worker stays registered - cover the zero-informer Subscribe lifecycle with a regression test Task 3 of workergateway-instancetypeflavor-listing. Signed-off-by: thxCode <thxcode0824@gmail.com>
- separate the built-in application settings with blank lines - collapse the Node/InstanceType management doc block into a section marker Signed-off-by: thxCode <thxcode0824@gmail.com>
thxCode
force-pushed
the
spec/workergateway-instancetypeflavor-listing
branch
from
204f474 to
5c9362c
Compare
| // them at runtime without restarting the operator. Consumers must read them | ||
| // per-reconcile via ShouldValueBool(ctx) (never cache a package-level value), | ||
| // so a flip re-converges on the next reconcile. | ||
| // InstanceType. |
| lister, ok := defaultListerFactories[gvk] | ||
| if !ok { | ||
| logger.Info("no informer or lister for gvk, skip") | ||
| continue | ||
| } |
What type of PR is this?
/kind enhancement
/area workergateway
What this PR does / why we need it:
The worker-gateway referenced two list operators (
OpListClusterInstanceTypeFlavors,OpListAggregateInstanceTypeFlavors) that were never defined, sopkg/workergateway/servicedid notcompile and
GET /instancetypeflavorsreturned nothing. This PR implements both and closes the wiringgap so the endpoint returns real data end-to-end:
GET /instancetypeflavors→ClusterInstanceTypeFlavorList: every subscribed cluster's flavors, tagged bycluster.GET /instancetypeflavors?aggregated=true→AggregatedInstanceTypeFlavorList: flavors deduplicated across clusters bySpec, sorted deterministically (accelerated first, then name).InstanceTypeFlavoris list-only (noWatch), so it cannot flow throughIterateWorkers' informerpath. Instead of special-casing it,
IterateWorkersnow falls back to a live per-clusterListfor anyGVK with no informer — a generic
listAll+kmeta.EachListItemover adefaultListerFactoriestablecovering all worker GVKs (reading from the apiserver cache). Consequently
defaultGVKsis dropped:subscribing a worker without GVKs builds no informers — it stays listable through the proxy, but a
watchfor it delivers no events. A latent bug where a zero-informer worker self-deleted andre-subscribed in a spin is fixed by blocking
Subscribe()on the worker context.Which issue(s) this PR fixes:
N/A — spec-driven change, no tracked issue.
Special notes for your reviewer:
defaultGVKsmeans an empty-GVK subscribe no longer starts every informer. List is served by the new live-list proxy; watch yields no events for such a worker (documented on theSubscribeWorkerinterface +handleSubscribeWorker).listAll/EachListItemboxing).GET /instancetypeflavors(?aggregated=true) against a running worker-gateway with ≥1 subscribed cluster.Does this PR introduce a user-facing change?