From 22872b7197563ee79e165adb8ca42d470c3b9e1e Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Thu, 12 May 2022 10:48:10 +0200 Subject: [PATCH 1/6] feat: Added option for configuring number of retries for http event sender, as well as additional logging Signed-off-by: Florian Bacher --- pkg/lib/v0_2_0/events.go | 21 ++++++++-- pkg/lib/v0_2_0/events_test.go | 74 ++++++++++++++++++----------------- 2 files changed, 57 insertions(+), 38 deletions(-) diff --git a/pkg/lib/v0_2_0/events.go b/pkg/lib/v0_2_0/events.go index 9f879b7f..abd6c131 100644 --- a/pkg/lib/v0_2_0/events.go +++ b/pkg/lib/v0_2_0/events.go @@ -41,16 +41,26 @@ const keptnSpecVersionCEExtension = "shkeptnspecversion" const triggeredIDCEExtension = "triggeredid" const keptnGitCommitIDCEExtension = "gitcommitid" +type HTTPSenderOption func(httpSender *HTTPEventSender) + +func WithSendRetries(retries int) HTTPSenderOption { + return func(httpSender *HTTPEventSender) { + httpSender.nrRetries = retries + } +} + // HTTPEventSender sends CloudEvents via HTTP type HTTPEventSender struct { // EventsEndpoint is the http endpoint the events are sent to EventsEndpoint string // Client is an implementation of the cloudevents.Client interface Client cloudevents.Client + // nrRetries is the number of retries that are attempted if the endpoint an event is forwarded to returns an http code outside the 2xx range + nrRetries int } // NewHTTPEventSender creates a new HTTPSender -func NewHTTPEventSender(endpoint string) (*HTTPEventSender, error) { +func NewHTTPEventSender(endpoint string, opts ...HTTPSenderOption) (*HTTPEventSender, error) { if endpoint == "" { endpoint = DefaultHTTPEventEndpoint } @@ -75,6 +85,11 @@ func NewHTTPEventSender(endpoint string) (*HTTPEventSender, error) { httpSender := &HTTPEventSender{ EventsEndpoint: endpoint, Client: c, + nrRetries: MAX_SEND_RETRIES, + } + + for _, o := range opts { + o(httpSender) } return httpSender, nil } @@ -88,7 +103,7 @@ func (httpSender HTTPEventSender) Send(ctx context.Context, event cloudevents.Ev ctx = cloudevents.ContextWithTarget(ctx, httpSender.EventsEndpoint) ctx = cloudevents.WithEncodingStructured(ctx) var result protocol.Result - for i := 0; i <= MAX_SEND_RETRIES; i++ { + for i := 0; i <= httpSender.nrRetries; i++ { result = httpSender.Client.Send(ctx, event) httpResult, ok := result.(*httpprotocol.Result) switch { @@ -103,7 +118,7 @@ func (httpSender HTTPEventSender) Send(ctx context.Context, event cloudevents.Ev return nil } } - return errors.New("Failed to send cloudevent: " + result.Error()) + return fmt.Errorf("could not send cloudevent after %d retries. Received the following result from the receiver: %w", httpSender.nrRetries, result) } // EventSender fakes the sending of CloudEvents diff --git a/pkg/lib/v0_2_0/events_test.go b/pkg/lib/v0_2_0/events_test.go index faa46b63..28283a95 100644 --- a/pkg/lib/v0_2_0/events_test.go +++ b/pkg/lib/v0_2_0/events_test.go @@ -1,6 +1,7 @@ package v0_2_0 import ( + "context" "net/http" "net/http/httptest" "testing" @@ -55,42 +56,45 @@ func TestKeptn_SendCloudEventWithRetry(t *testing.T) { ) defer ts.Close() - type args struct { - event cloudevents.Event - } - tests := []struct { - name string - fields fields - args args - wantErr bool - }{ - { - name: "", - fields: getKeptnFields(ts), - args: args{ - event: getTestEvent(), - }, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - httpSender, _ := NewHTTPEventSender(ts.URL) - k := &Keptn{ - KeptnBase: keptn.KeptnBase{ - KeptnContext: tt.fields.KeptnContext, - Event: tt.fields.KeptnBase, - EventSender: httpSender, - UseLocalFileSystem: tt.fields.useLocalFileSystem, - ResourceHandler: tt.fields.resourceHandler, - EventHandler: tt.fields.eventHandler, - }, - } - if err := k.SendCloudEvent(tt.args.event); (err != nil) != tt.wantErr { - t.Errorf("SendCloudEvent() error = %v, wantErr %v", err, tt.wantErr) + httpSender, _ := NewHTTPEventSender(ts.URL) + + err := httpSender.Send(context.TODO(), getTestEvent()) + + require.Nil(t, err) +} + +func TestKeptn_SendCloudEventWithOneRetry(t *testing.T) { + nrRequests := 0 + ts := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("Content-Type", "application/json") + if nrRequests < 2 { + nrRequests++ + w.WriteHeader(500) + w.Write([]byte(`{}`)) + return } - }) - } + w.WriteHeader(200) + w.Write([]byte(`{}`)) + }), + ) + defer ts.Close() + + httpSender, _ := NewHTTPEventSender(ts.URL, WithSendRetries(1)) + + err := httpSender.Send(context.TODO(), getTestEvent()) + + require.NotNil(t, err) + + // reset nrRequests + nrRequests = 0 + + //initialize a new sender with 2 retries + httpSender, _ = NewHTTPEventSender(ts.URL, WithSendRetries(5)) + + err = httpSender.Send(context.TODO(), getTestEvent()) + + require.Nil(t, err) } func getTestEvent() cloudevents.Event { From 98a550ee9a2ee3720415fd97d537a1db7b5fcbbd Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Thu, 12 May 2022 13:55:17 +0200 Subject: [PATCH 2/6] added option for injecting retry callback Signed-off-by: Florian Bacher --- pkg/lib/v0_2_0/events.go | 12 +++++++++++- pkg/lib/v0_2_0/events_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/pkg/lib/v0_2_0/events.go b/pkg/lib/v0_2_0/events.go index abd6c131..efb903bd 100644 --- a/pkg/lib/v0_2_0/events.go +++ b/pkg/lib/v0_2_0/events.go @@ -49,6 +49,12 @@ func WithSendRetries(retries int) HTTPSenderOption { } } +func WithRetryCallback(cb func()) HTTPSenderOption { + return func(httpSender *HTTPEventSender) { + httpSender.retryCallback = cb + } +} + // HTTPEventSender sends CloudEvents via HTTP type HTTPEventSender struct { // EventsEndpoint is the http endpoint the events are sent to @@ -56,7 +62,8 @@ type HTTPEventSender struct { // Client is an implementation of the cloudevents.Client interface Client cloudevents.Client // nrRetries is the number of retries that are attempted if the endpoint an event is forwarded to returns an http code outside the 2xx range - nrRetries int + nrRetries int + retryCallback func() } // NewHTTPEventSender creates a new HTTPSender @@ -104,6 +111,9 @@ func (httpSender HTTPEventSender) Send(ctx context.Context, event cloudevents.Ev ctx = cloudevents.WithEncodingStructured(ctx) var result protocol.Result for i := 0; i <= httpSender.nrRetries; i++ { + if i > 0 && httpSender.retryCallback != nil { + httpSender.retryCallback() + } result = httpSender.Client.Send(ctx, event) httpResult, ok := result.(*httpprotocol.Result) switch { diff --git a/pkg/lib/v0_2_0/events_test.go b/pkg/lib/v0_2_0/events_test.go index 28283a95..a5285db8 100644 --- a/pkg/lib/v0_2_0/events_test.go +++ b/pkg/lib/v0_2_0/events_test.go @@ -49,6 +49,7 @@ func TestKeptn_SendCloudEventWithRetry(t *testing.T) { failOnFirstTry = false w.WriteHeader(500) w.Write([]byte(`{}`)) + return } w.WriteHeader(200) w.Write([]byte(`{}`)) @@ -63,6 +64,35 @@ func TestKeptn_SendCloudEventWithRetry(t *testing.T) { require.Nil(t, err) } +func TestKeptn_SendCloudEventWithRetryCallback(t *testing.T) { + failOnFirstTry := true + ts := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("Content-Type", "application/json") + if failOnFirstTry { + failOnFirstTry = false + w.WriteHeader(500) + w.Write([]byte(`{}`)) + return + } + w.WriteHeader(200) + w.Write([]byte(`{}`)) + }), + ) + defer ts.Close() + + callbackCalled := false + httpSender, _ := NewHTTPEventSender(ts.URL, WithRetryCallback(func() { + callbackCalled = true + })) + + err := httpSender.Send(context.TODO(), getTestEvent()) + + require.Nil(t, err) + + require.True(t, callbackCalled) +} + func TestKeptn_SendCloudEventWithOneRetry(t *testing.T) { nrRequests := 0 ts := httptest.NewServer( From d574fe0cc6a95516734a24194b43fb4cd5059935 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Thu, 12 May 2022 13:56:59 +0200 Subject: [PATCH 3/6] added option for injecting retry callback Signed-off-by: Florian Bacher --- pkg/lib/v0_2_0/events.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/lib/v0_2_0/events.go b/pkg/lib/v0_2_0/events.go index efb903bd..d5cb7524 100644 --- a/pkg/lib/v0_2_0/events.go +++ b/pkg/lib/v0_2_0/events.go @@ -128,7 +128,7 @@ func (httpSender HTTPEventSender) Send(ctx context.Context, event cloudevents.Ev return nil } } - return fmt.Errorf("could not send cloudevent after %d retries. Received the following result from the receiver: %w", httpSender.nrRetries, result) + return fmt.Errorf("could not send cloudevent after %d retries. Received result from the receiver: %w", httpSender.nrRetries, result) } // EventSender fakes the sending of CloudEvents From da769aae4384acaa4be16bbd49582bc97d7b303d Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Thu, 12 May 2022 14:04:31 +0200 Subject: [PATCH 4/6] remove retry callback option Signed-off-by: Florian Bacher --- pkg/lib/v0_2_0/events.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/pkg/lib/v0_2_0/events.go b/pkg/lib/v0_2_0/events.go index d5cb7524..e9422a19 100644 --- a/pkg/lib/v0_2_0/events.go +++ b/pkg/lib/v0_2_0/events.go @@ -49,12 +49,6 @@ func WithSendRetries(retries int) HTTPSenderOption { } } -func WithRetryCallback(cb func()) HTTPSenderOption { - return func(httpSender *HTTPEventSender) { - httpSender.retryCallback = cb - } -} - // HTTPEventSender sends CloudEvents via HTTP type HTTPEventSender struct { // EventsEndpoint is the http endpoint the events are sent to @@ -62,8 +56,7 @@ type HTTPEventSender struct { // Client is an implementation of the cloudevents.Client interface Client cloudevents.Client // nrRetries is the number of retries that are attempted if the endpoint an event is forwarded to returns an http code outside the 2xx range - nrRetries int - retryCallback func() + nrRetries int } // NewHTTPEventSender creates a new HTTPSender @@ -111,9 +104,6 @@ func (httpSender HTTPEventSender) Send(ctx context.Context, event cloudevents.Ev ctx = cloudevents.WithEncodingStructured(ctx) var result protocol.Result for i := 0; i <= httpSender.nrRetries; i++ { - if i > 0 && httpSender.retryCallback != nil { - httpSender.retryCallback() - } result = httpSender.Client.Send(ctx, event) httpResult, ok := result.(*httpprotocol.Result) switch { From ddf2121a38c5d017cd114996b24ec372115a355b Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Thu, 12 May 2022 15:37:54 +0200 Subject: [PATCH 5/6] remove retry callback option unit test Signed-off-by: Florian Bacher --- pkg/lib/v0_2_0/events_test.go | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/pkg/lib/v0_2_0/events_test.go b/pkg/lib/v0_2_0/events_test.go index a5285db8..21fde3f6 100644 --- a/pkg/lib/v0_2_0/events_test.go +++ b/pkg/lib/v0_2_0/events_test.go @@ -64,35 +64,6 @@ func TestKeptn_SendCloudEventWithRetry(t *testing.T) { require.Nil(t, err) } -func TestKeptn_SendCloudEventWithRetryCallback(t *testing.T) { - failOnFirstTry := true - ts := httptest.NewServer( - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Add("Content-Type", "application/json") - if failOnFirstTry { - failOnFirstTry = false - w.WriteHeader(500) - w.Write([]byte(`{}`)) - return - } - w.WriteHeader(200) - w.Write([]byte(`{}`)) - }), - ) - defer ts.Close() - - callbackCalled := false - httpSender, _ := NewHTTPEventSender(ts.URL, WithRetryCallback(func() { - callbackCalled = true - })) - - err := httpSender.Send(context.TODO(), getTestEvent()) - - require.Nil(t, err) - - require.True(t, callbackCalled) -} - func TestKeptn_SendCloudEventWithOneRetry(t *testing.T) { nrRequests := 0 ts := httptest.NewServer( From 6e1f0d58fcb1be42b634ec63719151200b1644ba Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Thu, 12 May 2022 15:52:56 +0200 Subject: [PATCH 6/6] added comment Signed-off-by: Florian Bacher --- pkg/lib/v0_2_0/events.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/lib/v0_2_0/events.go b/pkg/lib/v0_2_0/events.go index e9422a19..96b4f518 100644 --- a/pkg/lib/v0_2_0/events.go +++ b/pkg/lib/v0_2_0/events.go @@ -43,6 +43,7 @@ const keptnGitCommitIDCEExtension = "gitcommitid" type HTTPSenderOption func(httpSender *HTTPEventSender) +// WithSendRetries allows to specify the number of retries that are performed if the receiver of an event returns a HTTP error code func WithSendRetries(retries int) HTTPSenderOption { return func(httpSender *HTTPEventSender) { httpSender.nrRetries = retries