fix(urunc config): Ensure default values for monitor configuration

Make sure that the default values defined for the monitor configuration
are not overwritten if the respective fields are not defined in the
configuration file.

PR: #829
Signed-off-by: Charalampos Mainas <cmainas@nubificus.co.uk>
Reviewed-by: Anastassios Nanos <ananos@nubificus.co.uk>
Approved-by: Anastassios Nanos <ananos@nubificus.co.uk>

Author: cmainas

pkg/unikontainers/urunc_config.go

@@ -78,13 +78,18 @@ func defaultTimestampsConfig() UruncTimestamps {
 	}
 }
 
+const (
+	defaultMonitorMemoryMB uint = 256
+	defaultMonitorVCPUs    uint = 1
+)
+
 func defaultMonitorsConfig() map[string]types.MonitorConfig {
 	return map[string]types.MonitorConfig{
-		"qemu":             {DefaultMemoryMB: 256, DefaultVCPUs: 1},
-		"hvt":              {DefaultMemoryMB: 256, DefaultVCPUs: 1},
-		"spt":              {DefaultMemoryMB: 256, DefaultVCPUs: 1},
-		"firecracker":      {DefaultMemoryMB: 256, DefaultVCPUs: 1},
-		"cloud-hypervisor": {DefaultMemoryMB: 256, DefaultVCPUs: 1},
+		"qemu":             {DefaultMemoryMB: defaultMonitorMemoryMB, DefaultVCPUs: defaultMonitorVCPUs},
+		"hvt":              {DefaultMemoryMB: defaultMonitorMemoryMB, DefaultVCPUs: defaultMonitorVCPUs},
+		"spt":              {DefaultMemoryMB: defaultMonitorMemoryMB, DefaultVCPUs: defaultMonitorVCPUs},
+		"firecracker":      {DefaultMemoryMB: defaultMonitorMemoryMB, DefaultVCPUs: defaultMonitorVCPUs},
+		"cloud-hypervisor": {DefaultMemoryMB: defaultMonitorMemoryMB, DefaultVCPUs: defaultMonitorVCPUs},
 	}
 }
 
@@ -108,11 +113,24 @@ func defaultUruncConfig() *UruncConfig {
 func LoadUruncConfig(path string) (*UruncConfig, error) {
 	cfg := defaultUruncConfig()
 	_, err := toml.DecodeFile(path, cfg)
-	if err == nil {
-		return cfg, nil
+	if err != nil {
+		uniklog.Warnf("Failed to load urunc config from %s: %v. Using default configuration.", path, err)
+		return defaultUruncConfig(), err
 	}
-	uniklog.Warnf("Failed to load urunc config from %s: %v. Using default configuration.", path, err)
-	return defaultUruncConfig(), err
+	// Decoding a partially-specified [monitors.<name>] section (e.g. one that
+	// only sets binary_path) zeroes the fields absent from the file, dropping
+	// the seeded defaults. Re-apply them for any zero value.
+	for name, mon := range cfg.Monitors {
+		if mon.DefaultMemoryMB == 0 {
+			mon.DefaultMemoryMB = defaultMonitorMemoryMB
+		}
+		if mon.DefaultVCPUs == 0 {
+			mon.DefaultVCPUs = defaultMonitorVCPUs
+		}
+		cfg.Monitors[name] = mon
+	}
+
+	return cfg, nil
 }
 
 func (p *UruncConfig) Map() map[string]string {

pkg/unikontainers/urunc_config_test.go

@@ -15,6 +15,8 @@
 package unikontainers
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -537,3 +539,82 @@ func TestDefaultConfigs(t *testing.T) {
 		assert.Equal(t, testTimestampsPath, config.Timestamps.Destination)
 	})
 }
+
+func writeTestConfig(t *testing.T, content string) string {
+	t.Helper()
+	path := filepath.Join(t.TempDir(), "config.toml")
+	if err := os.WriteFile(path, []byte(content), 0o600); err != nil {
+		t.Fatalf("failed to write test config: %v", err)
+	}
+
+	return path
+}
+
+func TestLoadUruncConfig(t *testing.T) {
+	t.Run("partial monitor section keeps default memory and vcpus", func(t *testing.T) {
+		t.Parallel()
+		path := writeTestConfig(t, `
+[monitors.qemu]
+path = "/opt/urunc/bin/qemu-system-x86_64"
+data_path = "/opt/urunc"
+`)
+		config, err := LoadUruncConfig(path)
+		assert.NoError(t, err)
+
+		qemu := config.Monitors["qemu"]
+		assert.Equal(t, defaultMonitorMemoryMB, qemu.DefaultMemoryMB)
+		assert.Equal(t, defaultMonitorVCPUs, qemu.DefaultVCPUs)
+		assert.Equal(t, "/opt/urunc/bin/qemu-system-x86_64", qemu.BinaryPath)
+		assert.Equal(t, "/opt/urunc", qemu.DataPath)
+	})
+
+	t.Run("explicit values are honored", func(t *testing.T) {
+		t.Parallel()
+		path := writeTestConfig(t, `
+[monitors.qemu]
+default_memory_mb = 512
+default_vcpus = 4
+`)
+		config, err := LoadUruncConfig(path)
+		assert.NoError(t, err)
+
+		qemu := config.Monitors["qemu"]
+		assert.Equal(t, uint(512), qemu.DefaultMemoryMB)
+		assert.Equal(t, uint(4), qemu.DefaultVCPUs)
+	})
+
+	t.Run("monitors absent from the file keep their defaults", func(t *testing.T) {
+		t.Parallel()
+		path := writeTestConfig(t, `
+[monitors.qemu]
+default_memory_mb = 512
+`)
+		config, err := LoadUruncConfig(path)
+		assert.NoError(t, err)
+
+		assert.Equal(t, uint(512), config.Monitors["qemu"].DefaultMemoryMB)
+		assert.Equal(t, defaultMonitorMemoryMB, config.Monitors["hvt"].DefaultMemoryMB)
+		assert.Equal(t, defaultMonitorVCPUs, config.Monitors["hvt"].DefaultVCPUs)
+	})
+
+	t.Run("custom monitor gets defaults for omitted fields", func(t *testing.T) {
+		t.Parallel()
+		path := writeTestConfig(t, `
+[monitors.mon]
+path = "/usr/bin/mon"
+`)
+		config, err := LoadUruncConfig(path)
+		assert.NoError(t, err)
+
+		mon := config.Monitors["mon"]
+		assert.Equal(t, defaultMonitorMemoryMB, mon.DefaultMemoryMB)
+		assert.Equal(t, defaultMonitorVCPUs, mon.DefaultVCPUs)
+	})
+
+	t.Run("missing file returns defaults", func(t *testing.T) {
+		t.Parallel()
+		config, err := LoadUruncConfig(filepath.Join(t.TempDir(), "does-not-exist.toml"))
+		assert.Error(t, err)
+		assert.Equal(t, defaultMonitorsConfig(), config.Monitors)
+	})
+}