Skip to content

Commit 8e071bf

Browse files
committed
chore: refine HTTP client timeout docs and simplify integration tests
Clarify documentation on `timeout` behavior and its independence from transport-level settings. Simplify OCI registry timeout integration tests by consolidating duplicate setups and removing unused toxic configurations. Signed-off-by: Piotr Janik <piotr.janik@sap.com>
1 parent b509b62 commit 8e071bf

2 files changed

Lines changed: 38 additions & 52 deletions

File tree

api/datacontext/attrs/httpcfgattr/config.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,14 @@ func NewDuration(d time.Duration) *Duration {
5454
// All timeout values use Duration (Go duration strings in config).
5555
// If not set (nil), the http.DefaultTransport value from the Go
5656
// standard library is used.
57+
//
58+
// Note: Timeout controls the overall http.Client deadline and is
59+
// independent of the transport-level timeouts below. Setting Timeout
60+
// alone does NOT override TCPDialTimeout, TLSHandshakeTimeout, etc.
5761
type HTTPSettings struct {
58-
// Timeout is the overall HTTP client timeout.
62+
// Timeout is the overall http.Client timeout — the maximum duration
63+
// for the entire request including connection, TLS, headers, and body.
64+
// It does NOT serve as a fallback for transport-level timeouts.
5965
// If not set, http.Client uses no timeout (0).
6066
Timeout *Duration `json:"timeout,omitempty"`
6167

@@ -209,4 +215,8 @@ HTTP client settings:
209215
All timeout values are Go duration strings (e.g. "30s", "5m", "1h").
210216
Use "0s" to disable a specific timeout. If not set, the <code>http.DefaultTransport</code>
211217
values from the Go standard library are used.
218+
219+
Note: <code>timeout</code> controls the overall <code>http.Client</code> request deadline and is
220+
independent of the transport-level settings. Setting only <code>timeout</code> does not
221+
affect <code>tcpDialTimeout</code>, <code>tlsHandshakeTimeout</code>, or other transport timeouts.
212222
`

api/oci/extensions/repositories/ocireg/timeout_integration_test.go

Lines changed: 27 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,28 @@ var _ = Describe("Registry timeout:", Ordered, func() {
7777

7878
env = NewTestEnv(envhelper.FileSystem(osfs.New()))
7979

80+
// Create temp dir and CTF for all tests.
81+
tempDir, err = os.MkdirTemp("", "ocm-timeout-*")
82+
Expect(err).To(Succeed())
83+
84+
ctfDir = filepath.Join(tempDir, "ctf")
85+
constructorFile := filepath.Join(tempDir, "constructor.yaml")
86+
constructor := `components:
87+
- name: ` + componentName + `
88+
version: ` + componentVersion + `
89+
provider:
90+
name: test
91+
`
92+
Expect(os.WriteFile(constructorFile, []byte(constructor), 0o644)).To(Succeed())
93+
94+
buf := bytes.NewBuffer(nil)
95+
Expect(env.CatchOutput(buf).Execute(
96+
"add", "componentversions",
97+
"--create",
98+
"--file", ctfDir,
99+
constructorFile,
100+
)).To(Succeed())
101+
80102
log.Info("Toxic Registry ready", "proxy", proxyHost, "ctf", ctfDir)
81103
})
82104

@@ -106,30 +128,6 @@ var _ = Describe("Registry timeout:", Ordered, func() {
106128
return cfgFile
107129
}
108130

109-
It("creates component version", func() {
110-
var err error
111-
tempDir, err = os.MkdirTemp("", "ocm-timeout-*")
112-
Expect(err).To(Succeed())
113-
114-
ctfDir = filepath.Join(tempDir, "ctf")
115-
constructorFile := filepath.Join(tempDir, "constructor.yaml")
116-
constructor := `components:
117-
- name: ` + componentName + `
118-
version: ` + componentVersion + `
119-
provider:
120-
name: test
121-
`
122-
Expect(os.WriteFile(constructorFile, []byte(constructor), 0o644)).To(Succeed())
123-
124-
buf := bytes.NewBuffer(nil)
125-
Expect(env.CatchOutput(buf).Execute(
126-
"add", "componentversions",
127-
"--create",
128-
"--file", ctfDir,
129-
constructorFile,
130-
)).To(Succeed())
131-
})
132-
133131
It("fails when timeout is shorter than proxy latency", func() {
134132
addLatency(proxy, 30_000, "upstream")
135133
defer removeToxic(proxy, "latency")
@@ -166,12 +164,11 @@ var _ = Describe("Registry timeout:", Ordered, func() {
166164
})
167165

168166
It("fails when response headers can't arrive within configured time", func() {
169-
addBandwidth(proxy, 1, "downstream")
170-
defer removeToxic(proxy, "bandwidth")
171-
addLatency(proxy, 3_000, "downstream")
167+
addLatency(proxy, 5_000, "downstream")
172168
defer removeToxic(proxy, "latency")
173169

174-
cfgFile := writeHTTPConfig(`"responseHeaderTimeout":"1s"`)
170+
// Cap overall timeout to prevent retries from dragging out.
171+
cfgFile := writeHTTPConfig(`"responseHeaderTimeout":"1s","timeout":"8s"`)
175172
registryURL := "http://" + proxyHost
176173
err := env.Execute(
177174
"--config", cfgFile,
@@ -188,8 +185,8 @@ var _ = Describe("Registry timeout:", Ordered, func() {
188185
})
189186

190187
It("succeeds when response header timeout is generous enough", func() {
191-
addBandwidth(proxy, 1, "downstream")
192-
defer removeToxic(proxy, "bandwidth")
188+
addLatency(proxy, 2_000, "downstream")
189+
defer removeToxic(proxy, "latency")
193190

194191
cfgFile := writeHTTPConfig(`"responseHeaderTimeout":"30s"`)
195192
registryURL := "http://" + proxyHost
@@ -219,20 +216,6 @@ var _ = Describe("Registry timeout:", Ordered, func() {
219216
))
220217
})
221218

222-
It("succeeds with generous tcp-dial-timeout", func() {
223-
addLatency(proxy, 500, "upstream")
224-
defer removeToxic(proxy, "latency")
225-
226-
cfgFile := writeHTTPConfig(`"tcpDialTimeout":"10s"`)
227-
registryURL := "http://" + proxyHost
228-
buf := bytes.NewBuffer(nil)
229-
Expect(env.CatchOutput(buf).Execute(
230-
"--config", cfgFile,
231-
"transfer", "componentversions",
232-
ctfDir+"//"+componentName+":"+componentVersion,
233-
registryURL,
234-
)).To(Succeed())
235-
})
236219
})
237220

238221
func addLatency(proxy *toxiproxy.Proxy, latencyMs int, stream string) {
@@ -242,13 +225,6 @@ func addLatency(proxy *toxiproxy.Proxy, latencyMs int, stream string) {
242225
Expect(err).To(Succeed())
243226
}
244227

245-
func addBandwidth(proxy *toxiproxy.Proxy, rateKBps int, stream string) {
246-
_, err := proxy.AddToxic("bandwidth", "bandwidth", stream, 1.0, toxiproxy.Attributes{
247-
"rate": rateKBps,
248-
})
249-
Expect(err).To(Succeed())
250-
}
251-
252228
func removeToxic(proxy *toxiproxy.Proxy, name string) {
253229
Expect(proxy.RemoveToxic(name)).To(Succeed())
254230
}

0 commit comments

Comments
 (0)