diff options
| -rwxr-xr-x | contrib/cirrus/check_go_changes.sh | 53 | ||||
| -rwxr-xr-x | contrib/cirrus/runner.sh | 20 | ||||
| -rw-r--r-- | libpod/runtime_volume_common.go (renamed from libpod/runtime_volume_linux.go) | 4 | ||||
| -rw-r--r-- | libpod/runtime_volume_unsupported.go | 4 | ||||
| -rw-r--r-- | libpod/util_freebsd.go | 36 | ||||
| -rw-r--r-- | libpod/util_unsupported.go | 4 | ||||
| -rw-r--r-- | libpod/volume_internal_common.go | 194 | ||||
| -rw-r--r-- | libpod/volume_internal_freebsd.go | 9 | ||||
| -rw-r--r-- | libpod/volume_internal_linux.go | 189 | ||||
| -rw-r--r-- | libpod/volume_internal_unsupported.go | 4 | ||||
| -rw-r--r-- | pkg/k8s.io/api/core/v1/types.go | 10 | ||||
| -rw-r--r-- | pkg/specgen/generate/kube/kube.go | 8 | ||||
| -rw-r--r-- | pkg/specgen/generate/kube/play_test.go | 1 |
13 files changed, 314 insertions, 222 deletions
diff --git a/contrib/cirrus/check_go_changes.sh b/contrib/cirrus/check_go_changes.sh new file mode 100755 index 000000000..3c35ce51a --- /dev/null +++ b/contrib/cirrus/check_go_changes.sh @@ -0,0 +1,53 @@ +#!/bin/bash + +set -eo pipefail + +# This script is intended to confirm new go code conforms to certain +# conventions and/or does not introduce use of old/deprecated packages +# or functions. It needs to run in the Cirrus CI environment, on behalf +# of PRs, via runner.sh. This ensures a consistent and predictable +# environment not easily reproduced by a `Makefile`. + +# shellcheck source=contrib/cirrus/lib.sh +source $(dirname $0)/lib.sh + +check_msg() { + msg "#####" # Cirrus-CI logs automatically squash empty lines + msg "##### $1" # Complains if $1 is empty +} + +# First arg is check description, second is regex to search $diffs for. +check_diffs() { + local check regex + check="$1" + regex="$2" + check_msg "Confirming changes have no $check" + req_env_vars check regex diffs + if egrep -q "$regex"<<<"$diffs"; then + # Show 5 context lines before/after as compromise for script simplicity + die "Found $check: +$(egrep -B 5 -A 5 "$regex"<<<"$diffs")" + fi +} + +if [[ -n "$CIRRUS_TAG" ]] || ! req_env_vars CIRRUS_CHANGE_IN_REPO CIRRUS_PR DEST_BRANCH +then + warn "Skipping: Golang code checks cannot run in this context" + exit 0 +fi + +base=$(git merge-base $DEST_BRANCH $CIRRUS_CHANGE_IN_REPO) +diffs=$(git diff $base $CIRRUS_CHANGE_IN_REPO -- '*.go' ':^vendor/') + +if [[ -z "$diffs" ]]; then + check_msg "There are no golang diffs to check between $base...$CIRRUS_CHANGE_IN_REPO" + exit 0 +fi + +check_diffs \ + "use of deprecated ioutil vs recommended io or os packages." \ + "^(\\+[^#]+io/ioutil)|(\\+.+ioutil\\..+)" + +check_diffs \ + "use of os.IsNotExists(err) vs recommended errors.Is(err, os.ErrNotExist)" \ + "^\\+[^#]*os\\.IsNotExists\\(" diff --git a/contrib/cirrus/runner.sh b/contrib/cirrus/runner.sh index 5b1bc8d5c..c44251e2f 100755 --- a/contrib/cirrus/runner.sh +++ b/contrib/cirrus/runner.sh @@ -233,25 +233,7 @@ function _run_consistency() { SUGGESTION="run 'make generate-bindings' and commit all changes" ./hack/tree_status.sh make completions SUGGESTION="run 'make completions' and commit all changes" ./hack/tree_status.sh - - if [[ -z "$CIRRUS_TAG" ]] && \ - req_env_vars CIRRUS_CHANGE_IN_REPO CIRRUS_PR DEST_BRANCH - then - local base diffs regex i - # Prevent this check from detecting itself - i=i - msg "#####" - msg "Verifying no change adds new calls to ${i}o/${i}outil." - base=$(git merge-base $DEST_BRANCH $CIRRUS_CHANGE_IN_REPO) - diffs=$(git diff $base $CIRRUS_CHANGE_IN_REPO -- '*.go' ':^vendor/') - regex=$(echo -e "^(\\+.+${i}o/${i}outil)|(\\+.+${i}outil\\..+)") - if egrep -q "$regex"<<<"$diffs"; then - die "Found attempted use of deprecated ${i}outils: -$(egrep -B 5 -A 5 "$regex"<<<"$diffs")" - fi - else - msg "Skipping check for ${i}o/${i}outil addition." - fi + $SCRIPT_BASE/check_go_changes.sh } function _run_build() { diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_common.go index c59417979..b1de2be86 100644 --- a/libpod/runtime_volume_linux.go +++ b/libpod/runtime_volume_common.go @@ -1,5 +1,5 @@ -//go:build linux -// +build linux +//go:build linux || freebsd +// +build linux freebsd package libpod diff --git a/libpod/runtime_volume_unsupported.go b/libpod/runtime_volume_unsupported.go index c2816b817..7b7758894 100644 --- a/libpod/runtime_volume_unsupported.go +++ b/libpod/runtime_volume_unsupported.go @@ -1,5 +1,5 @@ -//go:build !linux -// +build !linux +//go:build !linux && !freebsd +// +build !linux,!freebsd package libpod diff --git a/libpod/util_freebsd.go b/libpod/util_freebsd.go new file mode 100644 index 000000000..72019743c --- /dev/null +++ b/libpod/util_freebsd.go @@ -0,0 +1,36 @@ +//go:build freebsd +// +build freebsd + +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") +} + +// No equivalent on FreeBSD? +func LabelVolumePath(path string) error { + return nil +} + +// Unmount umounts a target directory +func Unmount(mount string) { + if err := unix.Unmount(mount, unix.MNT_FORCE); err != nil { + if err != syscall.EINVAL { + logrus.Warnf("Failed to unmount %s : %v", mount, err) + } else { + logrus.Debugf("failed to unmount %s : %v", mount, err) + } + } +} diff --git a/libpod/util_unsupported.go b/libpod/util_unsupported.go index d2ec3ae7b..fc3d00274 100644 --- a/libpod/util_unsupported.go +++ b/libpod/util_unsupported.go @@ -1,5 +1,5 @@ -//go:build !linux -// +build !linux +//go:build !linux && !freebsd +// +build !linux,!freebsd package libpod diff --git a/libpod/volume_internal_common.go b/libpod/volume_internal_common.go new file mode 100644 index 000000000..4ff7ac790 --- /dev/null +++ b/libpod/volume_internal_common.go @@ -0,0 +1,194 @@ +//go:build linux || freebsd +// +build linux freebsd + +package libpod + +import ( + "errors" + "fmt" + "os/exec" + "strings" + + "github.com/containers/podman/v4/libpod/define" + pluginapi "github.com/docker/go-plugins-helpers/volume" + "github.com/sirupsen/logrus" + "golang.org/x/sys/unix" +) + +// This is a pseudo-container ID to use when requesting a mount or unmount from +// the volume plugins. +// This is the shas256 of the string "placeholder\n". +const pseudoCtrID = "2f73349cfc4630255319c6c8dfc1b46a8996ace9d14d8e07563b165915918ec2" + +// mount mounts the volume if necessary. +// A mount is necessary if a volume has any options set. +// If a mount is necessary, v.state.MountCount will be incremented. +// If it was 0 when the increment occurred, the volume will be mounted on the +// host. Otherwise, we assume it is already mounted. +// Must be done while the volume is locked. +// Is a no-op on volumes that do not require a mount (as defined by +// volumeNeedsMount()). +func (v *Volume) mount() error { + if !v.needsMount() { + return nil + } + + // Update the volume from the DB to get an accurate mount counter. + if err := v.update(); err != nil { + return err + } + + // If the count is non-zero, the volume is already mounted. + // Nothing to do. + if v.state.MountCount > 0 { + v.state.MountCount++ + logrus.Debugf("Volume %s mount count now at %d", v.Name(), v.state.MountCount) + return v.save() + } + + // Volume plugins implement their own mount counter, based on the ID of + // the mounting container. But we already have one, and honestly I trust + // ours more. So hardcode container ID to something reasonable, and use + // the same one for everything. + if v.UsesVolumeDriver() { + if v.plugin == nil { + return fmt.Errorf("volume plugin %s (needed by volume %s) missing: %w", v.Driver(), v.Name(), define.ErrMissingPlugin) + } + + req := new(pluginapi.MountRequest) + req.Name = v.Name() + req.ID = pseudoCtrID + mountPoint, err := v.plugin.MountVolume(req) + if err != nil { + return err + } + + v.state.MountCount++ + v.state.MountPoint = mountPoint + return v.save() + } else if v.config.Driver == define.VolumeDriverImage { + mountPoint, err := v.runtime.storageService.MountContainerImage(v.config.StorageID) + if err != nil { + return fmt.Errorf("mounting volume %s image failed: %w", v.Name(), err) + } + + v.state.MountCount++ + v.state.MountPoint = mountPoint + return v.save() + } + + volDevice := v.config.Options["device"] + volType := v.config.Options["type"] + volOptions := v.config.Options["o"] + + // Some filesystems (tmpfs) don't have a device, but we still need to + // give the kernel something. + if volDevice == "" && volType != "" { + volDevice = volType + } + + // We need to use the actual mount command. + // Convincing unix.Mount to use the same semantics as the mount command + // itself seems prohibitively difficult. + // TODO: might want to cache this path in the runtime? + mountPath, err := exec.LookPath("mount") + if err != nil { + return fmt.Errorf("locating 'mount' binary: %w", err) + } + mountArgs := []string{} + if volOptions != "" { + mountArgs = append(mountArgs, "-o", volOptions) + } + switch volType { + case "": + case "bind": + mountArgs = append(mountArgs, "-o", volType) + default: + mountArgs = append(mountArgs, "-t", volType) + } + + mountArgs = append(mountArgs, volDevice, v.config.MountPoint) + mountCmd := exec.Command(mountPath, mountArgs...) + + logrus.Debugf("Running mount command: %s %s", mountPath, strings.Join(mountArgs, " ")) + if output, err := mountCmd.CombinedOutput(); err != nil { + logrus.Debugf("Mount %v failed with %v", mountCmd, err) + return errors.New(string(output)) + } + + logrus.Debugf("Mounted volume %s", v.Name()) + + // Increment the mount counter + v.state.MountCount++ + logrus.Debugf("Volume %s mount count now at %d", v.Name(), v.state.MountCount) + return v.save() +} + +// unmount unmounts the volume if necessary. +// Unmounting a volume that is not mounted is a no-op. +// Unmounting a volume that does not require a mount is a no-op. +// The volume must be locked for this to occur. +// The mount counter will be decremented if non-zero. If the counter reaches 0, +// the volume will really be unmounted, as no further containers are using the +// volume. +// If force is set, the volume will be unmounted regardless of mount counter. +func (v *Volume) unmount(force bool) error { + if !v.needsMount() { + return nil + } + + // Update the volume from the DB to get an accurate mount counter. + if err := v.update(); err != nil { + return err + } + + if v.state.MountCount == 0 { + logrus.Debugf("Volume %s already unmounted", v.Name()) + return nil + } + + if !force { + v.state.MountCount-- + } else { + v.state.MountCount = 0 + } + + logrus.Debugf("Volume %s mount count now at %d", v.Name(), v.state.MountCount) + + if v.state.MountCount == 0 { + if v.UsesVolumeDriver() { + if v.plugin == nil { + return fmt.Errorf("volume plugin %s (needed by volume %s) missing: %w", v.Driver(), v.Name(), define.ErrMissingPlugin) + } + + req := new(pluginapi.UnmountRequest) + req.Name = v.Name() + req.ID = pseudoCtrID + if err := v.plugin.UnmountVolume(req); err != nil { + return err + } + + v.state.MountPoint = "" + return v.save() + } else if v.config.Driver == define.VolumeDriverImage { + if _, err := v.runtime.storageService.UnmountContainerImage(v.config.StorageID, force); err != nil { + return fmt.Errorf("unmounting volume %s image: %w", v.Name(), err) + } + + v.state.MountPoint = "" + return v.save() + } + + // Unmount the volume + if err := detachUnmount(v.config.MountPoint); err != nil { + if err == unix.EINVAL { + // Ignore EINVAL - the mount no longer exists. + return nil + } + return fmt.Errorf("unmounting volume %s: %w", v.Name(), err) + } + logrus.Debugf("Unmounted volume %s", v.Name()) + } + + return v.save() +} diff --git a/libpod/volume_internal_freebsd.go b/libpod/volume_internal_freebsd.go new file mode 100644 index 000000000..cf71f1e32 --- /dev/null +++ b/libpod/volume_internal_freebsd.go @@ -0,0 +1,9 @@ +package libpod + +import ( + "golang.org/x/sys/unix" +) + +func detachUnmount(mountPoint string) error { + return unix.Unmount(mountPoint, unix.MNT_FORCE) +} diff --git a/libpod/volume_internal_linux.go b/libpod/volume_internal_linux.go index 440bceec3..eb4309dc3 100644 --- a/libpod/volume_internal_linux.go +++ b/libpod/volume_internal_linux.go @@ -1,194 +1,9 @@ -//go:build linux -// +build linux - package libpod import ( - "errors" - "fmt" - "os/exec" - "strings" - - "github.com/containers/podman/v4/libpod/define" - pluginapi "github.com/docker/go-plugins-helpers/volume" - "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) -// This is a pseudo-container ID to use when requesting a mount or unmount from -// the volume plugins. -// This is the shas256 of the string "placeholder\n". -const pseudoCtrID = "2f73349cfc4630255319c6c8dfc1b46a8996ace9d14d8e07563b165915918ec2" - -// mount mounts the volume if necessary. -// A mount is necessary if a volume has any options set. -// If a mount is necessary, v.state.MountCount will be incremented. -// If it was 0 when the increment occurred, the volume will be mounted on the -// host. Otherwise, we assume it is already mounted. -// Must be done while the volume is locked. -// Is a no-op on volumes that do not require a mount (as defined by -// volumeNeedsMount()). -func (v *Volume) mount() error { - if !v.needsMount() { - return nil - } - - // Update the volume from the DB to get an accurate mount counter. - if err := v.update(); err != nil { - return err - } - - // If the count is non-zero, the volume is already mounted. - // Nothing to do. - if v.state.MountCount > 0 { - v.state.MountCount++ - logrus.Debugf("Volume %s mount count now at %d", v.Name(), v.state.MountCount) - return v.save() - } - - // Volume plugins implement their own mount counter, based on the ID of - // the mounting container. But we already have one, and honestly I trust - // ours more. So hardcode container ID to something reasonable, and use - // the same one for everything. - if v.UsesVolumeDriver() { - if v.plugin == nil { - return fmt.Errorf("volume plugin %s (needed by volume %s) missing: %w", v.Driver(), v.Name(), define.ErrMissingPlugin) - } - - req := new(pluginapi.MountRequest) - req.Name = v.Name() - req.ID = pseudoCtrID - mountPoint, err := v.plugin.MountVolume(req) - if err != nil { - return err - } - - v.state.MountCount++ - v.state.MountPoint = mountPoint - return v.save() - } else if v.config.Driver == define.VolumeDriverImage { - mountPoint, err := v.runtime.storageService.MountContainerImage(v.config.StorageID) - if err != nil { - return fmt.Errorf("mounting volume %s image failed: %w", v.Name(), err) - } - - v.state.MountCount++ - v.state.MountPoint = mountPoint - return v.save() - } - - volDevice := v.config.Options["device"] - volType := v.config.Options["type"] - volOptions := v.config.Options["o"] - - // Some filesystems (tmpfs) don't have a device, but we still need to - // give the kernel something. - if volDevice == "" && volType != "" { - volDevice = volType - } - - // We need to use the actual mount command. - // Convincing unix.Mount to use the same semantics as the mount command - // itself seems prohibitively difficult. - // TODO: might want to cache this path in the runtime? - mountPath, err := exec.LookPath("mount") - if err != nil { - return fmt.Errorf("locating 'mount' binary: %w", err) - } - mountArgs := []string{} - if volOptions != "" { - mountArgs = append(mountArgs, "-o", volOptions) - } - switch volType { - case "": - case "bind": - mountArgs = append(mountArgs, "-o", volType) - default: - mountArgs = append(mountArgs, "-t", volType) - } - - mountArgs = append(mountArgs, volDevice, v.config.MountPoint) - mountCmd := exec.Command(mountPath, mountArgs...) - - logrus.Debugf("Running mount command: %s %s", mountPath, strings.Join(mountArgs, " ")) - if output, err := mountCmd.CombinedOutput(); err != nil { - logrus.Debugf("Mount %v failed with %v", mountCmd, err) - return errors.New(string(output)) - } - - logrus.Debugf("Mounted volume %s", v.Name()) - - // Increment the mount counter - v.state.MountCount++ - logrus.Debugf("Volume %s mount count now at %d", v.Name(), v.state.MountCount) - return v.save() -} - -// unmount unmounts the volume if necessary. -// Unmounting a volume that is not mounted is a no-op. -// Unmounting a volume that does not require a mount is a no-op. -// The volume must be locked for this to occur. -// The mount counter will be decremented if non-zero. If the counter reaches 0, -// the volume will really be unmounted, as no further containers are using the -// volume. -// If force is set, the volume will be unmounted regardless of mount counter. -func (v *Volume) unmount(force bool) error { - if !v.needsMount() { - return nil - } - - // Update the volume from the DB to get an accurate mount counter. - if err := v.update(); err != nil { - return err - } - - if v.state.MountCount == 0 { - logrus.Debugf("Volume %s already unmounted", v.Name()) - return nil - } - - if !force { - v.state.MountCount-- - } else { - v.state.MountCount = 0 - } - - logrus.Debugf("Volume %s mount count now at %d", v.Name(), v.state.MountCount) - - if v.state.MountCount == 0 { - if v.UsesVolumeDriver() { - if v.plugin == nil { - return fmt.Errorf("volume plugin %s (needed by volume %s) missing: %w", v.Driver(), v.Name(), define.ErrMissingPlugin) - } - - req := new(pluginapi.UnmountRequest) - req.Name = v.Name() - req.ID = pseudoCtrID - if err := v.plugin.UnmountVolume(req); err != nil { - return err - } - - v.state.MountPoint = "" - return v.save() - } else if v.config.Driver == define.VolumeDriverImage { - if _, err := v.runtime.storageService.UnmountContainerImage(v.config.StorageID, force); err != nil { - return fmt.Errorf("unmounting volume %s image: %w", v.Name(), err) - } - - v.state.MountPoint = "" - return v.save() - } - - // Unmount the volume - if err := unix.Unmount(v.config.MountPoint, unix.MNT_DETACH); err != nil { - if err == unix.EINVAL { - // Ignore EINVAL - the mount no longer exists. - return nil - } - return fmt.Errorf("unmounting volume %s: %w", v.Name(), err) - } - logrus.Debugf("Unmounted volume %s", v.Name()) - } - - return v.save() +func detachUnmount(mountPoint string) error { + return unix.Unmount(mountPoint, unix.MNT_DETACH) } diff --git a/libpod/volume_internal_unsupported.go b/libpod/volume_internal_unsupported.go index 50515e692..d138c15fb 100644 --- a/libpod/volume_internal_unsupported.go +++ b/libpod/volume_internal_unsupported.go @@ -1,5 +1,5 @@ -//go:build !linux -// +build !linux +//go:build !linux && !freebsd +// +build !linux,!freebsd package libpod diff --git a/pkg/k8s.io/api/core/v1/types.go b/pkg/k8s.io/api/core/v1/types.go index 6f20cd351..4447847e3 100644 --- a/pkg/k8s.io/api/core/v1/types.go +++ b/pkg/k8s.io/api/core/v1/types.go @@ -939,15 +939,15 @@ type HTTPHeader struct { // HTTPGetAction describes an action based on HTTP Get requests. type HTTPGetAction struct { - // Path to access on the HTTP server. + // Path to access on the HTTP server. Defaults to /. // +optional Path string `json:"path,omitempty"` // Name or number of the port to access on the container. // Number must be in the range 1 to 65535. // Name must be an IANA_SVC_NAME. Port intstr.IntOrString `json:"port"` - // Host name to connect to, defaults to the pod IP. You probably want to set - // "Host" in httpHeaders instead. + // Host name to connect to. You probably want to set "Host" in httpHeaders instead. + // Defaults to the pod IP in Kubernetes, in case of Podman to localhost. // +optional Host string `json:"host,omitempty"` // Scheme to use for connecting to the host. @@ -964,9 +964,9 @@ type URIScheme string const ( // URISchemeHTTP means that the scheme used will be http:// - URISchemeHTTP URIScheme = "HTTP" + URISchemeHTTP URIScheme = "http" // URISchemeHTTPS means that the scheme used will be https:// - URISchemeHTTPS URIScheme = "HTTPS" + URISchemeHTTPS URIScheme = "https" ) // TCPSocketAction describes an action based on opening a socket diff --git a/pkg/specgen/generate/kube/kube.go b/pkg/specgen/generate/kube/kube.go index 2a0d80cb8..5186a2f72 100644 --- a/pkg/specgen/generate/kube/kube.go +++ b/pkg/specgen/generate/kube/kube.go @@ -507,7 +507,7 @@ func setupLivenessProbe(s *specgen.SpecGenerator, containerYAML v1.Container, re commandString = fmt.Sprintf("%s || %s", execString, failureCmd) case probeHandler.HTTPGet != nil: // set defaults as in https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/#http-probes - var uriScheme v1.URIScheme = "http" + uriScheme := v1.URISchemeHTTP if probeHandler.HTTPGet.Scheme != "" { uriScheme = probeHandler.HTTPGet.Scheme } @@ -515,7 +515,11 @@ func setupLivenessProbe(s *specgen.SpecGenerator, containerYAML v1.Container, re if probeHandler.HTTPGet.Host != "" { host = probeHandler.HTTPGet.Host } - commandString = fmt.Sprintf("curl -f %s://%s:%d%s || %s", uriScheme, host, probeHandler.HTTPGet.Port.IntValue(), probeHandler.HTTPGet.Path, failureCmd) + path := "/" + if probeHandler.HTTPGet.Path != "" { + path = probeHandler.HTTPGet.Path + } + commandString = fmt.Sprintf("curl -f %s://%s:%d%s || %s", uriScheme, host, probeHandler.HTTPGet.Port.IntValue(), path, failureCmd) case probeHandler.TCPSocket != nil: commandString = fmt.Sprintf("nc -z -v %s %d || %s", probeHandler.TCPSocket.Host, probeHandler.TCPSocket.Port.IntValue(), failureCmd) } diff --git a/pkg/specgen/generate/kube/play_test.go b/pkg/specgen/generate/kube/play_test.go index efe2e51b1..adf9b979a 100644 --- a/pkg/specgen/generate/kube/play_test.go +++ b/pkg/specgen/generate/kube/play_test.go @@ -897,7 +897,6 @@ func TestHttpLivenessProbe(t *testing.T) { Handler: v1.Handler{ HTTPGet: &v1.HTTPGetAction{ Port: intstr.FromInt(80), - Path: "/", }, }, }, |