From b54e605224d3a8fcf6b8949304a8b972cd6a8ba4 Mon Sep 17 00:00:00 2001 From: Dominik Richter Date: Sun, 14 Jan 2024 17:13:38 -0800 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20introduce=20I/O=20retries=20for=20p?= =?UTF-8?q?rovider=20installs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/mondoohq/cnspec/issues/998 The implementation is flexible to be used in other places. We can consider packaging it separately down the line, but for now it is in a good place to be very specific and purpose-built for this use-case. Signed-off-by: Dominik Richter --- providers/providers.go | 59 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 4 deletions(-) diff --git a/providers/providers.go b/providers/providers.go index fab457dc90..f34e307ccb 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -13,6 +13,7 @@ import ( "path/filepath" "runtime" "strings" + "syscall" "time" "github.com/cockroachdb/errors" @@ -438,6 +439,46 @@ func InstallFile(path string, conf InstallConf) ([]*Provider, error) { return InstallIO(reader, conf) } +// kept a tad bit higher to give I/O more time to complete +const osRetryDuration = 100 * time.Millisecond + +// In the process of installing larger binaries, we will need time for +// antivirus software to scan it. This is currently set to retry for: +// 100ms (above) * 10 (=1sec) * 60 (=1min) * 3 (=3min) +const maxInstallBinaryRetries = 10 * 60 * 3 + +// The retries for config files (like JSON) are much shorter, since these +// files are considerably smaller: +// 100ms (above) * 10 (=1sec) * 20 (=20sec) +const maxInstallConfRetries = 10 * 20 + +// osRetry will try to re-run the given function as long as the resource is busy. +// This is helpful in e.g. Windows systems, which may get an antivirus tool +// check files while we create them (e.g. installing providers). +// It will look for common OS signals that the I/O is busy right now or that +// it asks the caller to run their call again later. +// It is retried every osRetryDuration. +// maxRetry has the maximum number of retries (or -1 for indefinite) +func osRetry(f func() error, maxRetry int) error { + for maxRetry != 0 { + err := f() + if err == nil { + return nil + } + + if errors.As(err, syscall.EBUSY) || errors.As(err, syscall.EAGAIN) { + time.Sleep(osRetryDuration) + } else { + return err + } + + if maxRetry > 0 { + maxRetry-- + } + } + return nil +} + func InstallIO(reader io.ReadCloser, conf InstallConf) ([]*Provider, error) { if conf.Dst == "" { conf.Dst = DefaultPath @@ -482,7 +523,11 @@ func InstallIO(reader io.ReadCloser, conf InstallConf) ([]*Provider, error) { // so we don't spam the system with unnecessary data. Optionally we could // keep them and re-use them, so they don't have to download again. defer func() { - if err = os.RemoveAll(tmpdir); err != nil { + // We don't set a max retry, since we can indefinitely try to remove this + err := osRetry(func() error { + return os.RemoveAll(tmpdir) + }, -1) + if err != nil { log.Error().Err(err).Msg("failed to remove temporary folder for unpacked provider") } }() @@ -517,7 +562,9 @@ func InstallIO(reader io.ReadCloser, conf InstallConf) ([]*Provider, error) { srcBin := filepath.Join(tmpdir, name) dstBin := filepath.Join(dstPath, name) log.Debug().Str("src", srcBin).Str("dst", dstBin).Msg("move provider binary") - if err = os.Rename(srcBin, dstBin); err != nil { + if err = osRetry(func() error { + return os.Rename(srcBin, dstBin) + }, maxInstallBinaryRetries); err != nil { return nil, err } if err = os.Chmod(dstBin, 0o755); err != nil { @@ -526,10 +573,14 @@ func InstallIO(reader io.ReadCloser, conf InstallConf) ([]*Provider, error) { srcMeta := filepath.Join(tmpdir, providerName) dstMeta := filepath.Join(dstPath, providerName) - if err = os.Rename(srcMeta+".json", dstMeta+".json"); err != nil { + if err = osRetry(func() error { + return os.Rename(srcMeta+".json", dstMeta+".json") + }, maxInstallConfRetries); err != nil { return nil, err } - if err = os.Rename(srcMeta+".resources.json", dstMeta+".resources.json"); err != nil { + if err = osRetry(func() error { + return os.Rename(srcMeta+".resources.json", dstMeta+".resources.json") + }, maxInstallConfRetries); err != nil { return nil, err }