From 3bb2e6b1e8602db9f872476b82d2bced2a2ee5e9 Mon Sep 17 00:00:00 2001 From: Andrew Dodds Date: Mon, 20 Apr 2026 15:37:24 +0000 Subject: [PATCH 1/8] Add config selection method --- server/bootserver.go | 30 ++++- server/config_selector.go | 156 +++++++++++++++++++++++ server/config_selector_test.go | 222 +++++++++++++++++++++++++++++++++ 3 files changed, 402 insertions(+), 6 deletions(-) create mode 100644 server/config_selector.go create mode 100644 server/config_selector_test.go diff --git a/server/bootserver.go b/server/bootserver.go index f1e9861..65e2fd1 100644 --- a/server/bootserver.go +++ b/server/bootserver.go @@ -136,7 +136,12 @@ func handleIPXE(w http.ResponseWriter, r *http.Request, k8sClient client.Client, return } - config := ipxeBootConfigList.Items[0] + config, err := selectIPXEBootConfig(ctx, k8sClient, log, ipxeBootConfigList.Items) + if err != nil { + log.Error(err, "Failed to select IPXEBootConfig") + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } if config.Spec.IPXEScriptSecretRef != nil { secret := &corev1.Secret{} err := k8sClient.Get(ctx, types.NamespacedName{Name: config.Spec.IPXEScriptSecretRef.Name, Namespace: config.Namespace}, secret) @@ -208,7 +213,12 @@ func handleIgnitionIPXEBoot(w http.ResponseWriter, r *http.Request, k8sClient cl return } - ipxeBootConfig := ipxeBootConfigList.Items[0] + ipxeBootConfig, err := selectIPXEBootConfig(ctx, k8sClient, log, ipxeBootConfigList.Items) + if err != nil { + log.Error(err, "Failed to select IPXEBootConfig") + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } ignitionSecret := corev1.Secret{ ObjectMeta: v1.ObjectMeta{ @@ -315,7 +325,12 @@ func handleIgnitionHTTPBoot(w http.ResponseWriter, r *http.Request, k8sClient cl return } - httpBootConfig := HTTPBootConfigList.Items[0] + httpBootConfig, err := selectHTTPBootConfig(ctx, k8sClient, log, HTTPBootConfigList.Items) + if err != nil { + log.Error(err, "Failed to select HTTPBootConfig") + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } ignitionSecret := corev1.Secret{ ObjectMeta: v1.ObjectMeta{ @@ -465,9 +480,12 @@ func handleHTTPBoot( "UKIURL": ukiURL, } } else { - // TODO: Pick the first HttpBootConfig if multiple CRs are found. - // Implement better validation in the future. - httpBootConfig := httpBootConfigs.Items[0] + httpBootConfig, err := selectHTTPBootConfig(ctx, k8sClient, log, httpBootConfigs.Items) + if err != nil { + log.Error(err, "Failed to select HTTPBootConfig") + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } httpBootResponseData = map[string]string{ "ClientIPs": strings.Join(clientIPs, ","), diff --git a/server/config_selector.go b/server/config_selector.go new file mode 100644 index 0000000..d3ac56f --- /dev/null +++ b/server/config_selector.go @@ -0,0 +1,156 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package server + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + bootv1alpha1 "github.com/ironcore-dev/boot-operator/api/v1alpha1" + metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// selectIPXEBootConfig picks the correct IPXEBootConfig when multiple configs +// match the same server. It resolves the owning Server, filters out orphaned +// configs, and prefers the maintenance config during maintenance. +func selectIPXEBootConfig(ctx context.Context, k8sClient client.Client, log logr.Logger, items []bootv1alpha1.IPXEBootConfig) (bootv1alpha1.IPXEBootConfig, error) { + if len(items) == 1 { + return items[0], nil + } + log.Info("Multiple IPXEBootConfigs found, resolving preferred config", "count", len(items)) + sbcNames := make([]string, len(items)) + for i := range items { + sbcNames[i] = ownerSBCName(items[i].OwnerReferences) + } + idx, err := preferredBootConfigIndex(ctx, k8sClient, log, items[0].Namespace, sbcNames) + if err != nil { + return bootv1alpha1.IPXEBootConfig{}, err + } + return items[idx], nil +} + +// selectHTTPBootConfig picks the correct HTTPBootConfig when multiple configs +// match the same server. It resolves the owning Server, filters out orphaned +// configs, and prefers the maintenance config during maintenance. +func selectHTTPBootConfig(ctx context.Context, k8sClient client.Client, log logr.Logger, items []bootv1alpha1.HTTPBootConfig) (bootv1alpha1.HTTPBootConfig, error) { + if len(items) == 1 { + return items[0], nil + } + log.Info("Multiple HTTPBootConfigs found, resolving preferred config", "count", len(items)) + sbcNames := make([]string, len(items)) + for i := range items { + sbcNames[i] = ownerSBCName(items[i].OwnerReferences) + } + idx, err := preferredBootConfigIndex(ctx, k8sClient, log, items[0].Namespace, sbcNames) + if err != nil { + return bootv1alpha1.HTTPBootConfig{}, err + } + return items[idx], nil +} + +// ownerSBCName extracts the ServerBootConfiguration name from an object's +// owner references. +func ownerSBCName(refs []metav1.OwnerReference) string { + for _, ref := range refs { + if ref.Kind == "ServerBootConfiguration" { + return ref.Name + } + } + return "" +} + +// preferredBootConfigIndex determines which boot config to serve when multiple +// configs target the same server. It looks up the Server via any owning +// ServerBootConfiguration, then filters out configs whose owner SBC is not +// recognized by the Server's bootConfigurationRef or maintenanceBootConfigurationRef. +// Among recognized configs, it prefers the maintenance config if the server is +// in maintenance. +func preferredBootConfigIndex(ctx context.Context, k8sClient client.Client, log logr.Logger, namespace string, sbcNames []string) (int, error) { + // Find the Server by looking up any SBC that owns one of the configs. + // All configs target the same server (queried by UUID/IP), so any valid + // SBC will lead to the same Server. + server, err := resolveServer(ctx, k8sClient, namespace, sbcNames) + if err != nil { + return 0, fmt.Errorf("failed to resolve Server from boot configs: %w", err) + } + + // Build the set of SBC names the Server recognizes. + recognized := make(map[string]bool, 2) + if server.Spec.BootConfigurationRef != nil { + recognized[server.Spec.BootConfigurationRef.Name] = true + } + if server.Spec.MaintenanceBootConfigurationRef != nil { + recognized[server.Spec.MaintenanceBootConfigurationRef.Name] = true + } + + // Filter items to only those whose owner SBC is recognized by the Server. + // Anything else is an orphan from a failed cleanup or a manual creation. + var validIndices []int + for i, name := range sbcNames { + if name != "" && recognized[name] { + validIndices = append(validIndices, i) + } else { + log.Info("Discarding orphaned boot config", "index", i, "ownerSBC", name, "server", server.Name) + } + } + + if len(validIndices) == 0 { + return 0, fmt.Errorf("all %d boot configs are orphaned — none match Server %q boot configuration refs", len(sbcNames), server.Name) + } + + if len(validIndices) == 1 { + return validIndices[0], nil + } + + // Multiple valid configs: prefer the maintenance one if the server is in maintenance. + if server.Spec.MaintenanceBootConfigurationRef != nil { + maintenanceSBCName := server.Spec.MaintenanceBootConfigurationRef.Name + for _, i := range validIndices { + if sbcNames[i] == maintenanceSBCName { + log.Info("Selecting maintenance boot config", "maintenanceSBC", maintenanceSBCName, "server", server.Name) + return i, nil + } + } + } + + // Fall back to the workload config. + if server.Spec.BootConfigurationRef != nil { + workloadSBCName := server.Spec.BootConfigurationRef.Name + for _, i := range validIndices { + if sbcNames[i] == workloadSBCName { + return i, nil + } + } + } + + // Should not be reachable: validIndices only contains indices whose sbcName + // is in the recognized set, and the loops above cover both recognized names. + return 0, fmt.Errorf("unexpected state: %d valid boot configs but none matched Server %q refs", len(validIndices), server.Name) +} + +// resolveServer finds the Server that the boot configs target by looking up +// any owning ServerBootConfiguration and following its serverRef. +func resolveServer(ctx context.Context, k8sClient client.Client, namespace string, sbcNames []string) (*metalv1alpha1.Server, error) { + for _, name := range sbcNames { + if name == "" { + continue + } + sbc := &metalv1alpha1.ServerBootConfiguration{} + if err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, sbc); err != nil { + // This SBC might be an orphan that's already been deleted. + // Try the next one. + continue + } + server := &metalv1alpha1.Server{} + if err := k8sClient.Get(ctx, types.NamespacedName{Name: sbc.Spec.ServerRef.Name}, server); err != nil { + return nil, fmt.Errorf("failed to get Server %q referenced by ServerBootConfiguration %q: %w", sbc.Spec.ServerRef.Name, name, err) + } + return server, nil + } + return nil, fmt.Errorf("none of the %d boot configs have a resolvable ServerBootConfiguration owner", len(sbcNames)) +} diff --git a/server/config_selector_test.go b/server/config_selector_test.go new file mode 100644 index 0000000..fb316a9 --- /dev/null +++ b/server/config_selector_test.go @@ -0,0 +1,222 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package server + +import ( + "context" + + "github.com/go-logr/logr" + bootv1alpha1 "github.com/ironcore-dev/boot-operator/api/v1alpha1" + metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func newTestClient(objs ...client.Object) client.Client { + scheme := runtime.NewScheme() + Expect(corev1.AddToScheme(scheme)).To(Succeed()) + Expect(bootv1alpha1.AddToScheme(scheme)).To(Succeed()) + Expect(metalv1alpha1.AddToScheme(scheme)).To(Succeed()) + return fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objs...). + WithStatusSubresource(&metalv1alpha1.Server{}). + Build() +} + +func ownerRef(name string) []metav1.OwnerReference { + return []metav1.OwnerReference{{ + APIVersion: "metal.ironcore.dev/v1alpha1", + Kind: "ServerBootConfiguration", + Name: name, + UID: "test-uid", + }} +} + +var _ = Describe("ConfigSelector", func() { + var ( + ctx = context.Background() + log = logr.Discard() + ) + + Context("selectIPXEBootConfig", func() { + It("returns the single item without any lookups", func() { + item := bootv1alpha1.IPXEBootConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "cfg-1", Namespace: "default"}, + } + result, err := selectIPXEBootConfig(ctx, newTestClient(), log, []bootv1alpha1.IPXEBootConfig{item}) + Expect(err).NotTo(HaveOccurred()) + Expect(result.Name).To(Equal("cfg-1")) + }) + }) + + Context("selectHTTPBootConfig", func() { + It("returns the single item without any lookups", func() { + item := bootv1alpha1.HTTPBootConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "cfg-1", Namespace: "default"}, + } + result, err := selectHTTPBootConfig(ctx, newTestClient(), log, []bootv1alpha1.HTTPBootConfig{item}) + Expect(err).NotTo(HaveOccurred()) + Expect(result.Name).To(Equal("cfg-1")) + }) + }) + + Context("preferredBootConfigIndex", func() { + It("selects the maintenance config when server is in maintenance", func() { + server := &metalv1alpha1.Server{ + ObjectMeta: metav1.ObjectMeta{Name: "server-1"}, + Spec: metalv1alpha1.ServerSpec{ + BootConfigurationRef: &metalv1alpha1.ObjectReference{ + Name: "workload-sbc", Namespace: "default", + }, + MaintenanceBootConfigurationRef: &metalv1alpha1.ObjectReference{ + Name: "maintenance-sbc", Namespace: "default", + }, + }, + } + workloadSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + maintenanceSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "maintenance-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + k8s := newTestClient(server, workloadSBC, maintenanceSBC) + + sbcNames := []string{"workload-sbc", "maintenance-sbc"} + idx, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) + Expect(err).NotTo(HaveOccurred()) + Expect(sbcNames[idx]).To(Equal("maintenance-sbc")) + }) + + It("selects the workload config when server is not in maintenance", func() { + server := &metalv1alpha1.Server{ + ObjectMeta: metav1.ObjectMeta{Name: "server-1"}, + Spec: metalv1alpha1.ServerSpec{ + BootConfigurationRef: &metalv1alpha1.ObjectReference{ + Name: "workload-sbc", Namespace: "default", + }, + }, + } + workloadSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + orphanSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "orphan-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + k8s := newTestClient(server, workloadSBC, orphanSBC) + + sbcNames := []string{"workload-sbc", "orphan-sbc"} + idx, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) + Expect(err).NotTo(HaveOccurred()) + Expect(sbcNames[idx]).To(Equal("workload-sbc")) + }) + + It("discards orphaned configs and returns the valid one", func() { + server := &metalv1alpha1.Server{ + ObjectMeta: metav1.ObjectMeta{Name: "server-1"}, + Spec: metalv1alpha1.ServerSpec{ + BootConfigurationRef: &metalv1alpha1.ObjectReference{ + Name: "workload-sbc", Namespace: "default", + }, + }, + } + workloadSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + // orphan-sbc exists but is not referenced by the Server + orphanSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "orphan-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + k8s := newTestClient(server, workloadSBC, orphanSBC) + + // orphan is at index 0, workload at index 1 + sbcNames := []string{"orphan-sbc", "workload-sbc"} + idx, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) + Expect(err).NotTo(HaveOccurred()) + Expect(idx).To(Equal(1)) + Expect(sbcNames[idx]).To(Equal("workload-sbc")) + }) + + It("returns an error when all configs are orphaned", func() { + server := &metalv1alpha1.Server{ + ObjectMeta: metav1.ObjectMeta{Name: "server-1"}, + Spec: metalv1alpha1.ServerSpec{ + BootConfigurationRef: &metalv1alpha1.ObjectReference{ + Name: "real-sbc", Namespace: "default", + }, + }, + } + orphanSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "orphan-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + k8s := newTestClient(server, orphanSBC) + + sbcNames := []string{"orphan-sbc", "another-orphan"} + _, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("orphaned")) + }) + + It("returns an error when no configs have owner references", func() { + sbcNames := []string{"", ""} + _, err := preferredBootConfigIndex(ctx, newTestClient(), log, "default", sbcNames) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("resolvable ServerBootConfiguration owner")) + }) + }) + + Context("resolveServer", func() { + It("skips deleted SBCs and resolves via the next one", func() { + server := &metalv1alpha1.Server{ + ObjectMeta: metav1.ObjectMeta{Name: "server-1"}, + } + // Only the second SBC exists + validSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "valid-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + k8s := newTestClient(server, validSBC) + + // "deleted-sbc" doesn't exist, "valid-sbc" does + result, err := resolveServer(ctx, k8s, "default", []string{"deleted-sbc", "valid-sbc"}) + Expect(err).NotTo(HaveOccurred()) + Expect(result.Name).To(Equal("server-1")) + }) + + It("returns an error when no SBC can be resolved", func() { + k8s := newTestClient() + _, err := resolveServer(ctx, k8s, "default", []string{"gone-sbc", ""}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("resolvable ServerBootConfiguration owner")) + }) + }) +}) From 4580338f76284b82bc84bba7044b899278857211 Mon Sep 17 00:00:00 2001 From: Andrew Dodds Date: Mon, 20 Apr 2026 15:49:24 +0000 Subject: [PATCH 2/8] remove unused owner-ref in config-selector-test.go --- server/config_selector_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/server/config_selector_test.go b/server/config_selector_test.go index fb316a9..b9eca76 100644 --- a/server/config_selector_test.go +++ b/server/config_selector_test.go @@ -30,15 +30,6 @@ func newTestClient(objs ...client.Object) client.Client { Build() } -func ownerRef(name string) []metav1.OwnerReference { - return []metav1.OwnerReference{{ - APIVersion: "metal.ironcore.dev/v1alpha1", - Kind: "ServerBootConfiguration", - Name: name, - UID: "test-uid", - }} -} - var _ = Describe("ConfigSelector", func() { var ( ctx = context.Background() From b1297de96076ff30568bf7d660dda2a06e9fe40b Mon Sep 17 00:00:00 2001 From: Andrew Dodds Date: Mon, 20 Apr 2026 16:01:42 +0000 Subject: [PATCH 3/8] Move tests into bootserver tests --- server/bootserver_suit_test.go | 14 +++ server/bootserver_test.go | 200 +++++++++++++++++++++++++++++++ server/config_selector_test.go | 213 --------------------------------- 3 files changed, 214 insertions(+), 213 deletions(-) delete mode 100644 server/config_selector_test.go diff --git a/server/bootserver_suit_test.go b/server/bootserver_suit_test.go index 37a54b3..942b8f9 100644 --- a/server/bootserver_suit_test.go +++ b/server/bootserver_suit_test.go @@ -11,6 +11,7 @@ import ( "github.com/go-logr/logr" bootv1alpha1 "github.com/ironcore-dev/boot-operator/api/v1alpha1" "github.com/ironcore-dev/boot-operator/internal/registry" + metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" @@ -34,10 +35,23 @@ func TestBootServer(t *testing.T) { RunSpecs(t, "Boot Server Suite") } +func newTestClient(objs ...client.Object) client.Client { + scheme := runtime.NewScheme() + Expect(corev1.AddToScheme(scheme)).To(Succeed()) + Expect(bootv1alpha1.AddToScheme(scheme)).To(Succeed()) + Expect(metalv1alpha1.AddToScheme(scheme)).To(Succeed()) + return fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objs...). + WithStatusSubresource(&metalv1alpha1.Server{}). + Build() +} + var _ = BeforeSuite(func() { scheme := runtime.NewScheme() Expect(corev1.AddToScheme(scheme)).To(Succeed()) Expect(bootv1alpha1.AddToScheme(scheme)).To(Succeed()) + Expect(metalv1alpha1.AddToScheme(scheme)).To(Succeed()) k8sClient = fake.NewClientBuilder(). WithScheme(scheme). diff --git a/server/bootserver_test.go b/server/bootserver_test.go index 83dc2b6..6f0dd81 100644 --- a/server/bootserver_test.go +++ b/server/bootserver_test.go @@ -10,6 +10,7 @@ import ( "github.com/go-logr/logr" bootv1alpha1 "github.com/ironcore-dev/boot-operator/api/v1alpha1" + metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" @@ -117,3 +118,202 @@ systemd: }) }) }) + +var _ = Describe("ConfigSelector", func() { + var ( + ctx = context.Background() + log = logr.Discard() + ) + + Context("selectIPXEBootConfig", func() { + It("returns the single item without any lookups", func() { + item := bootv1alpha1.IPXEBootConfig{ + ObjectMeta: v1.ObjectMeta{Name: "cfg-1", Namespace: "default"}, + } + result, err := selectIPXEBootConfig(ctx, newTestClient(), log, []bootv1alpha1.IPXEBootConfig{item}) + Expect(err).NotTo(HaveOccurred()) + Expect(result.Name).To(Equal("cfg-1")) + }) + }) + + Context("selectHTTPBootConfig", func() { + It("returns the single item without any lookups", func() { + item := bootv1alpha1.HTTPBootConfig{ + ObjectMeta: v1.ObjectMeta{Name: "cfg-1", Namespace: "default"}, + } + result, err := selectHTTPBootConfig(ctx, newTestClient(), log, []bootv1alpha1.HTTPBootConfig{item}) + Expect(err).NotTo(HaveOccurred()) + Expect(result.Name).To(Equal("cfg-1")) + }) + }) + + Context("preferredBootConfigIndex", func() { + It("selects the maintenance config when server is in maintenance", func() { + server := &metalv1alpha1.Server{ + ObjectMeta: v1.ObjectMeta{Name: "server-1"}, + Spec: metalv1alpha1.ServerSpec{ + BootConfigurationRef: &metalv1alpha1.ObjectReference{ + Name: "workload-sbc", Namespace: "default", + }, + MaintenanceBootConfigurationRef: &metalv1alpha1.ObjectReference{ + Name: "maintenance-sbc", Namespace: "default", + }, + }, + } + workloadSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + maintenanceSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "maintenance-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + k8s := newTestClient(server, workloadSBC, maintenanceSBC) + + sbcNames := []string{"workload-sbc", "maintenance-sbc"} + idx, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) + Expect(err).NotTo(HaveOccurred()) + Expect(sbcNames[idx]).To(Equal("maintenance-sbc")) + }) + + It("selects the workload config when server is not in maintenance", func() { + server := &metalv1alpha1.Server{ + ObjectMeta: v1.ObjectMeta{Name: "server-1"}, + Spec: metalv1alpha1.ServerSpec{ + BootConfigurationRef: &metalv1alpha1.ObjectReference{ + Name: "workload-sbc", Namespace: "default", + }, + }, + } + workloadSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + orphanSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "orphan-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + k8s := newTestClient(server, workloadSBC, orphanSBC) + + sbcNames := []string{"workload-sbc", "orphan-sbc"} + idx, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) + Expect(err).NotTo(HaveOccurred()) + Expect(sbcNames[idx]).To(Equal("workload-sbc")) + }) + + It("discards orphaned configs and returns the valid one", func() { + server := &metalv1alpha1.Server{ + ObjectMeta: v1.ObjectMeta{Name: "server-1"}, + Spec: metalv1alpha1.ServerSpec{ + BootConfigurationRef: &metalv1alpha1.ObjectReference{ + Name: "workload-sbc", Namespace: "default", + }, + }, + } + workloadSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + orphanSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "orphan-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + k8s := newTestClient(server, workloadSBC, orphanSBC) + + sbcNames := []string{"orphan-sbc", "workload-sbc"} + idx, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) + Expect(err).NotTo(HaveOccurred()) + Expect(idx).To(Equal(1)) + Expect(sbcNames[idx]).To(Equal("workload-sbc")) + }) + + It("returns an error when all configs are orphaned", func() { + server := &metalv1alpha1.Server{ + ObjectMeta: v1.ObjectMeta{Name: "server-1"}, + Spec: metalv1alpha1.ServerSpec{ + BootConfigurationRef: &metalv1alpha1.ObjectReference{ + Name: "real-sbc", Namespace: "default", + }, + }, + } + orphanSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "orphan-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + k8s := newTestClient(server, orphanSBC) + + sbcNames := []string{"orphan-sbc", "another-orphan"} + _, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("orphaned")) + }) + + It("returns an error when configs have no ServerBootConfiguration owner refs", func() { + // Items have owner refs of a different kind — ownerSBCName returns "" + // for both, so selectIPXEBootConfig delegates to preferredBootConfigIndex + // with all-empty names, which fails at resolveServer. + items := []bootv1alpha1.IPXEBootConfig{ + { + ObjectMeta: v1.ObjectMeta{ + Name: "cfg-a", Namespace: "default", + OwnerReferences: []v1.OwnerReference{{ + APIVersion: "v1", + Kind: "ConfigMap", + Name: "not-an-sbc", + UID: "uid-1", + }}, + }, + }, + { + ObjectMeta: v1.ObjectMeta{ + Name: "cfg-b", Namespace: "default", + // No owner refs at all. + }, + }, + } + _, err := selectIPXEBootConfig(ctx, newTestClient(), log, items) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("resolvable ServerBootConfiguration owner")) + }) + }) + + Context("resolveServer", func() { + It("skips deleted SBCs and resolves via the next one", func() { + server := &metalv1alpha1.Server{ + ObjectMeta: v1.ObjectMeta{Name: "server-1"}, + } + validSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "valid-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + k8s := newTestClient(server, validSBC) + + result, err := resolveServer(ctx, k8s, "default", []string{"deleted-sbc", "valid-sbc"}) + Expect(err).NotTo(HaveOccurred()) + Expect(result.Name).To(Equal("server-1")) + }) + + It("returns an error when no SBC can be resolved", func() { + k8s := newTestClient() + _, err := resolveServer(ctx, k8s, "default", []string{"gone-sbc", ""}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("resolvable ServerBootConfiguration owner")) + }) + }) +}) diff --git a/server/config_selector_test.go b/server/config_selector_test.go deleted file mode 100644 index b9eca76..0000000 --- a/server/config_selector_test.go +++ /dev/null @@ -1,213 +0,0 @@ -// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors -// SPDX-License-Identifier: Apache-2.0 - -package server - -import ( - "context" - - "github.com/go-logr/logr" - bootv1alpha1 "github.com/ironcore-dev/boot-operator/api/v1alpha1" - metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" -) - -func newTestClient(objs ...client.Object) client.Client { - scheme := runtime.NewScheme() - Expect(corev1.AddToScheme(scheme)).To(Succeed()) - Expect(bootv1alpha1.AddToScheme(scheme)).To(Succeed()) - Expect(metalv1alpha1.AddToScheme(scheme)).To(Succeed()) - return fake.NewClientBuilder(). - WithScheme(scheme). - WithObjects(objs...). - WithStatusSubresource(&metalv1alpha1.Server{}). - Build() -} - -var _ = Describe("ConfigSelector", func() { - var ( - ctx = context.Background() - log = logr.Discard() - ) - - Context("selectIPXEBootConfig", func() { - It("returns the single item without any lookups", func() { - item := bootv1alpha1.IPXEBootConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "cfg-1", Namespace: "default"}, - } - result, err := selectIPXEBootConfig(ctx, newTestClient(), log, []bootv1alpha1.IPXEBootConfig{item}) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Name).To(Equal("cfg-1")) - }) - }) - - Context("selectHTTPBootConfig", func() { - It("returns the single item without any lookups", func() { - item := bootv1alpha1.HTTPBootConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "cfg-1", Namespace: "default"}, - } - result, err := selectHTTPBootConfig(ctx, newTestClient(), log, []bootv1alpha1.HTTPBootConfig{item}) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Name).To(Equal("cfg-1")) - }) - }) - - Context("preferredBootConfigIndex", func() { - It("selects the maintenance config when server is in maintenance", func() { - server := &metalv1alpha1.Server{ - ObjectMeta: metav1.ObjectMeta{Name: "server-1"}, - Spec: metalv1alpha1.ServerSpec{ - BootConfigurationRef: &metalv1alpha1.ObjectReference{ - Name: "workload-sbc", Namespace: "default", - }, - MaintenanceBootConfigurationRef: &metalv1alpha1.ObjectReference{ - Name: "maintenance-sbc", Namespace: "default", - }, - }, - } - workloadSBC := &metalv1alpha1.ServerBootConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, - Spec: metalv1alpha1.ServerBootConfigurationSpec{ - ServerRef: corev1.LocalObjectReference{Name: "server-1"}, - }, - } - maintenanceSBC := &metalv1alpha1.ServerBootConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "maintenance-sbc", Namespace: "default"}, - Spec: metalv1alpha1.ServerBootConfigurationSpec{ - ServerRef: corev1.LocalObjectReference{Name: "server-1"}, - }, - } - k8s := newTestClient(server, workloadSBC, maintenanceSBC) - - sbcNames := []string{"workload-sbc", "maintenance-sbc"} - idx, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) - Expect(err).NotTo(HaveOccurred()) - Expect(sbcNames[idx]).To(Equal("maintenance-sbc")) - }) - - It("selects the workload config when server is not in maintenance", func() { - server := &metalv1alpha1.Server{ - ObjectMeta: metav1.ObjectMeta{Name: "server-1"}, - Spec: metalv1alpha1.ServerSpec{ - BootConfigurationRef: &metalv1alpha1.ObjectReference{ - Name: "workload-sbc", Namespace: "default", - }, - }, - } - workloadSBC := &metalv1alpha1.ServerBootConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, - Spec: metalv1alpha1.ServerBootConfigurationSpec{ - ServerRef: corev1.LocalObjectReference{Name: "server-1"}, - }, - } - orphanSBC := &metalv1alpha1.ServerBootConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "orphan-sbc", Namespace: "default"}, - Spec: metalv1alpha1.ServerBootConfigurationSpec{ - ServerRef: corev1.LocalObjectReference{Name: "server-1"}, - }, - } - k8s := newTestClient(server, workloadSBC, orphanSBC) - - sbcNames := []string{"workload-sbc", "orphan-sbc"} - idx, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) - Expect(err).NotTo(HaveOccurred()) - Expect(sbcNames[idx]).To(Equal("workload-sbc")) - }) - - It("discards orphaned configs and returns the valid one", func() { - server := &metalv1alpha1.Server{ - ObjectMeta: metav1.ObjectMeta{Name: "server-1"}, - Spec: metalv1alpha1.ServerSpec{ - BootConfigurationRef: &metalv1alpha1.ObjectReference{ - Name: "workload-sbc", Namespace: "default", - }, - }, - } - workloadSBC := &metalv1alpha1.ServerBootConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, - Spec: metalv1alpha1.ServerBootConfigurationSpec{ - ServerRef: corev1.LocalObjectReference{Name: "server-1"}, - }, - } - // orphan-sbc exists but is not referenced by the Server - orphanSBC := &metalv1alpha1.ServerBootConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "orphan-sbc", Namespace: "default"}, - Spec: metalv1alpha1.ServerBootConfigurationSpec{ - ServerRef: corev1.LocalObjectReference{Name: "server-1"}, - }, - } - k8s := newTestClient(server, workloadSBC, orphanSBC) - - // orphan is at index 0, workload at index 1 - sbcNames := []string{"orphan-sbc", "workload-sbc"} - idx, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) - Expect(err).NotTo(HaveOccurred()) - Expect(idx).To(Equal(1)) - Expect(sbcNames[idx]).To(Equal("workload-sbc")) - }) - - It("returns an error when all configs are orphaned", func() { - server := &metalv1alpha1.Server{ - ObjectMeta: metav1.ObjectMeta{Name: "server-1"}, - Spec: metalv1alpha1.ServerSpec{ - BootConfigurationRef: &metalv1alpha1.ObjectReference{ - Name: "real-sbc", Namespace: "default", - }, - }, - } - orphanSBC := &metalv1alpha1.ServerBootConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "orphan-sbc", Namespace: "default"}, - Spec: metalv1alpha1.ServerBootConfigurationSpec{ - ServerRef: corev1.LocalObjectReference{Name: "server-1"}, - }, - } - k8s := newTestClient(server, orphanSBC) - - sbcNames := []string{"orphan-sbc", "another-orphan"} - _, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("orphaned")) - }) - - It("returns an error when no configs have owner references", func() { - sbcNames := []string{"", ""} - _, err := preferredBootConfigIndex(ctx, newTestClient(), log, "default", sbcNames) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("resolvable ServerBootConfiguration owner")) - }) - }) - - Context("resolveServer", func() { - It("skips deleted SBCs and resolves via the next one", func() { - server := &metalv1alpha1.Server{ - ObjectMeta: metav1.ObjectMeta{Name: "server-1"}, - } - // Only the second SBC exists - validSBC := &metalv1alpha1.ServerBootConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "valid-sbc", Namespace: "default"}, - Spec: metalv1alpha1.ServerBootConfigurationSpec{ - ServerRef: corev1.LocalObjectReference{Name: "server-1"}, - }, - } - k8s := newTestClient(server, validSBC) - - // "deleted-sbc" doesn't exist, "valid-sbc" does - result, err := resolveServer(ctx, k8s, "default", []string{"deleted-sbc", "valid-sbc"}) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Name).To(Equal("server-1")) - }) - - It("returns an error when no SBC can be resolved", func() { - k8s := newTestClient() - _, err := resolveServer(ctx, k8s, "default", []string{"gone-sbc", ""}) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("resolvable ServerBootConfiguration owner")) - }) - }) -}) From 4a21aba0d824deb8fc8484b3dda007b2106ca4b7 Mon Sep 17 00:00:00 2001 From: Andrew Dodds Date: Mon, 20 Apr 2026 18:46:19 +0000 Subject: [PATCH 4/8] refine selector --- server/bootserver.go | 14 ++++---- server/bootserver_test.go | 71 +++++++++++++++++++++++++++++++++++---- server/config_selector.go | 52 ++++++++++++++-------------- 3 files changed, 97 insertions(+), 40 deletions(-) diff --git a/server/bootserver.go b/server/bootserver.go index 65e2fd1..106a0c6 100644 --- a/server/bootserver.go +++ b/server/bootserver.go @@ -136,7 +136,7 @@ func handleIPXE(w http.ResponseWriter, r *http.Request, k8sClient client.Client, return } - config, err := selectIPXEBootConfig(ctx, k8sClient, log, ipxeBootConfigList.Items) + config, err := selectBootConfig(ctx, k8sClient, log, toPointers(ipxeBootConfigList.Items)) if err != nil { log.Error(err, "Failed to select IPXEBootConfig") http.Error(w, "Internal Server Error", http.StatusInternalServerError) @@ -172,7 +172,7 @@ func handleIPXE(w http.ResponseWriter, r *http.Request, k8sClient client.Client, IPXEServerURL: ipxeServiceURL, }) - err = SetStatusCondition(ctx, k8sClient, log, &config, "IPXEScriptFetched") + err = SetStatusCondition(ctx, k8sClient, log, config, "IPXEScriptFetched") if err != nil { log.Error(err, "Failed to set IPXEScriptFetched status condition") } @@ -213,7 +213,7 @@ func handleIgnitionIPXEBoot(w http.ResponseWriter, r *http.Request, k8sClient cl return } - ipxeBootConfig, err := selectIPXEBootConfig(ctx, k8sClient, log, ipxeBootConfigList.Items) + ipxeBootConfig, err := selectBootConfig(ctx, k8sClient, log, toPointers(ipxeBootConfigList.Items)) if err != nil { log.Error(err, "Failed to select IPXEBootConfig") http.Error(w, "Internal Server Error", http.StatusInternalServerError) @@ -254,7 +254,7 @@ func handleIgnitionIPXEBoot(w http.ResponseWriter, r *http.Request, k8sClient cl return } - err = SetStatusCondition(ctx, k8sClient, log, &ipxeBootConfig, "IgnitionDataFetched") + err = SetStatusCondition(ctx, k8sClient, log, ipxeBootConfig, "IgnitionDataFetched") if err != nil { log.Error(err, "Failed to set IgnitionDataFetched status condition") } @@ -325,7 +325,7 @@ func handleIgnitionHTTPBoot(w http.ResponseWriter, r *http.Request, k8sClient cl return } - httpBootConfig, err := selectHTTPBootConfig(ctx, k8sClient, log, HTTPBootConfigList.Items) + httpBootConfig, err := selectBootConfig(ctx, k8sClient, log, toPointers(HTTPBootConfigList.Items)) if err != nil { log.Error(err, "Failed to select HTTPBootConfig") http.Error(w, "Internal Server Error", http.StatusInternalServerError) @@ -366,7 +366,7 @@ func handleIgnitionHTTPBoot(w http.ResponseWriter, r *http.Request, k8sClient cl return } - err = SetStatusCondition(ctx, k8sClient, log, &httpBootConfig, "IgnitionDataFetched") + err = SetStatusCondition(ctx, k8sClient, log, httpBootConfig, "IgnitionDataFetched") if err != nil { log.Error(err, "Failed to set IgnitionDataFetched status condition") } @@ -480,7 +480,7 @@ func handleHTTPBoot( "UKIURL": ukiURL, } } else { - httpBootConfig, err := selectHTTPBootConfig(ctx, k8sClient, log, httpBootConfigs.Items) + httpBootConfig, err := selectBootConfig(ctx, k8sClient, log, toPointers(httpBootConfigs.Items)) if err != nil { log.Error(err, "Failed to select HTTPBootConfig") http.Error(w, "Internal Server Error", http.StatusInternalServerError) diff --git a/server/bootserver_test.go b/server/bootserver_test.go index 6f0dd81..45daa25 100644 --- a/server/bootserver_test.go +++ b/server/bootserver_test.go @@ -125,26 +125,85 @@ var _ = Describe("ConfigSelector", func() { log = logr.Discard() ) - Context("selectIPXEBootConfig", func() { + Context("selectBootConfig with IPXEBootConfig", func() { It("returns the single item without any lookups", func() { item := bootv1alpha1.IPXEBootConfig{ ObjectMeta: v1.ObjectMeta{Name: "cfg-1", Namespace: "default"}, } - result, err := selectIPXEBootConfig(ctx, newTestClient(), log, []bootv1alpha1.IPXEBootConfig{item}) + result, err := selectBootConfig(ctx, newTestClient(), log, toPointers([]bootv1alpha1.IPXEBootConfig{item})) Expect(err).NotTo(HaveOccurred()) Expect(result.Name).To(Equal("cfg-1")) }) + + It("selects the maintenance config from multiple items", func() { + server := &metalv1alpha1.Server{ + ObjectMeta: v1.ObjectMeta{Name: "server-1"}, + Spec: metalv1alpha1.ServerSpec{ + BootConfigurationRef: &metalv1alpha1.ObjectReference{Name: "workload-sbc", Namespace: "default"}, + MaintenanceBootConfigurationRef: &metalv1alpha1.ObjectReference{Name: "maintenance-sbc", Namespace: "default"}, + }, + } + workloadSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ServerRef: corev1.LocalObjectReference{Name: "server-1"}}, + } + maintenanceSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "maintenance-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ServerRef: corev1.LocalObjectReference{Name: "server-1"}}, + } + items := []bootv1alpha1.IPXEBootConfig{ + {ObjectMeta: v1.ObjectMeta{Name: "ipxe-workload", Namespace: "default", OwnerReferences: []v1.OwnerReference{ + {APIVersion: "metal.ironcore.dev/v1alpha1", Kind: "ServerBootConfiguration", Name: "workload-sbc", UID: "uid-1"}, + }}}, + {ObjectMeta: v1.ObjectMeta{Name: "ipxe-maintenance", Namespace: "default", OwnerReferences: []v1.OwnerReference{ + {APIVersion: "metal.ironcore.dev/v1alpha1", Kind: "ServerBootConfiguration", Name: "maintenance-sbc", UID: "uid-2"}, + }}}, + } + k8s := newTestClient(server, workloadSBC, maintenanceSBC) + result, err := selectBootConfig(ctx, k8s, log, toPointers(items)) + Expect(err).NotTo(HaveOccurred()) + Expect(result.Name).To(Equal("ipxe-maintenance")) + }) }) - Context("selectHTTPBootConfig", func() { + Context("selectBootConfig with HTTPBootConfig", func() { It("returns the single item without any lookups", func() { item := bootv1alpha1.HTTPBootConfig{ ObjectMeta: v1.ObjectMeta{Name: "cfg-1", Namespace: "default"}, } - result, err := selectHTTPBootConfig(ctx, newTestClient(), log, []bootv1alpha1.HTTPBootConfig{item}) + result, err := selectBootConfig(ctx, newTestClient(), log, toPointers([]bootv1alpha1.HTTPBootConfig{item})) Expect(err).NotTo(HaveOccurred()) Expect(result.Name).To(Equal("cfg-1")) }) + + It("selects the workload config when not in maintenance", func() { + server := &metalv1alpha1.Server{ + ObjectMeta: v1.ObjectMeta{Name: "server-1"}, + Spec: metalv1alpha1.ServerSpec{ + BootConfigurationRef: &metalv1alpha1.ObjectReference{Name: "workload-sbc", Namespace: "default"}, + }, + } + workloadSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ServerRef: corev1.LocalObjectReference{Name: "server-1"}}, + } + orphanSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "orphan-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ServerRef: corev1.LocalObjectReference{Name: "server-1"}}, + } + items := []bootv1alpha1.HTTPBootConfig{ + {ObjectMeta: v1.ObjectMeta{Name: "http-orphan", Namespace: "default", OwnerReferences: []v1.OwnerReference{ + {APIVersion: "metal.ironcore.dev/v1alpha1", Kind: "ServerBootConfiguration", Name: "orphan-sbc", UID: "uid-1"}, + }}}, + {ObjectMeta: v1.ObjectMeta{Name: "http-workload", Namespace: "default", OwnerReferences: []v1.OwnerReference{ + {APIVersion: "metal.ironcore.dev/v1alpha1", Kind: "ServerBootConfiguration", Name: "workload-sbc", UID: "uid-2"}, + }}}, + } + k8s := newTestClient(server, workloadSBC, orphanSBC) + result, err := selectBootConfig(ctx, k8s, log, toPointers(items)) + Expect(err).NotTo(HaveOccurred()) + Expect(result.Name).To(Equal("http-workload")) + }) }) Context("preferredBootConfigIndex", func() { @@ -264,7 +323,7 @@ var _ = Describe("ConfigSelector", func() { It("returns an error when configs have no ServerBootConfiguration owner refs", func() { // Items have owner refs of a different kind — ownerSBCName returns "" - // for both, so selectIPXEBootConfig delegates to preferredBootConfigIndex + // for both, so selectBootConfig delegates to preferredBootConfigIndex // with all-empty names, which fails at resolveServer. items := []bootv1alpha1.IPXEBootConfig{ { @@ -285,7 +344,7 @@ var _ = Describe("ConfigSelector", func() { }, }, } - _, err := selectIPXEBootConfig(ctx, newTestClient(), log, items) + _, err := selectBootConfig(ctx, newTestClient(), log, toPointers(items)) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("resolvable ServerBootConfiguration owner")) }) diff --git a/server/config_selector.go b/server/config_selector.go index d3ac56f..61292db 100644 --- a/server/config_selector.go +++ b/server/config_selector.go @@ -8,49 +8,45 @@ import ( "fmt" "github.com/go-logr/logr" - bootv1alpha1 "github.com/ironcore-dev/boot-operator/api/v1alpha1" metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) -// selectIPXEBootConfig picks the correct IPXEBootConfig when multiple configs -// match the same server. It resolves the owning Server, filters out orphaned -// configs, and prefers the maintenance config during maintenance. -func selectIPXEBootConfig(ctx context.Context, k8sClient client.Client, log logr.Logger, items []bootv1alpha1.IPXEBootConfig) (bootv1alpha1.IPXEBootConfig, error) { +// selectBootConfig picks the correct boot config when multiple configs match +// the same server. It resolves the owning Server, filters out orphaned configs, +// and prefers the maintenance config during maintenance. T must implement +// client.Object (satisfied by *IPXEBootConfig, *HTTPBootConfig, etc.). +func selectBootConfig[T client.Object](ctx context.Context, k8sClient client.Client, log logr.Logger, items []T) (T, error) { + var zero T + if len(items) == 0 { + return zero, fmt.Errorf("no boot config items to select from") + } if len(items) == 1 { return items[0], nil } - log.Info("Multiple IPXEBootConfigs found, resolving preferred config", "count", len(items)) + log.Info("Multiple boot configs found, resolving preferred config", "count", len(items)) sbcNames := make([]string, len(items)) for i := range items { - sbcNames[i] = ownerSBCName(items[i].OwnerReferences) + sbcNames[i] = ownerSBCName(items[i].GetOwnerReferences()) } - idx, err := preferredBootConfigIndex(ctx, k8sClient, log, items[0].Namespace, sbcNames) + idx, err := preferredBootConfigIndex(ctx, k8sClient, log, items[0].GetNamespace(), sbcNames) if err != nil { - return bootv1alpha1.IPXEBootConfig{}, err + return zero, err } return items[idx], nil } -// selectHTTPBootConfig picks the correct HTTPBootConfig when multiple configs -// match the same server. It resolves the owning Server, filters out orphaned -// configs, and prefers the maintenance config during maintenance. -func selectHTTPBootConfig(ctx context.Context, k8sClient client.Client, log logr.Logger, items []bootv1alpha1.HTTPBootConfig) (bootv1alpha1.HTTPBootConfig, error) { - if len(items) == 1 { - return items[0], nil - } - log.Info("Multiple HTTPBootConfigs found, resolving preferred config", "count", len(items)) - sbcNames := make([]string, len(items)) +// toPointers converts a value slice to a pointer slice so that elements +// satisfy client.Object. +func toPointers[T any](items []T) []*T { + ptrs := make([]*T, len(items)) for i := range items { - sbcNames[i] = ownerSBCName(items[i].OwnerReferences) + ptrs[i] = &items[i] } - idx, err := preferredBootConfigIndex(ctx, k8sClient, log, items[0].Namespace, sbcNames) - if err != nil { - return bootv1alpha1.HTTPBootConfig{}, err - } - return items[idx], nil + return ptrs } // ownerSBCName extracts the ServerBootConfiguration name from an object's @@ -142,9 +138,11 @@ func resolveServer(ctx context.Context, k8sClient client.Client, namespace strin } sbc := &metalv1alpha1.ServerBootConfiguration{} if err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, sbc); err != nil { - // This SBC might be an orphan that's already been deleted. - // Try the next one. - continue + if apierrors.IsNotFound(err) { + // This SBC has been deleted (orphaned child). Try the next one. + continue + } + return nil, fmt.Errorf("failed to get ServerBootConfiguration %q: %w", name, err) } server := &metalv1alpha1.Server{} if err := k8sClient.Get(ctx, types.NamespacedName{Name: sbc.Spec.ServerRef.Name}, server); err != nil { From 5ad1fe8435c8da44bd5cf38d68f1e3befaf84097 Mon Sep 17 00:00:00 2001 From: Andrew Dodds Date: Tue, 21 Apr 2026 07:47:07 +0000 Subject: [PATCH 5/8] improve namsepace handling --- server/bootserver_test.go | 46 ++++++++++++++++------- server/config_selector.go | 77 +++++++++++++++++++++++++-------------- 2 files changed, 82 insertions(+), 41 deletions(-) diff --git a/server/bootserver_test.go b/server/bootserver_test.go index 45daa25..ed8a887 100644 --- a/server/bootserver_test.go +++ b/server/bootserver_test.go @@ -233,10 +233,13 @@ var _ = Describe("ConfigSelector", func() { } k8s := newTestClient(server, workloadSBC, maintenanceSBC) - sbcNames := []string{"workload-sbc", "maintenance-sbc"} - idx, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) + owners := []sbcRef{ + {namespace: "default", name: "workload-sbc"}, + {namespace: "default", name: "maintenance-sbc"}, + } + idx, err := preferredBootConfigIndex(ctx, k8s, log, owners) Expect(err).NotTo(HaveOccurred()) - Expect(sbcNames[idx]).To(Equal("maintenance-sbc")) + Expect(owners[idx].name).To(Equal("maintenance-sbc")) }) It("selects the workload config when server is not in maintenance", func() { @@ -262,10 +265,13 @@ var _ = Describe("ConfigSelector", func() { } k8s := newTestClient(server, workloadSBC, orphanSBC) - sbcNames := []string{"workload-sbc", "orphan-sbc"} - idx, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) + owners := []sbcRef{ + {namespace: "default", name: "workload-sbc"}, + {namespace: "default", name: "orphan-sbc"}, + } + idx, err := preferredBootConfigIndex(ctx, k8s, log, owners) Expect(err).NotTo(HaveOccurred()) - Expect(sbcNames[idx]).To(Equal("workload-sbc")) + Expect(owners[idx].name).To(Equal("workload-sbc")) }) It("discards orphaned configs and returns the valid one", func() { @@ -291,11 +297,14 @@ var _ = Describe("ConfigSelector", func() { } k8s := newTestClient(server, workloadSBC, orphanSBC) - sbcNames := []string{"orphan-sbc", "workload-sbc"} - idx, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) + owners := []sbcRef{ + {namespace: "default", name: "orphan-sbc"}, + {namespace: "default", name: "workload-sbc"}, + } + idx, err := preferredBootConfigIndex(ctx, k8s, log, owners) Expect(err).NotTo(HaveOccurred()) Expect(idx).To(Equal(1)) - Expect(sbcNames[idx]).To(Equal("workload-sbc")) + Expect(owners[idx].name).To(Equal("workload-sbc")) }) It("returns an error when all configs are orphaned", func() { @@ -315,8 +324,11 @@ var _ = Describe("ConfigSelector", func() { } k8s := newTestClient(server, orphanSBC) - sbcNames := []string{"orphan-sbc", "another-orphan"} - _, err := preferredBootConfigIndex(ctx, k8s, log, "default", sbcNames) + owners := []sbcRef{ + {namespace: "default", name: "orphan-sbc"}, + {namespace: "default", name: "another-orphan"}, + } + _, err := preferredBootConfigIndex(ctx, k8s, log, owners) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("orphaned")) }) @@ -363,14 +375,22 @@ var _ = Describe("ConfigSelector", func() { } k8s := newTestClient(server, validSBC) - result, err := resolveServer(ctx, k8s, "default", []string{"deleted-sbc", "valid-sbc"}) + owners := []sbcRef{ + {namespace: "default", name: "deleted-sbc"}, + {namespace: "default", name: "valid-sbc"}, + } + result, err := resolveServer(ctx, k8s, owners) Expect(err).NotTo(HaveOccurred()) Expect(result.Name).To(Equal("server-1")) }) It("returns an error when no SBC can be resolved", func() { k8s := newTestClient() - _, err := resolveServer(ctx, k8s, "default", []string{"gone-sbc", ""}) + owners := []sbcRef{ + {namespace: "default", name: "gone-sbc"}, + {namespace: "default", name: ""}, + } + _, err := resolveServer(ctx, k8s, owners) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("resolvable ServerBootConfiguration owner")) }) diff --git a/server/config_selector.go b/server/config_selector.go index 61292db..0f06401 100644 --- a/server/config_selector.go +++ b/server/config_selector.go @@ -15,6 +15,16 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +// sbcRef identifies an owning ServerBootConfiguration by namespace and name. +type sbcRef struct { + namespace string + name string +} + +func (r sbcRef) key() string { + return r.namespace + "/" + r.name +} + // selectBootConfig picks the correct boot config when multiple configs match // the same server. It resolves the owning Server, filters out orphaned configs, // and prefers the maintenance config during maintenance. T must implement @@ -28,11 +38,12 @@ func selectBootConfig[T client.Object](ctx context.Context, k8sClient client.Cli return items[0], nil } log.Info("Multiple boot configs found, resolving preferred config", "count", len(items)) - sbcNames := make([]string, len(items)) + owners := make([]sbcRef, len(items)) for i := range items { - sbcNames[i] = ownerSBCName(items[i].GetOwnerReferences()) + name := ownerSBCName(items[i].GetOwnerReferences()) + owners[i] = sbcRef{namespace: items[i].GetNamespace(), name: name} } - idx, err := preferredBootConfigIndex(ctx, k8sClient, log, items[0].GetNamespace(), sbcNames) + idx, err := preferredBootConfigIndex(ctx, k8sClient, log, owners) if err != nil { return zero, err } @@ -65,38 +76,41 @@ func ownerSBCName(refs []metav1.OwnerReference) string { // ServerBootConfiguration, then filters out configs whose owner SBC is not // recognized by the Server's bootConfigurationRef or maintenanceBootConfigurationRef. // Among recognized configs, it prefers the maintenance config if the server is -// in maintenance. -func preferredBootConfigIndex(ctx context.Context, k8sClient client.Client, log logr.Logger, namespace string, sbcNames []string) (int, error) { +// in maintenance. Each owner carries its own namespace so cross-namespace items +// are handled correctly. +func preferredBootConfigIndex(ctx context.Context, k8sClient client.Client, log logr.Logger, owners []sbcRef) (int, error) { // Find the Server by looking up any SBC that owns one of the configs. // All configs target the same server (queried by UUID/IP), so any valid // SBC will lead to the same Server. - server, err := resolveServer(ctx, k8sClient, namespace, sbcNames) + server, err := resolveServer(ctx, k8sClient, owners) if err != nil { return 0, fmt.Errorf("failed to resolve Server from boot configs: %w", err) } - // Build the set of SBC names the Server recognizes. + // Build the set of namespace/name keys the Server recognizes. recognized := make(map[string]bool, 2) if server.Spec.BootConfigurationRef != nil { - recognized[server.Spec.BootConfigurationRef.Name] = true + ref := sbcRef{namespace: server.Spec.BootConfigurationRef.Namespace, name: server.Spec.BootConfigurationRef.Name} + recognized[ref.key()] = true } if server.Spec.MaintenanceBootConfigurationRef != nil { - recognized[server.Spec.MaintenanceBootConfigurationRef.Name] = true + ref := sbcRef{namespace: server.Spec.MaintenanceBootConfigurationRef.Namespace, name: server.Spec.MaintenanceBootConfigurationRef.Name} + recognized[ref.key()] = true } // Filter items to only those whose owner SBC is recognized by the Server. // Anything else is an orphan from a failed cleanup or a manual creation. var validIndices []int - for i, name := range sbcNames { - if name != "" && recognized[name] { + for i, owner := range owners { + if owner.name != "" && recognized[owner.key()] { validIndices = append(validIndices, i) } else { - log.Info("Discarding orphaned boot config", "index", i, "ownerSBC", name, "server", server.Name) + log.Info("Discarding orphaned boot config", "index", i, "ownerSBC", owner.key(), "server", server.Name) } } if len(validIndices) == 0 { - return 0, fmt.Errorf("all %d boot configs are orphaned — none match Server %q boot configuration refs", len(sbcNames), server.Name) + return 0, fmt.Errorf("all %d boot configs are orphaned — none match Server %q boot configuration refs", len(owners), server.Name) } if len(validIndices) == 1 { @@ -105,10 +119,13 @@ func preferredBootConfigIndex(ctx context.Context, k8sClient client.Client, log // Multiple valid configs: prefer the maintenance one if the server is in maintenance. if server.Spec.MaintenanceBootConfigurationRef != nil { - maintenanceSBCName := server.Spec.MaintenanceBootConfigurationRef.Name + maintenanceKey := (sbcRef{ + namespace: server.Spec.MaintenanceBootConfigurationRef.Namespace, + name: server.Spec.MaintenanceBootConfigurationRef.Name, + }).key() for _, i := range validIndices { - if sbcNames[i] == maintenanceSBCName { - log.Info("Selecting maintenance boot config", "maintenanceSBC", maintenanceSBCName, "server", server.Name) + if owners[i].key() == maintenanceKey { + log.Info("Selecting maintenance boot config", "maintenanceSBC", maintenanceKey, "server", server.Name) return i, nil } } @@ -116,39 +133,43 @@ func preferredBootConfigIndex(ctx context.Context, k8sClient client.Client, log // Fall back to the workload config. if server.Spec.BootConfigurationRef != nil { - workloadSBCName := server.Spec.BootConfigurationRef.Name + workloadKey := (sbcRef{ + namespace: server.Spec.BootConfigurationRef.Namespace, + name: server.Spec.BootConfigurationRef.Name, + }).key() for _, i := range validIndices { - if sbcNames[i] == workloadSBCName { + if owners[i].key() == workloadKey { return i, nil } } } - // Should not be reachable: validIndices only contains indices whose sbcName - // is in the recognized set, and the loops above cover both recognized names. + // Should not be reachable: validIndices only contains indices whose owner + // is in the recognized set, and the loops above cover both recognized keys. return 0, fmt.Errorf("unexpected state: %d valid boot configs but none matched Server %q refs", len(validIndices), server.Name) } // resolveServer finds the Server that the boot configs target by looking up -// any owning ServerBootConfiguration and following its serverRef. -func resolveServer(ctx context.Context, k8sClient client.Client, namespace string, sbcNames []string) (*metalv1alpha1.Server, error) { - for _, name := range sbcNames { - if name == "" { +// any owning ServerBootConfiguration and following its serverRef. Each owner +// carries its own namespace for correct cross-namespace lookups. +func resolveServer(ctx context.Context, k8sClient client.Client, owners []sbcRef) (*metalv1alpha1.Server, error) { + for _, owner := range owners { + if owner.name == "" { continue } sbc := &metalv1alpha1.ServerBootConfiguration{} - if err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, sbc); err != nil { + if err := k8sClient.Get(ctx, types.NamespacedName{Name: owner.name, Namespace: owner.namespace}, sbc); err != nil { if apierrors.IsNotFound(err) { // This SBC has been deleted (orphaned child). Try the next one. continue } - return nil, fmt.Errorf("failed to get ServerBootConfiguration %q: %w", name, err) + return nil, fmt.Errorf("failed to get ServerBootConfiguration %q: %w", owner.key(), err) } server := &metalv1alpha1.Server{} if err := k8sClient.Get(ctx, types.NamespacedName{Name: sbc.Spec.ServerRef.Name}, server); err != nil { - return nil, fmt.Errorf("failed to get Server %q referenced by ServerBootConfiguration %q: %w", sbc.Spec.ServerRef.Name, name, err) + return nil, fmt.Errorf("failed to get Server %q referenced by ServerBootConfiguration %q: %w", sbc.Spec.ServerRef.Name, owner.key(), err) } return server, nil } - return nil, fmt.Errorf("none of the %d boot configs have a resolvable ServerBootConfiguration owner", len(sbcNames)) + return nil, fmt.Errorf("none of the %d boot configs have a resolvable ServerBootConfiguration owner", len(owners)) } From 6cd15020eeadb2018e4661dac9b3061dc4b05322 Mon Sep 17 00:00:00 2001 From: Andrew Dodds Date: Tue, 21 Apr 2026 09:49:38 +0000 Subject: [PATCH 6/8] Fix stale server refs --- server/bootserver_test.go | 28 ++++++++++++++++++++++++++++ server/config_selector.go | 9 +++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/server/bootserver_test.go b/server/bootserver_test.go index ed8a887..7ee80fc 100644 --- a/server/bootserver_test.go +++ b/server/bootserver_test.go @@ -394,5 +394,33 @@ var _ = Describe("ConfigSelector", func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("resolvable ServerBootConfiguration owner")) }) + + It("skips SBCs whose Server no longer exists and resolves via the next one", func() { + // First SBC exists but points to a deleted Server; second resolves. + staleSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "stale-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "deleted-server"}, + }, + } + server := &metalv1alpha1.Server{ + ObjectMeta: v1.ObjectMeta{Name: "server-1"}, + } + validSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "valid-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + k8s := newTestClient(staleSBC, server, validSBC) + + owners := []sbcRef{ + {namespace: "default", name: "stale-sbc"}, + {namespace: "default", name: "valid-sbc"}, + } + result, err := resolveServer(ctx, k8s, owners) + Expect(err).NotTo(HaveOccurred()) + Expect(result.Name).To(Equal("server-1")) + }) }) }) diff --git a/server/config_selector.go b/server/config_selector.go index 0f06401..02aabb9 100644 --- a/server/config_selector.go +++ b/server/config_selector.go @@ -61,10 +61,11 @@ func toPointers[T any](items []T) []*T { } // ownerSBCName extracts the ServerBootConfiguration name from an object's -// owner references. +// owner references, matching on both API version and kind to avoid false +// matches from other API groups. func ownerSBCName(refs []metav1.OwnerReference) string { for _, ref := range refs { - if ref.Kind == "ServerBootConfiguration" { + if ref.APIVersion == metalv1alpha1.GroupVersion.String() && ref.Kind == "ServerBootConfiguration" { return ref.Name } } @@ -167,6 +168,10 @@ func resolveServer(ctx context.Context, k8sClient client.Client, owners []sbcRef } server := &metalv1alpha1.Server{} if err := k8sClient.Get(ctx, types.NamespacedName{Name: sbc.Spec.ServerRef.Name}, server); err != nil { + if apierrors.IsNotFound(err) { + // The Server referenced by this SBC no longer exists. Try the next owner. + continue + } return nil, fmt.Errorf("failed to get Server %q referenced by ServerBootConfiguration %q: %w", sbc.Spec.ServerRef.Name, owner.key(), err) } return server, nil From c38d09c9ae4a421cc42a284816b4d85034e3c687 Mon Sep 17 00:00:00 2001 From: Andrew Dodds Date: Thu, 23 Apr 2026 08:35:38 +0000 Subject: [PATCH 7/8] Include metalv1alpha1.ServerStateMaintenance in server status check --- server/bootserver_test.go | 44 +++++++++++++++++++++++++++++++++++++++ server/config_selector.go | 8 ++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/server/bootserver_test.go b/server/bootserver_test.go index 7ee80fc..f135b1c 100644 --- a/server/bootserver_test.go +++ b/server/bootserver_test.go @@ -142,6 +142,9 @@ var _ = Describe("ConfigSelector", func() { BootConfigurationRef: &metalv1alpha1.ObjectReference{Name: "workload-sbc", Namespace: "default"}, MaintenanceBootConfigurationRef: &metalv1alpha1.ObjectReference{Name: "maintenance-sbc", Namespace: "default"}, }, + Status: metalv1alpha1.ServerStatus{ + State: metalv1alpha1.ServerStateMaintenance, + }, } workloadSBC := &metalv1alpha1.ServerBootConfiguration{ ObjectMeta: v1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, @@ -218,6 +221,9 @@ var _ = Describe("ConfigSelector", func() { Name: "maintenance-sbc", Namespace: "default", }, }, + Status: metalv1alpha1.ServerStatus{ + State: metalv1alpha1.ServerStateMaintenance, + }, } workloadSBC := &metalv1alpha1.ServerBootConfiguration{ ObjectMeta: v1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, @@ -242,6 +248,44 @@ var _ = Describe("ConfigSelector", func() { Expect(owners[idx].name).To(Equal("maintenance-sbc")) }) + It("selects the workload config when MaintenanceBootConfigurationRef is set but server is not in maintenance state", func() { + server := &metalv1alpha1.Server{ + ObjectMeta: v1.ObjectMeta{Name: "server-1"}, + Spec: metalv1alpha1.ServerSpec{ + BootConfigurationRef: &metalv1alpha1.ObjectReference{ + Name: "workload-sbc", Namespace: "default", + }, + MaintenanceBootConfigurationRef: &metalv1alpha1.ObjectReference{ + Name: "maintenance-sbc", Namespace: "default", + }, + }, + Status: metalv1alpha1.ServerStatus{ + State: metalv1alpha1.ServerStateReserved, // Not in Maintenance! + }, + } + workloadSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "workload-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + maintenanceSBC := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: v1.ObjectMeta{Name: "maintenance-sbc", Namespace: "default"}, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "server-1"}, + }, + } + k8s := newTestClient(server, workloadSBC, maintenanceSBC) + + owners := []sbcRef{ + {namespace: "default", name: "workload-sbc"}, + {namespace: "default", name: "maintenance-sbc"}, + } + idx, err := preferredBootConfigIndex(ctx, k8s, log, owners) + Expect(err).NotTo(HaveOccurred()) + Expect(owners[idx].name).To(Equal("workload-sbc")) // Falls back to workload! + }) + It("selects the workload config when server is not in maintenance", func() { server := &metalv1alpha1.Server{ ObjectMeta: v1.ObjectMeta{Name: "server-1"}, diff --git a/server/config_selector.go b/server/config_selector.go index 02aabb9..a51f59e 100644 --- a/server/config_selector.go +++ b/server/config_selector.go @@ -119,7 +119,7 @@ func preferredBootConfigIndex(ctx context.Context, k8sClient client.Client, log } // Multiple valid configs: prefer the maintenance one if the server is in maintenance. - if server.Spec.MaintenanceBootConfigurationRef != nil { + if server.Status.State == metalv1alpha1.ServerStateMaintenance && server.Spec.MaintenanceBootConfigurationRef != nil { maintenanceKey := (sbcRef{ namespace: server.Spec.MaintenanceBootConfigurationRef.Namespace, name: server.Spec.MaintenanceBootConfigurationRef.Name, @@ -153,6 +153,12 @@ func preferredBootConfigIndex(ctx context.Context, k8sClient client.Client, log // resolveServer finds the Server that the boot configs target by looking up // any owning ServerBootConfiguration and following its serverRef. Each owner // carries its own namespace for correct cross-namespace lookups. +// +// Note: This returns the first resolvable Server without validating that all +// owners point to the same Server. The query contract (boot configs matched by +// systemUUID or systemIPs) makes cross-server matches extremely unlikely in +// practice. Adding validation would catch only manual misconfigurations while +// adding complexity and latency to every boot request. func resolveServer(ctx context.Context, k8sClient client.Client, owners []sbcRef) (*metalv1alpha1.Server, error) { for _, owner := range owners { if owner.name == "" { From 87a3f7c9ec529a893ccc2075b7ed7dcf26f80b62 Mon Sep 17 00:00:00 2001 From: Andrew Dodds Date: Fri, 24 Apr 2026 08:52:03 +0000 Subject: [PATCH 8/8] Rename config_selector.go to helpler.go --- server/{config_selector.go => helper.go} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename server/{config_selector.go => helper.go} (99%) diff --git a/server/config_selector.go b/server/helper.go similarity index 99% rename from server/config_selector.go rename to server/helper.go index a51f59e..45960d1 100644 --- a/server/config_selector.go +++ b/server/helper.go @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors // SPDX-License-Identifier: Apache-2.0 package server