From 7661cbe86dbd6b237f95eac4dccbfd330b3424de Mon Sep 17 00:00:00 2001 From: Mysterio-17 Date: Sat, 24 Jan 2026 08:35:00 +0000 Subject: [PATCH 1/4] feat: add netkit network device support Signed-off-by: Mysterio-17 --- .github/linters/urunc-dict.txt | 1 + pkg/network/network.go | 70 ++++++++++++++++- pkg/network/network_netkit_test.go | 121 +++++++++++++++++++++++++++++ 3 files changed, 190 insertions(+), 2 deletions(-) create mode 100644 pkg/network/network_netkit_test.go diff --git a/.github/linters/urunc-dict.txt b/.github/linters/urunc-dict.txt index ab032d84..e555b7d3 100644 --- a/.github/linters/urunc-dict.txt +++ b/.github/linters/urunc-dict.txt @@ -224,6 +224,7 @@ nerdctl netcli netdev netgo +netkit netlink netlog netns diff --git a/pkg/network/network.go b/pkg/network/network.go index 6aa55e22..5f8333b6 100644 --- a/pkg/network/network.go +++ b/pkg/network/network.go @@ -18,6 +18,8 @@ import ( "errors" "fmt" "net" + "os" + "strconv" "strings" "github.com/jackpal/gateway" @@ -29,6 +31,9 @@ import ( const ( DefaultInterface = "eth0" // FIXME: Discover the veth endpoint name instead of using default "eth0". See: https://github.com/urunc-dev/urunc/issues/14 DefaultTap = "tapX_urunc" + // MinNetkitKernelVersion is the minimum kernel version (6.8) required for netkit support + MinNetkitKernelMajor = 6 + MinNetkitKernelMinor = 8 ) var netlog = logrus.WithField("subsystem", "network") @@ -78,6 +83,54 @@ func getTapIndex() (int, error) { return tapCount, nil } +// isNetkitSupported checks if the kernel version is >= 6.8 +func isNetkitSupported() bool { + var uname unix.Utsname + if err := unix.Uname(&uname); err != nil { + netlog.Debugf("failed to get kernel version: %v", err) + return false + } + + // Convert release to string (e.g., "6.8.0-31-generic") + release := string(uname.Release[:]) + release = strings.Split(release, "\x00")[0] // trim null bytes + parts := strings.Split(release, ".") + if len(parts) < 2 { + return false + } + + major, err := strconv.Atoi(parts[0]) + if err != nil { + return false + } + minor, err := strconv.Atoi(parts[1]) + if err != nil { + return false + } + + if major > MinNetkitKernelMajor { + return true + } + if major == MinNetkitKernelMajor && minor >= MinNetkitKernelMinor { + return true + } + return false +} + +// isNetkitEnabled checks if netkit support is enabled via environment variable +func isNetkitEnabled() bool { + val := os.Getenv("URUNC_ENABLE_NETKIT") + return val == "1" || strings.ToLower(val) == "true" +} + +// getLinkType returns the type of the network link (e.g., "veth", "netkit", "device") +func getLinkType(link netlink.Link) string { + if link == nil { + return "unknown" + } + return link.Type() +} + func createTapDevice(name string, mtu int, ownerUID, ownerGID uint32) (netlink.Link, error) { tapLinkAttrs := netlink.NewLinkAttrs() tapLinkAttrs.Name = name @@ -217,8 +270,21 @@ func addRedirectFilter(source netlink.Link, target netlink.Link) error { } func networkSetup(tapName string, ipAddress string, redirectLink netlink.Link, addTCRules bool, uid uint32, gid uint32) (netlink.Link, error) { - netlog.Debugf("starting for tapName=%s ipAddress=%s redirectLink=%s addTCRules=%v", - tapName, ipAddress, redirectLink.Attrs().Name, addTCRules) + linkType := getLinkType(redirectLink) + netlog.Debugf("starting for tapName=%s ipAddress=%s redirectLink=%s linkType=%s addTCRules=%v", + tapName, ipAddress, redirectLink.Attrs().Name, linkType, addTCRules) + + // Log netkit status if enabled + if isNetkitEnabled() && isNetkitSupported() { + if linkType == "netkit" { + netlog.Infof("netkit device detected: %s (kernel supports netkit)", redirectLink.Attrs().Name) + } else { + netlog.Debugf("netkit enabled but device %s is type %s (not netkit)", redirectLink.Attrs().Name, linkType) + } + } else if isNetkitEnabled() && !isNetkitSupported() { + netlog.Warnf("netkit enabled but kernel version < %d.%d (detected type: %s)", + MinNetkitKernelMajor, MinNetkitKernelMinor, linkType) + } // Sanity check: ensure eth0 exists in this namespace err := ensureEth0Exists() diff --git a/pkg/network/network_netkit_test.go b/pkg/network/network_netkit_test.go new file mode 100644 index 00000000..e09d0f44 --- /dev/null +++ b/pkg/network/network_netkit_test.go @@ -0,0 +1,121 @@ +// Copyright (c) 2023-2025, Nubificus LTD +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package network + +import ( + "os" + "testing" +) + +func TestIsNetkitEnabled(t *testing.T) { + tests := []struct { + name string + envValue string + want bool + }{ + { + name: "enabled with 1", + envValue: "1", + want: true, + }, + { + name: "enabled with true", + envValue: "true", + want: true, + }, + { + name: "enabled with TRUE", + envValue: "TRUE", + want: true, + }, + { + name: "disabled with 0", + envValue: "0", + want: false, + }, + { + name: "disabled with false", + envValue: "false", + want: false, + }, + { + name: "disabled with empty", + envValue: "", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Save original value + originalValue := os.Getenv("URUNC_ENABLE_NETKIT") + defer func() { + if originalValue != "" { + os.Setenv("URUNC_ENABLE_NETKIT", originalValue) + } else { + os.Unsetenv("URUNC_ENABLE_NETKIT") + } + }() + + // Set test value + if tt.envValue != "" { + os.Setenv("URUNC_ENABLE_NETKIT", tt.envValue) + } else { + os.Unsetenv("URUNC_ENABLE_NETKIT") + } + + if got := isNetkitEnabled(); got != tt.want { + t.Errorf("isNetkitEnabled() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestIsNetkitSupported(t *testing.T) { + // This test will check if the kernel version detection works + // The result depends on the actual kernel version + supported := isNetkitSupported() + t.Logf("Netkit support detected: %v", supported) + + // We can't assert a specific value since it depends on the kernel + // but we can verify the function doesn't panic + if supported { + t.Log("Kernel version >= 6.8 detected, netkit is supported") + } else { + t.Log("Kernel version < 6.8 detected, netkit is not supported") + } +} + +func TestGetLinkType(t *testing.T) { + tests := []struct { + name string + link interface{} + want string + }{ + { + name: "nil link", + link: nil, + want: "unknown", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := getLinkType(nil); got != tt.want { + t.Errorf("getLinkType() = %v, want %v", got, tt.want) + } + }) + } +} From 217cd44e3b98a06b27fc80b040971bff38cf4bc7 Mon Sep 17 00:00:00 2001 From: Mysterio-17 Date: Sat, 24 Jan 2026 11:26:42 +0000 Subject: [PATCH 2/4] test: add unit tests for vaccel, network utils, and rootfs selection Signed-off-by: Mysterio-17 --- pkg/network/network_test.go | 103 ++++++++++++++ pkg/unikontainers/rootfs_test.go | 230 +++++++++++++++++++++++++++++++ pkg/unikontainers/vaccel_test.go | 187 +++++++++++++++++++++++++ 3 files changed, 520 insertions(+) create mode 100644 pkg/network/network_test.go create mode 100644 pkg/unikontainers/rootfs_test.go create mode 100644 pkg/unikontainers/vaccel_test.go diff --git a/pkg/network/network_test.go b/pkg/network/network_test.go new file mode 100644 index 00000000..45d7d7d7 --- /dev/null +++ b/pkg/network/network_test.go @@ -0,0 +1,103 @@ +// Copyright (c) 2023-2025, Nubificus LTD +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package network + +import ( + "testing" +) + +func TestGetTapIndex(t *testing.T) { + // This test just verifies the function doesn't panic + // The actual count depends on the system's network interfaces + index, err := getTapIndex() + if err != nil { + t.Errorf("getTapIndex() error = %v", err) + return + } + + // Index should be non-negative + if index < 0 { + t.Errorf("getTapIndex() = %d, want >= 0", index) + } + + // Index should not exceed 255 (function returns error if > 255) + if index > 255 { + t.Errorf("getTapIndex() = %d, want <= 255", index) + } +} + +func TestNewNetworkManager(t *testing.T) { + tests := []struct { + name string + networkType string + wantErr bool + wantType string + }{ + { + name: "static network manager", + networkType: "static", + wantErr: false, + wantType: "*network.StaticNetwork", + }, + { + name: "dynamic network manager", + networkType: "dynamic", + wantErr: false, + wantType: "*network.DynamicNetwork", + }, + { + name: "invalid network type", + networkType: "invalid", + wantErr: true, + }, + { + name: "empty network type", + networkType: "", + wantErr: true, + }, + { + name: "unknown network type", + networkType: "bridge", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := NewNetworkManager(tt.networkType) + if (err != nil) != tt.wantErr { + t.Errorf("NewNetworkManager() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if !tt.wantErr { + if got == nil { + t.Error("NewNetworkManager() returned nil manager") + } + } + }) + } +} + +func TestEnsureEth0Exists(t *testing.T) { + // This test verifies the function works + // It might fail or succeed depending on the test environment + err := ensureEth0Exists() + + // We don't assert a specific result since it depends on + // whether eth0 exists in the test environment + // Just verify it doesn't panic + t.Logf("ensureEth0Exists() error = %v", err) +} diff --git a/pkg/unikontainers/rootfs_test.go b/pkg/unikontainers/rootfs_test.go new file mode 100644 index 00000000..35dfc6db --- /dev/null +++ b/pkg/unikontainers/rootfs_test.go @@ -0,0 +1,230 @@ +// Copyright (c) 2023-2025, Nubificus LTD +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package unikontainers + +import ( + "testing" + + "github.com/urunc-dev/urunc/pkg/unikontainers/types" +) + +func TestNewRootfsResult(t *testing.T) { + tests := []struct { + name string + rootfsType string + path string + mountedPath string + monRootfs string + want types.RootfsParams + }{ + { + name: "initrd rootfs", + rootfsType: "initrd", + path: "/path/to/initrd", + mountedPath: "", + monRootfs: "/run/urunc/mon", + want: types.RootfsParams{ + Type: "initrd", + Path: "/path/to/initrd", + MountedPath: "", + MonRootfs: "/run/urunc/mon", + }, + }, + { + name: "block rootfs", + rootfsType: "block", + path: "/dev/dm-0", + mountedPath: "/mnt/rootfs", + monRootfs: "/run/urunc/mon", + want: types.RootfsParams{ + Type: "block", + Path: "/dev/dm-0", + MountedPath: "/mnt/rootfs", + MonRootfs: "/run/urunc/mon", + }, + }, + { + name: "9pfs rootfs", + rootfsType: "9pfs", + path: "/container/rootfs", + mountedPath: "", + monRootfs: "/run/urunc/mon", + want: types.RootfsParams{ + Type: "9pfs", + Path: "/container/rootfs", + MountedPath: "", + MonRootfs: "/run/urunc/mon", + }, + }, + { + name: "empty paths", + rootfsType: "none", + path: "", + mountedPath: "", + monRootfs: "", + want: types.RootfsParams{ + Type: "none", + Path: "", + MountedPath: "", + MonRootfs: "", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := newRootfsResult(tt.rootfsType, tt.path, tt.mountedPath, tt.monRootfs) + + if got.Type != tt.want.Type { + t.Errorf("Type = %v, want %v", got.Type, tt.want.Type) + } + if got.Path != tt.want.Path { + t.Errorf("Path = %v, want %v", got.Path, tt.want.Path) + } + if got.MountedPath != tt.want.MountedPath { + t.Errorf("MountedPath = %v, want %v", got.MountedPath, tt.want.MountedPath) + } + if got.MonRootfs != tt.want.MonRootfs { + t.Errorf("MonRootfs = %v, want %v", got.MonRootfs, tt.want.MonRootfs) + } + }) + } +} + +func TestRootfsSelector_TryInitrd(t *testing.T) { + tests := []struct { + name string + annot map[string]string + wantFound bool + wantType string + wantPath string + }{ + { + name: "initrd present", + annot: map[string]string{ + annotInitrd: "/path/to/initrd.img", + }, + wantFound: true, + wantType: "initrd", + wantPath: "/path/to/initrd.img", + }, + { + name: "initrd missing", + annot: map[string]string{}, + wantFound: false, + }, + { + name: "initrd empty", + annot: map[string]string{ + annotInitrd: "", + }, + wantFound: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rs := &rootfsSelector{ + annot: tt.annot, + cntrRootfs: "/container/rootfs", + } + + got, found := rs.tryInitrd() + + if found != tt.wantFound { + t.Errorf("tryInitrd() found = %v, want %v", found, tt.wantFound) + return + } + + if found { + if got.Type != tt.wantType { + t.Errorf("tryInitrd() Type = %v, want %v", got.Type, tt.wantType) + } + if got.Path != tt.wantPath { + t.Errorf("tryInitrd() Path = %v, want %v", got.Path, tt.wantPath) + } + } + }) + } +} + +func TestRootfsSelector_ShouldMountContainerRootfs(t *testing.T) { + tests := []struct { + name string + annot map[string]string + want bool + }{ + { + name: "mount rootfs true", + annot: map[string]string{ + annotMountRootfs: "true", + }, + want: true, + }, + { + name: "mount rootfs 1", + annot: map[string]string{ + annotMountRootfs: "1", + }, + want: true, + }, + { + name: "mount rootfs false", + annot: map[string]string{ + annotMountRootfs: "false", + }, + want: false, + }, + { + name: "mount rootfs 0", + annot: map[string]string{ + annotMountRootfs: "0", + }, + want: false, + }, + { + name: "mount rootfs missing", + annot: map[string]string{}, + want: false, + }, + { + name: "mount rootfs empty", + annot: map[string]string{ + annotMountRootfs: "", + }, + want: false, + }, + { + name: "mount rootfs invalid", + annot: map[string]string{ + annotMountRootfs: "invalid", + }, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rs := &rootfsSelector{ + annot: tt.annot, + } + + got := rs.shouldMountContainerRootfs() + if got != tt.want { + t.Errorf("shouldMountContainerRootfs() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/unikontainers/vaccel_test.go b/pkg/unikontainers/vaccel_test.go new file mode 100644 index 00000000..bb088639 --- /dev/null +++ b/pkg/unikontainers/vaccel_test.go @@ -0,0 +1,187 @@ +// Copyright (c) 2023-2025, Nubificus LTD +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package unikontainers + +import ( + "testing" +) + +func TestIdToGuestCID(t *testing.T) { + tests := []struct { + name string + id string + wantMin int + wantMax int + checkCID int + }{ + { + name: "empty string", + id: "", + wantMin: 3, + wantMax: 99, + }, + { + name: "simple id", + id: "container123", + wantMin: 3, + wantMax: 99, + checkCID: -1, + }, + { + name: "long id", + id: "very-long-container-id-12345678", + wantMin: 3, + wantMax: 99, + checkCID: -1, + }, + { + name: "special characters", + id: "container-with-special_chars.123", + wantMin: 3, + wantMax: 99, + checkCID: -1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := idToGuestCID(tt.id) + + // Check if CID is within valid range + if got < tt.wantMin || got > tt.wantMax { + t.Errorf("idToGuestCID(%q) = %d, want value between %d and %d", + tt.id, got, tt.wantMin, tt.wantMax) + } + + // Test determinism - same input should always produce same output + got2 := idToGuestCID(tt.id) + if got != got2 { + t.Errorf("idToGuestCID(%q) is not deterministic: first=%d, second=%d", + tt.id, got, got2) + } + }) + } +} + +func TestIsValidVSockAddress(t *testing.T) { + tests := []struct { + name string + rpcAddress string + hypervisor string + wantValid bool + wantErr bool + wantPath string + }{ + { + name: "valid qemu vsock address", + rpcAddress: "vsock://2:1234", + hypervisor: "qemu", + wantValid: true, + wantErr: false, + }, + { + name: "invalid qemu vsock - wrong CID", + rpcAddress: "vsock://3:1234", + hypervisor: "qemu", + wantValid: false, + wantErr: true, + }, + { + name: "invalid qemu vsock - no port", + rpcAddress: "vsock://2:", + hypervisor: "qemu", + wantValid: false, + wantErr: true, + }, + { + name: "invalid qemu vsock - malformed", + rpcAddress: "vsock://invalid", + hypervisor: "qemu", + wantValid: false, + wantErr: true, + }, + { + name: "not a vsock address", + rpcAddress: "http://localhost:1234", + hypervisor: "qemu", + wantValid: false, + wantErr: true, + }, + { + name: "empty address", + rpcAddress: "", + hypervisor: "qemu", + wantValid: false, + wantErr: true, + }, + { + name: "valid firecracker unix socket", + rpcAddress: "unix:///tmp/vaccel.sock_1234", + hypervisor: "firecracker", + wantValid: true, + wantErr: false, + wantPath: "/tmp", + }, + { + name: "valid firecracker nested path", + rpcAddress: "unix:///var/run/urunc/vaccel.sock_5678", + hypervisor: "firecracker", + wantValid: true, + wantErr: false, + wantPath: "/var/run/urunc", + }, + { + name: "firecracker invalid - wrong socket name", + rpcAddress: "unix:///tmp/test.sock", + hypervisor: "firecracker", + wantValid: false, + wantErr: true, + }, + { + name: "firecracker invalid - no unix prefix", + rpcAddress: "/tmp/vaccel.sock_1234", + hypervisor: "firecracker", + wantValid: false, + wantErr: true, + }, + { + name: "unsupported hypervisor", + rpcAddress: "vsock://2:1234", + hypervisor: "kvm", + wantValid: false, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + addr := tt.rpcAddress + gotValid, gotPath, err := isValidVSockAddress(&addr, tt.hypervisor) + + if (err != nil) != tt.wantErr { + t.Errorf("isValidVSockAddress() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if gotValid != tt.wantValid { + t.Errorf("isValidVSockAddress() valid = %v, want %v", gotValid, tt.wantValid) + } + + if tt.wantPath != "" && gotPath != tt.wantPath { + t.Errorf("isValidVSockAddress() path = %v, want %v", gotPath, tt.wantPath) + } + }) + } +} From 24d5f9f69ff949e9901c48f0ba24df50769372ee Mon Sep 17 00:00:00 2001 From: Mradul Tiwari Date: Mon, 26 Jan 2026 14:29:19 +0530 Subject: [PATCH 3/4] chore: remove netkit changes from unit tests PR This PR should only contain unit test additions. The netkit-related changes (network_netkit_test.go, netkit support functions, and dictionary updates) have been removed to keep the PR focused on unit tests only. Signed-off-by: Mradul Tiwari --- .github/linters/urunc-dict.txt | 1 - pkg/network/network.go | 70 +---------------- pkg/network/network_netkit_test.go | 121 ----------------------------- 3 files changed, 2 insertions(+), 190 deletions(-) delete mode 100644 pkg/network/network_netkit_test.go diff --git a/.github/linters/urunc-dict.txt b/.github/linters/urunc-dict.txt index e555b7d3..ab032d84 100644 --- a/.github/linters/urunc-dict.txt +++ b/.github/linters/urunc-dict.txt @@ -224,7 +224,6 @@ nerdctl netcli netdev netgo -netkit netlink netlog netns diff --git a/pkg/network/network.go b/pkg/network/network.go index 5f8333b6..6aa55e22 100644 --- a/pkg/network/network.go +++ b/pkg/network/network.go @@ -18,8 +18,6 @@ import ( "errors" "fmt" "net" - "os" - "strconv" "strings" "github.com/jackpal/gateway" @@ -31,9 +29,6 @@ import ( const ( DefaultInterface = "eth0" // FIXME: Discover the veth endpoint name instead of using default "eth0". See: https://github.com/urunc-dev/urunc/issues/14 DefaultTap = "tapX_urunc" - // MinNetkitKernelVersion is the minimum kernel version (6.8) required for netkit support - MinNetkitKernelMajor = 6 - MinNetkitKernelMinor = 8 ) var netlog = logrus.WithField("subsystem", "network") @@ -83,54 +78,6 @@ func getTapIndex() (int, error) { return tapCount, nil } -// isNetkitSupported checks if the kernel version is >= 6.8 -func isNetkitSupported() bool { - var uname unix.Utsname - if err := unix.Uname(&uname); err != nil { - netlog.Debugf("failed to get kernel version: %v", err) - return false - } - - // Convert release to string (e.g., "6.8.0-31-generic") - release := string(uname.Release[:]) - release = strings.Split(release, "\x00")[0] // trim null bytes - parts := strings.Split(release, ".") - if len(parts) < 2 { - return false - } - - major, err := strconv.Atoi(parts[0]) - if err != nil { - return false - } - minor, err := strconv.Atoi(parts[1]) - if err != nil { - return false - } - - if major > MinNetkitKernelMajor { - return true - } - if major == MinNetkitKernelMajor && minor >= MinNetkitKernelMinor { - return true - } - return false -} - -// isNetkitEnabled checks if netkit support is enabled via environment variable -func isNetkitEnabled() bool { - val := os.Getenv("URUNC_ENABLE_NETKIT") - return val == "1" || strings.ToLower(val) == "true" -} - -// getLinkType returns the type of the network link (e.g., "veth", "netkit", "device") -func getLinkType(link netlink.Link) string { - if link == nil { - return "unknown" - } - return link.Type() -} - func createTapDevice(name string, mtu int, ownerUID, ownerGID uint32) (netlink.Link, error) { tapLinkAttrs := netlink.NewLinkAttrs() tapLinkAttrs.Name = name @@ -270,21 +217,8 @@ func addRedirectFilter(source netlink.Link, target netlink.Link) error { } func networkSetup(tapName string, ipAddress string, redirectLink netlink.Link, addTCRules bool, uid uint32, gid uint32) (netlink.Link, error) { - linkType := getLinkType(redirectLink) - netlog.Debugf("starting for tapName=%s ipAddress=%s redirectLink=%s linkType=%s addTCRules=%v", - tapName, ipAddress, redirectLink.Attrs().Name, linkType, addTCRules) - - // Log netkit status if enabled - if isNetkitEnabled() && isNetkitSupported() { - if linkType == "netkit" { - netlog.Infof("netkit device detected: %s (kernel supports netkit)", redirectLink.Attrs().Name) - } else { - netlog.Debugf("netkit enabled but device %s is type %s (not netkit)", redirectLink.Attrs().Name, linkType) - } - } else if isNetkitEnabled() && !isNetkitSupported() { - netlog.Warnf("netkit enabled but kernel version < %d.%d (detected type: %s)", - MinNetkitKernelMajor, MinNetkitKernelMinor, linkType) - } + netlog.Debugf("starting for tapName=%s ipAddress=%s redirectLink=%s addTCRules=%v", + tapName, ipAddress, redirectLink.Attrs().Name, addTCRules) // Sanity check: ensure eth0 exists in this namespace err := ensureEth0Exists() diff --git a/pkg/network/network_netkit_test.go b/pkg/network/network_netkit_test.go deleted file mode 100644 index e09d0f44..00000000 --- a/pkg/network/network_netkit_test.go +++ /dev/null @@ -1,121 +0,0 @@ -// Copyright (c) 2023-2025, Nubificus LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package network - -import ( - "os" - "testing" -) - -func TestIsNetkitEnabled(t *testing.T) { - tests := []struct { - name string - envValue string - want bool - }{ - { - name: "enabled with 1", - envValue: "1", - want: true, - }, - { - name: "enabled with true", - envValue: "true", - want: true, - }, - { - name: "enabled with TRUE", - envValue: "TRUE", - want: true, - }, - { - name: "disabled with 0", - envValue: "0", - want: false, - }, - { - name: "disabled with false", - envValue: "false", - want: false, - }, - { - name: "disabled with empty", - envValue: "", - want: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Save original value - originalValue := os.Getenv("URUNC_ENABLE_NETKIT") - defer func() { - if originalValue != "" { - os.Setenv("URUNC_ENABLE_NETKIT", originalValue) - } else { - os.Unsetenv("URUNC_ENABLE_NETKIT") - } - }() - - // Set test value - if tt.envValue != "" { - os.Setenv("URUNC_ENABLE_NETKIT", tt.envValue) - } else { - os.Unsetenv("URUNC_ENABLE_NETKIT") - } - - if got := isNetkitEnabled(); got != tt.want { - t.Errorf("isNetkitEnabled() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestIsNetkitSupported(t *testing.T) { - // This test will check if the kernel version detection works - // The result depends on the actual kernel version - supported := isNetkitSupported() - t.Logf("Netkit support detected: %v", supported) - - // We can't assert a specific value since it depends on the kernel - // but we can verify the function doesn't panic - if supported { - t.Log("Kernel version >= 6.8 detected, netkit is supported") - } else { - t.Log("Kernel version < 6.8 detected, netkit is not supported") - } -} - -func TestGetLinkType(t *testing.T) { - tests := []struct { - name string - link interface{} - want string - }{ - { - name: "nil link", - link: nil, - want: "unknown", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := getLinkType(nil); got != tt.want { - t.Errorf("getLinkType() = %v, want %v", got, tt.want) - } - }) - } -} From 8e50443e15ebd0a57f03e483e8cf4421d4d1e69d Mon Sep 17 00:00:00 2001 From: Mradul Tiwari Date: Wed, 28 Jan 2026 18:14:16 +0530 Subject: [PATCH 4/4] test: refactor unit tests per code review feedback - Use assert package for cleaner assertions (config_test.go style) - Simplify redundant test cases and use constants - Rename hypervisor to monitor in vaccel tests - Add t.Parallel() for parallel test execution Signed-off-by: Mradul Tiwari --- pkg/network/network_test.go | 81 ++++-------- pkg/unikontainers/rootfs_test.go | 165 +++++++----------------- pkg/unikontainers/vaccel_test.go | 214 ++++++++++++++----------------- 3 files changed, 169 insertions(+), 291 deletions(-) diff --git a/pkg/network/network_test.go b/pkg/network/network_test.go index 45d7d7d7..5676711e 100644 --- a/pkg/network/network_test.go +++ b/pkg/network/network_test.go @@ -16,88 +16,55 @@ package network import ( "testing" + + "github.com/stretchr/testify/assert" ) func TestGetTapIndex(t *testing.T) { // This test just verifies the function doesn't panic // The actual count depends on the system's network interfaces index, err := getTapIndex() - if err != nil { - t.Errorf("getTapIndex() error = %v", err) - return - } - - // Index should be non-negative - if index < 0 { - t.Errorf("getTapIndex() = %d, want >= 0", index) - } - - // Index should not exceed 255 (function returns error if > 255) - if index > 255 { - t.Errorf("getTapIndex() = %d, want <= 255", index) - } + assert.NoError(t, err, "getTapIndex() should not return an error") + assert.GreaterOrEqual(t, index, 0, "Index should be non-negative") + assert.LessOrEqual(t, index, 255, "Index should not exceed 255") } func TestNewNetworkManager(t *testing.T) { tests := []struct { - name string - networkType string - wantErr bool - wantType string + name string + networkType string + expectedErr bool + expectedType string }{ { - name: "static network manager", - networkType: "static", - wantErr: false, - wantType: "*network.StaticNetwork", + name: "static network manager", + networkType: "static", + expectedErr: false, + expectedType: "*network.StaticNetwork", }, { - name: "dynamic network manager", - networkType: "dynamic", - wantErr: false, - wantType: "*network.DynamicNetwork", + name: "dynamic network manager", + networkType: "dynamic", + expectedErr: false, + expectedType: "*network.DynamicNetwork", }, { name: "invalid network type", networkType: "invalid", - wantErr: true, - }, - { - name: "empty network type", - networkType: "", - wantErr: true, - }, - { - name: "unknown network type", - networkType: "bridge", - wantErr: true, + expectedErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() got, err := NewNetworkManager(tt.networkType) - if (err != nil) != tt.wantErr { - t.Errorf("NewNetworkManager() error = %v, wantErr %v", err, tt.wantErr) - return - } - - if !tt.wantErr { - if got == nil { - t.Error("NewNetworkManager() returned nil manager") - } + if tt.expectedErr { + assert.Error(t, err, "NewNetworkManager() should return an error") + } else { + assert.NoError(t, err, "NewNetworkManager() should not return an error") + assert.NotNil(t, got, "NewNetworkManager() should return a non-nil manager") } }) } } - -func TestEnsureEth0Exists(t *testing.T) { - // This test verifies the function works - // It might fail or succeed depending on the test environment - err := ensureEth0Exists() - - // We don't assert a specific result since it depends on - // whether eth0 exists in the test environment - // Just verify it doesn't panic - t.Logf("ensureEth0Exists() error = %v", err) -} diff --git a/pkg/unikontainers/rootfs_test.go b/pkg/unikontainers/rootfs_test.go index 35dfc6db..278394b1 100644 --- a/pkg/unikontainers/rootfs_test.go +++ b/pkg/unikontainers/rootfs_test.go @@ -17,144 +17,72 @@ package unikontainers import ( "testing" + "github.com/stretchr/testify/assert" "github.com/urunc-dev/urunc/pkg/unikontainers/types" ) func TestNewRootfsResult(t *testing.T) { - tests := []struct { - name string - rootfsType string - path string - mountedPath string - monRootfs string - want types.RootfsParams - }{ - { - name: "initrd rootfs", - rootfsType: "initrd", - path: "/path/to/initrd", - mountedPath: "", - monRootfs: "/run/urunc/mon", - want: types.RootfsParams{ - Type: "initrd", - Path: "/path/to/initrd", - MountedPath: "", - MonRootfs: "/run/urunc/mon", - }, - }, - { - name: "block rootfs", - rootfsType: "block", - path: "/dev/dm-0", - mountedPath: "/mnt/rootfs", - monRootfs: "/run/urunc/mon", - want: types.RootfsParams{ - Type: "block", - Path: "/dev/dm-0", - MountedPath: "/mnt/rootfs", - MonRootfs: "/run/urunc/mon", - }, - }, - { - name: "9pfs rootfs", - rootfsType: "9pfs", - path: "/container/rootfs", - mountedPath: "", - monRootfs: "/run/urunc/mon", - want: types.RootfsParams{ - Type: "9pfs", - Path: "/container/rootfs", - MountedPath: "", - MonRootfs: "/run/urunc/mon", - }, - }, - { - name: "empty paths", - rootfsType: "none", - path: "", - mountedPath: "", - monRootfs: "", - want: types.RootfsParams{ - Type: "none", - Path: "", - MountedPath: "", - MonRootfs: "", - }, - }, + expected := types.RootfsParams{ + Type: "initrd", + Path: "/path/to/initrd", + MountedPath: "/mnt/rootfs", + MonRootfs: "/run/urunc/mon", } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := newRootfsResult(tt.rootfsType, tt.path, tt.mountedPath, tt.monRootfs) - - if got.Type != tt.want.Type { - t.Errorf("Type = %v, want %v", got.Type, tt.want.Type) - } - if got.Path != tt.want.Path { - t.Errorf("Path = %v, want %v", got.Path, tt.want.Path) - } - if got.MountedPath != tt.want.MountedPath { - t.Errorf("MountedPath = %v, want %v", got.MountedPath, tt.want.MountedPath) - } - if got.MonRootfs != tt.want.MonRootfs { - t.Errorf("MonRootfs = %v, want %v", got.MonRootfs, tt.want.MonRootfs) - } - }) - } + got := newRootfsResult("initrd", "/path/to/initrd", "/mnt/rootfs", "/run/urunc/mon") + + assert.Equal(t, expected.Type, got.Type, "Type should match") + assert.Equal(t, expected.Path, got.Path, "Path should match") + assert.Equal(t, expected.MountedPath, got.MountedPath, "MountedPath should match") + assert.Equal(t, expected.MonRootfs, got.MonRootfs, "MonRootfs should match") } func TestRootfsSelector_TryInitrd(t *testing.T) { tests := []struct { - name string - annot map[string]string - wantFound bool - wantType string - wantPath string + name string + annot map[string]string + expectedFound bool + expectedType string + expectedPath string }{ { name: "initrd present", annot: map[string]string{ annotInitrd: "/path/to/initrd.img", }, - wantFound: true, - wantType: "initrd", - wantPath: "/path/to/initrd.img", + expectedFound: true, + expectedType: "initrd", + expectedPath: "/path/to/initrd.img", }, { - name: "initrd missing", - annot: map[string]string{}, - wantFound: false, + name: "initrd missing", + annot: map[string]string{}, + expectedFound: false, }, { name: "initrd empty", annot: map[string]string{ annotInitrd: "", }, - wantFound: false, + expectedFound: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() rs := &rootfsSelector{ annot: tt.annot, cntrRootfs: "/container/rootfs", } - + got, found := rs.tryInitrd() - - if found != tt.wantFound { - t.Errorf("tryInitrd() found = %v, want %v", found, tt.wantFound) - return - } - + + assert.Equal(t, tt.expectedFound, found, "tryInitrd() found mismatch") + if found { - if got.Type != tt.wantType { - t.Errorf("tryInitrd() Type = %v, want %v", got.Type, tt.wantType) - } - if got.Path != tt.wantPath { - t.Errorf("tryInitrd() Path = %v, want %v", got.Path, tt.wantPath) - } + assert.Equal(t, tt.expectedType, got.Type, "tryInitrd() Type mismatch") + assert.Equal(t, tt.expectedPath, got.Path, "tryInitrd() Path mismatch") } }) } @@ -162,69 +90,68 @@ func TestRootfsSelector_TryInitrd(t *testing.T) { func TestRootfsSelector_ShouldMountContainerRootfs(t *testing.T) { tests := []struct { - name string - annot map[string]string - want bool + name string + annot map[string]string + expected bool }{ { name: "mount rootfs true", annot: map[string]string{ annotMountRootfs: "true", }, - want: true, + expected: true, }, { name: "mount rootfs 1", annot: map[string]string{ annotMountRootfs: "1", }, - want: true, + expected: true, }, { name: "mount rootfs false", annot: map[string]string{ annotMountRootfs: "false", }, - want: false, + expected: false, }, { name: "mount rootfs 0", annot: map[string]string{ annotMountRootfs: "0", }, - want: false, + expected: false, }, { - name: "mount rootfs missing", - annot: map[string]string{}, - want: false, + name: "mount rootfs missing", + annot: map[string]string{}, + expected: false, }, { name: "mount rootfs empty", annot: map[string]string{ annotMountRootfs: "", }, - want: false, + expected: false, }, { name: "mount rootfs invalid", annot: map[string]string{ annotMountRootfs: "invalid", }, - want: false, + expected: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() rs := &rootfsSelector{ annot: tt.annot, } - + got := rs.shouldMountContainerRootfs() - if got != tt.want { - t.Errorf("shouldMountContainerRootfs() = %v, want %v", got, tt.want) - } + assert.Equal(t, tt.expected, got, "shouldMountContainerRootfs() mismatch") }) } } diff --git a/pkg/unikontainers/vaccel_test.go b/pkg/unikontainers/vaccel_test.go index bb088639..f887851d 100644 --- a/pkg/unikontainers/vaccel_test.go +++ b/pkg/unikontainers/vaccel_test.go @@ -16,171 +16,155 @@ package unikontainers import ( "testing" + + "github.com/stretchr/testify/assert" +) + +const ( + minCID = 3 + maxCID = 99 ) func TestIdToGuestCID(t *testing.T) { tests := []struct { - name string - id string - wantMin int - wantMax int - checkCID int + name string + id string + expectedCID int }{ { - name: "empty string", - id: "", - wantMin: 3, - wantMax: 99, + name: "empty string", + id: "", + expectedCID: idToGuestCID(""), }, { - name: "simple id", - id: "container123", - wantMin: 3, - wantMax: 99, - checkCID: -1, - }, - { - name: "long id", - id: "very-long-container-id-12345678", - wantMin: 3, - wantMax: 99, - checkCID: -1, - }, - { - name: "special characters", - id: "container-with-special_chars.123", - wantMin: 3, - wantMax: 99, - checkCID: -1, + name: "simple id", + id: "container123", + expectedCID: idToGuestCID("container123"), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() got := idToGuestCID(tt.id) - + // Check if CID is within valid range - if got < tt.wantMin || got > tt.wantMax { - t.Errorf("idToGuestCID(%q) = %d, want value between %d and %d", - tt.id, got, tt.wantMin, tt.wantMax) - } - - // Test determinism - same input should always produce same output - got2 := idToGuestCID(tt.id) - if got != got2 { - t.Errorf("idToGuestCID(%q) is not deterministic: first=%d, second=%d", - tt.id, got, got2) - } + assert.GreaterOrEqual(t, got, minCID, "CID should be >= %d", minCID) + assert.LessOrEqual(t, got, maxCID, "CID should be <= %d", maxCID) + + // Verify the expected result matches + assert.Equal(t, tt.expectedCID, got, "CID should match expected value") }) } } func TestIsValidVSockAddress(t *testing.T) { tests := []struct { - name string - rpcAddress string - hypervisor string - wantValid bool - wantErr bool - wantPath string + name string + rpcAddress string + monitor string + expectedValid bool + expectedErr bool + expectedPath string }{ { - name: "valid qemu vsock address", - rpcAddress: "vsock://2:1234", - hypervisor: "qemu", - wantValid: true, - wantErr: false, + name: "valid qemu vsock address", + rpcAddress: "vsock://2:1234", + monitor: "qemu", + expectedValid: true, + expectedErr: false, + expectedPath: "", }, { - name: "invalid qemu vsock - wrong CID", - rpcAddress: "vsock://3:1234", - hypervisor: "qemu", - wantValid: false, - wantErr: true, + name: "invalid qemu vsock - wrong CID", + rpcAddress: "vsock://3:1234", + monitor: "qemu", + expectedValid: false, + expectedErr: true, }, { - name: "invalid qemu vsock - no port", - rpcAddress: "vsock://2:", - hypervisor: "qemu", - wantValid: false, - wantErr: true, + name: "invalid qemu vsock - no port", + rpcAddress: "vsock://2:", + monitor: "qemu", + expectedValid: false, + expectedErr: true, }, { - name: "invalid qemu vsock - malformed", - rpcAddress: "vsock://invalid", - hypervisor: "qemu", - wantValid: false, - wantErr: true, + name: "invalid qemu vsock - malformed", + rpcAddress: "vsock://invalid", + monitor: "qemu", + expectedValid: false, + expectedErr: true, }, { - name: "not a vsock address", - rpcAddress: "http://localhost:1234", - hypervisor: "qemu", - wantValid: false, - wantErr: true, + name: "not a vsock address", + rpcAddress: "http://localhost:1234", + monitor: "qemu", + expectedValid: false, + expectedErr: true, }, { - name: "empty address", - rpcAddress: "", - hypervisor: "qemu", - wantValid: false, - wantErr: true, + name: "empty address", + rpcAddress: "", + monitor: "qemu", + expectedValid: false, + expectedErr: true, }, { - name: "valid firecracker unix socket", - rpcAddress: "unix:///tmp/vaccel.sock_1234", - hypervisor: "firecracker", - wantValid: true, - wantErr: false, - wantPath: "/tmp", + name: "valid firecracker unix socket", + rpcAddress: "unix:///tmp/vaccel.sock_1234", + monitor: "firecracker", + expectedValid: true, + expectedErr: false, + expectedPath: "/tmp", }, { - name: "valid firecracker nested path", - rpcAddress: "unix:///var/run/urunc/vaccel.sock_5678", - hypervisor: "firecracker", - wantValid: true, - wantErr: false, - wantPath: "/var/run/urunc", + name: "valid firecracker nested path", + rpcAddress: "unix:///var/run/urunc/vaccel.sock_5678", + monitor: "firecracker", + expectedValid: true, + expectedErr: false, + expectedPath: "/var/run/urunc", }, { - name: "firecracker invalid - wrong socket name", - rpcAddress: "unix:///tmp/test.sock", - hypervisor: "firecracker", - wantValid: false, - wantErr: true, + name: "firecracker invalid - wrong socket name", + rpcAddress: "unix:///tmp/test.sock", + monitor: "firecracker", + expectedValid: false, + expectedErr: true, }, { - name: "firecracker invalid - no unix prefix", - rpcAddress: "/tmp/vaccel.sock_1234", - hypervisor: "firecracker", - wantValid: false, - wantErr: true, + name: "firecracker invalid - no unix prefix", + rpcAddress: "/tmp/vaccel.sock_1234", + monitor: "firecracker", + expectedValid: false, + expectedErr: true, }, { - name: "unsupported hypervisor", - rpcAddress: "vsock://2:1234", - hypervisor: "kvm", - wantValid: false, - wantErr: true, + name: "unsupported monitor", + rpcAddress: "vsock://2:1234", + monitor: "kvm", + expectedValid: false, + expectedErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() addr := tt.rpcAddress - gotValid, gotPath, err := isValidVSockAddress(&addr, tt.hypervisor) - - if (err != nil) != tt.wantErr { - t.Errorf("isValidVSockAddress() error = %v, wantErr %v", err, tt.wantErr) - return - } - - if gotValid != tt.wantValid { - t.Errorf("isValidVSockAddress() valid = %v, want %v", gotValid, tt.wantValid) + gotValid, gotPath, err := isValidVSockAddress(&addr, tt.monitor) + + if tt.expectedErr { + assert.Error(t, err, "isValidVSockAddress() should return an error") + } else { + assert.NoError(t, err, "isValidVSockAddress() should not return an error") } - - if tt.wantPath != "" && gotPath != tt.wantPath { - t.Errorf("isValidVSockAddress() path = %v, want %v", gotPath, tt.wantPath) + + assert.Equal(t, tt.expectedValid, gotValid, "isValidVSockAddress() valid mismatch") + + if tt.expectedPath != "" { + assert.Equal(t, tt.expectedPath, gotPath, "isValidVSockAddress() path mismatch") } }) }