diff --git a/controller/internal/exporterset/reconciler.go b/controller/internal/exporterset/reconciler.go index 8491e875b..60a88d6f2 100644 --- a/controller/internal/exporterset/reconciler.go +++ b/controller/internal/exporterset/reconciler.go @@ -62,9 +62,18 @@ 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, 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 // kindExporter is the Kind string used in OwnerReference lookups. @@ -204,6 +213,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 +350,7 @@ func (r *ExporterSetReconciler) scaleUp( ObjectMeta: metav1.ObjectMeta{ GenerateName: es.Name + "-", Namespace: es.Namespace, - Labels: maps.Clone(es.Spec.Template.Metadata.Labels), + Labels: r.exporterLabels(es), Annotations: maps.Clone(es.Spec.Template.Metadata.Annotations), }, Spec: jumpstarterdevv1alpha1.ExporterSpec{ @@ -919,6 +934,72 @@ 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 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{} + } + maps.Copy(labels, r.identityLabels(es)) + 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) + + desired := r.identityLabels(es) + + for i := range owned { + exporter := &owned[i] + 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..9210cf330 100644 --- a/controller/internal/exporterset/reconciler_test.go +++ b/controller/internal/exporterset/reconciler_test.go @@ -2014,3 +2014,74 @@ 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") + } + // 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) + } +} + +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) + } + 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 + }) + r, _ := newReconciler(t, es, makeVTC()) + labels := r.exporterLabels(es) + if labels[labelExporterSetName] != "demo-set" { + t.Errorf("expected set name label, got %v", labels) + } +}