From 45af1d21de6951476d1a4204189c22e92d90015f Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Wed, 23 Nov 2022 11:52:18 -0500 Subject: [PATCH] Remove conmon probe 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 --- pkg/config/config.go | 26 +++-------- pkg/config/default.go | 90 -------------------------------------- pkg/config/default_test.go | 46 ------------------- 3 files changed, 5 insertions(+), 157 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 26c5d6e27..62b416b77 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -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 { @@ -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)", @@ -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. diff --git a/pkg/config/default.go b/pkg/config/default.go index f0f55f0df..0a4bd3078 100644 --- a/pkg/config/default.go +++ b/pkg/config/default.go @@ -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" @@ -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" @@ -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\d+).(?P\d+).(?P\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 diff --git a/pkg/config/default_test.go b/pkg/config/default_test.go index 2dbcd3110..542837149 100644 --- a/pkg/config/default_test.go +++ b/pkg/config/default_test.go @@ -1,7 +1,6 @@ package config import ( - "errors" "fmt" "os" "testing" @@ -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()