poc for fileconfiguration failover by cristiGuranIonos · Pull Request #55 · ionos-cloud/sdk-go-bundle

cristiGuranIonos · 2026-02-24T08:33:02Z

No description provided.

shared/failover_roundtripper.go

…e logic

) * refactor callAPI to be a wrapper function, so behaviour can actually be tested * tests, comments, refactor * do not retry if context is canceled

* fix all issues caught by sonarqube * error on non-existing failoverStrategy, refactor doFailoverAttempt, better logging * simplify doFailoverAttempt, check for NetworkError in orderedRoundTrip

Copilot

Pull request overview

This PR introduces transport-level endpoint failover (RoundTripper-based) and extracts the per-product HTTP retry logic into a shared shared/retry package, with initial YAML deserialization support for failover options in shared/fileconfiguration.

Changes:

Add shared/failover RoundTripper with RoundRobin strategy, backoff, and extensive unit/E2E tests.
Centralize application-level HTTP retry into shared/retry.DoWithApplicationRetry and switch compute client to use it.
Extend file configuration to deserialize a top-level failover: block into failover.Options.

Reviewed changes

Copilot reviewed 12 out of 14 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
shared/retry/retry.go	New shared application-level retry implementation used by product clients.
shared/logger.go	Adds `LogDebug`/`LogTrace` helpers for log-level-gated logging.
shared/go.mod	Adds dependencies needed by failover/retry + (currently) compute for E2E test.
shared/go.sum	Updates checksums for newly required modules.
shared/fileconfiguration/fileconfiguration.go	Adds `Failover *failover.Options` field and accessor.
shared/fileconfiguration/fileconfiguration_test.go	Tests YAML deserialization and nil behavior for failover options.
shared/failover/helpers_test.go	Test helpers for RoundTripper/network error simulation and backoff.
shared/failover/failover_roundtripper.go	New failover RoundTripper implementation and option types.
shared/failover/failover_roundtripper_test.go	Unit tests covering failover behavior across error/status scenarios.
shared/failover/failover_e2e_test.go	E2E test wiring compute client to failover transport via shared module.
shared/configuration.go	Exports `DefaultMaxRetries` and updates default initializations.
products/compute/go.mod	Adds local replace to shared (duplicated) and indirect deps for failover tests.
products/compute/go.sum	Updates module checksums for new deps.
products/compute/client.go	Replaces inline retry logic with `shared/retry.DoWithApplicationRetry`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

shared/failover/failover_roundtripper.go

shared/fileconfiguration/fileconfiguration.go

shared/failover/failover_roundtripper.go

products/compute/go.mod

shared/failover/failover_e2e_test.go

shared/retry/retry.go

shared/go.mod

shared/retry/retry.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 11 out of 13 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

shared/retry/retry.go

Copilot · 2026-03-05T12:13:19Z

shared/retry/retry.go

+	logRequest(clonedRequest, 0)
+
+	httpRequestStartTime := time.Now()
+	clonedRequest.Close = true


This retry implementation sets clonedRequest.Close = true, which disables connection reuse, but later drains the response body “so the connection can be reused”. Either remove Close = true to allow keep-alives (and keep draining), or drop the drain logic if connections are intentionally closed each attempt.

Suggested change

clonedRequest.Close = true

Copilot · 2026-03-05T12:13:19Z

shared/retry/retry.go

+	if !shared.SdkLogLevel.Satisfies(shared.Trace) {
+		logReq.Header.Del("Authorization")
+	}
+	dump, err := httputil.DumpRequestOut(logReq, true)


request.Clone() shares the same Body reader with the original request. httputil.DumpRequestOut(..., true) reads the body; with the current flow (logging the same cloned request you later send), debug logging can consume the body and cause the real request to be sent with an empty/truncated payload. To avoid this, ensure the request being dumped has an independent body reader (e.g., set logReq.Body from request.GetBody() when available) rather than reusing the same Body instance.

Suggested change

dump, err := httputil.DumpRequestOut(logReq, true)

dumpBody := false

if request.GetBody != nil {

if bodyCopy, err := request.GetBody(); err == nil && bodyCopy != nil {

defer bodyCopy.Close()

logReq.Body = bodyCopy

dumpBody = true

}

}

dump, err := httputil.DumpRequestOut(logReq, dumpBody)

Copilot · 2026-03-05T12:13:20Z

shared/failover/failover_roundtripper.go

+	// MaxRetries controls how many times the transport will attempt a request using the set strategy before giving up
+	// and returning the last error. If zero, it defaults to 3.


The Options.MaxRetries docs are inconsistent with the implementation: orderedRoundTrip uses totalAttempts := maxRetries + 1, but Options.MaxRetries is described as “how many times the transport will attempt a request”. Please clarify whether MaxRetries is “retries” or “attempts” and align the comment (and the worst-case attempts formula) accordingly.

Suggested change

// MaxRetries controls how many times the transport will attempt a request using the set strategy before giving up

// and returning the last error. If zero, it defaults to 3.

// MaxRetries controls how many times the transport will retry a request using the set strategy

// after the initial attempt before giving up and returning the last error. In other words, the

// worst-case total number of attempts is MaxRetries+1. If zero, it defaults to 3.

Copilot · 2026-03-05T12:13:20Z

products/compute/go.mod

+
+replace github.com/ionos-cloud/sdk-go-bundle/shared => ../../shared


Duplicate replace github.com/ionos-cloud/sdk-go-bundle/shared => ../../shared: this file declares it twice, which will make go mod tidy fail. Remove one of the duplicate replace directives.

Suggested change

replace github.com/ionos-cloud/sdk-go-bundle/shared => ../../shared

Copilot · 2026-03-05T12:13:20Z

shared/retry/retry.go

+// DoWithApplicationRetry executes an HTTP request with application-level retry
+// logic. This is the shared equivalent of the per-product callAPI method.
+//
+// Retry behaviour:
+//   - 502/503/504: retries with backoff (skips POST)
+//   - 429: retries honoring Retry-After header, falls back to waitTime
+//   - Other status codes or transport errors: returns immediately
+//   - Respects context cancellation during backoff


This PR introduces a new shared retry helper, but there are no unit tests covering the key behaviors (e.g., 429 Retry-After parsing, 502/503/504 retry rules, and POST not being retried). Please add focused tests under shared/retry to protect future migrations from regressions.

Copilot · 2026-03-05T12:13:20Z

shared/retry/retry.go

+) (*http.Response, time.Duration, error) {
+	if cfg == nil {
+		return nil, 0, errors.New("nil configuration")
+	}


DoWithApplicationRetry validates cfg but not request. A nil request will panic when accessing request.Method / request.Context(). Add an early request == nil check and return a clear error.

Suggested change

}

}

if request == nil {

return nil, 0, errors.New("nil request")

}

sonarqubecloud · 2026-03-06T10:30:14Z

Quality Gate failed

Failed conditions
1 New Critical Issues (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

poc for fileconfiguration failover

50098a7

adeatcu-ionos reviewed Feb 24, 2026

View reviewed changes

shared/failover_roundtripper.go Outdated Show resolved Hide resolved

shared/failover_roundtripper.go Outdated Show resolved Hide resolved

shared/failover_roundtripper.go Outdated Show resolved Hide resolved

shared/failover_roundtripper.go Outdated Show resolved Hide resolved

cristiGuranIonos and others added 6 commits February 25, 2026 10:16

add options struct,

e5bdbb3

add backoff options to failover config, did not handle 429 status cod…

66094f5

…e logic

fail on correct errors, fix tests, remove unused function

2048af9

add more comments to document behaviour, fix dns test to expect error

7581c72

feat: move callAPI logic to shared package, keep callAPI as wrapper (#58

e5852bc

) * refactor callAPI to be a wrapper function, so behaviour can actually be tested * tests, comments, refactor * do not retry if context is canceled

fix all issues caught by sonarqube (#60)

ba74617

* fix all issues caught by sonarqube * error on non-existing failoverStrategy, refactor doFailoverAttempt, better logging * simplify doFailoverAttempt, check for NetworkError in orderedRoundTrip

cristiGuranIonos requested a review from adeatcu-ionos March 2, 2026 11:29

cristiGuranIonos added 2 commits March 2, 2026 13:29

add e2e tests (#59)

f89cf0e

Pass only failover and servers and stricter error checks (#61)

e235960

cristiGuranIonos marked this pull request as ready for review March 5, 2026 10:45

cristiGuranIonos requested a review from Copilot March 5, 2026 11:44

Copilot started reviewing on behalf of cristiGuranIonos March 5, 2026 11:44 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

cristiGuranIonos and others added 4 commits March 5, 2026 13:57

Update shared/failover/failover_roundtripper.go

bce3fa0

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update shared/fileconfiguration/fileconfiguration.go

a35ce4e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update shared/retry/retry.go

cbe7076

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix: remove compute test from shared

56dd938

cristiGuranIonos requested a review from Copilot March 5, 2026 12:06

Copilot started reviewing on behalf of cristiGuranIonos March 5, 2026 12:06 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

fix: log attempt

a4d07d6

adeatcu-ionos approved these changes Mar 6, 2026

View reviewed changes

fix: remove replace

7dd68bc

cristiGuranIonos merged commit 4376df3 into v2 Mar 6, 2026
2 of 3 checks passed

-	dump, err := httputil.DumpRequestOut(logReq, true)
+	dumpBody := false
+	if request.GetBody != nil {
+		if bodyCopy, err := request.GetBody(); err == nil && bodyCopy != nil {
+			defer bodyCopy.Close()
+			logReq.Body = bodyCopy
+			dumpBody = true
+		}
+	}
+	dump, err := httputil.DumpRequestOut(logReq, dumpBody)

		// MaxRetries controls how many times the transport will attempt a request using the set strategy before giving up
		// and returning the last error. If zero, it defaults to 3.

-	// MaxRetries controls how many times the transport will attempt a request using the set strategy before giving up
-	// and returning the last error. If zero, it defaults to 3.
+	// MaxRetries controls how many times the transport will retry a request using the set strategy
+	// after the initial attempt before giving up and returning the last error. In other words, the
+	// worst-case total number of attempts is MaxRetries+1. If zero, it defaults to 3.


		replace github.com/ionos-cloud/sdk-go-bundle/shared => ../../shared

-	}
+	}
+	if request == nil {
+		return nil, 0, errors.New("nil request")
+	}

Conversation

cristiGuranIonos commented Feb 24, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Mar 6, 2026

Quality Gate failed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants