add options struct,

Author: cristiGuranIonos

products/compute/client.go

@@ -137,6 +137,24 @@ const (
 	FailoverRoundRobin FailoverStrategy = "roundRobin"
 )
 
+// FailoverOptions controls transport-level endpoint failover behaviour.
+// It is nested under Configuration so it can be grouped cleanly in JSON/YAML.
+type FailoverOptions struct {
+	Strategy FailoverStrategy `json:"strategy,omitempty" yaml:"strategy,omitempty"`
+
+	// RetryableMethods controls which HTTP methods are eligible for transport-level
+	// failover retries. If empty/nil, the default is safe/idempotent methods.
+	RetryableMethods []string `json:"retryableMethods,omitempty" yaml:"retryableMethods,omitempty"`
+
+	// RetryOnTimeout controls whether failover retries should also happen when
+	// the request fails due to context cancellation/deadline exceeded.
+	RetryOnTimeout bool `json:"retryOnTimeout,omitempty" yaml:"retryOnTimeout,omitempty"`
+
+	// FailoverOnStatusCodes controls whether the transport should fail over to the
+	// next server when it receives one of these HTTP status codes.
+	FailoverOnStatusCodes []int `json:"failoverOnStatusCodes,omitempty" yaml:"failoverOnStatusCodes,omitempty"`
+}
+
 // Configuration stores the configuration of the API client
 type Configuration struct {
 	Host               string                          `json:"host,omitempty"`
@@ -159,18 +177,7 @@ type Configuration struct {
 	MiddlewareWithError MiddlewareFunctionWithError `json:"-"`
 	ResponseMiddleware  ResponseMiddlewareFunction  `json:"-"`
 
-	FailoverStrategy FailoverStrategy `json:"failoverStrategy,omitempty"`
-
-	// RetryableMethods controls which HTTP methods are eligible for transport-level
-	// failover retries.
-	// If empty/nil, the default is to retry only safe/idempotent methods: GET, HEAD,
-	// PUT, DELETE, OPTIONS.
-	RetryableMethods []string `json:"retryableMethods,omitempty"`
-
-	// RetryOnTimeout controls whether transport-level failover retries should also
-	// happen when the request fails due to context cancellation/deadline exceeded.
-	// Default is false to avoid duplicate side effects for non-idempotent requests.
-	RetryOnTimeout bool `json:"retryOnTimeout,omitempty"`
+	Failover *FailoverOptions `json:"failover,omitempty"`
 }
 
 // NewConfiguration returns a new shared.Configuration object.

shared/configuration.go

@@ -7,6 +7,8 @@ package shared
 import (
 	"context"
 	"errors"
+	"fmt"
+	"io"
 	"net"
 	"net/http"
 	"net/url"
@@ -32,16 +34,19 @@ import (
 // preserving path and query.
 type FailoverRoundTripper struct {
 	cfg  *Configuration
+	opts *FailoverOptions
 	base http.RoundTripper
 }
 
-// NewFailoverRoundTripper creates a new FailoverRoundTripper with the given configuration and base RoundTripper.
-func NewFailoverRoundTripper(cfg *Configuration, base http.RoundTripper) http.RoundTripper {
+// NewFailoverRoundTripper creates a new FailoverRoundTripper.
+// If opts is nil, it will fall back to cfg.Failover.
+func NewFailoverRoundTripper(cfg *Configuration, opts *FailoverOptions, base http.RoundTripper) http.RoundTripper {
 	if base == nil {
 		base = http.DefaultTransport
 	}
 	return &FailoverRoundTripper{
 		cfg:  cfg,
+		opts: opts,
 		base: base,
 	}
 }
@@ -51,60 +56,86 @@ func (t *FailoverRoundTripper) RoundTrip(req *http.Request) (*http.Response, err
 	if req == nil {
 		return nil, errors.New("nil request")
 	}
-	if t == nil || t.cfg == nil {
-		if t != nil && t.base != nil {
-			return t.base.RoundTrip(req)
-		}
-		return http.DefaultTransport.RoundTrip(req)
+	if t == nil {
+		return nil, errors.New("nil FailoverRoundTripper")
+	}
+	if t.base == nil {
+		// Be resilient if instantiated without constructor.
+		t.base = http.DefaultTransport
+	}
+	if t.cfg == nil {
+		// No config => behave like the base transport.
+		return t.base.RoundTrip(req)
 	}
 
-	strategyName := strings.TrimSpace(strings.ToLower(string(t.cfg.FailoverStrategy)))
-	servers := len(t.cfg.Servers)
-	if strategyName == "" || strategyName == string(FailoverNone) || servers <= 1 {
+	fo := t.opts
+	if fo == nil {
+		fo = t.cfg.Failover
+	}
+	if fo == nil {
 		return t.base.RoundTrip(req)
 	}
 
-	if strategyName != strings.ToLower(string(FailoverRoundRobin)) {
+	servers := t.cfg.Servers
+	order := serverOrderFor(fo.Strategy, len(servers))
+	if order == nil {
+		// Unknown or disabled strategy => pass through.
 		return t.base.RoundTrip(req)
 	}
 
 	// Check if method is allowed for failover retries.
-	if !isRetryableMethod(t.cfg, req.Method) {
+	if !isRetryableMethod(fo, req.Method) {
 		return t.base.RoundTrip(req)
 	}
 
 	var lastErr error
-	for i := range servers {
-		// Always start from the first server in the list.
-		serverURL := t.cfg.Servers[i].URL
+	for attempt := range len(servers) {
+		serverURL := servers[order(attempt)].URL
 
 		targetURL, err := url.Parse(serverURL)
 		if err != nil {
-			lastErr = err
-			continue
+			return nil, fmt.Errorf("invalid server URL at Servers[%d]=%q: %w", order(attempt), serverURL, err)
 		}
 
 		attemptReq, err := cloneRequestForRetry(req)
 		if err != nil {
 			return nil, err
 		}
 
-		// Update both URL and the Host header field.
 		attemptReq.URL.Scheme = targetURL.Scheme
 		attemptReq.URL.Host = targetURL.Host
 		attemptReq.Host = targetURL.Host
 
+		if SdkLogLevel.Satisfies(Debug) {
+			SdkLogger.Printf("[Failover] attempt=%d method=%s url=%s", attempt+1, attemptReq.Method, attemptReq.URL.String())
+		}
+
 		resp, err := t.base.RoundTrip(attemptReq)
 		if err == nil {
+			if shouldFailoverOnStatus(fo, resp.StatusCode) {
+				if SdkLogLevel.Satisfies(Debug) {
+					SdkLogger.Printf("[Failover] status=%d triggers failover to next server", resp.StatusCode)
+				}
+				// Drain/close body to allow connection reuse.
+				if resp.Body != nil {
+					_, _ = io.Copy(io.Discard, resp.Body)
+					_ = resp.Body.Close()
+				}
+				lastErr = fmt.Errorf("failover status: %s", resp.Status)
+				continue
+			}
 			return resp, nil
 		}
 
 		lastErr = err
-		if !isNetworkErrorRT(attemptReq.Context(), err, t.cfg.RetryOnTimeout) {
+		retryable := isNetworkErrorRT(attemptReq.Context(), err, fo.RetryOnTimeout)
+		if !retryable {
 			return nil, err
 		}
+		if SdkLogLevel.Satisfies(Debug) {
+			SdkLogger.Printf("[Failover] network error: %v; trying next server", err)
+		}
 
-		// Ensure we don't spin too hot in case of immediate failures.
 		tinyBackoff(attemptReq.Context())
 	}
 
@@ -126,18 +157,17 @@ func cloneRequestForRetry(req *http.Request) (*http.Request, error) {
 	return clone, nil
 }
 
-func isRetryableMethod(cfg *Configuration, method string) bool {
+func isRetryableMethod(fo *FailoverOptions, method string) bool {
 	m := strings.ToUpper(strings.TrimSpace(method))
-	if cfg == nil {
+	if fo == nil {
 		return defaultRetryableMethods[m]
 	}
 
-	// If not configured, use defaults.
-	if len(cfg.RetryableMethods) == 0 {
+	if len(fo.RetryableMethods) == 0 {
 		return defaultRetryableMethods[m]
 	}
 
-	for _, v := range cfg.RetryableMethods {
+	for _, v := range fo.RetryableMethods {
 		if strings.ToUpper(strings.TrimSpace(v)) == m {
 			return true
 		}
@@ -208,3 +238,35 @@ func tinyBackoff(ctx context.Context) {
 	case <-ctx.Done():
 	}
 }
+
+func shouldFailoverOnStatus(fo *FailoverOptions, statusCode int) bool {
+	if fo == nil || len(fo.FailoverOnStatusCodes) == 0 {
+		return false
+	}
+	for _, sc := range fo.FailoverOnStatusCodes {
+		if sc == statusCode {
+			return true
+		}
+	}
+	return false
+}
+
+// serverOrder maps an attempt index (0, 1, 2, …) to a server index.
+// Different strategies produce different orderings.
+type serverOrder func(attempt int) int
+
+// serverOrderFor returns a serverOrder for the given strategy, or nil when
+// failover should not be applied (unknown/disabled strategy, ≤1 server).
+func serverOrderFor(strategy FailoverStrategy, numServers int) serverOrder {
+	s := strings.TrimSpace(strings.ToLower(string(strategy)))
+	if s == "" || s == string(FailoverNone) || numServers <= 1 {
+		return nil
+	}
+	switch s {
+	case strings.ToLower(string(FailoverRoundRobin)):
+		// Sequential: 0, 1, 2, …
+		return func(attempt int) int { return attempt % numServers }
+	default:
+		return nil
+	}
+}

shared/failover_roundtripper.go

@@ -38,17 +38,18 @@ func (f *fakeTransport) RoundTrip(r *http.Request) (*http.Response, error) {
 
 func TestFailoverRoundTripper_RoundRobin_NetworkError_FailsOverToNextServer(t *testing.T) {
 	cfg := &Configuration{
-		FailoverStrategy: FailoverRoundRobin,
+		Failover: &FailoverOptions{
+			Strategy:       FailoverRoundRobin,
+			RetryOnTimeout: false,
+		},
 		Servers: ServerConfigurations{
 			{URL: "https://s1.example"},
 			{URL: "https://s2.example"},
 		},
-		// default RetryableMethods covers GET
-		RetryOnTimeout: false,
 	}
 
 	ft := &fakeTransport{}
-	rt := NewFailoverRoundTripper(cfg, ft)
+	rt := NewFailoverRoundTripper(cfg, cfg.Failover, ft)
 
 	req, err := http.NewRequest(http.MethodGet, "https://s1.example/some/path?x=1", nil)
 	if err != nil {
@@ -76,16 +77,18 @@ func TestFailoverRoundTripper_RoundRobin_NetworkError_FailsOverToNextServer(t *t
 
 func TestFailoverRoundTripper_DoesNotRetry_WhenMethodNotRetryable(t *testing.T) {
 	cfg := &Configuration{
-		FailoverStrategy: FailoverRoundRobin,
+		Failover: &FailoverOptions{
+			Strategy:         FailoverRoundRobin,
+			RetryableMethods: []string{http.MethodGet},
+		},
 		Servers: ServerConfigurations{
 			{URL: "https://s1.example"},
 			{URL: "https://s2.example"},
 		},
-		RetryableMethods: []string{http.MethodGet},
 	}
 
 	ft := &fakeTransport{}
-	rt := NewFailoverRoundTripper(cfg, ft)
+	rt := NewFailoverRoundTripper(cfg, cfg.Failover, ft)
 
 	// POST is not retryable per config
 	req, err := http.NewRequest(http.MethodPost, "https://s1.example/some/path", io.NopCloser(bytes.NewBufferString("x")))
@@ -112,12 +115,14 @@ func TestFailoverRoundTripper_DoesNotRetry_WhenMethodNotRetryable(t *testing.T)
 
 func TestFailoverRoundTripper_RetriesOnNoSuchHost(t *testing.T) {
 	cfg := &Configuration{
-		FailoverStrategy: FailoverRoundRobin,
+		Failover: &FailoverOptions{
+			Strategy:       FailoverRoundRobin,
+			RetryOnTimeout: false,
+		},
 		Servers: ServerConfigurations{
 			{URL: "https://s1.example"},
 			{URL: "https://s2.example"},
 		},
-		RetryOnTimeout: false,
 	}
 
 	ft := &fakeTransport{}
@@ -126,7 +131,7 @@ func TestFailoverRoundTripper_RetriesOnNoSuchHost(t *testing.T) {
 	_ = ft2
 
 	// Inline transport to simulate DNS error.
-	rt := NewFailoverRoundTripper(cfg, roundTripperFunc(func(r *http.Request) (*http.Response, error) {
+	rt := NewFailoverRoundTripper(cfg, cfg.Failover, roundTripperFunc(func(r *http.Request) (*http.Response, error) {
 		host := ""
 		urlStr := ""
 		if r != nil && r.URL != nil {
@@ -160,6 +165,44 @@ func TestFailoverRoundTripper_RetriesOnNoSuchHost(t *testing.T) {
 	}
 }
 
+func TestFailoverRoundTripper_FailoverOnStatusCodes(t *testing.T) {
+	cfg := &Configuration{
+		Failover: &FailoverOptions{
+			Strategy:              FailoverRoundRobin,
+			FailoverOnStatusCodes: []int{http.StatusServiceUnavailable},
+		},
+		Servers: ServerConfigurations{
+			{URL: "https://s1.example"},
+			{URL: "https://s2.example"},
+		},
+	}
+
+	calls := []string{}
+	rt := NewFailoverRoundTripper(cfg, cfg.Failover, roundTripperFunc(func(r *http.Request) (*http.Response, error) {
+		calls = append(calls, r.URL.Host)
+		if r.URL.Host == "s1.example" {
+			return &http.Response{Status: "503 Service Unavailable", StatusCode: http.StatusServiceUnavailable, Body: io.NopCloser(bytes.NewBufferString("no")), Header: make(http.Header), Request: r}, nil
+		}
+		return &http.Response{Status: "200 OK", StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewBufferString("ok")), Header: make(http.Header), Request: r}, nil
+	}))
+
+	req, err := http.NewRequest(http.MethodGet, "https://s1.example/some/path", nil)
+	if err != nil {
+		t.Fatalf("unexpected error creating request: %v", err)
+	}
+
+	resp, err := rt.RoundTrip(req)
+	if err != nil {
+		t.Fatalf("expected success, got error: %v", err)
+	}
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("expected 200, got %d", resp.StatusCode)
+	}
+	if len(calls) != 2 || calls[0] != "s1.example" || calls[1] != "s2.example" {
+		t.Fatalf("unexpected call order: %+v", calls)
+	}
+}
+
 type roundTripperFunc func(*http.Request) (*http.Response, error)
 
 func (f roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }