Skip to content

Commit

Permalink
[builder] make retries configurable for faster tests (#10035)
Browse files Browse the repository at this point in the history
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
When running tests, waiting for `downloadModules()` to fail 3 times when
that's expected adds time to the test run. This updates tests to only
attempt downloading once. Note: if there's a network failure that could
cause `downloadModules()` to fail when it should normally succeed. Also
the wording here is `retries` when in actuality it's the number of
attempts. I didn't change this to keep the log wording the same, but I
can change the wording if that's preferable.

<!-- Issue number if applicable -->
#### Link to tracking issue
this will help for adding tests for
#9252
and
#9896

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Tests ran

---------

Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
  • Loading branch information
kristinapathak and mx-psi authored May 3, 2024
1 parent 174f003 commit ff7a485
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 19 deletions.
14 changes: 14 additions & 0 deletions cmd/builder/internal/builder/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"os/exec"
"path/filepath"
"strings"
"time"

"github.com/hashicorp/go-version"
"go.uber.org/multierr"
Expand Down Expand Up @@ -41,6 +42,8 @@ type Config struct {
Providers *[]Module `mapstructure:"providers"`
Replaces []string `mapstructure:"replaces"`
Excludes []string `mapstructure:"excludes"`

downloadModules retry `mapstructure:"-"`
}

// Distribution holds the parameters for the final binary
Expand All @@ -66,6 +69,11 @@ type Module struct {
Path string `mapstructure:"path"` // an optional path to the local version of this module
}

type retry struct {
numRetries int
wait time.Duration
}

// NewDefaultConfig creates a new config, with default values
func NewDefaultConfig() Config {
log, err := zap.NewDevelopment()
Expand All @@ -85,6 +93,12 @@ func NewDefaultConfig() Config {
OtelColVersion: defaultOtelColVersion,
Module: "go.opentelemetry.io/collector/cmd/builder",
},
// basic retry if error from go mod command (in case of transient network error).
// retry 3 times with 5 second spacing interval
downloadModules: retry{
numRetries: 3,
wait: 5 * time.Second,
},
}
}

Expand Down
9 changes: 3 additions & 6 deletions cmd/builder/internal/builder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,15 +189,12 @@ func GetModules(cfg Config) error {

func downloadModules(cfg Config) error {
cfg.Logger.Info("Getting go modules")
// basic retry if error from go mod command (in case of transient network error). This could be improved
// retry 3 times with 5 second spacing interval
retries := 3
failReason := "unknown"
for i := 1; i <= retries; i++ {
for i := 1; i <= cfg.downloadModules.numRetries; i++ {
if _, err := runGoCommand(cfg, "mod", "download"); err != nil {
failReason = err.Error()
cfg.Logger.Info("Failed modules download", zap.String("retry", fmt.Sprintf("%d/%d", i, retries)))
time.Sleep(5 * time.Second)
cfg.Logger.Info("Failed modules download", zap.String("retry", fmt.Sprintf("%d/%d", i, cfg.downloadModules.numRetries)))
time.Sleep(cfg.downloadModules.wait)
continue
}
return nil
Expand Down
33 changes: 20 additions & 13 deletions cmd/builder/internal/builder/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,15 @@ require (
)`)
)

func newInitializedConfig(t *testing.T) Config {
func newTestConfig() Config {
cfg := NewDefaultConfig()
cfg.downloadModules.wait = 0
cfg.downloadModules.numRetries = 1
return cfg
}

func newInitializedConfig(t *testing.T) Config {
cfg := newTestConfig()
// Validate and ParseModules will be called before the config is
// given to Generate.
assert.NoError(t, cfg.Validate())
Expand Down Expand Up @@ -66,7 +73,7 @@ func TestVersioning(t *testing.T) {
{
description: "defaults",
cfgBuilder: func() Config {
cfg := NewDefaultConfig()
cfg := newTestConfig()
cfg.Distribution.Go = "go"
cfg.Replaces = append(cfg.Replaces, replaces...)
return cfg
Expand All @@ -76,7 +83,7 @@ func TestVersioning(t *testing.T) {
{
description: "require otelcol",
cfgBuilder: func() Config {
cfg := NewDefaultConfig()
cfg := newTestConfig()
cfg.Distribution.Go = "go"
cfg.Distribution.RequireOtelColModule = true
cfg.Replaces = append(cfg.Replaces, replaces...)
Expand All @@ -87,7 +94,7 @@ func TestVersioning(t *testing.T) {
{
description: "only gomod file, skip generate",
cfgBuilder: func() Config {
cfg := NewDefaultConfig()
cfg := newTestConfig()
tempDir := t.TempDir()
err := makeModule(tempDir, goModTestFile)
require.NoError(t, err)
Expand All @@ -101,7 +108,7 @@ func TestVersioning(t *testing.T) {
{
description: "old otel version",
cfgBuilder: func() Config {
cfg := NewDefaultConfig()
cfg := newTestConfig()
cfg.Verbose = true
cfg.Distribution.Go = "go"
cfg.Distribution.OtelColVersion = "0.97.0"
Expand Down Expand Up @@ -133,7 +140,7 @@ func TestVersioning(t *testing.T) {
{
description: "old component version",
cfgBuilder: func() Config {
cfg := NewDefaultConfig()
cfg := newTestConfig()
cfg.Distribution.Go = "go"
cfg.Exporters = []Module{
{
Expand All @@ -149,7 +156,7 @@ func TestVersioning(t *testing.T) {
{
description: "old component version without strict mode",
cfgBuilder: func() Config {
cfg := NewDefaultConfig()
cfg := newTestConfig()
cfg.Distribution.Go = "go"
cfg.SkipStrictVersioning = true
cfg.Exporters = []Module{
Expand Down Expand Up @@ -200,7 +207,7 @@ func TestGenerateAndCompile(t *testing.T) {
{
testCase: "Default Configuration Compilation",
cfgBuilder: func(t *testing.T) Config {
cfg := NewDefaultConfig()
cfg := newTestConfig()
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
return cfg
Expand All @@ -209,7 +216,7 @@ func TestGenerateAndCompile(t *testing.T) {
{
testCase: "LDFlags Compilation",
cfgBuilder: func(t *testing.T) Config {
cfg := NewDefaultConfig()
cfg := newTestConfig()
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
cfg.LDFlags = `-X "test.gitVersion=0743dc6c6411272b98494a9b32a63378e84c34da" -X "test.gitTag=local-testing" -X "test.goVersion=go version go1.20.7 darwin/amd64"`
Expand All @@ -219,7 +226,7 @@ func TestGenerateAndCompile(t *testing.T) {
{
testCase: "Debug Compilation",
cfgBuilder: func(t *testing.T) Config {
cfg := NewDefaultConfig()
cfg := newTestConfig()
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
cfg.Logger = zap.NewNop()
Expand All @@ -230,7 +237,7 @@ func TestGenerateAndCompile(t *testing.T) {
{
testCase: "No providers",
cfgBuilder: func(t *testing.T) Config {
cfg := NewDefaultConfig()
cfg := newTestConfig()
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
cfg.Providers = &[]Module{}
Expand All @@ -240,7 +247,7 @@ func TestGenerateAndCompile(t *testing.T) {
{
testCase: "Pre-confmap factories",
cfgBuilder: func(t *testing.T) Config {
cfg := NewDefaultConfig()
cfg := newTestConfig()
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
cfg.Distribution.OtelColVersion = "0.98.0"
Expand All @@ -251,7 +258,7 @@ func TestGenerateAndCompile(t *testing.T) {
{
testCase: "With confmap factories",
cfgBuilder: func(t *testing.T) Config {
cfg := NewDefaultConfig()
cfg := newTestConfig()
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
cfg.Distribution.OtelColVersion = "0.99.0"
Expand Down

0 comments on commit ff7a485

Please sign in to comment.