From d021129bcde899b7fbfaaa69eb3c1ad7d817eca9 Mon Sep 17 00:00:00 2001
From: Nuckal777 <erik.schubert@sap.com>
Date: Thu, 5 Dec 2024 16:39:48 +0100
Subject: [PATCH] Add systemUUID to Server CRD

---
 api/v1alpha1/server_types.go                  |  4 ++
 .../crd/bases/metal.ironcore.dev_servers.yaml |  7 +++-
 docs/api-reference/api.md                     | 30 +++++++++++++--
 internal/controller/bmc_controller.go         |  5 ++-
 internal/controller/bmc_controller_test.go    |  2 +
 internal/controller/server_controller.go      | 38 +++++++++----------
 internal/controller/server_controller_test.go | 12 +++---
 .../controller/serverclaim_controller_test.go |  3 +-
 8 files changed, 70 insertions(+), 31 deletions(-)

diff --git a/api/v1alpha1/server_types.go b/api/v1alpha1/server_types.go
index 401b9a8..fd0a911 100644
--- a/api/v1alpha1/server_types.go
+++ b/api/v1alpha1/server_types.go
@@ -77,8 +77,12 @@ type BIOSSettings struct {
 // ServerSpec defines the desired state of a Server.
 type ServerSpec struct {
 	// UUID is the unique identifier for the server.
+	// Deprecated in favor of systemUUID.
 	UUID string `json:"uuid"`
 
+	// SystemUUID is the unique identifier for the server.
+	SystemUUID string `json:"systemUUID,omitempty"`
+
 	// Power specifies the desired power state of the server.
 	Power Power `json:"power,omitempty"`
 
diff --git a/config/crd/bases/metal.ironcore.dev_servers.yaml b/config/crd/bases/metal.ironcore.dev_servers.yaml
index 7294fb3..be46967 100644
--- a/config/crd/bases/metal.ironcore.dev_servers.yaml
+++ b/config/crd/bases/metal.ironcore.dev_servers.yaml
@@ -271,8 +271,13 @@ spec:
                     type: string
                 type: object
                 x-kubernetes-map-type: atomic
+              systemUUID:
+                description: SystemUUID is the unique identifier for the server.
+                type: string
               uuid:
-                description: UUID is the unique identifier for the server.
+                description: |-
+                  UUID is the unique identifier for the server.
+                  Deprecated in favor of systemUUID.
                 type: string
             required:
             - uuid
diff --git a/docs/api-reference/api.md b/docs/api-reference/api.md
index ae4c3d2..e764b4c 100644
--- a/docs/api-reference/api.md
+++ b/docs/api-reference/api.md
@@ -1215,7 +1215,19 @@ string
 </em>
 </td>
 <td>
-<p>UUID is the unique identifier for the server.</p>
+<p>UUID is the unique identifier for the server.
+Deprecated in favor of systemUUID.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>systemUUID</code><br/>
+<em>
+string
+</em>
+</td>
+<td>
+<p>SystemUUID is the unique identifier for the server.</p>
 </td>
 </tr>
 <tr>
@@ -1851,7 +1863,19 @@ string
 </em>
 </td>
 <td>
-<p>UUID is the unique identifier for the server.</p>
+<p>UUID is the unique identifier for the server.
+Deprecated in favor of systemUUID.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>systemUUID</code><br/>
+<em>
+string
+</em>
+</td>
+<td>
+<p>SystemUUID is the unique identifier for the server.</p>
 </td>
 </tr>
 <tr>
@@ -2119,7 +2143,7 @@ k8s.io/apimachinery/pkg/api/resource.Quantity
 </em>
 </td>
 <td>
-<p>TotalSystemMemoryBytes is the total amount of memory in bytes available on the server.</p>
+<p>TotalSystemMemory is the total amount of memory in bytes available on the server.</p>
 </td>
 </tr>
 <tr>
diff --git a/internal/controller/bmc_controller.go b/internal/controller/bmc_controller.go
index 273fbda..8880d86 100644
--- a/internal/controller/bmc_controller.go
+++ b/internal/controller/bmc_controller.go
@@ -169,8 +169,9 @@ func (r *BMCReconciler) discoverServers(ctx context.Context, log logr.Logger, bm
 				Name: GetServerNameFromBMCandIndex(i, bmcObj),
 			},
 			Spec: metalv1alpha1.ServerSpec{
-				UUID:   strings.ToLower(s.UUID), // always use lower-case uuids
-				BMCRef: &v1.LocalObjectReference{Name: bmcObj.Name},
+				UUID:       strings.ToLower(s.UUID), // always use lower-case uuids
+				SystemUUID: strings.ToLower(s.UUID), // always use lower-case uuids
+				BMCRef:     &v1.LocalObjectReference{Name: bmcObj.Name},
 			},
 		}
 
diff --git a/internal/controller/bmc_controller_test.go b/internal/controller/bmc_controller_test.go
index b1bcbb2..6c5c03d 100644
--- a/internal/controller/bmc_controller_test.go
+++ b/internal/controller/bmc_controller_test.go
@@ -85,6 +85,7 @@ var _ = Describe("BMC Controller", func() {
 				BlockOwnerDeletion: ptr.To(true),
 			})),
 			HaveField("Spec.UUID", "38947555-7742-3448-3784-823347823834"),
+			HaveField("Spec.SystemUUID", "38947555-7742-3448-3784-823347823834"),
 			HaveField("Spec.BMCRef.Name", endpoint.Name),
 		))
 	})
@@ -150,6 +151,7 @@ var _ = Describe("BMC Controller", func() {
 				BlockOwnerDeletion: ptr.To(true),
 			})),
 			HaveField("Spec.UUID", "38947555-7742-3448-3784-823347823834"),
+			HaveField("Spec.SystemUUID", "38947555-7742-3448-3784-823347823834"),
 			HaveField("Spec.BMCRef.Name", bmc.Name),
 		))
 	})
diff --git a/internal/controller/server_controller.go b/internal/controller/server_controller.go
index d608519..288fa87 100644
--- a/internal/controller/server_controller.go
+++ b/internal/controller/server_controller.go
@@ -276,7 +276,7 @@ func (r *ServerReconciler) handleDiscoveryState(ctx context.Context, log logr.Lo
 	if err != nil {
 		return false, fmt.Errorf("failed to create BMC client: %w", err)
 	}
-	storages, err := bmcClient.GetStorages(ctx, server.Spec.UUID)
+	storages, err := bmcClient.GetStorages(ctx, server.Spec.SystemUUID)
 	if err != nil {
 		return false, fmt.Errorf("failed to get storages for Server: %w", err)
 	}
@@ -422,7 +422,7 @@ func (r *ServerReconciler) updateServerStatus(ctx context.Context, log logr.Logg
 	}
 	defer bmcClient.Logout()
 
-	systemInfo, err := bmcClient.GetSystemInfo(ctx, server.Spec.UUID)
+	systemInfo, err := bmcClient.GetSystemInfo(ctx, server.Spec.SystemUUID)
 	if err != nil {
 		return fmt.Errorf("failed to get system info for Server: %w", err)
 	}
@@ -436,7 +436,7 @@ func (r *ServerReconciler) updateServerStatus(ctx context.Context, log logr.Logg
 	server.Status.IndicatorLED = metalv1alpha1.IndicatorLED(systemInfo.IndicatorLED)
 	server.Status.TotalSystemMemory = &systemInfo.TotalSystemMemory
 
-	currentBiosVersion, err := bmcClient.GetBiosVersion(ctx, server.Spec.UUID)
+	currentBiosVersion, err := bmcClient.GetBiosVersion(ctx, server.Spec.SystemUUID)
 	if err != nil {
 		return fmt.Errorf("failed to load bios version: %w", err)
 	}
@@ -448,7 +448,7 @@ func (r *ServerReconciler) updateServerStatus(ctx context.Context, log logr.Logg
 			for k := range bios.Settings {
 				keys = append(keys, k)
 			}
-			attributes, err := bmcClient.GetBiosAttributeValues(ctx, server.Spec.UUID, keys)
+			attributes, err := bmcClient.GetBiosAttributeValues(ctx, server.Spec.SystemUUID, keys)
 			if err != nil {
 				return fmt.Errorf("failed load bios settings: %w", err)
 			}
@@ -507,7 +507,7 @@ func (r *ServerReconciler) applyBootConfigurationAndIgnitionForDiscovery(ctx con
 }
 
 func (r *ServerReconciler) applyDefaultIgnitionForServer(ctx context.Context, log logr.Logger, server *metalv1alpha1.Server, bootConfig *metalv1alpha1.ServerBootConfiguration, registryURL string) error {
-	probeFlags := fmt.Sprintf("--registry-url=%s --server-uuid=%s", registryURL, server.Spec.UUID)
+	probeFlags := fmt.Sprintf("--registry-url=%s --server-uuid=%s", registryURL, server.Spec.SystemUUID)
 	ignitionData, err := r.generateDefaultIgnitionDataForServer(probeFlags)
 	if err != nil {
 		return fmt.Errorf("failed to generate default ignitionSecret data: %w", err)
@@ -625,14 +625,14 @@ func (r *ServerReconciler) pxeBootServer(ctx context.Context, log logr.Logger, s
 	if err != nil {
 		return fmt.Errorf("failed to get BMC client: %w", err)
 	}
-	if err := bmcClient.SetPXEBootOnce(ctx, server.Spec.UUID); err != nil {
+	if err := bmcClient.SetPXEBootOnce(ctx, server.Spec.SystemUUID); err != nil {
 		return fmt.Errorf("failed to set PXE boot one for server: %w", err)
 	}
 	return nil
 }
 
 func (r *ServerReconciler) extractServerDetailsFromRegistry(ctx context.Context, log logr.Logger, server *metalv1alpha1.Server) (bool, error) {
-	resp, err := http.Get(fmt.Sprintf("%s/systems/%s", r.RegistryURL, server.Spec.UUID))
+	resp, err := http.Get(fmt.Sprintf("%s/systems/%s", r.RegistryURL, server.Spec.SystemUUID))
 	if resp != nil && resp.StatusCode == http.StatusNotFound {
 		log.V(1).Info("Did not find server information in registry")
 		return false, nil
@@ -718,11 +718,11 @@ func (r *ServerReconciler) ensureServerPowerState(ctx context.Context, log logr.
 
 	switch powerOp {
 	case powerOpOn:
-		if err := bmcClient.PowerOn(ctx, server.Spec.UUID); err != nil {
+		if err := bmcClient.PowerOn(ctx, server.Spec.SystemUUID); err != nil {
 			return fmt.Errorf("failed to power on server: %w", err)
 		}
 		if err := bmcClient.WaitForServerPowerState(
-			ctx, server.Spec.UUID,
+			ctx, server.Spec.SystemUUID,
 			redfish.OnPowerState,
 		); err != nil {
 			return fmt.Errorf("failed to wait for server power on server: %w", err)
@@ -730,17 +730,17 @@ func (r *ServerReconciler) ensureServerPowerState(ctx context.Context, log logr.
 	case powerOpOff:
 		powerOffType := bmcClient.PowerOff
 
-		if err := powerOffType(ctx, server.Spec.UUID); err != nil {
+		if err := powerOffType(ctx, server.Spec.SystemUUID); err != nil {
 			return fmt.Errorf("failed to power off server: %w", err)
 		}
-		if err := bmcClient.WaitForServerPowerState(ctx, server.Spec.UUID, redfish.OffPowerState); err != nil {
+		if err := bmcClient.WaitForServerPowerState(ctx, server.Spec.SystemUUID, redfish.OffPowerState); err != nil {
 			if r.EnforcePowerOff {
 				log.V(1).Info("Failed to wait for server graceful shutdown, retrying with force power off")
 				powerOffType = bmcClient.ForcePowerOff
-				if err := powerOffType(ctx, server.Spec.UUID); err != nil {
+				if err := powerOffType(ctx, server.Spec.SystemUUID); err != nil {
 					return fmt.Errorf("failed to power off server: %w", err)
 				}
-				if err := bmcClient.WaitForServerPowerState(ctx, server.Spec.UUID, redfish.OffPowerState); err != nil {
+				if err := bmcClient.WaitForServerPowerState(ctx, server.Spec.SystemUUID, redfish.OffPowerState); err != nil {
 					return fmt.Errorf("failed to wait for server force power off: %w", err)
 				}
 			} else {
@@ -787,7 +787,7 @@ func (r *ServerReconciler) ensureInitialBootConfigurationIsDeleted(ctx context.C
 }
 
 func (r *ServerReconciler) invalidateRegistryEntryForServer(log logr.Logger, server *metalv1alpha1.Server) error {
-	url := fmt.Sprintf("%s/delete/%s", r.RegistryURL, server.Spec.UUID)
+	url := fmt.Sprintf("%s/delete/%s", r.RegistryURL, server.Spec.SystemUUID)
 
 	c := &http.Client{}
 
@@ -822,7 +822,7 @@ func (r *ServerReconciler) applyBootOrder(ctx context.Context, log logr.Logger,
 	}
 	defer bmcClient.Logout()
 
-	order, err := bmcClient.GetBootOrder(ctx, server.Spec.UUID)
+	order, err := bmcClient.GetBootOrder(ctx, server.Spec.SystemUUID)
 	if err != nil {
 		return fmt.Errorf("failed to create BMC client: %w", err)
 	}
@@ -839,7 +839,7 @@ func (r *ServerReconciler) applyBootOrder(ctx context.Context, log logr.Logger,
 		}
 	}
 	if change {
-		return bmcClient.SetBootOrder(ctx, server.Spec.UUID, newOrder)
+		return bmcClient.SetBootOrder(ctx, server.Spec.SystemUUID, newOrder)
 	}
 	return nil
 }
@@ -856,7 +856,7 @@ func (r *ServerReconciler) applyBiosSettings(ctx context.Context, log logr.Logge
 	}
 	defer bmcClient.Logout()
 
-	version, err := bmcClient.GetBiosVersion(ctx, server.Spec.UUID)
+	version, err := bmcClient.GetBiosVersion(ctx, server.Spec.SystemUUID)
 	if err != nil {
 		return fmt.Errorf("failed to create BMC client: %w", err)
 	}
@@ -873,7 +873,7 @@ func (r *ServerReconciler) applyBiosSettings(ctx context.Context, log logr.Logge
 					}
 				}
 			}
-			reset, err := bmcClient.SetBiosAttributes(ctx, server.Spec.UUID, diff)
+			reset, err := bmcClient.SetBiosAttributes(ctx, server.Spec.SystemUUID, diff)
 			if err != nil {
 				return err
 			}
@@ -908,7 +908,7 @@ func (r *ServerReconciler) handleAnnotionOperations(ctx context.Context, log log
 	}
 	defer bmcClient.Logout()
 	log.V(1).Info("Handling operation", "Operation", operation)
-	if err := bmcClient.Reset(ctx, server.Spec.UUID, redfish.ResetType(operation)); err != nil {
+	if err := bmcClient.Reset(ctx, server.Spec.SystemUUID, redfish.ResetType(operation)); err != nil {
 		return false, fmt.Errorf("failed to reset server: %w", err)
 	}
 	log.V(1).Info("Operation completed", "Operation", operation)
diff --git a/internal/controller/server_controller_test.go b/internal/controller/server_controller_test.go
index c09d6f8..d4c7016 100644
--- a/internal/controller/server_controller_test.go
+++ b/internal/controller/server_controller_test.go
@@ -75,6 +75,7 @@ var _ = Describe("Server Controller", func() {
 				BlockOwnerDeletion: ptr.To(true),
 			})),
 			HaveField("Spec.UUID", "38947555-7742-3448-3784-823347823834"),
+			HaveField("Spec.SystemUUID", "38947555-7742-3448-3784-823347823834"),
 			HaveField("Spec.Power", metalv1alpha1.PowerOff),
 			HaveField("Spec.IndicatorLED", metalv1alpha1.IndicatorLED("")),
 			HaveField("Spec.ServerClaimRef", BeNil()),
@@ -150,7 +151,7 @@ var _ = Describe("Server Controller", func() {
 		))
 
 		By("Starting the probe agent")
-		probeAgent := probe.NewAgent(server.Spec.UUID, registryURL, 100*time.Millisecond)
+		probeAgent := probe.NewAgent(server.Spec.SystemUUID, registryURL, 100*time.Millisecond)
 		go func() {
 			defer GinkgoRecover()
 			Expect(probeAgent.Start(ctx)).To(Succeed(), "failed to start probe agent")
@@ -168,7 +169,7 @@ var _ = Describe("Server Controller", func() {
 		Consistently(Get(bootConfig)).Should(Satisfy(apierrors.IsNotFound))
 
 		By("Ensuring that the server is removed from the registry")
-		response, err := http.Get(registryURL + "/systems/" + server.Spec.UUID)
+		response, err := http.Get(registryURL + "/systems/" + server.Spec.SystemUUID)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(response.StatusCode).To(Equal(http.StatusNotFound))
 	})
@@ -193,7 +194,8 @@ var _ = Describe("Server Controller", func() {
 				GenerateName: "server-",
 			},
 			Spec: metalv1alpha1.ServerSpec{
-				UUID: "38947555-7742-3448-3784-823347823834",
+				UUID:       "38947555-7742-3448-3784-823347823834",
+				SystemUUID: "38947555-7742-3448-3784-823347823834",
 				BMC: &metalv1alpha1.BMCAccess{
 					Protocol: metalv1alpha1.Protocol{
 						Name: metalv1alpha1.ProtocolRedfishLocal,
@@ -275,7 +277,7 @@ var _ = Describe("Server Controller", func() {
 		))
 
 		By("Starting the probe agent")
-		probeAgent := probe.NewAgent(server.Spec.UUID, registryURL, 50*time.Millisecond)
+		probeAgent := probe.NewAgent(server.Spec.SystemUUID, registryURL, 50*time.Millisecond)
 		go func() {
 			defer GinkgoRecover()
 			Expect(probeAgent.Start(ctx)).To(Succeed(), "failed to start probe agent")
@@ -327,7 +329,7 @@ var _ = Describe("Server Controller", func() {
 		Consistently(Get(bootConfig)).Should(Satisfy(apierrors.IsNotFound))
 
 		By("Ensuring that the server is removed from the registry")
-		response, err := http.Get(registryURL + "/systems/" + server.Spec.UUID)
+		response, err := http.Get(registryURL + "/systems/" + server.Spec.SystemUUID)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(response.StatusCode).To(Equal(http.StatusNotFound))
 	})
diff --git a/internal/controller/serverclaim_controller_test.go b/internal/controller/serverclaim_controller_test.go
index 046f762..88b9b91 100644
--- a/internal/controller/serverclaim_controller_test.go
+++ b/internal/controller/serverclaim_controller_test.go
@@ -40,7 +40,8 @@ var _ = Describe("ServerClaim Controller", func() {
 				GenerateName: "test-",
 			},
 			Spec: metalv1alpha1.ServerSpec{
-				UUID: "38947555-7742-3448-3784-823347823834",
+				UUID:       "38947555-7742-3448-3784-823347823834",
+				SystemUUID: "38947555-7742-3448-3784-823347823834",
 				BMC: &metalv1alpha1.BMCAccess{
 					Protocol: metalv1alpha1.Protocol{
 						Name: metalv1alpha1.ProtocolRedfishLocal,