[TomerKarniol]

What this PR does / why we need it:

ComputerSystem already links to /Memory, /Storage and /EthernetInterfaces, but the router had no handlers for them. Every one of those URLs fell through and returned the plain ComputerSystem body with HTTP 200, so Redfish clients that walk those links (Ironic/sushy, inventory tooling) got no hardware data.

This PR implements them read-only:

Path	Returns
/Systems/{id}/Processors[/CPU{n}]	One processor per socket: Socket, TotalCores, TotalThreads, Model, ProcessorArchitecture, InstructionSet
/Systems/{id}/Memory[/1]	One memory module with CapacityMiB
/Systems/{id}/Storage[/1], .../Drives/{disk}	All disks of the VM (CD-ROMs excluded, they are VirtualMedia) with CapacityBytes where declared
/Systems/{id}/EthernetInterfaces[/{nic}]	Every NIC: MACAddress, LinkStatus (LinkUp while the VM runs and the link is up, NoLink when it is stopped or KubeVirt reports the link as down), IPv4 and IPv6 addresses

Unknown members return 404 and non-GET requests get 405 with Allow: GET. That includes PATCH and the Reset action path below these resources: both used to be caught earlier in handleSystem (every PATCH under a system was treated as a boot update, and the Reset check matched any path containing /Actions/ComputerSystem.Reset), so PATCH /Systems/{id}/Memory/1 answered 200 and POST /Systems/{id}/Memory/1/Actions/ComputerSystem.Reset reached the power handler. The hardware resources are now routed first and the Reset action only matches /Systems/{id}/Actions/ComputerSystem.Reset. The existing access check (resolveSystemVMandCheckAccess) is reused. If the cluster cannot be read (for example the controller is not allowed to read PVCs), the endpoint answers 500 and logs the reason, instead of returning a made-up answer.

It also fixes inaccuracies in the existing ComputerSystem output:

1. CPU count ignored sockets and threads. cores: 2, sockets: 2 reported Count: 2. ProcessorSummary now follows the Redfish definitions: Count = sockets, CoreCount = sockets x cores, LogicalProcessorCount = sockets x cores x threads (the vCPU count), plus ThreadingEnabled and the configured Model (domain.cpu.model, omitted when unset). The exact layout is under Oem.KubeVirt.Processors. If the topology cannot be read, these fields are left out instead of showing a guessed 1/1/1. An existing test that expected sockets to be ignored is updated.
2. Memory fell back to an invented 2 GiB. Only domain.memory.guest was read, so a VM that sets resources.requests.memory got 2 GiB. It now reads guest, then requests, then limits (see "Running and stopped VMs" below for which spec). If none is set the field is omitted instead of guessed, and a failed VM lookup returns an error instead of being swallowed.
3. Memory was a non-standard MemorySummary containing an @odata.id. Memory is now a link to the collection and the total moved to the standard MemorySummary.
4. @odata.type versions were older than the properties they carry. Everything was v1_0_0. The versions are now ComputerSystem.v1_15_0, Memory.v1_1_0, EthernetInterface.v1_1_0 and VirtualMedia.v1_2_0 (needed for InsertMedia/EjectMedia), checked with the DMTF Redfish Service Validator. Processor, Storage and Drive stay at v1_0_0, which already defines everything they emit.

Notes and things to know:

• CapacityBytes / CapacityMiB are the only capacity fields the Redfish schemas define and clients read them. A human-readable GiB copy is added under Oem.KubeVirt.CapacityGiB for drives and memory.
• Disk size comes from the PVC (status.capacity, then spec.requests), then the DataVolume template, or emptyDisk/hostDisk. A containerDisk or cloud-init disk has no declared size, so none is shown.
• A drive request reads only that disk's size (one PVC lookup) and the drive list needs none, so the cost does not grow with the number of disks. A 24-disk VM took about 4.7 s per request when every disk was looked up and about 0.5 s now.
• If a PVC cannot be read for any reason other than "not found" (forbidden, timeout), the drive request answers 500 instead of falling back to another size. The same applies to the VMI: only "no VMI" (or a VMI in a final phase) means the VM is stopped, any other failure to read it is a 500. It is a 500 and not a 403 on purpose: the caller already passed the access check, and it is the controller's own service account that lacks the permission, which is a server fault the client cannot fix.
• NIC MAC, IPs and link state come from the running VMI when present, otherwise the MAC comes from the VM spec. SpeedMbps is intentionally omitted: virtual NICs have no link speed. A NIC the VMI reports as linkState: down is NoLink; a running NIC with no reported state is LinkUp.
• Model is the configured virtual CPU model, not a hardware model; KubeVirt does not expose the node's physical CPU here.
• ProcessorArchitecture / InstructionSet come from spec.architecture of the VMI, or of the VM when it is stopped (amd64 -> x86 / x86-64, arm64 -> ARM / ARM-A64). The Redfish values are fixed enums, so the mapping is needed; an unknown architecture (for example s390x, which has no Redfish value) is omitted, so a new platform never produces a wrong value. If no architecture is declared (KubeVirt normally fills it in on creation), both fields are omitted rather than guessed. sushy builds its summary architecture from these fields.
• Running and stopped VMs. The VM spec holds the staged values, the VMI is what the guest runs now, so a running VM reports its VMI and a stopped VM its VM spec:
  • CPU: status.currentCPUTopology, then the VMI spec, then the VM spec.
  • Memory: status.memory.guestCurrent, then the VMI spec, then the VM spec.
  • Disks and NICs: the VMI spec, then the VM spec. A volume hot-plugged without --persist is listed; one only staged in the VM is not.
  • A change that cannot be applied live (the VM then has RestartRequired) and a hotplug in progress do not show up before the guest has them. A VMI in a final phase (Succeeded/Failed, which KubeVirt can leave behind after a shutdown) counts as stopped.
• The vCPU layout is under the system-level Oem because ProcessorSummary has no Oem property in any schema version.
• New code lives in pkg/kubevirt/devices.go and pkg/server/system_devices.go to avoid growing client.go / server.go further. GetVMCPU is removed (nothing used it any more) and the test mock now returns a typed NotFound like the real API.
• Breaking changes:
  • Memory in the ComputerSystem body changes from {@odata.id, TotalSystemMemoryGiB} to a plain link; the total moves to MemorySummary.TotalSystemMemoryGiB.
  • ProcessorSummary.Count now means the number of sockets (it used to be the number of cores). A 1-socket, 4-core VM goes from Count: 4 to Count: 1 with CoreCount: 4 and LogicalProcessorCount: 4.
  • Clients reading the old shapes need to move to the new fields.

Testing:

Unit tests and checks:

• Tests for the running/stopped rule: VMI over VM for CPU, memory, architecture, disks and NICs, hotplug status (sockets, cores and threads, memory), nil status fields, VMI in a final phase, hot-plugged disks, link state, and a VMI that cannot be read. I broke each of these behaviours and the 405 routing in turn and checked that a test fails.
• make fmt, make validate and make test pass. New tests cover CPU topology, model and architecture, memory sources, disk capacity (PVC, DataVolume template, emptyDisk including size 0, containerDisk, LUN), the number of PVC lookups for a 24-disk VM, forbidden / timeout / server error / not found for PVC and VMI reads, users of another chassis and unauthenticated requests on every new path, a running NIC with IPv4 and IPv6, VMs that declare nothing (empty collections are [], not null), the schema versions, and every endpoint including 404s and method checks.
• I re-introduced the old behaviour (errors swallowed, access check removed) to confirm the new tests fail without the fixes.
• golangci-lint reports no new issues. I used a v2-format equivalent of the config: the repo's .golangci.yml is in the v1 format and does not load in current golangci-lint, which is probably worth a separate fix.
• go test -race passes for pkg/kubevirt, pkg/redfish and the new tests. pkg/server still fails under -race: data races in connection_pool.go, memory_monitor.go, job_scheduler.go and the logger, and TestServerShutdown times out. The same tests fail the same way on current main (2e38e46), so they are not caused by this PR.

Manual tests, kind (KubeVirt v1.9.0, software emulation):

• 1 and 2x2 CPU topologies, emptyDisk, PVC and cloud-init disks and a fixed MAC.
• Running VMs reported LinkUp, the KubeVirt-assigned MAC and an IPv4 address; stopped VMs reported NoLink.

Manual tests, OpenShift 4.20 with OpenShift Virtualization 4.20 (software emulation), controller run locally against the cluster:

• VM layouts: a containerDisk with a CDI DataVolume disk and cloud-init (the DataVolume disk reported its real 2 GiB from the PVC), 2 sockets x 2 cores with host-model and a fixed MAC, LUN / Filesystem-mode / Block-mode PVCs with a CD-ROM (left out of Storage), 24 PVC-backed disks, and a VM that declares no disks, NICs, CPU or memory.
• threads: 2 set on a running VM and restarted through the Redfish ForceRestart: ThreadingEnabled: true, LogicalProcessorCount: 8 and each socket 2 cores / 4 threads.
• Both power states (the OpenShift MAC pool gives stopped VMs a MAC too), 404 for unknown members, 405 for a non-GET request, and the enhanced system ID convention (namespace.vm) on every new path.
• Two chassis and two users: each user only sees their own VMs and gets 404 for the other's on all new paths, and missing or wrong credentials get 401. vm_selector by labels and by names behaves the same way.
• sushy 5.14.0 (the client Ironic uses) read power state, processors (sockets, cores, threads, architecture), storage drives with capacities and Ethernet interfaces without errors.
• DMTF Redfish Service Validator 3.1.8 on a VM's resource tree: 152 pass / 12 fail before the schema version changes, 168 pass / 1 fail after. The same single failure shows on the other VMs: a stray members_identities key in the VirtualMedia collection that is already on main. The warnings are the deprecated /Systems/{id}/Storage/{id}/Drives/... URI.
• 630 requests with 40 in parallel and about 60 odd requests (//, dot segments, encoded and very long names, odd methods): all answered, no panics and no 500s.
• Helm chart installed in the cluster (image built from this branch) and tested through its Route over HTTPS: sizes were read from the PVCs with the chart's ClusterRole. With get on PVCs removed from the role, PVC-backed drives answered 500 with the forbidden PVC in the log, while the drive list and emptyDisk drives still worked. Everything was removed afterwards.

Manual tests of the running/stopped rule and the routing, same OpenShift cluster, controller built from this branch and installed with the Helm chart:

• A staged edit that cannot be applied live (cores, CPU model, memory) on a running VM: RestartRequired is set and the endpoints keep the VMI values; after a stop they show the VM spec; after a restart the new values.
• Live CPU hotplug (1 to 3 sockets, real migration) polled throughout: only the old and the new value appear, nothing in between. Memory hotplug reports guestCurrent, so it stays at the old value until the guest has the memory.
• A VM with runStrategy: Once whose QEMU process was killed: the VMI stays in Failed, the VM spec is edited, and the endpoints show the VM spec and NoLink.
• PATCH, POST, PUT and DELETE on every hardware path answer 405; POST of the Reset action below a hardware path answers 405, and the real action path still reaches the power handler.
• About 60 odd paths (CPU-1, CPU0x1, CPU00, encoded dots and NULs, a 5000-character name, query strings) all answer 404 or 400, never 500; every link in the resource tree of three VMs resolves; deleting the VMI, halting and deleting the VM while polling gave 200 and then 404, with no 500.

[kubevirt-prow]

Hi @TomerKarniol. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[MarSik]

Is this accounting for cpu and memory hotplug OR at least for staged changes? In other words, should it return the current values from VMI or the staged values from VM?

[MarSik]

Same comment about staged and hotplug values. Should it return data from VM or VMI?

[MarSik]

nit: 2026

[MarSik]

It might be better to show the whole path as an example, not just the trailing element to explain the constant 5 in the code.

[MarSik]

Should ppc be supported? Is this mapping necessary? It will require contant maintenance in case new platform designations apear?

[TomerKarniol]

The mapping is needed because Redfish only accepts its own architecture names (like x86 and ARM), so KubeVirt's amd64 and arm64 can't be passed through. This matters because sushy, the Python Redfish client that OpenStack Ironic uses to manage machines, reads these fields to work out each system's CPU architecture. I didn't add ppc: the KubeVirt API marks ppc64le as "no longer supported", so it could never be reached. It's a two-case switch, and an unknown architecture is simply left out, so a new platform gives a missing value, never a wrong one.

[MarSik]

This is a bit weird, isn't it? You get the VMI from outside and then create a VM with constant device specification. How does it make sure the VM and VMI form a reasonably consistent pair?

[MarSik]

Why not 403 Forbidden directly?

[TomerKarniol]

A 403 would tell the client that they aren't allowed to do this. Here the caller has already passed the access check. What is forbidden is the controller's own service account, which can't read the PVC. That is a server misconfiguration the client can't fix, so 500 is the right status. 403 would also be misleading, because a client might retry with other credentials or stop asking.

[MarSik]

I wonder if it might be a good idea to extract the access check and do it here instead of using it in every single sub-handler.

[TomerKarniol]

Good idea, done. The access check for the hardware sub-resources (Memory, Processors, Storage, EthernetInterfaces) now happens once in handleSystem, right after the method check, and handleSystemDevices receives the resolved namespace and VM name.
I kept the other handlers (power, boot, virtual media, the main GET) as they are. They call the check themselves and have many direct unit tests. Moving them changes handler signatures and the order of body validation versus access check, which seems out of scope for a PR about read-only hardware data. I'm happy to do that as a follow-up PR, or open an issue for it if you prefer.

[kubevirt-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign yanirq for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[TomerKarniol]

Hi @MarSik, friendly ping on this one. I addressed your last comment by
moving the access check into handleSystem (so the Memory, Processors,
Storage and EthernetInterfaces sub-handlers share it), and all checks are
green. Could you take another look when you have a moment? If anything
else needs changing, I'm happy to adjust.

[TomerKarniol]

Rebased onto main after #34. The only code change: the hardware resources now use setNoCacheHeaders(w), since #34 removed setCacheHeaders (the PR text-merged cleanly but no longer compiled). The fix is folded into the commit that introduced the call, so every commit builds.

go vet, go build and go test ./... pass. go test -race passes for pkg/kubevirt and pkg/redfish; pkg/server still fails under -race in the connection pool, memory monitor and job scheduler tests, the same as on current main. I updated the PR description to match.

[itayherzberg]

Two follow-up PRs now build on this one, so a heads-up in case it affects how you'd like to land it:

• feat(server): report SerialNumber, UUID, Manufacturer and Model on the ComputerSystem #41 reports SerialNumber, UUID, Manufacturer and Model on the ComputerSystem (from the VM's firmware and machine type).
• feat(server): report the node a running VM is scheduled on as Oem.KubeVirt.NodeName #42 reports the node a running VM is scheduled on as Oem.KubeVirt.NodeName.

Both use this PR's getRunningVMI / processorFields, so until this merges their diffs also show its 8 commits. Each adds a single commit on top of 05be86f, and I'll rebase both once this is in. Nothing in them changes this PR.

[MarSik]

I apologize for yet another merge conflict, I merged the VM/VMI informers PR first.

[TomerKarniol]

Rebased onto current main to resolve the merge conflict (upstream's informer-cache change in #… touched the same client.go and mock code).
Kept the GetVMCPUTopology / specMemoryBytes code and took main's notFoundError helper in the mock client.
The hardware reads (getRunningVMI, GetVMDisks, GetVMNICs, GetVMCPUTopology, memory) now use getVMForRead / getVMIForRead, so they are served from the informer cache like the other read-only lookups. Nothing in the PR calls GetVM/GetVMI directly any more, since none of these paths patch the VM.
@MarSik let me know if you need anything else or have any questions.