Removing global state modification on unit tests (fix #10033 #10034) (#10935)

* Removing global state modification on unit tests (fix #10033 #10034)

The config.DefaultRemoteReadConfig and config.DefaultRemoteWriteConfig
instances hold global state. Unit tests were changing their url.URL reference
globally causing false positives when tests were ran through package.
Two helper functions were created to copy those global values instead of changing
them in place to fix null point when running unit tests by method instead of
by package.

Signed-off-by: Leonardo Zamariola <leonardo.zamariola@gmail.com>

* Fixing pull request suggestions

Copying by value from default config

Signed-off-by: Leonardo Zamariola <leonardo.zamariola@gmail.com>

storage/remote/queue_manager_test.go

@@ -17,7 +17,6 @@ import (
 	"context"
 	"fmt"
 	"math"
-	"net/url"
 	"os"
 	"runtime/pprof"
 	"sort"
@@ -32,7 +31,6 @@ import (
 	"github.com/golang/snappy"
 	"github.com/prometheus/client_golang/prometheus"
 	client_testutil "github.com/prometheus/client_golang/prometheus/testutil"
-	common_config "github.com/prometheus/common/config"
 	"github.com/prometheus/common/model"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/atomic"
@@ -84,20 +82,15 @@ func TestSampleDelivery(t *testing.T) {
 	queueConfig.BatchSendDeadline = model.Duration(100 * time.Millisecond)
 	queueConfig.MaxShards = 1
 
-	writeConfig := config.DefaultRemoteWriteConfig
 	// We need to set URL's so that metric creation doesn't panic.
-	writeConfig.URL = &common_config.URL{
-		URL: &url.URL{
-			Host: "http://test-storage.com",
-		},
-	}
+	writeConfig := baseRemoteWriteConfig("http://test-storage.com")
 	writeConfig.QueueConfig = queueConfig
 	writeConfig.SendExemplars = true
 
 	conf := &config.Config{
 		GlobalConfig: config.DefaultGlobalConfig,
 		RemoteWriteConfigs: []*config.RemoteWriteConfig{
-			&writeConfig,
+			writeConfig,
 		},
 	}
 

storage/remote/storage_test.go

@@ -31,21 +31,11 @@ func TestStorageLifecycle(t *testing.T) {
 	conf := &config.Config{
 		GlobalConfig: config.DefaultGlobalConfig,
 		RemoteWriteConfigs: []*config.RemoteWriteConfig{
-			&config.DefaultRemoteWriteConfig,
+			// We need to set URL's so that metric creation doesn't panic.
+			baseRemoteWriteConfig("http://test-storage.com"),
 		},
 		RemoteReadConfigs: []*config.RemoteReadConfig{
-			&config.DefaultRemoteReadConfig,
-		},
-	}
-	// We need to set URL's so that metric creation doesn't panic.
-	conf.RemoteWriteConfigs[0].URL = &common_config.URL{
-		URL: &url.URL{
-			Host: "http://test-storage.com",
-		},
-	}
-	conf.RemoteReadConfigs[0].URL = &common_config.URL{
-		URL: &url.URL{
-			Host: "http://test-storage.com",
+			baseRemoteReadConfig("http://test-storage.com"),
 		},
 	}
 
@@ -73,7 +63,7 @@ func TestUpdateRemoteReadConfigs(t *testing.T) {
 	require.Equal(t, 0, len(s.queryables))
 
 	conf.RemoteReadConfigs = []*config.RemoteReadConfig{
-		&config.DefaultRemoteReadConfig,
+		baseRemoteReadConfig("http://test-storage.com"),
 	}
 	require.NoError(t, s.ApplyConfig(conf))
 	require.Equal(t, 1, len(s.queryables))
@@ -96,7 +86,7 @@ func TestFilterExternalLabels(t *testing.T) {
 	require.Equal(t, 0, len(s.queryables))
 
 	conf.RemoteReadConfigs = []*config.RemoteReadConfig{
-		&config.DefaultRemoteReadConfig,
+		baseRemoteReadConfig("http://test-storage.com"),
 	}
 
 	require.NoError(t, s.ApplyConfig(conf))
@@ -121,7 +111,7 @@ func TestIgnoreExternalLabels(t *testing.T) {
 	require.Equal(t, 0, len(s.queryables))
 
 	conf.RemoteReadConfigs = []*config.RemoteReadConfig{
-		&config.DefaultRemoteReadConfig,
+		baseRemoteReadConfig("http://test-storage.com"),
 	}
 
 	conf.RemoteReadConfigs[0].FilterExternalLabels = false
@@ -133,3 +123,27 @@ func TestIgnoreExternalLabels(t *testing.T) {
 	err := s.Close()
 	require.NoError(t, err)
 }
+
+// baseRemoteWriteConfig copy values from global Default Write config
+// to avoid change global state and cross impact test execution
+func baseRemoteWriteConfig(host string) *config.RemoteWriteConfig {
+	cfg := config.DefaultRemoteWriteConfig
+	cfg.URL = &common_config.URL{
+		URL: &url.URL{
+			Host: host,
+		},
+	}
+	return &cfg
+}
+
+// baseRemoteReadConfig copy values from global Default Read config
+// to avoid change global state and cross impact test execution
+func baseRemoteReadConfig(host string) *config.RemoteReadConfig {
+	cfg := config.DefaultRemoteReadConfig
+	cfg.URL = &common_config.URL{
+		URL: &url.URL{
+			Host: host,
+		},
+	}
+	return &cfg
+}

storage/remote/write_test.go

@@ -202,7 +202,7 @@ func TestWriteStorageLifecycle(t *testing.T) {
 	conf := &config.Config{
 		GlobalConfig: config.DefaultGlobalConfig,
 		RemoteWriteConfigs: []*config.RemoteWriteConfig{
-			&config.DefaultRemoteWriteConfig,
+			baseRemoteWriteConfig("http://test-storage.com"),
 		},
 	}
 	require.NoError(t, s.ApplyConfig(conf))
@@ -249,18 +249,7 @@ func TestWriteStorageApplyConfigsIdempotent(t *testing.T) {
 	conf := &config.Config{
 		GlobalConfig: config.GlobalConfig{},
 		RemoteWriteConfigs: []*config.RemoteWriteConfig{
-			{
-				RemoteTimeout:    config.DefaultRemoteWriteConfig.RemoteTimeout,
-				QueueConfig:      config.DefaultRemoteWriteConfig.QueueConfig,
-				MetadataConfig:   config.DefaultRemoteWriteConfig.MetadataConfig,
-				HTTPClientConfig: config.DefaultRemoteWriteConfig.HTTPClientConfig,
-				// We need to set URL's so that metric creation doesn't panic.
-				URL: &common_config.URL{
-					URL: &url.URL{
-						Host: "http://test-storage.com",
-					},
-				},
-			},
+			baseRemoteWriteConfig("http://test-storage.com"),
 		},
 	}
 	hash, err := toHash(conf.RemoteWriteConfigs[0])