Skip to content

Commit b5d6661

Browse files
authored
fix(policy): respect timeout field on gateway API HTTPRoutes (#14750)
Fixes #14713 The [Linkerd documentation](https://linkerd.io/2-edge/reference/timeouts/#configuring-timeouts) specifies that: > If the [request timeout](https://gateway-api.sigs.k8s.io/api-types/httproute/#timeouts-optional) field is set on an HTTPRoute resource, it will be used as the timeout.linkerd.io/request timeout. However, if both the field and the annotation are specified, the annotation will take priority. This is true for policy.linkerd.io/HTTPRoute resources but not for `gateway.networking.k8s.io/HTTPRoute` resources. We update the policy controller to consider the request timeout field on `gateway.networking.k8s.io/HTTPRoute` and add API tests. Signed-off-by: Alex Leong <[email protected]>
1 parent eae8566 commit b5d6661

File tree

3 files changed

+166
-2
lines changed

3 files changed

+166
-2
lines changed

policy-controller/k8s/index/src/outbound/index/http.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ fn convert_gateway_rule(
159159
rule: gateway::HTTPRouteRules,
160160
cluster: &ClusterInfo,
161161
resource_info: &HashMap<ResourceRef, ResourceInfo>,
162-
timeouts: RouteTimeouts,
162+
mut timeouts: RouteTimeouts,
163163
retry: Option<RouteRetry<HttpRetryCondition>>,
164164
) -> Result<OutboundRouteRule<HttpRouteMatch, HttpRetryCondition>> {
165165
let matches = rule
@@ -183,6 +183,19 @@ fn convert_gateway_rule(
183183
.map(convert_gateway_filter)
184184
.collect::<Result<_>>()?;
185185

186+
timeouts.request = timeouts.request.or_else(|| {
187+
rule.timeouts.as_ref().and_then(|timeouts| {
188+
let timeout = parse_duration(timeouts.request.as_ref()?).ok()?;
189+
190+
// zero means "no timeout", per GEP-1742
191+
if timeout == time::Duration::ZERO {
192+
return None;
193+
}
194+
195+
Some(timeout)
196+
})
197+
});
198+
186199
Ok(OutboundRouteRule {
187200
matches,
188201
backends,

policy-test/tests/outbound_api_http.rs

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::time::Duration;
2+
13
use futures::StreamExt;
24
use linkerd2_proxy_api::{self as api, outbound};
35
use linkerd_policy_controller_k8s_api::{self as k8s, gateway, policy};
@@ -701,6 +703,155 @@ async fn http_route_retries_and_timeouts() {
701703
test::<policy::EgressNetwork, policy::HttpRoute>().await;
702704
}
703705

706+
#[tokio::test(flavor = "current_thread")]
707+
async fn http_route_linkerd_timeouts() {
708+
async fn test<P: TestParent>() {
709+
tracing::debug!(
710+
parent = %P::kind(&P::DynamicType::default()),
711+
);
712+
with_temp_ns(|client, ns| async move {
713+
// Create a parent
714+
let parent = create(&client, P::make_parent(&ns)).await;
715+
let port = 4191;
716+
// Create a backend
717+
let backend_port = 8888;
718+
let backend = match P::make_backend(&ns) {
719+
Some(b) => create(&client, b).await,
720+
None => parent.clone(),
721+
};
722+
723+
let route = policy::HttpRoute {
724+
metadata: k8s::ObjectMeta {
725+
namespace: Some(ns.to_string()),
726+
name: Some("foo-route".to_string()),
727+
..Default::default()
728+
},
729+
spec: policy::HttpRouteSpec {
730+
parent_refs: Some(vec![parent.obj_ref()]),
731+
hostnames: None,
732+
rules: Some(vec![policy::httproute::HttpRouteRule {
733+
matches: Some(vec![]),
734+
filters: None,
735+
timeouts: Some(policy::httproute::HttpRouteTimeouts {
736+
request: Some(Duration::from_secs(5).into()),
737+
backend_request: None,
738+
}),
739+
backend_refs: Some(vec![backend.backend_ref(backend_port)]),
740+
}]),
741+
},
742+
status: None,
743+
};
744+
745+
let route = create(&client, route).await;
746+
await_route_accepted(&client, &route).await;
747+
748+
let mut rx = retry_watch_outbound_policy(&client, &ns, parent.ip(), port).await;
749+
let config = rx
750+
.next()
751+
.await
752+
.expect("watch must not fail")
753+
.expect("watch must return an initial config");
754+
tracing::trace!(?config);
755+
assert_resource_meta(&config.metadata, parent.obj_ref(), port);
756+
757+
policy::HttpRoute::routes(&config, |routes| {
758+
let outbound_route = routes.first().expect("route must exist");
759+
assert!(route.meta_eq(policy::HttpRoute::extract_meta(outbound_route)));
760+
let rule = assert_singleton(&outbound_route.rules);
761+
let timeout = rule
762+
.timeouts
763+
.as_ref()
764+
.expect("timeouts expected")
765+
.request
766+
.as_ref()
767+
.expect("request timeout expected");
768+
assert_eq!(timeout.seconds, 5);
769+
});
770+
})
771+
.await;
772+
}
773+
774+
test::<k8s::Service>().await;
775+
test::<policy::EgressNetwork>().await;
776+
}
777+
778+
// The timeout field on HTTPRoute is only available in Gateway API v1.2+.
779+
#[cfg(feature = "gateway-api-experimental")]
780+
#[tokio::test(flavor = "current_thread")]
781+
async fn http_route_gateway_timeouts() {
782+
async fn test<P: TestParent>() {
783+
tracing::debug!(
784+
parent = %P::kind(&P::DynamicType::default()),
785+
);
786+
with_temp_ns(|client, ns| async move {
787+
// Create a parent
788+
let parent = create(&client, P::make_parent(&ns)).await;
789+
let port = 4191;
790+
// Create a backend
791+
let backend_port = 8888;
792+
let backend = match P::make_backend(&ns) {
793+
Some(b) => create(&client, b).await,
794+
None => parent.clone(),
795+
};
796+
797+
let route = gateway::HTTPRoute {
798+
metadata: k8s::ObjectMeta {
799+
namespace: Some(ns.to_string()),
800+
name: Some("foo-route".to_string()),
801+
..Default::default()
802+
},
803+
spec: gateway::HTTPRouteSpec {
804+
parent_refs: Some(vec![parent.obj_ref()]),
805+
hostnames: None,
806+
rules: Some(vec![gateway::HTTPRouteRules {
807+
name: None,
808+
matches: Some(vec![]),
809+
filters: None,
810+
timeouts: Some(gateway::HTTPRouteRulesTimeouts {
811+
request: Some("5s".to_string()),
812+
backend_request: None,
813+
}),
814+
retry: None,
815+
backend_refs: Some(vec![backend.backend_ref(backend_port)]),
816+
session_persistence: None,
817+
}]),
818+
},
819+
status: None,
820+
};
821+
822+
let route = create(&client, route).await;
823+
await_route_accepted(&client, &route).await;
824+
825+
let mut rx = retry_watch_outbound_policy(&client, &ns, parent.ip(), port).await;
826+
let config = rx
827+
.next()
828+
.await
829+
.expect("watch must not fail")
830+
.expect("watch must return an initial config");
831+
tracing::trace!(?config);
832+
assert_resource_meta(&config.metadata, parent.obj_ref(), port);
833+
834+
gateway::HTTPRoute::routes(&config, |routes| {
835+
let outbound_route = routes.first().expect("route must exist");
836+
assert!(route.meta_eq(gateway::HTTPRoute::extract_meta(outbound_route)));
837+
let rule = assert_singleton(&outbound_route.rules);
838+
let timeout = rule
839+
.timeouts
840+
.as_ref()
841+
.expect("timeouts expected")
842+
.request
843+
.as_ref()
844+
.expect("request timeout expected");
845+
assert_eq!(timeout.seconds, 5);
846+
});
847+
})
848+
.await;
849+
}
850+
851+
test::<k8s::Service>().await;
852+
test::<policy::EgressNetwork>().await;
853+
}
854+
704855
#[tokio::test(flavor = "current_thread")]
705856
async fn parent_retries_and_timeouts() {
706857
async fn test<P: TestParent, R: TestRoute<Route = outbound::HttpRoute>>() {

testutil/test_helper.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
corev1 "k8s.io/api/core/v1"
2323
)
2424

25-
const GATEWAY_API_VERSION = "v1.1.1"
25+
const GATEWAY_API_VERSION = "v1.2.0"
2626

2727
// TestHelper provides helpers for running the linkerd integration tests.
2828
type TestHelper struct {

0 commit comments

Comments
 (0)