Merge pull request #25743 from kolyshkin/freebsd-golangci-lint

Add freebsd golangci lint run; fix remaining freebsd warnings

.cirrus.yml

@@ -334,8 +334,20 @@ freebsd_alt_build_task:
         ALT_NAME: 'FreeBSD Cross'
     freebsd_instance:
         image_family: freebsd-13-4
+        # golangci-lint is a very, very hungry beast.
+        cpu: 4
+        memory: 8Gb
     setup_script:
         - pkg install -y gpgme bash go-md2man gmake gsed gnugrep go pkgconf zstd
+        - go version # Downloads a new go version based on go.mod's go directive.
+    golint_cache:
+        folder: ~/.cache/golangci-lint
+        reupload_on_changes: true
+        fingerprint_script:
+            - go version
+            - grep GOLANGCI_LINT_VERSION Makefile | head -1
+    lint_script:
+        - gmake golangci-lint
     build_amd64_script:
         - gmake podman-release
     # This task cannot make use of the shared repo.tar.zst artifact and must

cmd/podman-testing/store_supported.go

@@ -1,4 +1,4 @@
-//go:build linux && !remote
+//go:build (linux || freebsd) && !remote
 
 package main
 

hack/golangci-lint.sh

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 # Run golangci-lint with different sets of build tags.
 set -e
@@ -8,23 +8,29 @@ set -e
 # a very old version, where modern features (like `declare -A`) are
 # absent.
 
-echo "Linting for GOOS=$GOOS"
-# Special case: for Darwin and Windows only "remote" linting is possible and required.
-if [[ "$GOOS" == "windows" || "$GOOS" == "darwin" ]]; then
-  (
-    set -x
-    ./bin/golangci-lint run --build-tags="remote,containers_image_openpgp" "$@"
-  )
-  exit 0
-fi
+declare -a EXTRA_TAGS
 
-# Normal case (Linux): run linter for various sets of build tags.
-TAGS="apparmor,seccomp,selinux"
-for EXTRA_TAGS in "" ",systemd" ",remote"; do
+echo "Linting for GOOS=$GOOS"
+case "$GOOS" in
+  windows|darwin)
+    # For Darwin and Windows, only "remote" linting is possible and required.
+    TAGS="remote,containers_image_openpgp"
+    ;;
+  freebsd)
+    TAGS="containers_image_openpgp"
+    EXTRA_TAGS=(",remote")
+    ;;
+  *)
+    # Assume Linux: run linter for various sets of build tags.
+    TAGS="apparmor,seccomp,selinux"
+    EXTRA_TAGS=(",systemd" ",remote")
+esac
+
+for EXTRA in "" "${EXTRA_TAGS[@]}"; do
+  # Use set -x in a subshell to make it easy for a developer to copy-paste
+  # the command-line to focus or debug a single, specific linting category.
   (
-    # Make it really easy for a developer to copy-paste the command-line
-    # to focus or debug a single, specific linting category.
     set -x
-    ./bin/golangci-lint run --build-tags="${TAGS}${EXTRA_TAGS}" "$@"
+    ./bin/golangci-lint run --build-tags="${TAGS}${EXTRA}" "$@"
   )
 done

libpod/container_inspect.go

@@ -11,7 +11,6 @@ import (
 	"github.com/containers/podman/v5/libpod/driver"
 	"github.com/containers/podman/v5/pkg/signal"
 	"github.com/containers/podman/v5/pkg/util"
-	"github.com/containers/storage/types"
 	"github.com/docker/go-units"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
@@ -491,17 +490,6 @@ func (c *Container) generateInspectContainerConfig(spec *spec.Spec) *define.Insp
 	return ctrConfig
 }
 
-func generateIDMappings(idMappings types.IDMappingOptions) *define.InspectIDMappings {
-	var inspectMappings define.InspectIDMappings
-	for _, uid := range idMappings.UIDMap {
-		inspectMappings.UIDMap = append(inspectMappings.UIDMap, fmt.Sprintf("%d:%d:%d", uid.ContainerID, uid.HostID, uid.Size))
-	}
-	for _, gid := range idMappings.GIDMap {
-		inspectMappings.GIDMap = append(inspectMappings.GIDMap, fmt.Sprintf("%d:%d:%d", gid.ContainerID, gid.HostID, gid.Size))
-	}
-	return &inspectMappings
-}
-
 // Generate the InspectContainerHostConfig struct for the HostConfig field of
 // Inspect.
 func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, namedVolumes []*ContainerNamedVolume, mounts []spec.Mount) (*define.InspectContainerHostConfig, error) {
@@ -659,29 +647,6 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named
 	return hostConfig, nil
 }
 
-// Return true if the container is running in the host's PID NS.
-func (c *Container) inHostPidNS() (bool, error) {
-	if c.config.PIDNsCtr != "" {
-		return false, nil
-	}
-	ctrSpec, err := c.specFromState()
-	if err != nil {
-		return false, err
-	}
-	if ctrSpec.Linux != nil {
-		// Locate the spec's PID namespace.
-		// If there is none, it's pid=host.
-		// If there is one and it has a path, it's "ns:".
-		// If there is no path, it's default - the empty string.
-		for _, ns := range ctrSpec.Linux.Namespaces {
-			if ns.Type == spec.PIDNamespace {
-				return false, nil
-			}
-		}
-	}
-	return true, nil
-}
-
 func (c *Container) GetDevices(priv bool, ctrSpec spec.Spec, deviceNodes map[string]string) ([]define.InspectDevice, error) {
 	devices := []define.InspectDevice{}
 	if ctrSpec.Linux != nil && !priv {

libpod/container_inspect_linux.go

@@ -10,6 +10,7 @@ import (
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/podman/v5/libpod/define"
 	"github.com/containers/podman/v5/pkg/util"
+	"github.com/containers/storage/types"
 	"github.com/moby/sys/capability"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
@@ -309,3 +310,37 @@ func (c *Container) platformInspectContainerHostConfig(ctrSpec *spec.Spec, hostC
 
 	return nil
 }
+
+func generateIDMappings(idMappings types.IDMappingOptions) *define.InspectIDMappings {
+	var inspectMappings define.InspectIDMappings
+	for _, uid := range idMappings.UIDMap {
+		inspectMappings.UIDMap = append(inspectMappings.UIDMap, fmt.Sprintf("%d:%d:%d", uid.ContainerID, uid.HostID, uid.Size))
+	}
+	for _, gid := range idMappings.GIDMap {
+		inspectMappings.GIDMap = append(inspectMappings.GIDMap, fmt.Sprintf("%d:%d:%d", gid.ContainerID, gid.HostID, gid.Size))
+	}
+	return &inspectMappings
+}
+
+// Return true if the container is running in the host's PID NS.
+func (c *Container) inHostPidNS() (bool, error) {
+	if c.config.PIDNsCtr != "" {
+		return false, nil
+	}
+	ctrSpec, err := c.specFromState()
+	if err != nil {
+		return false, err
+	}
+	if ctrSpec.Linux != nil {
+		// Locate the spec's PID namespace.
+		// If there is none, it's pid=host.
+		// If there is one and it has a path, it's "ns:".
+		// If there is no path, it's default - the empty string.
+		for _, ns := range ctrSpec.Linux.Namespaces {
+			if ns.Type == spec.PIDNamespace {
+				return false, nil
+			}
+		}
+	}
+	return true, nil
+}

libpod/container_internal_freebsd.go

@@ -3,6 +3,7 @@
 package libpod
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -180,7 +181,9 @@ func (c *Container) addNetworkContainer(g *generate.Generator, ctr string) error
 	if err != nil {
 		return fmt.Errorf("retrieving dependency %s of container %s from state: %w", ctr, c.ID(), err)
 	}
-	c.runtime.state.UpdateContainer(nsCtr)
+	if err := c.runtime.state.UpdateContainer(nsCtr); err != nil {
+		return err
+	}
 	if nsCtr.state.NetNS != "" {
 		g.AddAnnotation("org.freebsd.parentJail", nsCtr.state.NetNS)
 	}
@@ -252,10 +255,9 @@ func (c *Container) addSharedNamespaces(g *generate.Generator) error {
 	// the user (already present in OCI spec). If we don't have a UTS ns,
 	// set it to the host's hostname instead.
 	hostname := c.Hostname()
-	foundUTS := false
 
 	// TODO: make this optional, needs progress on adding FreeBSD section to the spec
-	foundUTS = true
+	foundUTS := true
 	g.SetHostname(hostname)
 
 	if !foundUTS {
@@ -390,7 +392,7 @@ func (c *Container) getPlatformRunPath() (string, error) {
 		if err != nil {
 			return "", err
 		}
-		inspectData, err := image.Inspect(nil, nil)
+		inspectData, err := image.Inspect(context.TODO(), nil)
 		if err != nil {
 			return "", err
 		}

libpod/info.go

@@ -231,14 +231,15 @@ func (r *Runtime) storeInfo() (*define.StoreInfo, error) {
 	if err := syscall.Statfs(r.store.GraphRoot(), &grStats); err != nil {
 		return nil, fmt.Errorf("unable to collect graph root usage for %q: %w", r.store.GraphRoot(), err)
 	}
-	allocated := uint64(grStats.Bsize) * grStats.Blocks
+	bsize := uint64(grStats.Bsize) //nolint:unconvert,nolintlint // Bsize is not always uint64 on Linux.
+	allocated := bsize * grStats.Blocks
 	info := define.StoreInfo{
 		ImageStore:         imageInfo,
 		ImageCopyTmpDir:    os.Getenv("TMPDIR"),
 		ContainerStore:     conInfo,
 		GraphRoot:          r.store.GraphRoot(),
 		GraphRootAllocated: allocated,
-		GraphRootUsed:      allocated - (uint64(grStats.Bsize) * grStats.Bfree),
+		GraphRootUsed:      allocated - (bsize * grStats.Bfree),
 		RunRoot:            r.store.RunRoot(),
 		GraphDriverName:    r.store.GraphDriverName(),
 		GraphOptions:       nil,

libpod/networking_freebsd.go

@@ -9,12 +9,10 @@ import (
 	"fmt"
 	"net"
 	"os/exec"
-	"path/filepath"
 
 	"github.com/containers/buildah/pkg/jail"
 	"github.com/containers/common/libnetwork/types"
 	"github.com/containers/podman/v5/libpod/define"
-	"github.com/containers/storage/pkg/lockfile"
 	"github.com/sirupsen/logrus"
 )
 
@@ -46,38 +44,6 @@ type NetstatAddress struct {
 	Collisions uint64 `json:"collisions"`
 }
 
-type RootlessNetNS struct {
-	dir  string
-	Lock *lockfile.LockFile
-}
-
-// getPath will join the given path to the rootless netns dir
-func (r *RootlessNetNS) getPath(path string) string {
-	return filepath.Join(r.dir, path)
-}
-
-// Do - run the given function in the rootless netns.
-// It does not lock the rootlessCNI lock, the caller
-// should only lock when needed, e.g. for network operations.
-func (r *RootlessNetNS) Do(toRun func() error) error {
-	return errors.New("not supported on freebsd")
-}
-
-// Cleanup the rootless network namespace if needed.
-// It checks if we have running containers with the bridge network mode.
-// Cleanup() expects that r.Lock is locked
-func (r *RootlessNetNS) Cleanup(runtime *Runtime) error {
-	return errors.New("not supported on freebsd")
-}
-
-// GetRootlessNetNs returns the rootless netns object. If create is set to true
-// the rootless network namespace will be created if it does not already exist.
-// If called as root it returns always nil.
-// On success the returned RootlessCNI lock is locked and must be unlocked by the caller.
-func (r *Runtime) GetRootlessNetNs(new bool) (*RootlessNetNS, error) {
-	return nil, nil
-}
-
 func getSlirp4netnsIP(subnet *net.IPNet) (*net.IP, error) {
 	return nil, errors.New("not implemented GetSlirp4netnsIP")
 }
@@ -145,7 +111,7 @@ func (r *Runtime) createNetNS(ctr *Container) (n string, q map[string]types.Stat
 	jconf.Set("securelevel", -1)
 	j, err := jail.Create(jconf)
 	if err != nil {
-		return "", nil, fmt.Errorf("Failed to create vnet jail %s for container %s: %w", netns, ctr.ID(), err)
+		return "", nil, fmt.Errorf("failed to create vnet jail %s for container %s: %w", netns, ctr.ID(), err)
 	}
 
 	logrus.Debugf("Created vnet jail %s for container %s", netns, ctr.ID())
@@ -157,7 +123,7 @@ func (r *Runtime) createNetNS(ctr *Container) (n string, q map[string]types.Stat
 		jconf.Set("persist", false)
 		if err := j.Set(jconf); err != nil {
 			// Log this error and return the error from configureNetNS
-			logrus.Errorf("failed to destroy vnet jail %s: %w", netns, err)
+			logrus.Errorf("failed to destroy vnet jail %s: %v", netns, err)
 		}
 	}
 	return netns, networkStatus, err
@@ -258,13 +224,8 @@ func (c *Container) joinedNetworkNSPath() (string, bool) {
 func (c *Container) inspectJoinedNetworkNS(networkns string) (q types.StatusBlock, retErr error) {
 	// TODO: extract interface information from the vnet jail
 	return types.StatusBlock{}, nil
-
 }
 
 func (c *Container) reloadRootlessRLKPortMapping() error {
 	return errors.New("unsupported (*Container).reloadRootlessRLKPortMapping")
 }
-
-func (c *Container) setupRootlessNetwork() error {
-	return nil
-}

libpod/oci_conmon_linux.go

@@ -164,6 +164,11 @@ func (r *ConmonOCIRuntime) withContainerSocketLabel(ctr *Container, closure func
 	return err
 }
 
+// Create systemd unit name for cgroup scopes.
+func createUnitName(prefix string, name string) string {
+	return fmt.Sprintf("%s-%s.scope", prefix, name)
+}
+
 // moveConmonToCgroupAndSignal gets a container's cgroupParent and moves the conmon process to that cgroup
 // it then signals for conmon to start by sending nonce data down the start fd
 func (r *ConmonOCIRuntime) moveConmonToCgroupAndSignal(ctr *Container, cmd *exec.Cmd, startFd *os.File) error {

libpod/oci_util.go

@@ -27,11 +27,6 @@ type ociError struct {
 	Msg   string `json:"msg,omitempty"`
 }
 
-// Create systemd unit name for cgroup scopes
-func createUnitName(prefix string, name string) string {
-	return fmt.Sprintf("%s-%s.scope", prefix, name)
-}
-
 // Bind ports to keep them closed on the host
 func bindPorts(ports []types.PortMapping) ([]*os.File, error) {
 	var files []*os.File

libpod/stats_freebsd.go

@@ -35,7 +35,7 @@ func (c *Container) getPlatformContainerStats(stats *define.ContainerStats, prev
 	// in a new jail
 	if dur, ok := entries["wallclock"]; ok {
 		if previousStats.Duration > dur*1000000000 {
-			previousStats = &define.ContainerStats{}
+			previousStats = &define.ContainerStats{} //nolint:wastedassign // TODO: figure this out.
 		}
 	}
 

libpod/util_freebsd.go

@@ -3,26 +3,12 @@
 package libpod
 
 import (
-	"errors"
 	"syscall"
 
-	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
 
-// systemdSliceFromPath makes a new systemd slice under the given parent with
-// the given name.
-// The parent must be a slice. The name must NOT include ".slice"
-func systemdSliceFromPath(parent, name string, resources *spec.LinuxResources) (string, error) {
-	return "", errors.New("not implemented systemdSliceFromPath")
-}
-
-// deleteSystemdCgroup deletes the systemd cgroup at the given location
-func deleteSystemdCgroup(path string, resources *spec.LinuxResources) error {
-	return nil
-}
-
 // No equivalent on FreeBSD?
 func LabelVolumePath(path, mountLabel string) error {
 	return nil

pkg/emulation/elf.go

@@ -1,4 +1,4 @@
-//go:build !remote
+//go:build linux && !remote
 
 package emulation
 

pkg/machine/e2e/config_freebsd_test.go

@@ -0,0 +1,7 @@
+package e2e_test
+
+const podmanBinary = "../../../bin/podman-remote"
+
+func getOtherProvider() string {
+	return ""
+}

pkg/rootless/rootless_freebsd.go

@@ -21,7 +21,7 @@ func IsRootless() bool {
 // If podman was re-executed the caller needs to propagate the error code returned by the child
 // process.  It is a convenience function for BecomeRootInUserNSWithOpts with a default configuration.
 func BecomeRootInUserNS(pausePid string) (bool, int, error) {
-	return false, -1, errors.New("Rootless mode is not supported on FreeBSD - run podman as root")
+	return false, -1, errors.New("rootless mode is not supported on FreeBSD - run podman as root")
 }
 
 // GetRootlessUID returns the UID of the user in the parent userNS

pkg/specgen/generate/config_freebsd.go

@@ -48,7 +48,9 @@ func DevicesFromPath(g *generate.Generator, devicePath string, config *config.Co
 	}
 	if st.IsDir() {
 		// For devfs, we need to add the directory as well
-		addDevice(g, resolvedDevicePath)
+		if err := addDevice(g, resolvedDevicePath); err != nil {
+			return err
+		}
 
 		found := false
 		src := resolvedDevicePath

pkg/specgen/generate/namespaces_freebsd.go

@@ -57,5 +57,5 @@ func specConfigureNamespaces(s *specgen.SpecGenerator, g *generate.Generator, rt
 // only if we can initialise the network after the OCI container is created -
 // the OCI container will own the vnet in this case.
 func needPostConfigureNetNS(s *specgen.SpecGenerator) bool {
-	return jail.NeedVnetJail() == false
+	return !jail.NeedVnetJail()
 }

pkg/specgen/generate/oci_freebsd.go

@@ -12,7 +12,6 @@ import (
 	"github.com/containers/podman/v5/libpod"
 	"github.com/containers/podman/v5/libpod/define"
 	"github.com/containers/podman/v5/pkg/specgen"
-	"github.com/opencontainers/runtime-spec/specs-go"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/runtime-tools/generate"
 )
@@ -175,6 +174,6 @@ func WeightDevices(wtDevices map[string]spec.LinuxWeightDevice) ([]spec.LinuxWei
 	return devs, nil
 }
 
-func subNegativeOne(u specs.POSIXRlimit) specs.POSIXRlimit {
+func subNegativeOne(u spec.POSIXRlimit) spec.POSIXRlimit {
 	return u
 }