Added retry for Network errors containing i/o timeout

ravinitp · ravinitp · commit 410101b24c05 · 2024-11-26T18:45:49.000+05:30
diff --git a/internal/globalvar/constants.go b/internal/globalvar/constants.go
@@ -13,22 +13,30 @@ const (
 	ResourcePrincipal                     = "ResourcePrincipal"
 	RequestHeaderOpcOboToken              = "opc-obo-token"
 	RequestHeaderOpcHostSerial            = "opc-host-serial"
-	DefaultRequestTimeout                 = 0
-	DefaultConnectionTimeout              = 10 * time.Second
-	DefaultTLSHandshakeTimeout            = 10 * time.Second
-	DefaultUserAgentProviderName          = "Oracle-TerraformProvider"
-	UnknownTerraformCLIVersion            = "unknown"
-	TestTerraformCLIVersion               = "test"
-	UserAgentFormatter                    = "Oracle-GoSDK/%s (go/%s; %s/%s; terraform/%s; terraform-cli/%s) %s/%s"
-	UserAgentProviderNameEnv              = "USER_AGENT_PROVIDER_NAME"
-	UserAgentTerraformNameEnv             = "TF_APPEND_USER_AGENT"
-	UserAgentSDKNameEnv                   = "OCI_SDK_APPEND_USER_AGENT"
-	DomainNameOverrideEnv                 = "domain_name_override"
-	HasCorrectDomainNameEnv               = "has_correct_domain_name"
-	ClientHostOverridesEnv                = "CLIENT_HOST_OVERRIDES"
-	CustomCertLocationEnv                 = "custom_cert_location"
-	AcceptLocalCerts                      = "accept_local_certs"
-	JobOCID                               = "job-ocid"
+
+	//	HTTPRequestTimeout specifies the maximum duration for completing an HTTP request.
+	HTTPRequestTimeOut    = "HTTP_REQUEST_TIMEOUT"
+	DefaultRequestTimeout = 0
+	// DialContextConnectionTimeout defines the timeout for establishing a connection during a network dial operation.
+	DialContextConnectionTimeout = "DIAL_CONTEXT_CONNECTION_TIMEOUT"
+	DefaultConnectionTimeout     = 10 * time.Second
+	// TLSHandshakeTimeout indicates the maximum time allowed for the TLS handshake process.
+	TLSHandshakeTimeout        = "TLS_HANDSHAKE_TIMEOUT"
+	DefaultTLSHandshakeTimeout = 10 * time.Second
+
+	DefaultUserAgentProviderName = "Oracle-TerraformProvider"
+	UnknownTerraformCLIVersion   = "unknown"
+	TestTerraformCLIVersion      = "test"
+	UserAgentFormatter           = "Oracle-GoSDK/%s (go/%s; %s/%s; terraform/%s; terraform-cli/%s) %s/%s"
+	UserAgentProviderNameEnv     = "USER_AGENT_PROVIDER_NAME"
+	UserAgentTerraformNameEnv    = "TF_APPEND_USER_AGENT"
+	UserAgentSDKNameEnv          = "OCI_SDK_APPEND_USER_AGENT"
+	DomainNameOverrideEnv        = "domain_name_override"
+	HasCorrectDomainNameEnv      = "has_correct_domain_name"
+	ClientHostOverridesEnv       = "CLIENT_HOST_OVERRIDES"
+	CustomCertLocationEnv        = "custom_cert_location"
+	AcceptLocalCerts             = "accept_local_certs"
+	JobOCID                      = "job-ocid"
 
 	AuthAttrName                                = "auth"
 	TenancyOcidAttrName                         = "tenancy_ocid"
diff --git a/internal/provider/provider.go b/internal/provider/provider.go
@@ -568,19 +568,27 @@ func (p ResourceDataConfigProvider) PrivateRSAKey() (key *rsa.PrivateKey, err er
 }
 func BuildHttpClient() (httpClient *http.Client) {
 	httpClient = &http.Client{
-		Timeout: globalvar.DefaultRequestTimeout,
+		Timeout: getFromEnvVar(globalvar.HTTPRequestTimeOut, globalvar.DefaultRequestTimeout),
 		Transport: &http.Transport{
 			DialContext: (&net.Dialer{
-				Timeout: globalvar.DefaultConnectionTimeout,
+				Timeout: getFromEnvVar(globalvar.DialContextConnectionTimeout, globalvar.DefaultConnectionTimeout),
 			}).DialContext,
-			TLSHandshakeTimeout: globalvar.DefaultTLSHandshakeTimeout,
+			TLSHandshakeTimeout: getFromEnvVar(globalvar.TLSHandshakeTimeout, globalvar.DefaultTLSHandshakeTimeout),
 			TLSClientConfig:     &tls.Config{MinVersion: tls.VersionTLS12},
 			Proxy:               http.ProxyFromEnvironment,
 		},
 	}
 	return
 }
-
+func getFromEnvVar(varName string, defaultValue time.Duration) time.Duration {
+	valueStr := utils.GetEnvSettingWithDefault(varName, fmt.Sprint(defaultValue))
+	duration, err := time.ParseDuration(valueStr)
+	if err != nil {
+		utils.Debugf("ERROR while parsing env variable %s value: %v", varName, err)
+		return defaultValue
+	}
+	return duration
+}
 func UserAgentFromEnv() string {
 
 	userAgentFromEnv, err := schemaMultiEnvDefaultFuncVar([]string{globalvar.UserAgentProviderNameEnv, globalvar.UserAgentSDKNameEnv, globalvar.UserAgentTerraformNameEnv}, globalvar.DefaultUserAgentProviderName)()
diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go
@@ -3,6 +3,7 @@ package provider
 import (
 	"context"
 	"crypto/tls"
+	"fmt"
 	"io/ioutil"
 	"log"
 	"net/http"
@@ -877,3 +878,44 @@ func TestUnitUserAgentFromEnv(t *testing.T) {
 		})
 	}
 }
+
+func TestUnitGetFromEnvVar(t *testing.T) {
+	varName := "TEST_ENV_VAR"
+	defaultValue := 5 * time.Second
+
+	tests := []struct {
+		name          string
+		envValue      string
+		expectedValue time.Duration
+	}{
+		{
+			name:          "Valid duration in environment variable",
+			envValue:      "10s",
+			expectedValue: 10 * time.Second,
+		},
+		{
+			name:          "Invalid duration in environment variable",
+			envValue:      "invalid",
+			expectedValue: defaultValue, // Should fall back to default value
+		},
+		{
+			name:          "Empty environment variable",
+			envValue:      "",
+			expectedValue: defaultValue, // Should fall back to default value
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Set the environment variable
+			os.Setenv(varName, tt.envValue)
+			defer os.Unsetenv(varName) // Clean up after the test
+
+			// Call the function
+			result := getFromEnvVar(varName, defaultValue)
+
+			// Assert the result
+			assert.Equal(t, tt.expectedValue, result, fmt.Sprintf("Expected %v, but got %v", tt.expectedValue, result))
+		})
+	}
+}
diff --git a/internal/tfresource/retry.go b/internal/tfresource/retry.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"log"
 	"math/rand"
+	"net"
 	"strconv"
 	"strings"
 	"time"
@@ -122,7 +123,7 @@ func getExpectedRetryDuration(response oci_common.OCIOperationResponse, disableN
 func GetDefaultExpectedRetryDuration(response oci_common.OCIOperationResponse, disableNotFoundRetries bool) time.Duration {
 	defaultRetryTime := ShortRetryTime
 
-	if oci_common.IsNetworkError(response.Error) {
+	if IsNetworkError(response.Error) {
 		log.Printf("[DEBUG] Retrying for network error...")
 		return defaultRetryTime
 	}
@@ -215,7 +216,7 @@ func getRemainingEventualConsistencyDuration(r oci_common.OCIOperationResponse)
 
 func getIdentityExpectedRetryDuration(response oci_common.OCIOperationResponse, disableNotFoundRetries bool, optionals ...interface{}) time.Duration {
 	defaultRetryTime := GetDefaultExpectedRetryDuration(response, disableNotFoundRetries)
-	if oci_common.IsNetworkError(response.Error) {
+	if IsNetworkError(response.Error) {
 		return defaultRetryTime
 	}
 
@@ -251,7 +252,7 @@ func getIdentityExpectedRetryDuration(response oci_common.OCIOperationResponse,
 
 func getDatabaseExpectedRetryDuration(response oci_common.OCIOperationResponse, disableNotFoundRetries bool, optionals ...interface{}) time.Duration {
 	defaultRetryTime := GetDefaultExpectedRetryDuration(response, disableNotFoundRetries)
-	if oci_common.IsNetworkError(response.Error) {
+	if IsNetworkError(response.Error) {
 		return defaultRetryTime
 	}
 
@@ -283,7 +284,7 @@ func getDatabaseExpectedRetryDuration(response oci_common.OCIOperationResponse,
 
 func getObjectstorageServiceExpectedRetryDuration(response oci_common.OCIOperationResponse, disableNotFoundRetries bool, optionals ...interface{}) time.Duration {
 	defaultRetryTime := GetDefaultExpectedRetryDuration(response, disableNotFoundRetries)
-	if oci_common.IsNetworkError(response.Error) {
+	if IsNetworkError(response.Error) {
 		return defaultRetryTime
 	}
 
@@ -510,3 +511,13 @@ func GetDbHomeRetryDurationFunction(retryTimeout time.Duration) expectedRetryDur
 		return defaultRetryTime
 	}
 }
+
+// IsNetworkError checks if the given error is a network-related timeout error.
+// It returns true if the error is of type *net.OpError and indicates a timeout, or if the error message contains "i/o timeout".
+func IsNetworkError(err error) bool {
+	if opErr, ok := err.(*net.OpError); ok && (opErr.Timeout() || strings.Contains(err.Error(), "i/o timeout")) {
+		return true
+	}
+	// // IsNetworkError validates if an error is a net.Error and check if it's temporary or timeout
+	return oci_common.IsNetworkError(err)
+}
diff --git a/internal/tfresource/retry_test.go b/internal/tfresource/retry_test.go
@@ -4,6 +4,7 @@
 package tfresource
 
 import (
+	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -841,3 +842,53 @@ func TestUnitDefaultExpectedRetryDuration(t *testing.T) {
 
 	}
 }
+
+// Unit test for IsNetworkError function.
+func TestUnit_IsNetworkError(t *testing.T) {
+	// Create a timeout error using net package.
+	timeoutErr := &net.OpError{
+		Op:  "dial",
+		Net: "tcp",
+		Err: &net.DNSError{
+			IsTimeout: true,
+		},
+	}
+
+	// Create a non-timeout error using net package.
+	nonTimeoutErr := &net.OpError{
+		Op:  "read",
+		Net: "tcp",
+		Err: errors.New("connection reset by peer"),
+	}
+
+	tests := []struct {
+		name     string
+		inputErr error
+		expected bool
+	}{
+		{
+			name:     "Network timeout error",
+			inputErr: timeoutErr,
+			expected: true,
+		},
+		{
+			name:     "Non-timeout network error",
+			inputErr: nonTimeoutErr,
+			expected: false,
+		},
+		{
+			name:     "Nil error",
+			inputErr: nil,
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := IsNetworkError(tt.inputErr)
+			if result != tt.expected {
+				t.Errorf("IsNetworkTimeoutError(%v) = %v; expected %v", tt.inputErr, result, tt.expected)
+			}
+		})
+	}
+}
