From e78af75d6d7ab9c878b50008df154558d18d51e5 Mon Sep 17 00:00:00 2001 From: Khurram Baig Date: Mon, 21 Sep 2020 11:57:20 +0530 Subject: [PATCH] Add Timeout via Config for EventListener Server Added Timeout to avoid running out of file descriptor and leaking connections. These timeouts are configurable via config. --- cmd/eventlistenersink/main.go | 9 ++++--- config/controller.yaml | 4 +++ pkg/interceptors/bitbucket/bitbucket_test.go | 1 + pkg/interceptors/cel/cel_test.go | 3 ++- pkg/interceptors/github/github_test.go | 1 + pkg/interceptors/gitlab/gitlab_test.go | 1 + .../v1alpha1/eventlistener/eventlistener.go | 16 +++++++++++ .../eventlistener/eventlistener_test.go | 4 +++ pkg/sink/initialization.go | 27 ++++++++++++++++--- 9 files changed, 59 insertions(+), 7 deletions(-) diff --git a/cmd/eventlistenersink/main.go b/cmd/eventlistenersink/main.go index 0a9b22dfc5..23e12e0830 100644 --- a/cmd/eventlistenersink/main.go +++ b/cmd/eventlistenersink/main.go @@ -107,9 +107,12 @@ func main() { }) srv := &http.Server{ - Addr: fmt.Sprintf(":%s", sinkArgs.Port), - IdleTimeout: 120 * time.Second, - Handler: mux, + Addr: fmt.Sprintf(":%s", sinkArgs.Port), + ReadTimeout: sinkArgs.ELReadTimeOut * time.Second, + WriteTimeout: sinkArgs.ELWriteTimeOut * time.Second, + IdleTimeout: sinkArgs.ELIdleTimeOut * time.Second, + Handler: http.TimeoutHandler(mux, + sinkArgs.ELTimeOutHandler*time.Second, "EventListener Timeout!\n"), } if err := srv.ListenAndServe(); err != nil { diff --git a/config/controller.yaml b/config/controller.yaml index d496df19eb..085a03a3af 100644 --- a/config/controller.yaml +++ b/config/controller.yaml @@ -57,6 +57,10 @@ spec: "-stderrthreshold", "INFO", "-el-image", "ko://github.com/tektoncd/triggers/cmd/eventlistenersink", "-el-port", "8080", + "-el-readtimeout", "5", + "-el-writetimeout", "40", + "-el-idletimeout", "120", + "-el-timeouthandler", "30", "-period-seconds", "10", "-failure-threshold", "1" ] diff --git a/pkg/interceptors/bitbucket/bitbucket_test.go b/pkg/interceptors/bitbucket/bitbucket_test.go index 749ecd8a2f..52d64980ac 100644 --- a/pkg/interceptors/bitbucket/bitbucket_test.go +++ b/pkg/interceptors/bitbucket/bitbucket_test.go @@ -24,6 +24,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/interceptors/cel/cel_test.go b/pkg/interceptors/cel/cel_test.go index 0116a0799b..c127d98e43 100644 --- a/pkg/interceptors/cel/cel_test.go +++ b/pkg/interceptors/cel/cel_test.go @@ -31,12 +31,13 @@ import ( "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" "github.com/google/go-cmp/cmp" - triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" fakekubeclient "knative.dev/pkg/client/injection/kube/client/fake" "knative.dev/pkg/logging" rtesting "knative.dev/pkg/reconciler/testing" + + triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1" ) const testNS = "testing-ns" diff --git a/pkg/interceptors/github/github_test.go b/pkg/interceptors/github/github_test.go index b195cad354..3d454f1fc7 100644 --- a/pkg/interceptors/github/github_test.go +++ b/pkg/interceptors/github/github_test.go @@ -24,6 +24,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/interceptors/gitlab/gitlab_test.go b/pkg/interceptors/gitlab/gitlab_test.go index eadc6c9fb7..17a8d3a158 100644 --- a/pkg/interceptors/gitlab/gitlab_test.go +++ b/pkg/interceptors/gitlab/gitlab_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/reconciler/v1alpha1/eventlistener/eventlistener.go b/pkg/reconciler/v1alpha1/eventlistener/eventlistener.go index bdfbdc3a44..6d0bb50a80 100644 --- a/pkg/reconciler/v1alpha1/eventlistener/eventlistener.go +++ b/pkg/reconciler/v1alpha1/eventlistener/eventlistener.go @@ -67,6 +67,18 @@ var ( // ElPort defines the port for the EventListener to listen on ElPort = flag.Int("el-port", 8080, "The container port for the EventListener to listen on.") + // ELReadTimeOut defines the read timeout for EventListener Server + ELReadTimeOut = flag.Int64("el-readtimeout", 5, + "The read timeout for EventListener Server.") + // ELWriteTimeOut defines the write timeout for EventListener Server + ELWriteTimeOut = flag.Int64("el-writetimeout", 40, + "The write timeout for EventListener Server.") + // ELIdleTimeOut defines the read timeout for EventListener Server + ELIdleTimeOut = flag.Int64("el-idletimeout", 30, + "The idle timeout for EventListener Server.") + // ELTimeOutHandler defines the timeout for Timeout Handler of EventListener Server + ELTimeOutHandler = flag.Int64("el-timeouthandler", 5, + "The timeout for Timeout Handler of EventListener Server.") // PeriodSeconds defines Period Seconds for the EventListener Liveness and Readiness Probes PeriodSeconds = flag.Int("period-seconds", 10, "The Period Seconds for the EventListener Liveness and Readiness Probes.") @@ -293,6 +305,10 @@ func (r *Reconciler) reconcileDeployment(logger *zap.SugaredLogger, el *v1alpha1 "-el-name", el.Name, "-el-namespace", el.Namespace, "-port", strconv.Itoa(*ElPort), + "-readtimeout", strconv.FormatInt(*ELReadTimeOut, 10), + "-writetimeout", strconv.FormatInt(*ELWriteTimeOut, 10), + "-idletimeout", strconv.FormatInt(*ELIdleTimeOut, 10), + "-timeouthandler", strconv.FormatInt(*ELTimeOutHandler, 10), }, VolumeMounts: []corev1.VolumeMount{{ Name: "config-logging", diff --git a/pkg/reconciler/v1alpha1/eventlistener/eventlistener_test.go b/pkg/reconciler/v1alpha1/eventlistener/eventlistener_test.go index c411a0513e..4a102ed5c8 100644 --- a/pkg/reconciler/v1alpha1/eventlistener/eventlistener_test.go +++ b/pkg/reconciler/v1alpha1/eventlistener/eventlistener_test.go @@ -197,6 +197,10 @@ func makeDeployment(ops ...func(d *appsv1.Deployment)) *appsv1.Deployment { "-el-name", eventListenerName, "-el-namespace", namespace, "-port", strconv.Itoa(*ElPort), + "readtimeout", strconv.FormatInt(*ELReadTimeOut, 10), + "writetimeout", strconv.FormatInt(*ELWriteTimeOut, 10), + "idletimeout", strconv.FormatInt(*ELIdleTimeOut, 10), + "timeouthandler", strconv.FormatInt(*ELTimeOutHandler, 10), }, VolumeMounts: []corev1.VolumeMount{{ Name: "config-logging", diff --git a/pkg/sink/initialization.go b/pkg/sink/initialization.go index 8454018772..cd85366275 100644 --- a/pkg/sink/initialization.go +++ b/pkg/sink/initialization.go @@ -18,6 +18,7 @@ package sink import ( "flag" + "time" triggersclientset "github.com/tektoncd/triggers/pkg/client/clientset/versioned" "golang.org/x/xerrors" @@ -41,6 +42,14 @@ var ( "The namespace of the EventListener resource for this sink.") portFlag = flag.String("port", "", "The port for the EventListener sink to listen on.") + elReadTimeOut = flag.Int64("readtimeout", 5, + "The read timeout for EventListener Server.") + elWriteTimeOut = flag.Int64("writetimeout", 40, + "The write timeout for EventListener Server.") + elIdleTimeOut = flag.Int64("idletimeout", 30, + "The idle timeout for EventListener Server.") + elTimeOutHandler = flag.Int64("timeouthandler", 5, + "The timeout for Timeout Handler of EventListener Server.") ) // Args define the arguments for Sink. @@ -51,6 +60,14 @@ type Args struct { ElNamespace string // Port is the port the Sink should listen on. Port string + // ELReadTimeOut defines the read timeout for EventListener Server + ELReadTimeOut time.Duration + // ELWriteTimeOut defines the write timeout for EventListener Server + ELWriteTimeOut time.Duration + // ELIdleTimeOut defines the read timeout for EventListener Server + ELIdleTimeOut time.Duration + // ELTimeOutHandler defines the timeout for Timeout Handler of EventListener Server + ELTimeOutHandler time.Duration } // Clients define the set of client dependencies Sink requires. @@ -73,9 +90,13 @@ func GetArgs() (Args, error) { return Args{}, xerrors.Errorf("-%s arg not found", port) } return Args{ - ElName: *nameFlag, - ElNamespace: *namespaceFlag, - Port: *portFlag, + ElName: *nameFlag, + ElNamespace: *namespaceFlag, + Port: *portFlag, + ELReadTimeOut: time.Duration(*elReadTimeOut), + ELWriteTimeOut: time.Duration(*elWriteTimeOut), + ELIdleTimeOut: time.Duration(*elIdleTimeOut), + ELTimeOutHandler: time.Duration(*elTimeOutHandler), }, nil }