From 7bad89228b0f7f827f7067f5816a0b8ef6884acc Mon Sep 17 00:00:00 2001 From: Kirk Brauer Date: Sun, 30 Aug 2026 09:24:07 -0400 Subject: [PATCH 1/2] feat(exporterset): label exporters with the set that owns them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- controller/internal/exporterset/reconciler.go | 71 ++++++++++++++++++- .../internal/exporterset/reconciler_test.go | 63 ++++++++++++++++ 2 files changed, 132 insertions(+), 2 deletions(-) diff --git a/controller/internal/exporterset/reconciler.go b/controller/internal/exporterset/reconciler.go index 8491e875b..0cb0bb7b3 100644 --- a/controller/internal/exporterset/reconciler.go +++ b/controller/internal/exporterset/reconciler.go @@ -62,9 +62,15 @@ import ( const ( annotationSurplusSince = "exporterset.jumpstarter.dev/surplus-since" - // 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, so clients can tell how an + // exporter is provisioned without cluster access. + labelVirtualTargetClass = "exporterset.jumpstarter.dev/class" + defaultScaleDownCooldown = 5 * time.Minute // kindExporter is the Kind string used in OwnerReference lookups. @@ -204,6 +210,12 @@ func (r *ExporterSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } + // Keep identity labels current on exporters created before them, or after + // the set's class changed. + if err := r.reconcileExporterLabels(ctx, &exporterSet, ownedExporters); err != nil { + return ctrl.Result{}, err + } + // Single Pod List shared by terminal cleanup and ensureExporterPods. podsByExporter, err := r.listPodsGroupedByExporter(ctx, &exporterSet) if err != nil { @@ -335,7 +347,7 @@ func (r *ExporterSetReconciler) scaleUp( ObjectMeta: metav1.ObjectMeta{ GenerateName: es.Name + "-", Namespace: es.Namespace, - Labels: maps.Clone(es.Spec.Template.Metadata.Labels), + Labels: exporterLabels(es), Annotations: maps.Clone(es.Spec.Template.Metadata.Annotations), }, Spec: jumpstarterdevv1alpha1.ExporterSpec{ @@ -919,6 +931,61 @@ func (r *ExporterSetReconciler) clearSurplusAnnotation(ctx context.Context, es * } } +// exporterLabels are the labels an Exporter of this set carries: the set's +// template labels plus the identity labels that let a client tell which pool +// an exporter belongs to and how it is provisioned. +func exporterLabels(es *virtualtargetv1alpha1.ExporterSet) map[string]string { + labels := maps.Clone(es.Spec.Template.Metadata.Labels) + if labels == nil { + labels = map[string]string{} + } + labels[labelExporterSetName] = es.Name + if es.Spec.VirtualTargetClassName != "" { + labels[labelVirtualTargetClass] = es.Spec.VirtualTargetClassName + } + return labels +} + +// reconcileExporterLabels stamps the identity labels on exporters that predate +// them, so a pool created before this controller version becomes groupable by +// clients without waiting for its exporters to be recycled. +func (r *ExporterSetReconciler) reconcileExporterLabels( + ctx context.Context, + es *virtualtargetv1alpha1.ExporterSet, + owned []jumpstarterdevv1alpha1.Exporter, +) error { + logger := log.FromContext(ctx) + + for i := range owned { + exporter := &owned[i] + desired := map[string]string{labelExporterSetName: es.Name} + if es.Spec.VirtualTargetClassName != "" { + desired[labelVirtualTargetClass] = es.Spec.VirtualTargetClassName + } + + missing := map[string]string{} + for key, value := range desired { + if exporter.Labels[key] != value { + missing[key] = value + } + } + if len(missing) == 0 { + continue + } + + patch := client.MergeFrom(exporter.DeepCopy()) + if exporter.Labels == nil { + exporter.Labels = map[string]string{} + } + maps.Copy(exporter.Labels, missing) + if err := r.Patch(ctx, exporter, patch); err != nil { + return fmt.Errorf("unable to label Exporter %s: %w", exporter.Name, err) + } + logger.Info("stamped exporter set labels", "exporter", exporter.Name, "labels", missing) + } + return nil +} + func (r *ExporterSetReconciler) listOwnedExporters( ctx context.Context, es *virtualtargetv1alpha1.ExporterSet, diff --git a/controller/internal/exporterset/reconciler_test.go b/controller/internal/exporterset/reconciler_test.go index a860b629a..1d7dbf0cd 100644 --- a/controller/internal/exporterset/reconciler_test.go +++ b/controller/internal/exporterset/reconciler_test.go @@ -2014,3 +2014,66 @@ func TestMergeImages_esOverridesVtc(t *testing.T) { t.Errorf("runtime should be overridden by es, got %v", got.Runtime) } } + +// --- client-visible identity labels ----------------------------------------- + +func TestScaleUp_stampsIdentityLabels(t *testing.T) { + es := makeExporterSet(func(es *virtualtargetv1alpha1.ExporterSet) { + es.Spec.MinReplicas = 1 + es.Spec.MinAvailableReplicas = 0 + }) + r, c := newReconciler(t, es, makeVTC()) + reconcileOnce(t, r) + + exporters := listExporters(t, c) + if len(exporters) != 1 { + t.Fatalf("expected 1 exporter, got %d", len(exporters)) + } + // Set membership otherwise lives only in ownerReferences, which the client + // API never exposes. + if got := exporters[0].Labels[labelExporterSetName]; got != "demo-set" { + t.Errorf("%s = %q, want %q", labelExporterSetName, got, "demo-set") + } + if got := exporters[0].Labels[labelVirtualTargetClass]; got != "qemu-class" { + t.Errorf("%s = %q, want %q", labelVirtualTargetClass, got, "qemu-class") + } + // Template labels still come through. + if got := exporters[0].Labels["exporterset"]; got != "demo-set" { + t.Errorf("template label lost: got %q", got) + } +} + +func TestReconcile_backfillsIdentityLabelsOnExistingExporters(t *testing.T) { + es := makeExporterSet(func(es *virtualtargetv1alpha1.ExporterSet) { + es.Spec.MinReplicas = 1 + es.Spec.MinAvailableReplicas = 0 + }) + // An exporter from before these labels existed. + existing := makeExporter("demo-set-old", true, false, true) + delete(existing.Labels, labelExporterSetName) + + r, c := newReconciler(t, es, makeVTC(), existing) + reconcileOnce(t, r) + + var got jumpstarterdevv1alpha1.Exporter + if err := c.Get(context.Background(), + types.NamespacedName{Name: "demo-set-old", Namespace: nsDefault}, &got); err != nil { + t.Fatalf("get exporter: %v", err) + } + if got.Labels[labelExporterSetName] != "demo-set" { + t.Errorf("existing exporter not labelled: %v", got.Labels) + } + if got.Labels[labelVirtualTargetClass] != "qemu-class" { + t.Errorf("existing exporter missing class label: %v", got.Labels) + } +} + +func TestExporterLabels_survivesNilTemplateLabels(t *testing.T) { + es := makeExporterSet(func(es *virtualtargetv1alpha1.ExporterSet) { + es.Spec.Template.Metadata.Labels = nil + }) + labels := exporterLabels(es) + if labels[labelExporterSetName] != "demo-set" { + t.Errorf("expected set name label, got %v", labels) + } +} From 62d62b92058b1080b1fc440ddf7e97c2108a3396 Mon Sep 17 00:00:00 2001 From: Kirk Brauer Date: Sun, 30 Aug 2026 09:31:14 -0400 Subject: [PATCH 2/2] feat(exporterset): label exporters with their provisioner 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 --- controller/internal/exporterset/reconciler.go | 44 ++++++++++++------- .../internal/exporterset/reconciler_test.go | 10 ++++- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/controller/internal/exporterset/reconciler.go b/controller/internal/exporterset/reconciler.go index 0cb0bb7b3..60a88d6f2 100644 --- a/controller/internal/exporterset/reconciler.go +++ b/controller/internal/exporterset/reconciler.go @@ -67,9 +67,12 @@ const ( // lives only in ownerReferences, which the client API does not expose. labelExporterSetName = "exporterset.jumpstarter.dev/name" - // The VirtualTargetClass backing the pool, so clients can tell how an - // exporter is provisioned without cluster access. + // 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" defaultScaleDownCooldown = 5 * time.Minute @@ -347,7 +350,7 @@ func (r *ExporterSetReconciler) scaleUp( ObjectMeta: metav1.ObjectMeta{ GenerateName: es.Name + "-", Namespace: es.Namespace, - Labels: exporterLabels(es), + Labels: r.exporterLabels(es), Annotations: maps.Clone(es.Spec.Template.Metadata.Annotations), }, Spec: jumpstarterdevv1alpha1.ExporterSpec{ @@ -931,18 +934,32 @@ func (r *ExporterSetReconciler) clearSurplusAnnotation(ctx context.Context, es * } } +// identityLabels mark which pool an exporter belongs to and how it is +// provisioned. Reconcile has already established that the referenced class +// names this reconciler's provisioner, so it is the provisioner in effect. +func (r *ExporterSetReconciler) identityLabels( + es *virtualtargetv1alpha1.ExporterSet, +) map[string]string { + labels := map[string]string{labelExporterSetName: es.Name} + if es.Spec.VirtualTargetClassName != "" { + labels[labelVirtualTargetClass] = es.Spec.VirtualTargetClassName + } + if r.Provisioner != nil { + labels[labelProvisioner] = r.Provisioner.Name() + } + return labels +} + // exporterLabels are the labels an Exporter of this set carries: the set's -// template labels plus the identity labels that let a client tell which pool -// an exporter belongs to and how it is provisioned. -func exporterLabels(es *virtualtargetv1alpha1.ExporterSet) map[string]string { +// template labels plus the identity labels above. +func (r *ExporterSetReconciler) exporterLabels( + es *virtualtargetv1alpha1.ExporterSet, +) map[string]string { labels := maps.Clone(es.Spec.Template.Metadata.Labels) if labels == nil { labels = map[string]string{} } - labels[labelExporterSetName] = es.Name - if es.Spec.VirtualTargetClassName != "" { - labels[labelVirtualTargetClass] = es.Spec.VirtualTargetClassName - } + maps.Copy(labels, r.identityLabels(es)) return labels } @@ -956,13 +973,10 @@ func (r *ExporterSetReconciler) reconcileExporterLabels( ) error { logger := log.FromContext(ctx) + desired := r.identityLabels(es) + for i := range owned { exporter := &owned[i] - desired := map[string]string{labelExporterSetName: es.Name} - if es.Spec.VirtualTargetClassName != "" { - desired[labelVirtualTargetClass] = es.Spec.VirtualTargetClassName - } - missing := map[string]string{} for key, value := range desired { if exporter.Labels[key] != value { diff --git a/controller/internal/exporterset/reconciler_test.go b/controller/internal/exporterset/reconciler_test.go index 1d7dbf0cd..9210cf330 100644 --- a/controller/internal/exporterset/reconciler_test.go +++ b/controller/internal/exporterset/reconciler_test.go @@ -2037,6 +2037,10 @@ func TestScaleUp_stampsIdentityLabels(t *testing.T) { if got := exporters[0].Labels[labelVirtualTargetClass]; got != "qemu-class" { t.Errorf("%s = %q, want %q", labelVirtualTargetClass, got, "qemu-class") } + // The provisioner lives on the class, which a client cannot read. + if got := exporters[0].Labels[labelProvisioner]; got != qemu.ProvisionerName { + t.Errorf("%s = %q, want %q", labelProvisioner, got, qemu.ProvisionerName) + } // Template labels still come through. if got := exporters[0].Labels["exporterset"]; got != "demo-set" { t.Errorf("template label lost: got %q", got) @@ -2066,13 +2070,17 @@ func TestReconcile_backfillsIdentityLabelsOnExistingExporters(t *testing.T) { if got.Labels[labelVirtualTargetClass] != "qemu-class" { t.Errorf("existing exporter missing class label: %v", got.Labels) } + if got.Labels[labelProvisioner] != qemu.ProvisionerName { + t.Errorf("existing exporter missing provisioner label: %v", got.Labels) + } } func TestExporterLabels_survivesNilTemplateLabels(t *testing.T) { es := makeExporterSet(func(es *virtualtargetv1alpha1.ExporterSet) { es.Spec.Template.Metadata.Labels = nil }) - labels := exporterLabels(es) + r, _ := newReconciler(t, es, makeVTC()) + labels := r.exporterLabels(es) if labels[labelExporterSetName] != "demo-set" { t.Errorf("expected set name label, got %v", labels) }