Skip to content

Commit

Permalink
Remove conmon probe
Browse files Browse the repository at this point in the history
This is taking time on every podman call, and provide limited
protection. Versioning should be handled in the packaging system
and this is an unlikely to happen.

Every exec done by container tools hurts us as we try to get container
startup team to absolute minimal amounts.

Signed-off-by: Daniel J Walsh <[email protected]>
  • Loading branch information
rhatdan committed Nov 27, 2022
1 parent 33a7a35 commit 45af1d2
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 157 deletions.
26 changes: 5 additions & 21 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -959,11 +959,10 @@ func (c *NetworkConfig) Validate() error {
// to first (version) matching conmon binary. If non is found, we try
// to do a path lookup of "conmon".
func (c *Config) FindConmon() (string, error) {
return findConmonPath(c.Engine.ConmonPath, "conmon", _conmonMinMajorVersion, _conmonMinMinorVersion, _conmonMinPatchVersion)
return findConmonPath(c.Engine.ConmonPath, "conmon")
}

func findConmonPath(paths []string, binaryName string, major int, minor int, patch int) (string, error) {
foundOutdatedConmon := false
func findConmonPath(paths []string, binaryName string) (string, error) {
for _, path := range paths {
stat, err := os.Stat(path)
if err != nil {
Expand All @@ -972,29 +971,14 @@ func findConmonPath(paths []string, binaryName string, major int, minor int, pat
if stat.IsDir() {
continue
}
if err := probeConmon(path); err != nil {
logrus.Warnf("Conmon at %s invalid: %v", path, err)
foundOutdatedConmon = true
continue
}
logrus.Debugf("Using conmon: %q", path)
return path, nil
}

// Search the $PATH as last fallback
if path, err := exec.LookPath(binaryName); err == nil {
if err := probeConmon(path); err != nil {
logrus.Warnf("Conmon at %s is invalid: %v", path, err)
foundOutdatedConmon = true
} else {
logrus.Debugf("Using conmon from $PATH: %q", path)
return path, nil
}
}

if foundOutdatedConmon {
return "", fmt.Errorf("please update to v%d.%d.%d or later: %w",
major, minor, patch, ErrConmonOutdated)
logrus.Debugf("Using conmon from $PATH: %q", path)
return path, nil
}

return "", fmt.Errorf("could not find a working conmon binary (configured options: %v: %w)",
Expand All @@ -1005,7 +989,7 @@ func findConmonPath(paths []string, binaryName string, major int, minor int, pat
// to first (version) matching conmonrs binary. If non is found, we try
// to do a path lookup of "conmonrs".
func (c *Config) FindConmonRs() (string, error) {
return findConmonPath(c.Engine.ConmonRsPath, "conmonrs", _conmonrsMinMajorVersion, _conmonrsMinMinorVersion, _conmonrsMinPatchVersion)
return findConmonPath(c.Engine.ConmonRsPath, "conmonrs")
}

// GetDefaultEnv returns the environment variables for the container.
Expand Down
90 changes: 0 additions & 90 deletions pkg/config/default.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
package config

import (
"bytes"
"errors"
"fmt"
"net"
"os"
"os/exec"
"path/filepath"
"regexp"
"strconv"
"strings"

nettypes "github.com/containers/common/libnetwork/types"
Expand All @@ -24,28 +20,6 @@ import (
)

const (
// _conmonMinMajorVersion is the major version required for conmon.
_conmonMinMajorVersion = 2

// _conmonMinMinorVersion is the minor version required for conmon.
_conmonMinMinorVersion = 0

// _conmonMinPatchVersion is the sub-minor version required for conmon.
_conmonMinPatchVersion = 1

// _conmonrsMinMajorVersion is the major version required for conmonrs.
_conmonrsMinMajorVersion = 0

// _conmonrsMinMinorVersion is the minor version required for conmonrs.
_conmonrsMinMinorVersion = 1

// _conmonrsMinPatchVersion is the sub-minor version required for conmonrs.
_conmonrsMinPatchVersion = 0

// _conmonVersionFormatErr is used when the expected versio-format of conmon
// has changed.
_conmonVersionFormatErr = "conmon version changed format: %w"

// _defaultGraphRoot points to the default path of the graph root.
_defaultGraphRoot = "/var/lib/containers/storage"

Expand Down Expand Up @@ -470,70 +444,6 @@ func defaultTmpDir() (string, error) {
return filepath.Join(libpodRuntimeDir, "tmp"), nil
}

// probeConmon calls conmon --version and verifies it is a new enough version for
// the runtime expectations the container engine currently has.
func probeConmon(conmonBinary string) error {
cmd := exec.Command(conmonBinary, "--version")
var out bytes.Buffer
cmd.Stdout = &out
if err := cmd.Run(); err != nil {
return err
}
r := regexp.MustCompile(`^(version:|conmon version)? (?P<Major>\d+).(?P<Minor>\d+).(?P<Patch>\d+)`)

matches := r.FindStringSubmatch(out.String())
if len(matches) != 5 {
return fmt.Errorf(_conmonVersionFormatErr, errors.New("invalid version format"))
}
major, err := strconv.Atoi(matches[2])

var minMajor, minMinor, minPatch int
// conmon-rs returns "^version:"
if matches[1] == "version:" {
minMajor = _conmonrsMinMajorVersion
minMinor = _conmonrsMinMinorVersion
minPatch = _conmonrsMinPatchVersion
} else {
minMajor = _conmonMinMajorVersion
minMinor = _conmonMinMinorVersion
minPatch = _conmonMinPatchVersion
}

if err != nil {
return fmt.Errorf(_conmonVersionFormatErr, err)
}
if major < minMajor {
return ErrConmonOutdated
}
if major > minMajor {
return nil
}

minor, err := strconv.Atoi(matches[3])
if err != nil {
return fmt.Errorf(_conmonVersionFormatErr, err)
}
if minor < minMinor {
return ErrConmonOutdated
}
if minor > minMinor {
return nil
}

patch, err := strconv.Atoi(matches[4])
if err != nil {
return fmt.Errorf(_conmonVersionFormatErr, err)
}
if patch < minPatch {
return ErrConmonOutdated
}
if patch > minPatch {
return nil
}

return nil
}

// NetNS returns the default network namespace.
func (c *Config) NetNS() string {
return c.Containers.NetNS
Expand Down
46 changes: 0 additions & 46 deletions pkg/config/default_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package config

import (
"errors"
"fmt"
"os"
"testing"
Expand All @@ -24,51 +23,6 @@ func prepareProbeBinary(t *testing.T, expectedOutput string) (path string) {
return f.Name()
}

func TestProbeConmon(t *testing.T) {
t.Parallel()

for _, tc := range []struct {
msg string
output string
assert func(error, string)
}{
{
msg: "success conmon 2",
output: "conmon version 2.1.0",
assert: func(err error, msg string) {
assert.Nil(t, err, msg)
},
},
{
msg: "success conmon 3",
output: "conmon version 3.0.0-dev",
assert: func(x error, msg string) {
assert.Nil(t, x, msg)
},
},
{
msg: "failure outdated version",
output: "conmon version 1.0.0",
assert: func(err error, msg string) {
assert.Equal(t, err, ErrConmonOutdated, msg)
},
},
{
msg: "failure invalid format",
output: "invalid",
assert: func(err error, msg string) {
expectedErr := fmt.Errorf(_conmonVersionFormatErr, errors.New("invalid version format"))
assert.Equal(t, err, expectedErr, msg)
},
},
} {
filePath := prepareProbeBinary(t, tc.output)
defer os.RemoveAll(filePath)
err := probeConmon(filePath)
tc.assert(err, tc.msg)
}
}

func TestMachineVolumes(t *testing.T) {
t.Parallel()

Expand Down

0 comments on commit 45af1d2

Please sign in to comment.