From 689f229c72b8447dc7ead217d8e533f3e842e52e Mon Sep 17 00:00:00 2001 From: rcourtman Date: Fri, 12 Dec 2025 19:12:06 +0000 Subject: [PATCH] fix: Allow OCI detection for stopped containers - Refactored enrichContainerMetadata to not return early when container is stopped - Status API calls are still skipped for stopped containers (as expected) - Config fetch now runs regardless of status, enabling OCI detection - Added test for OCI detection on stopped containers Discovered: Proxmox 9.1 requires VM.Config.Options permission to read OCI container configs (not just VM.Audit). Document this in setup guides. --- .../enrich_container_metadata_test.go | 65 ++++++ internal/monitoring/monitor.go | 204 +++++++++--------- 2 files changed, 172 insertions(+), 97 deletions(-) create mode 100644 internal/monitoring/enrich_container_metadata_test.go diff --git a/internal/monitoring/enrich_container_metadata_test.go b/internal/monitoring/enrich_container_metadata_test.go new file mode 100644 index 0000000..7d9ddb5 --- /dev/null +++ b/internal/monitoring/enrich_container_metadata_test.go @@ -0,0 +1,65 @@ +package monitoring + +import ( + "context" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" +) + +type stubPVEClientContainerMetadata struct { + stubPVEClient + + config map[string]interface{} + + configCalls int + statusCalls int +} + +func (s *stubPVEClientContainerMetadata) GetContainerConfig(ctx context.Context, node string, vmid int) (map[string]interface{}, error) { + s.configCalls++ + return s.config, nil +} + +func (s *stubPVEClientContainerMetadata) GetContainerStatus(ctx context.Context, node string, vmid int) (*proxmox.Container, error) { + s.statusCalls++ + return nil, nil +} + +func TestEnrichContainerMetadata_DetectsOCIForStoppedContainer(t *testing.T) { + t.Parallel() + + monitor := &Monitor{} + client := &stubPVEClientContainerMetadata{ + config: map[string]interface{}{ + "entrypoint": "/bin/sh", + "ostype": "unmanaged", + "cmode": "console", + "lxc": "lxc.signal.halt: SIGTERM", + }, + } + + container := &models.Container{ + VMID: 300, + Name: "oci-alpine", + Status: "stopped", + Type: "lxc", + } + + monitor.enrichContainerMetadata(context.Background(), client, "delly", "delly", container) + + if client.configCalls != 1 { + t.Fatalf("expected 1 config call, got %d", client.configCalls) + } + if client.statusCalls != 0 { + t.Fatalf("expected 0 status calls for stopped container, got %d", client.statusCalls) + } + if !container.IsOCI { + t.Fatalf("expected container.IsOCI true, got false") + } + if container.Type != "oci" { + t.Fatalf("expected container.Type oci, got %q", container.Type) + } +} + diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index ca922ea..5810351 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -2358,25 +2358,28 @@ func (m *Monitor) enrichContainerMetadata(ctx context.Context, client PVEClientI ensureContainerRootDiskEntry(container) - if client == nil || container.Status != "running" { + if client == nil { return } - statusCtx, cancel := context.WithTimeout(ctx, 5*time.Second) - status, err := client.GetContainerStatus(statusCtx, nodeName, container.VMID) - cancel() - if err != nil { - log.Debug(). - Err(err). - Str("instance", instanceName). - Str("node", nodeName). - Str("container", container.Name). - Int("vmid", container.VMID). - Msg("Container status metadata unavailable") - return - } - if status == nil { - return + isRunning := container.Status == "running" + + var status *proxmox.Container + if isRunning { + statusCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + statusResp, err := client.GetContainerStatus(statusCtx, nodeName, container.VMID) + cancel() + if err != nil { + log.Debug(). + Err(err). + Str("instance", instanceName). + Str("node", nodeName). + Str("container", container.Name). + Int("vmid", container.VMID). + Msg("Container status metadata unavailable") + } else { + status = statusResp + } } rootDeviceHint := "" @@ -2396,55 +2399,60 @@ func (m *Monitor) enrichContainerMetadata(ctx context.Context, client PVEClientI addressOrder = append(addressOrder, addr) } - for _, addr := range sanitizeGuestAddressStrings(status.IP) { - addAddress(addr) - } - for _, addr := range sanitizeGuestAddressStrings(status.IP6) { - addAddress(addr) - } - for _, addr := range parseContainerRawIPs(status.IPv4) { - addAddress(addr) - } - for _, addr := range parseContainerRawIPs(status.IPv6) { - addAddress(addr) + if status != nil { + for _, addr := range sanitizeGuestAddressStrings(status.IP) { + addAddress(addr) + } + for _, addr := range sanitizeGuestAddressStrings(status.IP6) { + addAddress(addr) + } + for _, addr := range parseContainerRawIPs(status.IPv4) { + addAddress(addr) + } + for _, addr := range parseContainerRawIPs(status.IPv6) { + addAddress(addr) + } } - networkIfaces := make([]models.GuestNetworkInterface, 0, len(status.Network)) - for rawName, cfg := range status.Network { - if cfg == (proxmox.ContainerNetworkConfig{}) { - continue - } + networkIfaces := make([]models.GuestNetworkInterface, 0, 4) + if status != nil { + networkIfaces = make([]models.GuestNetworkInterface, 0, len(status.Network)) + for rawName, cfg := range status.Network { + if cfg == (proxmox.ContainerNetworkConfig{}) { + continue + } - iface := models.GuestNetworkInterface{} - name := strings.TrimSpace(cfg.Name) - if name == "" { - name = strings.TrimSpace(rawName) - } - if name != "" { - iface.Name = name - } - if mac := strings.TrimSpace(cfg.HWAddr); mac != "" { - iface.MAC = mac - } + iface := models.GuestNetworkInterface{} + name := strings.TrimSpace(cfg.Name) + if name == "" { + name = strings.TrimSpace(rawName) + } + if name != "" { + iface.Name = name + } + if mac := strings.TrimSpace(cfg.HWAddr); mac != "" { + iface.MAC = mac + } - addrCandidates := make([]string, 0, 4) - addrCandidates = append(addrCandidates, collectIPsFromInterface(cfg.IP)...) - addrCandidates = append(addrCandidates, collectIPsFromInterface(cfg.IP6)...) - addrCandidates = append(addrCandidates, collectIPsFromInterface(cfg.IPv4)...) - addrCandidates = append(addrCandidates, collectIPsFromInterface(cfg.IPv6)...) + addrCandidates := make([]string, 0, 4) + addrCandidates = append(addrCandidates, collectIPsFromInterface(cfg.IP)...) + addrCandidates = append(addrCandidates, collectIPsFromInterface(cfg.IP6)...) + addrCandidates = append(addrCandidates, collectIPsFromInterface(cfg.IPv4)...) + addrCandidates = append(addrCandidates, collectIPsFromInterface(cfg.IPv6)...) - if len(addrCandidates) > 0 { - deduped := dedupeStringsPreserveOrder(addrCandidates) - if len(deduped) > 0 { - iface.Addresses = deduped - for _, addr := range deduped { - addAddress(addr) + if len(addrCandidates) > 0 { + deduped := dedupeStringsPreserveOrder(addrCandidates) + if len(deduped) > 0 { + iface.Addresses = deduped + for _, addr := range deduped { + addAddress(addr) + } } } - } - if iface.Name != "" || iface.MAC != "" || len(iface.Addresses) > 0 { - networkIfaces = append(networkIfaces, iface) + if iface.Name != "" || iface.MAC != "" || len(iface.Addresses) > 0 { + networkIfaces = append(networkIfaces, iface) + } } } @@ -2510,38 +2518,26 @@ func (m *Monitor) enrichContainerMetadata(ctx context.Context, client PVEClientI } if len(addressOrder) == 0 { - interfacesCtx, cancelInterfaces := context.WithTimeout(ctx, 5*time.Second) - ifaceDetails, ifaceErr := client.GetContainerInterfaces(interfacesCtx, nodeName, container.VMID) - cancelInterfaces() - if ifaceErr != nil { - log.Debug(). - Err(ifaceErr). - Str("instance", instanceName). - Str("node", nodeName). - Str("container", container.Name). - Int("vmid", container.VMID). - Msg("Container interface metadata unavailable") - } else if len(ifaceDetails) > 0 { - for _, detail := range ifaceDetails { - parsed := containerNetworkDetails{} - parsed.Name = strings.TrimSpace(detail.Name) - parsed.MAC = strings.ToUpper(strings.TrimSpace(detail.HWAddr)) + if isRunning { + interfacesCtx, cancelInterfaces := context.WithTimeout(ctx, 5*time.Second) + ifaceDetails, ifaceErr := client.GetContainerInterfaces(interfacesCtx, nodeName, container.VMID) + cancelInterfaces() + if ifaceErr != nil { + log.Debug(). + Err(ifaceErr). + Str("instance", instanceName). + Str("node", nodeName). + Str("container", container.Name). + Int("vmid", container.VMID). + Msg("Container interface metadata unavailable") + } else if len(ifaceDetails) > 0 { + for _, detail := range ifaceDetails { + parsed := containerNetworkDetails{} + parsed.Name = strings.TrimSpace(detail.Name) + parsed.MAC = strings.ToUpper(strings.TrimSpace(detail.HWAddr)) - for _, addr := range detail.IPAddresses { - stripped := strings.TrimSpace(addr.Address) - if stripped == "" { - continue - } - if slash := strings.Index(stripped, "/"); slash > 0 { - stripped = stripped[:slash] - } - parsed.Addresses = append(parsed.Addresses, sanitizeGuestAddressStrings(stripped)...) - } - - if len(parsed.Addresses) == 0 && strings.TrimSpace(detail.Inet) != "" { - parts := strings.Fields(detail.Inet) - for _, part := range parts { - stripped := strings.TrimSpace(part) + for _, addr := range detail.IPAddresses { + stripped := strings.TrimSpace(addr.Address) if stripped == "" { continue } @@ -2550,18 +2546,32 @@ func (m *Monitor) enrichContainerMetadata(ctx context.Context, client PVEClientI } parsed.Addresses = append(parsed.Addresses, sanitizeGuestAddressStrings(stripped)...) } - } - parsed.Addresses = dedupeStringsPreserveOrder(parsed.Addresses) - - if len(parsed.Addresses) > 0 { - for _, addr := range parsed.Addresses { - addAddress(addr) + if len(parsed.Addresses) == 0 && strings.TrimSpace(detail.Inet) != "" { + parts := strings.Fields(detail.Inet) + for _, part := range parts { + stripped := strings.TrimSpace(part) + if stripped == "" { + continue + } + if slash := strings.Index(stripped, "/"); slash > 0 { + stripped = stripped[:slash] + } + parsed.Addresses = append(parsed.Addresses, sanitizeGuestAddressStrings(stripped)...) + } } - } - if parsed.Name != "" || parsed.MAC != "" || len(parsed.Addresses) > 0 { - mergeContainerNetworkInterface(&networkIfaces, parsed) + parsed.Addresses = dedupeStringsPreserveOrder(parsed.Addresses) + + if len(parsed.Addresses) > 0 { + for _, addr := range parsed.Addresses { + addAddress(addr) + } + } + + if parsed.Name != "" || parsed.MAC != "" || len(parsed.Addresses) > 0 { + mergeContainerNetworkInterface(&networkIfaces, parsed) + } } } }