From d389091ed22a0db84812f23355db51ce218abd8f Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Wed, 12 Jan 2022 16:33:39 +0000 Subject: [PATCH 01/14] Changed Silences.expire(id) to not return error for already expired silence Signed-off-by: Soon-Ping Phang --- silence/silence.go | 2 +- silence/silence_test.go | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/silence/silence.go b/silence/silence.go index 6f3b177dd9..5c021fc298 100644 --- a/silence/silence.go +++ b/silence/silence.go @@ -622,7 +622,7 @@ func (s *Silences) expire(id string) error { switch getState(sil, now) { case types.SilenceStateExpired: - return errors.Errorf("silence %s already expired", id) + return nil case types.SilenceStateActive: sil.EndsAt = now case types.SilenceStatePending: diff --git a/silence/silence_test.go b/silence/silence_test.go index c39b7804cf..2f116b5b52 100644 --- a/silence/silence_test.go +++ b/silence/silence_test.go @@ -835,9 +835,7 @@ func TestSilenceExpire(t *testing.T) { require.NoError(t, s.Expire("pending")) require.NoError(t, s.Expire("active")) - err = s.Expire("expired") - require.Error(t, err) - require.Contains(t, err.Error(), "already expired") + require.NoError(t, s.Expire("expired")) sil, err := s.QueryOne(QIDs("pending")) require.NoError(t, err) @@ -935,9 +933,7 @@ func TestSilenceExpireWithZeroRetention(t *testing.T) { require.NoError(t, s.Expire("pending")) require.NoError(t, s.Expire("active")) - err = s.Expire("expired") - require.Error(t, err) - require.Contains(t, err.Error(), "already expired") + require.NoError(t, s.Expire("expired")) _, err = s.QueryOne(QIDs("pending")) require.NoError(t, err) From 342fef5a04bd4066513977369b04530425d8efeb Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Mon, 17 Jan 2022 23:00:06 +0000 Subject: [PATCH 02/14] Added comment explaining idempotency change for Silences.expire() Signed-off-by: Soon-Ping Phang --- silence/silence.go | 1 + 1 file changed, 1 insertion(+) diff --git a/silence/silence.go b/silence/silence.go index 5c021fc298..82efccd072 100644 --- a/silence/silence.go +++ b/silence/silence.go @@ -622,6 +622,7 @@ func (s *Silences) expire(id string) error { switch getState(sil, now) { case types.SilenceStateExpired: + // returning nil ensures idempotent behaviour, at least in the short term before the silence is gc'd return nil case types.SilenceStateActive: sil.EndsAt = now From 3ddd3e6b9ca3d8c17ae0559ff84eabbc89cd9766 Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Tue, 18 Jan 2022 01:00:23 +0000 Subject: [PATCH 03/14] Trigger build Signed-off-by: Soon-Ping Phang From 1e7879f691719932d0964a42730c559660bc5a7d Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Tue, 18 Jan 2022 02:51:08 +0000 Subject: [PATCH 04/14] Trigger build Signed-off-by: Soon-Ping Phang From 2a7ef46d13e2a4573d6b62a855267165664b0c14 Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Tue, 18 Jan 2022 17:39:33 +0000 Subject: [PATCH 05/14] Fixed typo in comment Signed-off-by: Soon-Ping Phang --- silence/silence.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/silence/silence.go b/silence/silence.go index 82efccd072..5d01311cc6 100644 --- a/silence/silence.go +++ b/silence/silence.go @@ -622,7 +622,7 @@ func (s *Silences) expire(id string) error { switch getState(sil, now) { case types.SilenceStateExpired: - // returning nil ensures idempotent behaviour, at least in the short term before the silence is gc'd + // Returning nil ensures idempotent behaviour, at least in the short term before the silence is gc'd return nil case types.SilenceStateActive: sil.EndsAt = now From 53a300ec9544664cbc445b05a7dd4b13f1a7b2e0 Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Tue, 18 Jan 2022 18:16:19 +0000 Subject: [PATCH 06/14] Trigger build Signed-off-by: Soon-Ping Phang From cc0283c1dbea32bd02c44d4d982cfa386c44d6d3 Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Tue, 18 Jan 2022 19:40:16 +0000 Subject: [PATCH 07/14] Trigger build Signed-off-by: Soon-Ping Phang From e3f9f46c33d25999aeee06b77d2f367c1439a92e Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Wed, 19 Jan 2022 18:12:35 +0000 Subject: [PATCH 08/14] Fixed another typo in comment Signed-off-by: Soon-Ping Phang --- silence/silence.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/silence/silence.go b/silence/silence.go index 5d01311cc6..ad903f2f39 100644 --- a/silence/silence.go +++ b/silence/silence.go @@ -622,7 +622,7 @@ func (s *Silences) expire(id string) error { switch getState(sil, now) { case types.SilenceStateExpired: - // Returning nil ensures idempotent behaviour, at least in the short term before the silence is gc'd + // Returning nil ensures idempotent behaviour, at least in the short term before the silence is gc'd. return nil case types.SilenceStateActive: sil.EndsAt = now From 8e605a4bae3a745f494985d992de07d36d8b867b Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Thu, 20 Jan 2022 02:35:12 +0000 Subject: [PATCH 09/14] Promoted comment to function-level Signed-off-by: Soon-Ping Phang --- silence/silence.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/silence/silence.go b/silence/silence.go index ad903f2f39..b2bebe93f8 100644 --- a/silence/silence.go +++ b/silence/silence.go @@ -612,6 +612,8 @@ func (s *Silences) Expire(id string) error { } // Expire the silence with the given ID immediately. +// It is idempotent, nil is returned if the silence already expired before it is GC'd. +// If the silence is not found an error is returned. func (s *Silences) expire(id string) error { sil, ok := s.getSilence(id) if !ok { @@ -622,7 +624,6 @@ func (s *Silences) expire(id string) error { switch getState(sil, now) { case types.SilenceStateExpired: - // Returning nil ensures idempotent behaviour, at least in the short term before the silence is gc'd. return nil case types.SilenceStateActive: sil.EndsAt = now From 4b776b98e2ac920211d0e77fa7e8cb0cc3c7cf0b Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Wed, 26 Jan 2022 02:47:33 +0000 Subject: [PATCH 10/14] Added API v2 test for DeleteSilence, PostSilence Signed-off-by: Soon-Ping Phang --- api/v2/api_test.go | 182 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) diff --git a/api/v2/api_test.go b/api/v2/api_test.go index 5ff72fee66..6e0c901005 100644 --- a/api/v2/api_test.go +++ b/api/v2/api_test.go @@ -14,20 +14,31 @@ package v2 import ( + "bytes" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" "strconv" "testing" "time" + "github.com/go-openapi/runtime" "github.com/go-openapi/strfmt" "github.com/prometheus/common/model" "github.com/stretchr/testify/require" open_api_models "github.com/prometheus/alertmanager/api/v2/models" general_ops "github.com/prometheus/alertmanager/api/v2/restapi/operations/general" + silence_ops "github.com/prometheus/alertmanager/api/v2/restapi/operations/silence" "github.com/prometheus/alertmanager/config" "github.com/prometheus/alertmanager/pkg/labels" + "github.com/prometheus/alertmanager/silence" "github.com/prometheus/alertmanager/silence/silencepb" "github.com/prometheus/alertmanager/types" + + "github.com/go-kit/log" ) // If api.peers == nil, Alertmanager cluster feature is disabled. Make sure to @@ -72,6 +83,13 @@ var ( createdBy = "test" ) +func newSilences(t *testing.T) *silence.Silences { + silences, err := silence.New(silence.Options{}) + require.NoError(t, err) + + return silences +} + func gettableSilence(id string, state string, updatedAt string, start string, end string, ) *open_api_models.GettableSilence { @@ -131,6 +149,170 @@ func TestGetSilencesHandler(t *testing.T) { } } +func TestDeleteSilenceHandler(t *testing.T) { + now := time.Now() + silences := newSilences(t) + + m := &silencepb.Matcher{Type: silencepb.Matcher_EQUAL, Name: "a", Pattern: "b"} + + unexpiredSil := &silencepb.Silence{ + Matchers: []*silencepb.Matcher{m}, + StartsAt: now, + EndsAt: now.Add(time.Hour), + UpdatedAt: now, + } + unexpiredSid, err := silences.Set(unexpiredSil) + require.NoError(t, err) + + expiredSil := &silencepb.Silence{ + Matchers: []*silencepb.Matcher{m}, + StartsAt: now.Add(-time.Hour), + EndsAt: now.Add(time.Hour), + UpdatedAt: now, + } + expiredSid, err := silences.Set(expiredSil) + require.NoError(t, err) + require.NoError(t, silences.Expire(expiredSid)) + + for i, tc := range []struct { + sid string + expectedCode int + }{ + { + "unknownSid", + 500, + }, + { + unexpiredSid, + 200, + }, + { + expiredSid, + 200, + }, + } { + api := API{ + uptime: time.Now(), + silences: silences, + logger: log.NewNopLogger(), + } + + r, err := http.NewRequest("DELETE", "/api/v2/silence/${tc.sid}", nil) + require.NoError(t, err) + + w := httptest.NewRecorder() + p := runtime.TextProducer() + responder := api.deleteSilenceHandler(silence_ops.DeleteSilenceParams{ + SilenceID: strfmt.UUID(tc.sid), + HTTPRequest: r, + }) + responder.WriteResponse(w, p) + body, _ := ioutil.ReadAll(w.Result().Body) + + require.Equal(t, tc.expectedCode, w.Code, fmt.Sprintf(fmt.Sprintf("test case: %d, response: %s", i, string(body)))) + } +} + +func TestPostSilencesHandler(t *testing.T) { + now := time.Now() + silences := newSilences(t) + + m := &silencepb.Matcher{Type: silencepb.Matcher_EQUAL, Name: "a", Pattern: "b"} + + unexpiredSil := &silencepb.Silence{ + Matchers: []*silencepb.Matcher{m}, + StartsAt: now, + EndsAt: now.Add(time.Hour), + UpdatedAt: now, + } + unexpiredSid, err := silences.Set(unexpiredSil) + require.NoError(t, err) + + expiredSil := &silencepb.Silence{ + Matchers: []*silencepb.Matcher{m}, + StartsAt: now.Add(-time.Hour), + EndsAt: now.Add(time.Hour), + UpdatedAt: now, + } + expiredSid, err := silences.Set(expiredSil) + require.NoError(t, err) + require.NoError(t, silences.Expire(expiredSid)) + + for i, tc := range []struct { + sid string + start, end time.Time + expectedCode int + }{ + { + "unknownSid", + now.Add(time.Hour), + now.Add(time.Hour * 2), + 404, + }, + { + "", + now.Add(time.Hour), + now.Add(time.Hour * 2), + 200, + }, + { + unexpiredSid, + now.Add(time.Hour), + now.Add(time.Hour * 2), + 200, + }, + { + expiredSid, + now.Add(time.Hour), + now.Add(time.Hour * 2), + 200, + }, + } { + createdBy := "silenceCreator" + comment := "test" + matcherName := "a" + matcherValue := "b" + isRegex := false + startsAt := strfmt.DateTime(tc.start) + endsAt := strfmt.DateTime(tc.end) + + sil := open_api_models.PostableSilence{ + ID: tc.sid, + Silence: open_api_models.Silence{ + Matchers: open_api_models.Matchers{&open_api_models.Matcher{Name: &matcherName, Value: &matcherValue, IsRegex: &isRegex}}, + StartsAt: &startsAt, + EndsAt: &endsAt, + CreatedBy: &createdBy, + Comment: &comment, + }, + } + b, err := json.Marshal(&sil) + if err != nil { + t.Errorf("Unexpected error %v", err) + } + + api := API{ + uptime: time.Now(), + silences: silences, + logger: log.NewNopLogger(), + } + + r, err := http.NewRequest("POST", "/api/v2/silence/${tc.sid}", bytes.NewReader(b)) + require.NoError(t, err) + + w := httptest.NewRecorder() + p := runtime.TextProducer() + responder := api.postSilencesHandler(silence_ops.PostSilencesParams{ + HTTPRequest: r, + Silence: &sil, + }) + responder.WriteResponse(w, p) + body, _ := ioutil.ReadAll(w.Result().Body) + + require.Equal(t, tc.expectedCode, w.Code, fmt.Sprintf(fmt.Sprintf("test case: %d, response: %s", i, string(body)))) + } +} + func createSilenceMatcher(name string, pattern string, matcherType silencepb.Matcher_Type) *silencepb.Matcher { return &silencepb.Matcher{ Name: name, From 5d0d557ed1ed12a1078e58f23291bd5b7c6d35a0 Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Wed, 26 Jan 2022 03:55:09 +0000 Subject: [PATCH 11/14] Fixed lint errors Signed-off-by: Soon-Ping Phang --- api/v2/api_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/v2/api_test.go b/api/v2/api_test.go index 6e0c901005..2527c96d47 100644 --- a/api/v2/api_test.go +++ b/api/v2/api_test.go @@ -209,7 +209,7 @@ func TestDeleteSilenceHandler(t *testing.T) { responder.WriteResponse(w, p) body, _ := ioutil.ReadAll(w.Result().Body) - require.Equal(t, tc.expectedCode, w.Code, fmt.Sprintf(fmt.Sprintf("test case: %d, response: %s", i, string(body)))) + require.Equal(t, tc.expectedCode, w.Code, fmt.Sprintf("test case: %d, response: %s", i, string(body))) } } @@ -309,7 +309,7 @@ func TestPostSilencesHandler(t *testing.T) { responder.WriteResponse(w, p) body, _ := ioutil.ReadAll(w.Result().Body) - require.Equal(t, tc.expectedCode, w.Code, fmt.Sprintf(fmt.Sprintf("test case: %d, response: %s", i, string(body)))) + require.Equal(t, tc.expectedCode, w.Code, fmt.Sprintf("test case: %d, response: %s", i, string(body))) } } From 619b390387ca8a26be79be8d163793d2a860d2ec Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Wed, 26 Jan 2022 04:02:20 +0000 Subject: [PATCH 12/14] Trigger build Signed-off-by: Soon-Ping Phang From 451452de742a0ef4919cd2c335dd745443c2165c Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Wed, 26 Jan 2022 04:19:49 +0000 Subject: [PATCH 13/14] Trigger build Signed-off-by: Soon-Ping Phang From 4c3657957c016459b280a51355295d2984ef1e20 Mon Sep 17 00:00:00 2001 From: Soon-Ping Phang Date: Wed, 26 Jan 2022 13:01:53 +0000 Subject: [PATCH 14/14] Trigger build Signed-off-by: Soon-Ping Phang