feat(exporterset): label pool members with the set that owns them - #1047
feat(exporterset): label pool members with the set that owns them#1047kirkbrauer wants to merge 2 commits into
Conversation
An exporter's membership of a set lived only in its ownerReferences, which the client API never serializes, so a client had no way to tell a pooled exporter from a standalone one — or to group a pool's exporters together. Stamp exporterset.jumpstarter.dev/name and .../class on the exporters a set creates. Labels are already returned verbatim to clients and the hidden-label filter is a denylist, so this needs no protocol change, and ListExporters can filter on it server-side. Existing exporters are relabelled during reconcile rather than only on creation, so a pool that predates this becomes groupable immediately instead of when its exporters happen to be recycled. Signed-off-by: Kirk Brauer <kirkebrauer@gmail.com>
The class label names a VirtualTargetClass a client cannot read, so it does not tell a client how an exporter is provisioned. Carry the provisioner itself as well. Reconcile has already established that the referenced class names this reconciler's provisioner before any exporter is created, so the provisioner in effect is known without another lookup. Signed-off-by: Kirk Brauer <kirkebrauer@gmail.com>
kirkbrauer
added
enhancement
|
Warning Review limit reachedNext included review available in 46 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
||
| // Bridges the grandparent lookup (ExporterSet -> Exporter -> Pod). | ||
| // Bridges the grandparent lookup (ExporterSet -> Exporter -> Pod), and | ||
| // tells clients which pool an exporter came from: membership otherwise | ||
| // lives only in ownerReferences, which the client API does not expose. | ||
| labelExporterSetName = "exporterset.jumpstarter.dev/name" | ||
|
|
||
| // The VirtualTargetClass backing the pool, and the provisioner that | ||
| // class names, so clients can tell how an exporter is provisioned without | ||
| // cluster access. The provisioner is a property of the class, which a | ||
| // client cannot read, so it has to be carried here. | ||
| labelVirtualTargetClass = "exporterset.jumpstarter.dev/class" | ||
| labelProvisioner = "exporterset.jumpstarter.dev/provisioner" |
There was a problem hiding this comment.
These three labels will be visible to every client by default. The PR description says they "pass the controller's label denylist", but that's because hiddenLabels.keys defaults to empty , nothing is hidden unless an admin explicitly configures it.
In practice, every ExporterSet-managed exporter shown via jmp get exporters or the MCP jmp_list_exporters tool will render all three labels in the LABELS column:
exporterset.jumpstarter.dev/class=qemu-class,exporterset.jumpstarter.dev/name=demo-set,exporterset.jumpstarter.dev/provisioner=qemu.jumpstarter.dev,exporterset=demo-set
That's a lot of noise compared to a standalone exporter with just board=rpi4. A few options to consider:
I would add these to hiddenLabels.keys by default in the operator's default config, so they're hidden unless requested via --show-hidden-labels. Clients that want to filter by pool can still use them in --filter selectors even when hidden.
| labelExporterSetName = "exporterset.jumpstarter.dev/name" | ||
|
|
||
| // The VirtualTargetClass backing the pool, and the provisioner that | ||
| // class names, so clients can tell how an exporter is provisioned without |
There was a problem hiding this comment.
Nit: the comment seems like cut in the middle? "The VirtualTargetClass backing the pool, and the provisioner that class names"
2 participants
An exporter provisioned by an ExporterSet carried nothing saying which set it came from, so a client looking at a list of exporters could not tell a pool member from a standalone exporter, or one pool's members from another's. Working that out meant reading the ExporterSet's
ownerReferences, which needs a kubeconfig.The reconciler now labels each member with the set, class and provisioner that own it:
Existing members are backfilled on reconcile, not only newly provisioned ones. These pass the controller's label denylist, so clients see them over the normal exporter listing with no protocol change.