Skip to content

Commit a512ea6

Browse files
hawkwhawkw
authored
outbound: implement OutboundPolicies route request timeouts (#2418)
The latest proxy-api release, v0.10.0, adds fields to the `OutboundPolicies` API for configuring HTTP request timeouts, based on the proposed changes to HTTPRoute in kubernetes-sigs/gateway-api#1997. This branch updates the proxy-api dependency to v0.10.0 and adds the new timeout configuration fields to the proxy's internal client policy types. In addition, this branch adds a timeout middleware to the HTTP client policy stack, so that the timeout described by the `Rule.request_timeout` field is now applied. Implementing the `RouteBackend.request_timeout` field with semantics as close as possible to those described in GEP-1742 will be somewhat more complex, and will be added in a separate PR.
1 parent 864a5db commit a512ea6

File tree

23 files changed

+169
-21
lines changed

23 files changed

+169
-21
lines changed

‎Cargo.lock

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1892,9 +1892,9 @@ dependencies = [
18921892

18931893
[[package]]
18941894
name = "linkerd2-proxy-api"
1895-
version = "0.9.0"
1895+
version = "0.10.0"
18961896
source = "registry+https://github.com/rust-lang/crates.io-index"
1897-
checksum = "3c5191a6b6a0d97519b4746c09a5e92cb9f586cb808d1828f6d7f9889e9ba24d"
1897+
checksum = "597facef5c3f12aece4d18a5e3dbba88288837b0b5d8276681d063e4c9b98a14"
18981898
dependencies = [
18991899
"h2",
19001900
"http",

‎linkerd/app/inbound/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ linkerd-meshtls = { path = "../../meshtls", optional = true }
2929
linkerd-meshtls-rustls = { path = "../../meshtls/rustls", optional = true }
3030
linkerd-proxy-client-policy = { path = "../../proxy/client-policy" }
3131
linkerd-tonic-watch = { path = "../../tonic-watch" }
32-
linkerd2-proxy-api = { version = "0.9", features = ["inbound"] }
32+
linkerd2-proxy-api = { version = "0.10", features = ["inbound"] }
3333
once_cell = "1"
3434
parking_lot = "0.12"
3535
rangemap = "1"

‎linkerd/app/integration/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ ipnet = "2"
3434
linkerd-app = { path = "..", features = ["allow-loopback"] }
3535
linkerd-app-core = { path = "../core" }
3636
linkerd-metrics = { path = "../../metrics", features = ["test_util"] }
37-
linkerd2-proxy-api = { version = "0.9", features = [
37+
linkerd2-proxy-api = { version = "0.10", features = [
3838
"destination",
3939
"arbitrary",
4040
] }

‎linkerd/app/integration/src/policy.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ pub fn outbound_default_http_route(dst: impl ToString) -> outbound::HttpRoute {
151151
}],
152152
filters: Vec::new(),
153153
backends: Some(http_first_available(std::iter::once(backend(dst)))),
154+
request_timeout: None,
154155
}],
155156
}
156157
}
@@ -214,6 +215,7 @@ pub fn http_first_available(
214215
.map(|backend| http_route::RouteBackend {
215216
backend: Some(backend),
216217
filters: Vec::new(),
218+
request_timeout: None,
217219
})
218220
.collect(),
219221
},

‎linkerd/app/integration/src/tests/client_policy.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ async fn header_based_routing() {
223223
backends: Some(policy::http_first_available(std::iter::once(
224224
policy::backend(dst),
225225
))),
226+
request_timeout: None,
226227
};
227228

228229
let route = outbound::HttpRoute {
@@ -236,6 +237,7 @@ async fn header_based_routing() {
236237
backends: Some(policy::http_first_available(std::iter::once(
237238
policy::backend(&dst_world),
238239
))),
240+
request_timeout: None,
239241
},
240242
// x-hello-city: sf | x-hello-city: san francisco
241243
mk_header_rule(
@@ -398,6 +400,8 @@ async fn path_based_routing() {
398400
backends: Some(policy::http_first_available(std::iter::once(
399401
policy::backend(dst),
400402
))),
403+
404+
request_timeout: None,
401405
};
402406

403407
let route = outbound::HttpRoute {
@@ -411,6 +415,7 @@ async fn path_based_routing() {
411415
backends: Some(policy::http_first_available(std::iter::once(
412416
policy::backend(&dst_world),
413417
))),
418+
request_timeout: None,
414419
},
415420
// /goodbye/*
416421
mk_path_rule(

‎linkerd/app/outbound/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ ahash = "0.8"
2020
bytes = "1"
2121
http = "0.2"
2222
futures = { version = "0.3", default-features = false }
23-
linkerd2-proxy-api = { version = "0.9", features = ["outbound"] }
23+
linkerd2-proxy-api = { version = "0.10", features = ["outbound"] }
2424
linkerd-app-core = { path = "../core" }
2525
linkerd-app-test = { path = "../test", optional = true }
2626
linkerd-distribute = { path = "../../distribute" }

‎linkerd/app/outbound/src/discover.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,12 @@ pub fn synthesize_forward_policy(
206206
meta: meta.clone(),
207207
filters: NO_OPAQ_FILTERS.clone(),
208208
failure_policy: Default::default(),
209+
request_timeout: None,
209210
distribution: policy::RouteDistribution::FirstAvailable(Arc::new([
210211
policy::RouteBackend {
211212
filters: NO_OPAQ_FILTERS.clone(),
212213
backend: backend.clone(),
214+
request_timeout: None,
213215
},
214216
])),
215217
}),
@@ -223,10 +225,12 @@ pub fn synthesize_forward_policy(
223225
meta: meta.clone(),
224226
filters: NO_HTTP_FILTERS.clone(),
225227
failure_policy: Default::default(),
228+
request_timeout: None,
226229
distribution: policy::RouteDistribution::FirstAvailable(Arc::new([
227230
policy::RouteBackend {
228231
filters: NO_HTTP_FILTERS.clone(),
229232
backend: backend.clone(),
233+
request_timeout: None,
230234
},
231235
])),
232236
},

‎linkerd/app/outbound/src/http/logical/policy/route.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ pub(crate) struct Route<T, F, E> {
3030
pub(super) filters: Arc<[F]>,
3131
pub(super) distribution: BackendDistribution<T, F>,
3232
pub(super) failure_policy: E,
33+
pub(super) request_timeout: Option<std::time::Duration>,
3334
}
3435

3536
pub(crate) type MatchedRoute<T, M, F, E> = Matched<M, Route<T, F, E>>;
@@ -111,6 +112,8 @@ where
111112
.push_on_service(svc::LoadShed::layer())
112113
// TODO(ver) attach the `E` typed failure policy to requests.
113114
.push(filters::NewApplyFilters::<Self, _, _>::layer())
115+
// Sets an optional request timeout.
116+
.push(http::NewTimeout::layer())
114117
.push(classify::NewClassify::layer())
115118
.push(svc::ArcNewService::layer())
116119
.into_inner()
@@ -124,6 +127,12 @@ impl<T: Clone, M, F, E> svc::Param<BackendDistribution<T, F>> for MatchedRoute<T
124127
}
125128
}
126129

130+
impl<T, M, F, E> svc::Param<http::timeout::ResponseTimeout> for MatchedRoute<T, M, F, E> {
131+
fn param(&self) -> http::timeout::ResponseTimeout {
132+
http::timeout::ResponseTimeout(self.params.request_timeout)
133+
}
134+
}
135+
127136
impl<T> filters::Apply for Http<T> {
128137
#[inline]
129138
fn apply<B>(&self, req: &mut ::http::Request<B>) -> Result<()> {

‎linkerd/app/outbound/src/http/logical/policy/router.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ where
204204
filters,
205205
distribution,
206206
failure_policy,
207+
request_timeout,
207208
}| {
208209
let route_ref = RouteRef(meta);
209210
let distribution = mk_distribution(&route_ref, &distribution);
@@ -214,6 +215,7 @@ where
214215
filters,
215216
failure_policy,
216217
distribution,
218+
request_timeout,
217219
}
218220
};
219221

‎linkerd/app/outbound/src/http/logical/policy/tests.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,11 @@ async fn header_based_route() {
4848
}),
4949
filters: Arc::new([]),
5050
failure_policy: Default::default(),
51+
request_timeout: None,
5152
distribution: policy::RouteDistribution::FirstAvailable(Arc::new([policy::RouteBackend {
5253
filters: Arc::new([]),
5354
backend,
55+
request_timeout: None,
5456
}])),
5557
};
5658

@@ -197,6 +199,7 @@ async fn http_filter_request_headers() {
197199
policy: policy::RoutePolicy {
198200
meta: policy::Meta::new_default("turtles"),
199201
failure_policy: Default::default(),
202+
request_timeout: None,
200203
filters: Arc::new([policy::http::Filter::RequestHeaders(
201204
policy::http::filter::ModifyHeader {
202205
add: vec![(PIZZA.clone(), TUBULAR.clone())],
@@ -212,6 +215,7 @@ async fn http_filter_request_headers() {
212215
..Default::default()
213216
},
214217
)]),
218+
request_timeout: None,
215219
},
216220
])),
217221
},

‎linkerd/app/outbound/src/http/logical/tests.rs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,66 @@ async fn balancer_doesnt_select_tripped_breakers() {
285285
}
286286
}
287287

288+
#[tokio::test(flavor = "current_thread")]
289+
async fn route_request_timeout() {
290+
tokio::time::pause();
291+
let _trace = trace::test::trace_init();
292+
const REQUEST_TIMEOUT: Duration = std::time::Duration::from_secs(2);
293+
294+
let addr = SocketAddr::new([192, 0, 2, 41].into(), PORT);
295+
let dest: NameAddr = format!("{AUTHORITY}:{PORT}")
296+
.parse::<NameAddr>()
297+
.expect("dest addr is valid");
298+
let (svc, mut handle) = tower_test::mock::pair();
299+
let connect = HttpConnect::default().service(addr, svc);
300+
let resolve = support::resolver().endpoint_exists(dest.clone(), addr, Default::default());
301+
let (rt, _shutdown) = runtime();
302+
let stack = Outbound::new(default_config(), rt)
303+
.with_stack(connect)
304+
.push_http_cached(resolve)
305+
.into_inner();
306+
307+
let (_route_tx, routes) = {
308+
let backend = default_backend(&dest);
309+
let mut route = default_route(backend.clone());
310+
// set the request timeout on the route.
311+
route.rules[0].policy.request_timeout = Some(REQUEST_TIMEOUT);
312+
watch::channel(Routes::Policy(policy::Params::Http(policy::HttpParams {
313+
addr: dest.into(),
314+
meta: ParentRef(client_policy::Meta::new_default("parent")),
315+
backends: Arc::new([backend]),
316+
routes: Arc::new([route]),
317+
failure_accrual: client_policy::FailureAccrual::None,
318+
})))
319+
};
320+
let target = Target {
321+
num: 1,
322+
version: http::Version::H2,
323+
routes,
324+
};
325+
let svc = stack.new_service(target);
326+
327+
handle.allow(1);
328+
let rsp = send_req(svc.clone(), http::Request::get("/"));
329+
serve_req(&mut handle, mk_rsp(StatusCode::OK, "good")).await;
330+
assert_eq!(
331+
rsp.await.expect("request must succeed").status(),
332+
http::StatusCode::OK
333+
);
334+
335+
// now, time out...
336+
let rsp = send_req(svc.clone(), http::Request::get("/"));
337+
tokio::time::sleep(REQUEST_TIMEOUT).await;
338+
let error = rsp.await.expect_err("request must fail with a timeout");
339+
assert!(
340+
error.is::<LogicalError>(),
341+
"error must originate in the logical stack"
342+
);
343+
assert!(errors::is_caused_by::<http::timeout::ResponseTimeoutError>(
344+
error.as_ref()
345+
));
346+
}
347+
288348
#[derive(Clone, Debug)]
289349
struct Target {
290350
num: usize,
@@ -448,9 +508,11 @@ fn default_route(backend: client_policy::Backend) -> client_policy::http::Route
448508
meta: Meta::new_default("test_route"),
449509
filters: NO_FILTERS.clone(),
450510
failure_policy: Default::default(),
511+
request_timeout: None,
451512
distribution: RouteDistribution::FirstAvailable(Arc::new([RouteBackend {
452513
filters: NO_FILTERS.clone(),
453514
backend,
515+
request_timeout: None,
454516
}])),
455517
},
456518
}],

‎linkerd/app/test/src/resolver/client_policy.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,11 @@ impl ClientPolicies {
7373
meta: Meta::new_default("default"),
7474
filters: Arc::new([]),
7575
failure_policy: Default::default(),
76+
request_timeout: None,
7677
distribution: RouteDistribution::FirstAvailable(Arc::new([RouteBackend {
7778
filters: Arc::new([]),
7879
backend: backend.clone(),
80+
request_timeout: None,
7981
}])),
8082
},
8183
}],
@@ -96,9 +98,11 @@ impl ClientPolicies {
9698
meta: Meta::new_default("default"),
9799
filters: Arc::new([]),
98100
failure_policy: Default::default(),
101+
request_timeout: None,
99102
distribution: RouteDistribution::FirstAvailable(Arc::new([RouteBackend {
100103
filters: Arc::new([]),
101104
backend: backend.clone(),
105+
request_timeout: None,
102106
}])),
103107
}),
104108
},

‎linkerd/http-route/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ tracing = "0.1"
1717
url = "2"
1818

1919
[dependencies.linkerd2-proxy-api]
20-
version = "0.9"
20+
version = "0.10"
2121
features = ["http-route", "grpc-route"]
2222
optional = true
2323

‎linkerd/proxy/api-resolve/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ async-stream = "0.3"
1414
futures = { version = "0.3", default-features = false }
1515
linkerd-addr = { path = "../../addr" }
1616
linkerd-error = { path = "../../error" }
17-
linkerd2-proxy-api = { version = "0.9", features = ["destination"] }
17+
linkerd2-proxy-api = { version = "0.10", features = ["destination"] }
1818
linkerd-proxy-core = { path = "../core" }
1919
linkerd-stack = { path = "../../stack" }
2020
linkerd-tls = { path = "../../tls" }

‎linkerd/proxy/client-policy/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ proto = [
1818
ahash = "0.8"
1919
ipnet = "2"
2020
http = "0.2"
21-
linkerd2-proxy-api = { version = "0.9", optional = true, features = [
21+
linkerd2-proxy-api = { version = "0.10", optional = true, features = [
2222
"outbound",
2323
] }
2424
linkerd-error = { path = "../../error" }

‎linkerd/proxy/client-policy/src/grpc.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ pub fn default(distribution: crate::RouteDistribution<Filter>) -> Route {
3737
filters: Arc::new([]),
3838
distribution,
3939
failure_policy: Codes::default(),
40+
request_timeout: None,
4041
},
4142
}],
4243
}
@@ -101,6 +102,7 @@ pub mod proto {
101102
r#match::host::{proto::InvalidHostMatch, MatchHost},
102103
},
103104
};
105+
use std::time::Duration;
104106

105107
#[derive(Debug, thiserror::Error)]
106108
pub enum InvalidGrpcRoute {
@@ -124,6 +126,9 @@ pub mod proto {
124126

125127
#[error("invalid failure accrual policy: {0}")]
126128
Breaker(#[from] InvalidFailureAccrual),
129+
130+
#[error("invalid duration: {0}")]
131+
Duration(#[from] prost_types::DurationError),
127132
}
128133

129134
#[derive(Debug, thiserror::Error)]
@@ -198,6 +203,7 @@ pub mod proto {
198203
matches,
199204
backends,
200205
filters,
206+
request_timeout,
201207
} = proto;
202208

203209
let matches = matches
@@ -214,13 +220,16 @@ pub mod proto {
214220
.ok_or(InvalidGrpcRoute::Missing("distribution"))?
215221
.try_into()?;
216222

223+
let request_timeout = request_timeout.map(Duration::try_from).transpose()?;
224+
217225
Ok(Rule {
218226
matches,
219227
policy: Policy {
220228
meta: meta.clone(),
221229
filters,
222230
distribution,
223231
failure_policy: Codes::default(),
232+
request_timeout,
224233
},
225234
})
226235
}
@@ -270,10 +279,14 @@ pub mod proto {
270279
impl TryFrom<grpc_route::RouteBackend> for RouteBackend<Filter> {
271280
type Error = InvalidBackend;
272281
fn try_from(
273-
grpc_route::RouteBackend { backend, filters }: grpc_route::RouteBackend,
282+
grpc_route::RouteBackend {
283+
backend,
284+
filters,
285+
request_timeout,
286+
}: grpc_route::RouteBackend,
274287
) -> Result<RouteBackend<Filter>, InvalidBackend> {
275288
let backend = backend.ok_or(InvalidBackend::Missing("backend"))?;
276-
RouteBackend::try_from_proto(backend, filters)
289+
RouteBackend::try_from_proto(backend, filters, request_timeout)
277290
}
278291
}
279292

0 commit comments

Comments
 (0)