Merge pull request #28812 from bit-hope/improvements-for-libpod

Some improvements for libpod

libpod/container_inspect.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"maps"
+	"slices"
 	"strings"
 
 	"github.com/docker/go-units"
@@ -81,7 +82,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *define.Driver
 		args = args[1:]
 	}
 
-	execIDs := []string{}
+	execIDs := make([]string, 0, len(c.state.ExecSessions))
 	for id := range c.state.ExecSessions {
 		execIDs = append(execIDs, id)
 	}
@@ -252,6 +253,8 @@ func (c *Container) GetMounts(namedVolumes []*ContainerNamedVolume, imageVolumes
 		return inspectMounts, nil
 	}
 
+	inspectMounts = make([]define.InspectMount, 0, len(namedVolumes)+len(imageVolumes)+len(mounts))
+
 	for _, volume := range namedVolumes {
 		mountStruct := define.InspectMount{}
 		mountStruct.Type = "volume"
@@ -513,17 +516,10 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named
 		hostConfig.Dns = append(hostConfig.Dns, dns.String())
 	}
 
-	hostConfig.DnsOptions = make([]string, 0, len(c.config.DNSOption))
-	hostConfig.DnsOptions = append(hostConfig.DnsOptions, c.config.DNSOption...)
-
-	hostConfig.DnsSearch = make([]string, 0, len(c.config.DNSSearch))
-	hostConfig.DnsSearch = append(hostConfig.DnsSearch, c.config.DNSSearch...)
-
-	hostConfig.ExtraHosts = make([]string, 0, len(c.config.HostAdd))
-	hostConfig.ExtraHosts = append(hostConfig.ExtraHosts, c.config.HostAdd...)
-
-	hostConfig.GroupAdd = make([]string, 0, len(c.config.Groups))
-	hostConfig.GroupAdd = append(hostConfig.GroupAdd, c.config.Groups...)
+	hostConfig.DnsOptions = slices.Clone(c.config.DNSOption)
+	hostConfig.DnsSearch = slices.Clone(c.config.DNSSearch)
+	hostConfig.ExtraHosts = slices.Clone(c.config.HostAdd)
+	hostConfig.GroupAdd = slices.Clone(c.config.Groups)
 
 	hostConfig.HostsFile = c.config.BaseHostsFile
 

libpod/pod_api.go

@@ -625,10 +625,7 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) {
 	}
 	slices.SortFunc(ctrs, func(a, b define.InspectPodContainerInfo) int { return strings.Compare(a.ID, b.ID) })
 
-	podState, err := createPodStatusResults(ctrStatuses)
-	if err != nil {
-		return nil, err
-	}
+	podState := createPodStatusResults(ctrStatuses)
 
 	namespaces := map[string]bool{
 		"pid":    p.config.UsePodPID,

libpod/pod_status.go

@@ -12,52 +12,52 @@ func (p *Pod) GetPodStatus() (string, error) {
 	if err != nil {
 		return define.PodStateErrored, err
 	}
-	return createPodStatusResults(ctrStatuses)
+	return createPodStatusResults(ctrStatuses), nil
 }
 
-func createPodStatusResults(ctrStatuses map[string]define.ContainerStatus) (string, error) {
+func createPodStatusResults(ctrStatuses map[string]define.ContainerStatus) string {
 	ctrNum := len(ctrStatuses)
 	if ctrNum == 0 {
-		return define.PodStateCreated, nil
-	}
-	statuses := map[string]int{
-		define.PodStateStopped: 0,
-		define.PodStateRunning: 0,
-		define.PodStatePaused:  0,
-		define.PodStateCreated: 0,
-		define.PodStateErrored: 0,
+		return define.PodStateCreated
 	}
+
+	statusPodStateStopped := 0
+	statusPodStateRunning := 0
+	statusPodStatePaused := 0
+	statusPodStateCreated := 0
+	statusPodStateErrored := 0
+
 	for _, ctrStatus := range ctrStatuses {
 		switch ctrStatus {
 		case define.ContainerStateExited:
 			fallthrough
 		case define.ContainerStateStopped:
-			statuses[define.PodStateStopped]++
+			statusPodStateStopped++
 		case define.ContainerStateRunning:
-			statuses[define.PodStateRunning]++
+			statusPodStateRunning++
 		case define.ContainerStatePaused:
-			statuses[define.PodStatePaused]++
+			statusPodStatePaused++
 		case define.ContainerStateCreated, define.ContainerStateConfigured:
-			statuses[define.PodStateCreated]++
+			statusPodStateCreated++
 		default:
-			statuses[define.PodStateErrored]++
+			statusPodStateErrored++
 		}
 	}
 
 	switch {
-	case statuses[define.PodStateRunning] == ctrNum:
-		return define.PodStateRunning, nil
-	case statuses[define.PodStateRunning] > 0:
-		return define.PodStateDegraded, nil
-	case statuses[define.PodStatePaused] == ctrNum:
-		return define.PodStatePaused, nil
-	case statuses[define.PodStateStopped] == ctrNum:
-		return define.PodStateExited, nil
-	case statuses[define.PodStateStopped] > 0:
-		return define.PodStateStopped, nil
-	case statuses[define.PodStateErrored] > 0:
-		return define.PodStateErrored, nil
+	case statusPodStateRunning == ctrNum:
+		return define.PodStateRunning
+	case statusPodStateRunning > 0:
+		return define.PodStateDegraded
+	case statusPodStatePaused == ctrNum:
+		return define.PodStatePaused
+	case statusPodStateStopped == ctrNum:
+		return define.PodStateExited
+	case statusPodStateStopped > 0:
+		return define.PodStateStopped
+	case statusPodStateErrored > 0:
+		return define.PodStateErrored
 	default:
-		return define.PodStateCreated, nil
+		return define.PodStateCreated
 	}
 }

libpod/pod_status_test.go

@@ -0,0 +1,103 @@
+//go:build !remote && (linux || freebsd)
+
+package libpod
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"go.podman.io/podman/v6/libpod/define"
+)
+
+func Test_createPodStatusResults(t *testing.T) {
+	tests := []struct {
+		name        string
+		ctrStatuses map[string]define.ContainerStatus
+		want        string
+	}{
+		{
+			name:        "EmptyPod",
+			ctrStatuses: map[string]define.ContainerStatus{},
+			want:        define.PodStateCreated,
+		},
+		{
+			name: "AllRunning",
+			ctrStatuses: map[string]define.ContainerStatus{
+				"ctr1": define.ContainerStateRunning,
+				"ctr2": define.ContainerStateRunning,
+				"ctr3": define.ContainerStateRunning,
+			},
+			want: define.PodStateRunning,
+		},
+		{
+			name: "Degraded",
+			ctrStatuses: map[string]define.ContainerStatus{
+				"ctr1": define.ContainerStateRunning,
+				"ctr2": define.ContainerStateStopped,
+			},
+			want: define.PodStateDegraded,
+		},
+		{
+			name: "AllPaused",
+			ctrStatuses: map[string]define.ContainerStatus{
+				"ctr1": define.ContainerStatePaused,
+				"ctr2": define.ContainerStatePaused,
+			},
+			want: define.PodStatePaused,
+		},
+		{
+			name: "AllExited",
+			ctrStatuses: map[string]define.ContainerStatus{
+				"ctr1": define.ContainerStateExited,
+				"ctr2": define.ContainerStateStopped,
+			},
+			want: define.PodStateExited,
+		},
+		{
+			name: "PartialStopped",
+			ctrStatuses: map[string]define.ContainerStatus{
+				"ctr1": define.ContainerStateStopped,
+				"ctr2": define.ContainerStateCreated,
+			},
+			want: define.PodStateStopped,
+		},
+		{
+			name: "Errored",
+			ctrStatuses: map[string]define.ContainerStatus{
+				"ctr1": define.ContainerStateCreated,
+				"ctr2": define.ContainerStateUnknown,
+			},
+			want: define.PodStateErrored,
+		},
+		{
+			name: "AllCreated",
+			ctrStatuses: map[string]define.ContainerStatus{
+				"ctr1": define.ContainerStateCreated,
+				"ctr2": define.ContainerStateConfigured,
+			},
+			want: define.PodStateCreated,
+		},
+		{
+			name: "SingleRunning",
+			ctrStatuses: map[string]define.ContainerStatus{
+				"ctr1": define.ContainerStateRunning,
+			},
+			want: define.PodStateRunning,
+		},
+		{
+			name: "ExitedCountsAsStopped",
+			ctrStatuses: map[string]define.ContainerStatus{
+				"ctr1": define.ContainerStateExited,
+				"ctr2": define.ContainerStateExited,
+			},
+			want: define.PodStateExited,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := createPodStatusResults(tt.ctrStatuses)
+			assert.Equalf(t, tt.want, got, "createPodStatusResults(%v)", tt.ctrStatuses)
+		})
+	}
+}

libpod/runtime_ctr.go

@@ -11,7 +11,6 @@ import (
 	"os"
 	"path"
 	"path/filepath"
-	"slices"
 	"strings"
 	"time"
 
@@ -260,17 +259,17 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
 	if len(ctr.config.Networks) > 0 {
 		normalizeNetworks := make([]types.NamedPerNetworkOptions, 0, len(ctr.config.Networks))
 		// first get the already used interface names so we do not conflict
-		usedIfNames := make([]string, 0, len(ctr.config.Networks))
+		usedIfNames := make(map[string]struct{}, len(ctr.config.Networks))
 		for _, opts := range ctr.config.Networks {
 			if opts.InterfaceName != "" {
 				// check that no name is assigned to more than network
-				if slices.Contains(usedIfNames, opts.InterfaceName) {
+				if _, exists := usedIfNames[opts.InterfaceName]; exists {
 					return nil, fmt.Errorf("network interface name %q is already assigned to another network", opts.InterfaceName)
 				}
-				usedIfNames = append(usedIfNames, opts.InterfaceName)
+				usedIfNames[opts.InterfaceName] = struct{}{}
 			}
 		}
-		i := 0
+		nextEthIdx := 0
 		for _, network := range ctr.config.Networks {
 			netName, nicName, err := r.normalizeNetworkName(network.Name)
 			if err != nil {
@@ -285,14 +284,14 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
 
 			// assign default interface name if empty
 			if network.InterfaceName == "" {
-				for i < 100000 {
-					ifName := fmt.Sprintf("eth%d", i)
-					if !slices.Contains(usedIfNames, ifName) {
+				for nextEthIdx < 100000 {
+					ifName := fmt.Sprintf("eth%d", nextEthIdx)
+					if _, exists := usedIfNames[ifName]; !exists {
 						network.InterfaceName = ifName
-						usedIfNames = append(usedIfNames, ifName)
+						usedIfNames[ifName] = struct{}{}
 						break
 					}
-					i++
+					nextEthIdx++
 				}
 				// if still empty we did not find a free name
 				if network.InterfaceName == "" {