Address code review feedback

- Use io.MultiWriter to send output to both console and log file
- Add warning logs for cgroup degradation instead of silent failures
- Remove duplicate command/args extraction in run function
- Improve verify-new.sh with proper binary validation and sudo usage

Co-authored-by: j143 <[email protected]>

Author: Copilot

cgroup.go

@@ -95,15 +95,16 @@ func SetupCgroupsV2(containerID string, memoryLimit int64) error {
 
 	// Try to enable memory controller
 	if err := os.WriteFile(parentControl, []byte("+memory"), 0644); err != nil {
-		// It's OK if this fails - may not have permission
-		// Continue without memory limits
+		// Log warning but continue - cgroup limits may not be available
+		fmt.Printf("Warning: Cannot enable memory controller in cgroup v2 (degraded mode): %v\n", err)
 		return nil
 	}
 
 	// Set memory limit if supported
 	memoryMaxFile := filepath.Join(cgroupPath, "memory.max")
 	if err := os.WriteFile(memoryMaxFile, []byte(strconv.FormatInt(memoryLimit, 10)), 0644); err != nil {
-		// Not fatal - just means we can't set limits
+		// Log warning - limits won't be enforced but container can still run
+		fmt.Printf("Warning: Cannot set memory limit in cgroup v2 (degraded mode): %v\n", err)
 		return nil
 	}
 
@@ -129,7 +130,8 @@ func SetupCgroupsV1(containerID string, memoryLimit int64) error {
 	// Set memory limit
 	memoryLimitFile := filepath.Join(cgroupPath, "memory.limit_in_bytes")
 	if err := os.WriteFile(memoryLimitFile, []byte(strconv.FormatInt(memoryLimit, 10)), 0644); err != nil {
-		// Not fatal - just means we can't set limits
+		// Log warning - limits won't be enforced but container can still run
+		fmt.Printf("Warning: Cannot set memory limit in cgroup v1 (degraded mode): %v\n", err)
 		return nil
 	}
 

main.go

@@ -3,6 +3,7 @@ package main
 import (
 	"encoding/json"
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -581,15 +582,18 @@ func run() {
 		os.Exit(1)
 	}
 
-	// Create container metadata
-	createdAt := time.Now()
-	command := ""
-	args := []string{}
-	if len(os.Args) >= 4 {
-		command = os.Args[3]
-		args = os.Args[4:]
+	// Execute the command in the container
+	if len(os.Args) < 4 {
+		fmt.Println("Error: Command required for run")
+		os.Exit(1)
 	}
 
+	// Extract command and args once
+	command := os.Args[3]
+	args := os.Args[4:]
+
+	// Create container metadata
+	createdAt := time.Now()
 	metadata := ContainerMetadata{
 		ID:         containerID,
 		State:      StateCreated,
@@ -608,12 +612,6 @@ func run() {
 
 	fmt.Printf("Starting container %s\n", containerID)
 
-	// Execute the command in the container
-	if len(os.Args) < 4 {
-		fmt.Println("Error: Command required for run")
-		os.Exit(1)
-	}
-
 	runWithoutNamespaces(containerID, rootfs, command, args)
 }
 
@@ -783,13 +781,14 @@ func runWithoutNamespaces(containerID, rootfs, command string, args []string) {
 
 	cmd := exec.Command(command, args...)
 	cmd.Stdin = os.Stdin
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
 
-	// Also write to log file if available
+	// Use MultiWriter to send output to both console and log file
 	if logFd != nil {
-		cmd.Stdout = logFd
-		cmd.Stderr = logFd
+		cmd.Stdout = io.MultiWriter(os.Stdout, logFd)
+		cmd.Stderr = io.MultiWriter(os.Stderr, logFd)
+	} else {
+		cmd.Stdout = os.Stdout
+		cmd.Stderr = os.Stderr
 	}
 
 	err = cmd.Run()

verify-new.sh

@@ -83,10 +83,42 @@ fi
 section "Test 2: Creating Test Image"
 info "Setting up test image with basic binaries"
 TEST_IMAGE_DIR="/tmp/basic-docker/images/test-image/rootfs"
-mkdir -p "$TEST_IMAGE_DIR/bin"
-cp /bin/echo "$TEST_IMAGE_DIR/bin/" 2>/dev/null || cp /usr/bin/echo "$TEST_IMAGE_DIR/bin/"
-cp /bin/sh "$TEST_IMAGE_DIR/bin/" 2>/dev/null || cp /usr/bin/sh "$TEST_IMAGE_DIR/bin/" || cp /bin/bash "$TEST_IMAGE_DIR/bin/sh"
-success "Test image created"
+sudo mkdir -p "$TEST_IMAGE_DIR/bin"
+
+# Try to copy echo binary
+if [ -f /bin/echo ]; then
+    sudo cp /bin/echo "$TEST_IMAGE_DIR/bin/"
+elif [ -f /usr/bin/echo ]; then
+    sudo cp /usr/bin/echo "$TEST_IMAGE_DIR/bin/"
+else
+    error "Could not find echo binary"
+    exit 1
+fi
+
+# Try to copy shell binary
+if [ -f /bin/sh ]; then
+    sudo cp /bin/sh "$TEST_IMAGE_DIR/bin/"
+elif [ -f /usr/bin/sh ]; then
+    sudo cp /usr/bin/sh "$TEST_IMAGE_DIR/bin/"
+elif [ -f /bin/bash ]; then
+    sudo cp /bin/bash "$TEST_IMAGE_DIR/bin/sh"
+else
+    error "Could not find shell binary"
+    exit 1
+fi
+
+# Verify binaries are actually present
+if [ ! -f "$TEST_IMAGE_DIR/bin/echo" ]; then
+    error "Failed to copy echo binary to test image"
+    exit 1
+fi
+
+if [ ! -f "$TEST_IMAGE_DIR/bin/sh" ]; then
+    error "Failed to copy shell binary to test image"
+    exit 1
+fi
+
+success "Test image created with required binaries"
 
 # Test 3: Container Lifecycle - Run and State Tracking
 section "Test 3: Container Lifecycle - Run Command"