diff --git a/cmd/activator/main.go b/cmd/activator/main.go index b4bf5a36d69c..36ae25c214b4 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -19,6 +19,7 @@ import ( "net/http" "net/http/httputil" "net/url" + "time" "github.com/golang/glog" "github.com/knative/serving/pkg/activator" @@ -29,10 +30,39 @@ import ( "k8s.io/client-go/rest" ) +const ( + maxRetry = 60 + retryInterval = 1 * time.Second +) + type activationHandler struct { act activator.Activator } +// retryRoundTripper retries on 503's for up to 60 seconds. The reason is there is +// a small delay for k8s to include the ready IP in service. +// https://github.com/knative/serving/issues/660#issuecomment-384062553 +type retryRoundTripper struct{} + +func (rrt retryRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) { + transport := http.DefaultTransport + resp, err := transport.RoundTrip(r) + // TODO: Activator should retry with backoff. + // https://github.com/knative/serving/issues/1229 + i := 1 + for ; i < maxRetry; i++ { + if err == nil && resp != nil && resp.StatusCode != 503 { + break + } + resp.Body.Close() + time.Sleep(retryInterval) + resp, err = transport.RoundTrip(r) + } + // TODO: add metrics for number of tries and the response code. + glog.Infof("It took %d tries to get response code %d", i, resp.StatusCode) + return resp, nil +} + func (a *activationHandler) handler(w http.ResponseWriter, r *http.Request) { namespace := r.Header.Get(controller.GetRevisionHeaderNamespace()) name := r.Header.Get(controller.GetRevisionHeaderName()) @@ -48,6 +78,7 @@ func (a *activationHandler) handler(w http.ResponseWriter, r *http.Request) { Host: fmt.Sprintf("%s:%d", endpoint.FQDN, endpoint.Port), } proxy := httputil.NewSingleHostReverseProxy(target) + proxy.Transport = retryRoundTripper{} // TODO: Clear the host to avoid 404's. // https://github.com/elafros/elafros/issues/964 r.Host = "" diff --git a/pkg/activator/revision.go b/pkg/activator/revision.go index c45107f216be..2a56bd1e1744 100644 --- a/pkg/activator/revision.go +++ b/pkg/activator/revision.go @@ -105,13 +105,6 @@ func (r *revisionActivator) ActiveEndpoint(namespace, name string) (end Endpoint } else { log.Printf("Revision %s/%s is ready", rev.namespace, rev.name) } - // After a pod goes ready, there is a poll loop to publish that fact, then there are - // controllers that wake up to propagate the info to each node which configures iptables on - // a max frequency loop, so it's always possible that there's a small delay. - // The delay should be O(seconds) max, most of the time. - // TODO: rely on readinessProbe instead of a hard-coded sleep. - // https://github.com/elafros/elafros/issues/974 - time.Sleep(2 * time.Second) break RevisionReady } else { return internalError("Unexpected result type for revision %s/%s: %v", rev.namespace, rev.name, event) diff --git a/pkg/apis/istio/v1alpha2/routerule_types.go b/pkg/apis/istio/v1alpha2/routerule_types.go index 2a9f339c993a..48bc181bb823 100644 --- a/pkg/apis/istio/v1alpha2/routerule_types.go +++ b/pkg/apis/istio/v1alpha2/routerule_types.go @@ -87,7 +87,7 @@ type RouteRuleSpec struct { Destination IstioService `json:"destination"` Match Match `json:"match,omitempty"` Route []DestinationWeight `json:"route"` - AppendHeaders map[string]string `json:"appendHeaders"` + AppendHeaders map[string]string `json:"appendHeaders,omitempty"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go index cb4256eb2b70..a8fbe520b5e2 100644 --- a/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go @@ -57,6 +57,7 @@ func (in *Configuration) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ConfigurationCondition) DeepCopyInto(out *ConfigurationCondition) { *out = *in + in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) return } @@ -136,7 +137,9 @@ func (in *ConfigurationStatus) DeepCopyInto(out *ConfigurationStatus) { if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]ConfigurationCondition, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } @@ -338,6 +341,7 @@ func (in *Route) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RouteCondition) DeepCopyInto(out *RouteCondition) { *out = *in + in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) return } @@ -417,7 +421,9 @@ func (in *RouteStatus) DeepCopyInto(out *RouteStatus) { if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]RouteCondition, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } @@ -481,6 +487,7 @@ func (in *Service) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ServiceCondition) DeepCopyInto(out *ServiceCondition) { *out = *in + in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) return } @@ -568,7 +575,9 @@ func (in *ServiceStatus) DeepCopyInto(out *ServiceStatus) { if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]ServiceCondition, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } diff --git a/pkg/controller/route/istio_route.go b/pkg/controller/route/istio_route.go index 783c9915f581..12f292d5ee7f 100644 --- a/pkg/controller/route/istio_route.go +++ b/pkg/controller/route/istio_route.go @@ -20,12 +20,14 @@ import ( "fmt" "regexp" - "github.com/knative/serving/pkg/apis/serving/v1alpha1" istiov1alpha2 "github.com/knative/serving/pkg/apis/istio/v1alpha2" + "github.com/knative/serving/pkg/apis/serving/v1alpha1" "github.com/knative/serving/pkg/controller" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const requestTimeoutMs = "60000" + // makeIstioRouteSpec creates an Istio route func makeIstioRouteSpec(u *v1alpha1.Route, tt *v1alpha1.TrafficTarget, ns string, routes []RevisionRoute, domain string, inactiveRev string) istiov1alpha2.RouteRuleSpec { destinationWeights := calculateDestinationWeights(u, tt, routes) @@ -60,6 +62,9 @@ func makeIstioRouteSpec(u *v1alpha1.Route, tt *v1alpha1.TrafficTarget, ns string appendHeaders := make(map[string]string) appendHeaders[controller.GetRevisionHeaderName()] = inactiveRev appendHeaders[controller.GetRevisionHeaderNamespace()] = u.Namespace + // Set the Envoy upstream timeout to be 60 seconds, in case the revision needs longer time to come up + // https://www.envoyproxy.io/docs/envoy/v1.5.0/configuration/http_filters/router_filter#x-envoy-upstream-rq-timeout-ms + appendHeaders["x-envoy-upstream-rq-timeout-ms"] = requestTimeoutMs spec.AppendHeaders = appendHeaders } return spec diff --git a/pkg/controller/route/istio_route_test.go b/pkg/controller/route/istio_route_test.go index a8269bae67a5..ef7a6f7f0005 100644 --- a/pkg/controller/route/istio_route_test.go +++ b/pkg/controller/route/istio_route_test.go @@ -17,10 +17,10 @@ import ( "regexp" "testing" - "github.com/knative/serving/pkg/apis/serving/v1alpha1" + "github.com/google/go-cmp/cmp" istiov1alpha2 "github.com/knative/serving/pkg/apis/istio/v1alpha2" + "github.com/knative/serving/pkg/apis/serving/v1alpha1" "github.com/knative/serving/pkg/controller" - "github.com/google/go-cmp/cmp" ) const ( @@ -77,6 +77,7 @@ func TestMakeIstioRouteSpecRevisionInactive(t *testing.T) { appendHeaders := make(map[string]string) appendHeaders[controller.GetRevisionHeaderName()] = testInactiveRev appendHeaders[controller.GetRevisionHeaderNamespace()] = testNamespace + appendHeaders["x-envoy-upstream-rq-timeout-ms"] = requestTimeoutMs expectedIstioRouteSpec := istiov1alpha2.RouteRuleSpec{ Destination: istiov1alpha2.IstioService{ Name: "test-route-service", diff --git a/pkg/controller/route/route_test.go b/pkg/controller/route/route_test.go index b825554c3307..fca631ff9d15 100644 --- a/pkg/controller/route/route_test.go +++ b/pkg/controller/route/route_test.go @@ -412,6 +412,7 @@ func TestCreateRouteForOneReserveRevision(t *testing.T) { appendHeaders := make(map[string]string) appendHeaders[ctrl.GetRevisionHeaderName()] = "test-rev" appendHeaders[ctrl.GetRevisionHeaderNamespace()] = testNamespace + appendHeaders["x-envoy-upstream-rq-timeout-ms"] = requestTimeoutMs expectedRouteSpec := v1alpha2.RouteRuleSpec{ Destination: v1alpha2.IstioService{ Name: "test-route-service", @@ -509,14 +510,13 @@ func TestCreateRouteFromConfigsWithMultipleRevs(t *testing.T) { Namespace: testNamespace, }, Weight: 100, - }, getActivatorDestinationWeight(0), - { - Destination: v1alpha2.IstioService{ - Name: fmt.Sprintf("%s-service", otherRev.Name), - Namespace: testNamespace, - }, - Weight: 0, - }}, + }, getActivatorDestinationWeight(0), { + Destination: v1alpha2.IstioService{ + Name: fmt.Sprintf("%s-service", otherRev.Name), + Namespace: testNamespace, + }, + Weight: 0, + }}, } if diff := cmp.Diff(expectedRouteSpec, routerule.Spec); diff != "" { @@ -646,6 +646,7 @@ func TestCreateRouteWithOneTargetReserve(t *testing.T) { appendHeaders := make(map[string]string) appendHeaders[ctrl.GetRevisionHeaderName()] = "test-rev" appendHeaders[ctrl.GetRevisionHeaderNamespace()] = testNamespace + appendHeaders["x-envoy-upstream-rq-timeout-ms"] = requestTimeoutMs expectedRouteSpec := v1alpha2.RouteRuleSpec{ Destination: v1alpha2.IstioService{ Name: fmt.Sprintf("%s-service", route.Name),