Skip to content

Commit

Permalink
Set 60 seconds as the timeout of route (knative#1667)
Browse files Browse the repository at this point in the history
* Set a long default timeout for route

* Only use one timeout DefaultRouteTimeout in Route
  • Loading branch information
Zhimin Xiang committed Aug 2, 2018
1 parent 342cc20 commit 0405fc8
Show file tree
Hide file tree
Showing 8 changed files with 302 additions and 11 deletions.
14 changes: 7 additions & 7 deletions pkg/controller/route/resources/virtual_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const (
IstioTimeoutHackHeaderKey = "x-envoy-upstream-rq-timeout-ms"
IstioTimeoutHackHeaderValue = "0"

DefaultActivatorTimeout = "60s"
DefaultRouteTimeout = "60s"
)

// MakeVirtualService creates an Istio VirtualService to set up routing rules. Such VirtualService specifies
Expand Down Expand Up @@ -142,6 +142,10 @@ func makeVirtualServiceRoute(domains []string, ns string, targets []traffic.Revi
route := v1alpha3.HTTPRoute{
Match: matches,
Route: weights,
Timeout: DefaultRouteTimeout,
AppendHeaders: map[string]string{
IstioTimeoutHackHeaderKey: IstioTimeoutHackHeaderValue,
},
}
// Add traffic rules for activator.
return addActivatorRoutes(&route, ns, inactive)
Expand Down Expand Up @@ -189,12 +193,8 @@ func addActivatorRoutes(r *v1alpha3.HTTPRoute, ns string, inactive []traffic.Rev
},
Weight: totalInactivePercent,
})
r.AppendHeaders = map[string]string{
controller.GetRevisionHeaderName(): maxInactiveTarget.RevisionName,
controller.GetRevisionHeaderNamespace(): ns,
IstioTimeoutHackHeaderKey: IstioTimeoutHackHeaderValue,
}
r.Timeout = DefaultActivatorTimeout
r.AppendHeaders[controller.GetRevisionHeaderName()] = maxInactiveTarget.RevisionName
r.AppendHeaders[controller.GetRevisionHeaderNamespace()] = ns
return r
}

Expand Down
28 changes: 26 additions & 2 deletions pkg/controller/route/resources/virtual_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ func TestMakeVirtualServiceSpec_CorrectRoutes(t *testing.T) {
},
Weight: 100,
}},
Timeout: DefaultRouteTimeout,
AppendHeaders: map[string]string{
IstioTimeoutHackHeaderKey: IstioTimeoutHackHeaderValue,
},
}, {
Match: []v1alpha3.HTTPMatchRequest{{
Authority: &v1alpha3.StringMatch{Exact: "v1.domain.com"},
Expand All @@ -133,6 +137,10 @@ func TestMakeVirtualServiceSpec_CorrectRoutes(t *testing.T) {
},
Weight: 100,
}},
Timeout: DefaultRouteTimeout,
AppendHeaders: map[string]string{
IstioTimeoutHackHeaderKey: IstioTimeoutHackHeaderValue,
},
}}
routes := MakeVirtualService(r, &traffic.TrafficConfig{Targets: targets}).Spec.Http
if diff := cmp.Diff(expected, routes); diff != "" {
Expand Down Expand Up @@ -205,6 +213,10 @@ func TestMakeVirtualServiceRoute_Vanilla(t *testing.T) {
},
Weight: 100,
}},
Timeout: DefaultRouteTimeout,
AppendHeaders: map[string]string{
IstioTimeoutHackHeaderKey: IstioTimeoutHackHeaderValue,
},
}
if diff := cmp.Diff(&expected, route); diff != "" {
t.Errorf("Unexpected route (-want +got): %v", diff)
Expand Down Expand Up @@ -241,6 +253,10 @@ func TestMakeVirtualServiceRoute_ZeroPercentTarget(t *testing.T) {
},
Weight: 100,
}},
Timeout: DefaultRouteTimeout,
AppendHeaders: map[string]string{
IstioTimeoutHackHeaderKey: IstioTimeoutHackHeaderValue,
},
}
if diff := cmp.Diff(&expected, route); diff != "" {
t.Errorf("Unexpected route (-want +got): %v", diff)
Expand Down Expand Up @@ -284,6 +300,10 @@ func TestMakeVirtualServiceRoute_TwoTargets(t *testing.T) {
},
Weight: 10,
}},
Timeout: DefaultRouteTimeout,
AppendHeaders: map[string]string{
IstioTimeoutHackHeaderKey: IstioTimeoutHackHeaderValue,
},
}
if diff := cmp.Diff(&expected, route); diff != "" {
t.Errorf("Unexpected route (-want +got): %v", diff)
Expand Down Expand Up @@ -321,7 +341,7 @@ func TestMakeVirtualServiceRoute_VanillaScaledToZero(t *testing.T) {
"Knative-Serving-Namespace": "test-ns",
IstioTimeoutHackHeaderKey: IstioTimeoutHackHeaderValue,
},
Timeout: DefaultActivatorTimeout,
Timeout: DefaultRouteTimeout,
}
if diff := cmp.Diff(&expected, route); diff != "" {
t.Errorf("Unexpected route (-want +got): %v", diff)
Expand Down Expand Up @@ -364,7 +384,7 @@ func TestMakeVirtualServiceRoute_TwoInactiveTargets(t *testing.T) {
"Knative-Serving-Namespace": "test-ns",
IstioTimeoutHackHeaderKey: IstioTimeoutHackHeaderValue,
},
Timeout: DefaultActivatorTimeout,
Timeout: DefaultRouteTimeout,
}
if diff := cmp.Diff(&expected, route); diff != "" {
t.Errorf("Unexpected route (-want +got): %v", diff)
Expand Down Expand Up @@ -402,6 +422,10 @@ func TestMakeVirtualServiceRoute_ZeroPercentNamedTargetScaledToZero(t *testing.T
},
Weight: 100,
}},
Timeout: DefaultRouteTimeout,
AppendHeaders: map[string]string{
IstioTimeoutHackHeaderKey: IstioTimeoutHackHeaderValue,
},
}
if diff := cmp.Diff(&expected, route); diff != "" {
t.Errorf("Unexpected route (-want +got): %v", diff)
Expand Down
32 changes: 30 additions & 2 deletions pkg/controller/route/route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ func TestCreateRouteForOneReserveRevision(t *testing.T) {
ctrl.GetRevisionHeaderNamespace(): testNamespace,
resources.IstioTimeoutHackHeaderKey: resources.IstioTimeoutHackHeaderValue,
},
Timeout: resources.DefaultActivatorTimeout,
Timeout: resources.DefaultRouteTimeout,
}},
}
if diff := cmp.Diff(expectedSpec, vs.Spec); diff != "" {
Expand Down Expand Up @@ -406,6 +406,10 @@ func TestCreateRouteWithMultipleTargets(t *testing.T) {
},
Weight: 10,
}},
Timeout: resources.DefaultRouteTimeout,
AppendHeaders: map[string]string{
resources.IstioTimeoutHackHeaderKey: resources.IstioTimeoutHackHeaderValue,
},
}},
}
if diff := cmp.Diff(expectedSpec, vs.Spec); diff != "" {
Expand Down Expand Up @@ -493,7 +497,7 @@ func TestCreateRouteWithOneTargetReserve(t *testing.T) {
ctrl.GetRevisionHeaderNamespace(): testNamespace,
resources.IstioTimeoutHackHeaderKey: resources.IstioTimeoutHackHeaderValue,
},
Timeout: resources.DefaultActivatorTimeout,
Timeout: resources.DefaultRouteTimeout,
}},
}
if diff := cmp.Diff(expectedSpec, vs.Spec); diff != "" {
Expand Down Expand Up @@ -595,6 +599,10 @@ func TestCreateRouteWithDuplicateTargets(t *testing.T) {
},
Weight: 50,
}},
Timeout: resources.DefaultRouteTimeout,
AppendHeaders: map[string]string{
resources.IstioTimeoutHackHeaderKey: resources.IstioTimeoutHackHeaderValue,
},
}, {
Match: []v1alpha3.HTTPMatchRequest{{Authority: &v1alpha3.StringMatch{Exact: "test-revision-1." + domain}}},
Route: []v1alpha3.DestinationWeight{{
Expand All @@ -604,6 +612,10 @@ func TestCreateRouteWithDuplicateTargets(t *testing.T) {
},
Weight: 100,
}},
Timeout: resources.DefaultRouteTimeout,
AppendHeaders: map[string]string{
resources.IstioTimeoutHackHeaderKey: resources.IstioTimeoutHackHeaderValue,
},
}, {
Match: []v1alpha3.HTTPMatchRequest{{Authority: &v1alpha3.StringMatch{Exact: "test-revision-2." + domain}}},
Route: []v1alpha3.DestinationWeight{{
Expand All @@ -613,6 +625,10 @@ func TestCreateRouteWithDuplicateTargets(t *testing.T) {
},
Weight: 100,
}},
Timeout: resources.DefaultRouteTimeout,
AppendHeaders: map[string]string{
resources.IstioTimeoutHackHeaderKey: resources.IstioTimeoutHackHeaderValue,
},
}},
}
if diff := cmp.Diff(expectedSpec, vs.Spec); diff != "" {
Expand Down Expand Up @@ -699,6 +715,10 @@ func TestCreateRouteWithNamedTargets(t *testing.T) {
},
Weight: 50,
}},
Timeout: resources.DefaultRouteTimeout,
AppendHeaders: map[string]string{
resources.IstioTimeoutHackHeaderKey: resources.IstioTimeoutHackHeaderValue,
},
}, {
Match: []v1alpha3.HTTPMatchRequest{{Authority: &v1alpha3.StringMatch{Exact: "bar." + domain}}},
Route: []v1alpha3.DestinationWeight{{
Expand All @@ -708,6 +728,10 @@ func TestCreateRouteWithNamedTargets(t *testing.T) {
},
Weight: 100,
}},
Timeout: resources.DefaultRouteTimeout,
AppendHeaders: map[string]string{
resources.IstioTimeoutHackHeaderKey: resources.IstioTimeoutHackHeaderValue,
},
}, {
Match: []v1alpha3.HTTPMatchRequest{{Authority: &v1alpha3.StringMatch{Exact: "foo." + domain}}},
Route: []v1alpha3.DestinationWeight{{
Expand All @@ -717,6 +741,10 @@ func TestCreateRouteWithNamedTargets(t *testing.T) {
},
Weight: 100,
}},
Timeout: resources.DefaultRouteTimeout,
AppendHeaders: map[string]string{
resources.IstioTimeoutHackHeaderKey: resources.IstioTimeoutHackHeaderValue,
},
}},
}
if diff := cmp.Diff(expectedSpec, vs.Spec); diff != "" {
Expand Down
20 changes: 20 additions & 0 deletions test/crd.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,26 @@ func Configuration(namespace string, names ResourceNames, imagePath string) *v1a
}
}

func ConfigurationWithEnvVars(namespace string, names ResourceNames, imagePath string, envVars []corev1.EnvVar) *v1alpha1.Configuration {

return &v1alpha1.Configuration{
ObjectMeta: metav1.ObjectMeta{
Namespace: namespace,
Name: names.Config,
},
Spec: v1alpha1.ConfigurationSpec{
RevisionTemplate: v1alpha1.RevisionTemplateSpec{
Spec: v1alpha1.RevisionSpec{
Container: corev1.Container{
Image: imagePath,
Env: envVars,
},
},
},
},
}
}

func ConfigurationWithBuild(namespace string, names ResourceNames, build *buildv1alpha1.BuildSpec, imagePath string) *v1alpha1.Configuration {
return &v1alpha1.Configuration{
ObjectMeta: metav1.ObjectMeta{
Expand Down
16 changes: 16 additions & 0 deletions test/e2e/e2e.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"go.uber.org/zap"

corev1 "k8s.io/api/core/v1"
// Mysteriously required to support GCP auth (required by k8s libs).
// Apparently just importing it is enough. @_@ side effects @_@.
// https://github.com/kubernetes/client-go/issues/242
Expand Down Expand Up @@ -72,3 +73,18 @@ func CreateRouteAndConfig(clients *test.Clients, logger *zap.SugaredLogger, imag
test.Route(test.Flags.Namespace, names))
return names, err
}

func CreateRouteAndConfigWithEnv(clients *test.Clients, logger *zap.SugaredLogger, imagePath string, envVars []corev1.EnvVar) (test.ResourceNames, error) {
var names test.ResourceNames
names.Config = test.AppendRandomString(configName, logger)
names.Route = test.AppendRandomString(routeName, logger)

_, err := clients.Configs.Create(
test.ConfigurationWithEnvVars(test.Flags.Namespace, names, imagePath, envVars))
if err != nil {
return test.ResourceNames{}, err
}
_, err = clients.Routes.Create(
test.Route(test.Flags.Namespace, names))
return names, err
}
126 changes: 126 additions & 0 deletions test/e2e/service_to_service_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
// +build e2e

/*
Copyright 2018 The Knative Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package e2e

import (
"fmt"
"net/http"
"strings"
"testing"

"github.com/knative/serving/test"
"github.com/knative/serving/test/spoof"

"go.uber.org/zap"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
corev1 "k8s.io/api/core/v1"
)

var (
clients *test.Clients
logger *zap.SugaredLogger
)
const (
targetHostEnv = "TARGET_HOST"
helloworldResponse = "Hello World! How about some tasty noodles?"
)

func createTargetHostEnvVars(routeName string, t *testing.T) []corev1.EnvVar {
helloWorldRoute, err := clients.Routes.Get(routeName, metav1.GetOptions{})
if err != nil {
t.Fatalf("Failed to get Route of helloworld app: %v", err)
}
internalDomain := helloWorldRoute.Status.DomainInternal
logger.Infof("helloworld internal domain is %v.", internalDomain)
return []corev1.EnvVar{{
Name: targetHostEnv,
Value: internalDomain,
}}
}

func sendRequest(resolvableDomain bool, domain string) (*spoof.Response, error) {
logger.Infof("The domain of request is %s.", domain)
client, err := spoof.New(clients.Kube, logger, domain, resolvableDomain)
if err != nil {
return nil, err
}

req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("http://%s", domain), nil)
if err != nil {
return nil, err
}
return client.Do(req)
}

// In this test, we set up two apps: helloworld and httpproxy.
// helloworld is a simple app that displays a plaintext string.
// httpproxy is a proxy that redirects request to internal service of helloworld app
// with FQDN {route}.{namespace}.svc.cluster.local.
// The expected result is that the request sent to httpproxy app is successfully redirected
// to helloworld app.
func TestServiceToServiceCall(t *testing.T) {
logger = test.GetContextLogger("TestServiceToServiceCall")
clients = Setup(t)

// Set up helloworld app.
helloWorldImagePath := strings.Join([]string{test.Flags.DockerRepo, "helloworld"}, "/")
logger.Infof("Creating a Route and Configuration for helloworld test app.")
helloWorldNames, err := CreateRouteAndConfig(clients, logger, helloWorldImagePath)
if err != nil {
t.Fatalf("Failed to create Route and Configuration: %v", err)
}
test.CleanupOnInterrupt(func() { TearDown(clients, helloWorldNames, logger) }, logger)
defer TearDown(clients, helloWorldNames, logger)
if err := test.WaitForRouteState(clients.Routes, helloWorldNames.Route, test.IsRouteReady, "RouteIsReady"); err != nil {
t.Fatalf("The Route %s was not marked as Ready to serve traffic: %v", helloWorldNames.Route, err)
}

// Set up httpproxy app.
httpProxyImagePath := strings.Join([]string{test.Flags.DockerRepo, "httpproxy"}, "/")
logger.Infof("Creating a Route and Configuration for httpproxy test app.")
envVars := createTargetHostEnvVars(helloWorldNames.Route, t)
httpProxyNames, err := CreateRouteAndConfigWithEnv(clients, logger, httpProxyImagePath, envVars)
if err != nil {
t.Fatalf("Failed to create Route and Configuration: %v", err)
}
test.CleanupOnInterrupt(func() { TearDown(clients, httpProxyNames, logger) }, logger)
defer TearDown(clients, httpProxyNames, logger)
if err := test.WaitForRouteState(clients.Routes, httpProxyNames.Route, test.IsRouteReady, "RouteIsReady"); err != nil {
t.Fatalf("The Route %s was not marked as Ready to serve traffic: %v", httpProxyNames.Route, err)
}
httpProxyRoute, err := clients.Routes.Get(httpProxyNames.Route, metav1.GetOptions{})
if err != nil {
t.Fatalf("Failed to get Route %s: %v", httpProxyNames.Route, err)
}
if err = test.WaitForEndpointState(clients.Kube, logger, test.Flags.ResolvableDomain, httpProxyRoute.Status.Domain, test.MatchesAny, "HttpProxy"); err != nil {
t.Fatalf("Failed to start endpoint of httpproxy: %v", err)
}
logger.Info("httpproxy is ready.")


// Send request to httpproxy to trigger the http call from httpproxy Pod to internal service of helloworld app.
response, err := sendRequest(test.Flags.ResolvableDomain, httpProxyRoute.Status.Domain)
if err != nil {
t.Fatalf("Failed to send request to httpproxy. %v", err)
}
// We expect the response from httpproxy is equal to the response from htlloworld
if helloworldResponse != strings.TrimSpace(string(response.Body)) {
t.Fatalf("The httpproxy response %s is not equal to helloworld response %s.", string(response.Body), helloworldResponse)
}
}
Loading

0 comments on commit 0405fc8

Please sign in to comment.