From 65a45309880e8756abd2f8515eb9c1a388992923 Mon Sep 17 00:00:00 2001 From: Stefan Hipfel Date: Wed, 16 Oct 2024 10:33:44 +0200 Subject: [PATCH] Extract storage details of a `Server` using Redfish (#143) * adds storage details to server crd * uses k8s resource quantity * removes unused types * minor fix * use pointer for Capacity; load storages in discovery * adds missing comment * Regenerate api reference docs --------- Co-authored-by: Andreas Fritzler --- api/v1alpha1/server_types.go | 36 +++++ api/v1alpha1/zz_generated.deepcopy.go | 27 ++++ bmc/bmc.go | 19 +++ bmc/redfish.go | 50 ++++++ .../crd/bases/metal.ironcore.dev_servers.yaml | 34 ++++ docs/api-reference/api.md | 149 ++++++++++++++++++ internal/controller/server_controller.go | 24 +++ internal/controller/server_controller_test.go | 10 ++ 8 files changed, 349 insertions(+) diff --git a/api/v1alpha1/server_types.go b/api/v1alpha1/server_types.go index aac6906..6d8c3f7 100644 --- a/api/v1alpha1/server_types.go +++ b/api/v1alpha1/server_types.go @@ -5,6 +5,7 @@ package v1alpha1 import ( v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -142,6 +143,20 @@ const ( OffIndicatorLED IndicatorLED = "Off" ) +// StorageState represents Storage states +type StorageState string + +const ( + // StorageStateEnabled indicates that the storage device is enabled. + StorageStateEnabled StorageState = "Enabled" + + // StorageStateDisabled indicates that the storage device is disabled. + StorageStateDisabled StorageState = "Disabled" + + // StorageStateAbsent indicates that the storage device is absent. + StorageStateAbsent StorageState = "Absent" +) + // ServerStatus defines the observed state of Server. type ServerStatus struct { // Manufacturer is the name of the server manufacturer. @@ -168,6 +183,9 @@ type ServerStatus struct { // NetworkInterfaces is a list of network interfaces associated with the server. NetworkInterfaces []NetworkInterface `json:"networkInterfaces,omitempty"` + // Storages is a list of storages associated with the server. + Storages []Storage `json:"storages,omitempty"` + BIOS BIOSSettings `json:"BIOS,omitempty"` // Conditions represents the latest available observations of the server's current state. @@ -192,6 +210,24 @@ type NetworkInterface struct { MACAddress string `json:"macAddress"` } +// Storage defines the details of one storage device +type Storage struct { + // Name is the name of the storage interface. + Name string `json:"name,omitempty"` + // Rotational specifies whether the storage device is rotational. + Rotational bool `json:"rotational,omitempty"` + // Type specifies the type of the storage device. + Type string `json:"type,omitempty"` + // SizeBytes specifies the size of the storage device in bytes. + Capacity *resource.Quantity `json:"capacity,omitempty"` + // Vendor specifies the vendor of the storage device. + Vendor string `json:"vendor,omitempty"` + // Model specifies the model of the storage device. + Model string `json:"model,omitempty"` + // State specifies the state of the storage device. + State StorageState `json:"state,omitempty"` +} + //+kubebuilder:object:root=true //+kubebuilder:subresource:status //+kubebuilder:resource:scope=Cluster diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 24bd8c0..bedf466 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -708,6 +708,13 @@ func (in *ServerStatus) DeepCopyInto(out *ServerStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Storages != nil { + in, out := &in.Storages, &out.Storages + *out = make([]Storage, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } in.BIOS.DeepCopyInto(&out.BIOS) if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions @@ -727,3 +734,23 @@ func (in *ServerStatus) DeepCopy() *ServerStatus { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Storage) DeepCopyInto(out *Storage) { + *out = *in + if in.Capacity != nil { + in, out := &in.Capacity, &out.Capacity + x := (*in).DeepCopy() + *out = &x + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Storage. +func (in *Storage) DeepCopy() *Storage { + if in == nil { + return nil + } + out := new(Storage) + in.DeepCopyInto(out) + return out +} diff --git a/bmc/bmc.go b/bmc/bmc.go index 300092e..67b478c 100644 --- a/bmc/bmc.go +++ b/bmc/bmc.go @@ -46,6 +46,8 @@ type BMC interface { GetBiosVersion(systemUUID string) (string, error) SetBootOrder(systemUUID string, order []string) error + + GetStorages(systemUUID string) ([]Storage, error) } type Bios struct { @@ -105,6 +107,23 @@ type Server struct { SerialNumber string } +type Storage struct { + // Name is the name of the storage interface. + Name string `json:"name,omitempty"` + // Rotational specifies whether the storage device is rotational. + Rotational bool `json:"rotational,omitempty"` + // Type specifies the type of the storage device. + Type redfish.FormFactor `json:"type,omitempty"` + // SizeBytes specifies the size of the storage device in bytes. + SizeBytes int64 `json:"sizeBytes,omitempty"` + // Vendor specifies the vendor of the storage device. + Vendor string `json:"vendor,omitempty"` + // Model specifies the model of the storage device. + Model string `json:"model,omitempty"` + // State specifies the state of the storage device. + State common.State `json:"state,omitempty"` +} + // PowerState is the power state of the system. type PowerState string diff --git a/bmc/redfish.go b/bmc/redfish.go index e458e35..c6e45c9 100644 --- a/bmc/redfish.go +++ b/bmc/redfish.go @@ -342,6 +342,56 @@ func (r *RedfishBMC) checkBiosAttributes(attrs map[string]string) (reset bool, e return } +func (r *RedfishBMC) GetStorages(systemUUID string) ([]Storage, error) { + system, err := r.getSystemByUUID(systemUUID) + if err != nil { + return nil, err + } + systemStorage, err := system.Storage() + if err != nil { + return nil, err + } + result := make([]Storage, 0, len(systemStorage)) + for _, s := range systemStorage { + drives, err := s.Drives() + if err != nil { + return nil, err + } + for _, d := range drives { + result = append(result, Storage{ + Name: d.Name, + Rotational: d.RotationSpeedRPM != 0, + Type: d.DriveFormFactor, + SizeBytes: d.CapacityBytes, + Vendor: d.Manufacturer, + Model: d.Model, + State: d.Status.State, + }) + } + } + if len(result) == 0 { + // if no storage is found, fall back to simpleStorage (outdated storage API) + simpleStorages, err := system.SimpleStorages() + result = make([]Storage, 0, len(systemStorage)) + if err != nil { + return nil, err + } + for _, s := range simpleStorages { + for _, d := range s.Devices { + result = append(result, Storage{ + Name: d.Name, + SizeBytes: d.CapacityBytes, + Vendor: d.Manufacturer, + Model: d.Model, + State: d.Status.State, + }) + } + } + return result, nil + } + return result, nil +} + func (r *RedfishBMC) getSystemByUUID(systemUUID string) (*redfish.ComputerSystem, error) { service := r.client.GetService() systems, err := service.Systems() diff --git a/config/crd/bases/metal.ironcore.dev_servers.yaml b/config/crd/bases/metal.ironcore.dev_servers.yaml index 491c962..50265a7 100644 --- a/config/crd/bases/metal.ironcore.dev_servers.yaml +++ b/config/crd/bases/metal.ironcore.dev_servers.yaml @@ -400,6 +400,40 @@ spec: state: description: State represents the current state of the server. type: string + storages: + description: Storages is a list of storages associated with the server. + items: + description: Storage defines the details of one storage device + properties: + capacity: + anyOf: + - type: integer + - type: string + description: SizeBytes specifies the size of the storage device + in bytes. + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + model: + description: Model specifies the model of the storage device. + type: string + name: + description: Name is the name of the storage interface. + type: string + rotational: + description: Rotational specifies whether the storage device + is rotational. + type: boolean + state: + description: State specifies the state of the storage device. + type: string + type: + description: Type specifies the type of the storage device. + type: string + vendor: + description: Vendor specifies the vendor of the storage device. + type: string + type: object + type: array type: object type: object served: true diff --git a/docs/api-reference/api.md b/docs/api-reference/api.md index b605de6..818befe 100644 --- a/docs/api-reference/api.md +++ b/docs/api-reference/api.md @@ -1952,6 +1952,17 @@ string +model
+ +string + + + +

Model is the model of the server.

+ + + + sku
string @@ -2026,6 +2037,19 @@ ServerState +storages
+ + +[]Storage + + + + +

Storages is a list of storages associated with the server.

+ + + + BIOS
@@ -2052,6 +2076,131 @@ BIOSSettings +

Storage +

+

+(Appears on:ServerStatus) +

+
+

Storage defines the details of one storage device

+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+name
+ +string + +		 +

Name is the name of the storage interface.

+
+rotational
+ +bool + +		 +

Rotational specifies whether the storage device is rotational.

+
+type
+ +string + +		 +

Type specifies the type of the storage device.

+
+capacity
+ + +k8s.io/apimachinery/pkg/api/resource.Quantity + + +		 +

SizeBytes specifies the size of the storage device in bytes.

+
+vendor
+ +string + +		 +

Vendor specifies the vendor of the storage device.

+
+model
+ +string + +		 +

Model specifies the model of the storage device.

+
+state
+ + +StorageState + + +		 +

State specifies the state of the storage device.

+
+

StorageState +(string alias)

+

+(Appears on:Storage) +

+
+

StorageState represents Storage states

+
+ + + + + + + + + + + + + + +
ValueDescription

"Absent"

StorageStateAbsent indicates that the storage device is absent.

+

"Disabled"

StorageStateDisabled indicates that the storage device is disabled.

+

"Enabled"

StorageStateEnabled indicates that the storage device is enabled.

+

Generated with gen-crd-api-reference-docs diff --git a/internal/controller/server_controller.go b/internal/controller/server_controller.go index cd865dd..9b677b6 100644 --- a/internal/controller/server_controller.go +++ b/internal/controller/server_controller.go @@ -24,6 +24,7 @@ import ( v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" meta "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -274,6 +275,29 @@ func (r *ServerReconciler) handleDiscoveryState(ctx context.Context, log logr.Lo } log.V(1).Info("Server state set to power on") + bmcClient, err := GetBMCClientForServer(ctx, r.Client, server, r.Insecure) + if err != nil { + return false, fmt.Errorf("failed to create BMC client: %w", err) + } + storages, err := bmcClient.GetStorages(server.Spec.UUID) + if err != nil { + return false, fmt.Errorf("failed to get storages for Server: %w", err) + } + server.Status.Storages = nil + for _, storage := range storages { + server.Status.Storages = append(server.Status.Storages, metalv1alpha1.Storage{ + Name: storage.Name, + Model: storage.Model, + Capacity: resource.NewQuantity(storage.SizeBytes, resource.BinarySI), + Type: string(storage.Type), + Vendor: storage.Vendor, + State: metalv1alpha1.StorageState(storage.State), + }) + } + if err := r.Status().Patch(ctx, server, client.MergeFrom(serverBase)); err != nil { + return false, fmt.Errorf("failed to patch Server status: %w", err) + } + if r.checkLastStatusUpdateAfter(r.DiscoveryTimeout, server) { log.V(1).Info("Server did not post info to registry in time, back to initial state") if modified, err := r.patchServerState(ctx, server, metalv1alpha1.ServerStateInitial); err != nil || modified { diff --git a/internal/controller/server_controller_test.go b/internal/controller/server_controller_test.go index f513139..522d6c2 100644 --- a/internal/controller/server_controller_test.go +++ b/internal/controller/server_controller_test.go @@ -9,6 +9,7 @@ import ( "time" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" "github.com/ironcore-dev/metal-operator/internal/controller/testdata" @@ -290,6 +291,15 @@ var _ = Describe("Server Controller", func() { HaveField("Status.State", metalv1alpha1.ServerStateAvailable), HaveField("Status.PowerState", metalv1alpha1.ServerOffPowerState), HaveField("Status.NetworkInterfaces", Not(BeEmpty())), + HaveField("Status.Storages", ContainElement(metalv1alpha1.Storage{ + Name: "SATA Bay 1", + Rotational: false, + Capacity: resource.NewQuantity(8000000000000, resource.BinarySI), + Vendor: "Contoso", + Model: "3000GT8", + State: metalv1alpha1.StorageStateEnabled, + })), + HaveField("Status.Storages", HaveLen(4)), )) By("Ensuring that the boot configuration has been removed")