RepoMirrors
/
alertmanager
mirror of https://github.com/prometheus/alertmanager
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3881 from grobinson-grafana/grobinson/remove-id-from-set 

Remove Id return from silences.Set(*pb.Silence)
This commit is contained in:
Simon Pasquier 2024-06-21 16:03:14 +02:00 committed by GitHub
parent e690fbe250 cc6de9c666
commit 02a017311e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 86 additions and 106 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
api/v2/api.go
View File

@ -660,8 +660,7 @@ func (api *API) postSilencesHandler(params silence_ops.PostSilencesParams) middl
return silence_ops.NewPostSilencesBadRequest().WithPayload(msg)
}
sid, err := api.silences.Set(sil)
if err != nil {
if err = api.silences.Set(sil); err != nil {
level.Error(logger).Log("msg", "Failed to create silence", "err", err)
if errors.Is(err, silence.ErrNotFound) {
return silence_ops.NewPostSilencesNotFound().WithPayload(err.Error())
@ -670,7 +669,7 @@ func (api *API) postSilencesHandler(params silence_ops.PostSilencesParams) middl
}
return silence_ops.NewPostSilencesOK().WithPayload(&silence_ops.PostSilencesOKBody{
SilenceID: sid,
SilenceID: sil.Id,
})
}

24
api/v2/api_test.go
View File

@ -159,8 +159,7 @@ func TestDeleteSilenceHandler(t *testing.T) {
EndsAt: now.Add(time.Hour),
UpdatedAt: now,
}
unexpiredSid, err := silences.Set(unexpiredSil)
require.NoError(t, err)
require.NoError(t, silences.Set(unexpiredSil))
expiredSil := &silencepb.Silence{
Matchers: []*silencepb.Matcher{m},
@ -168,9 +167,8 @@ func TestDeleteSilenceHandler(t *testing.T) {
EndsAt: now.Add(time.Hour),
UpdatedAt: now,
}
expiredSid, err := silences.Set(expiredSil)
require.NoError(t, err)
require.NoError(t, silences.Expire(expiredSid))
require.NoError(t, silences.Set(expiredSil))
require.NoError(t, silences.Expire(expiredSil.Id))
for i, tc := range []struct {
sid string
@ -181,11 +179,11 @@ func TestDeleteSilenceHandler(t *testing.T) {
404,
},
{
unexpiredSid,
unexpiredSil.Id,
200,
},
{
expiredSid,
expiredSil.Id,
200,
},
} {
@ -223,8 +221,7 @@ func TestPostSilencesHandler(t *testing.T) {
EndsAt: now.Add(time.Hour),
UpdatedAt: now,
}
unexpiredSid, err := silences.Set(unexpiredSil)
require.NoError(t, err)
require.NoError(t, silences.Set(unexpiredSil))
expiredSil := &silencepb.Silence{
Matchers: []*silencepb.Matcher{m},
@ -232,9 +229,8 @@ func TestPostSilencesHandler(t *testing.T) {
EndsAt: now.Add(time.Hour),
UpdatedAt: now,
}
expiredSid, err := silences.Set(expiredSil)
require.NoError(t, err)
require.NoError(t, silences.Expire(expiredSid))
require.NoError(t, silences.Set(expiredSil))
require.NoError(t, silences.Expire(expiredSil.Id))
t.Run("Silences CRUD", func(t *testing.T) {
for i, tc := range []struct {
@ -259,14 +255,14 @@ func TestPostSilencesHandler(t *testing.T) {
},
{
"with an active silence ID - it extends the silence",
unexpiredSid,
unexpiredSil.Id,
now.Add(time.Hour),
now.Add(time.Hour * 2),
200,
},
{
"with an expired silence ID - it re-creates the silence",
expiredSid,
expiredSil.Id,
now.Add(time.Hour),
now.Add(time.Hour * 2),
200,

8
notify/notify_test.go
View File

@ -718,11 +718,11 @@ func TestMuteStageWithSilences(t *testing.T) {
if err != nil {
t.Fatal(err)
}
silID, err := silences.Set(&silencepb.Silence{
sil := &silencepb.Silence{
EndsAt: utcNow().Add(time.Hour),
Matchers: []*silencepb.Matcher{{Name: "mute", Pattern: "me"}},
})
if err != nil {
}
if err = silences.Set(sil); err != nil {
t.Fatal(err)
}
@ -799,7 +799,7 @@ func TestMuteStageWithSilences(t *testing.T) {
}
// Expire the silence and verify that no alerts are silenced now.
if err := silences.Expire(silID); err != nil {
if err := silences.Expire(sil.Id); err != nil {
t.Fatal(err)
}

18
silence/silence.go
View File

@ -602,24 +602,24 @@ func (s *Silences) setSilence(sil *pb.Silence, now time.Time, skipValidate bool)
// Set the specified silence. If a silence with the ID already exists and the modification
// modifies history, the old silence gets expired and a new one is created.
func (s *Silences) Set(sil *pb.Silence) (string, error) {
func (s *Silences) Set(sil *pb.Silence) error {
s.mtx.Lock()
defer s.mtx.Unlock()
now := s.nowUTC()
prev, ok := s.getSilence(sil.Id)
if sil.Id != "" && !ok {
return "", ErrNotFound
return ErrNotFound
}
if ok {
if canUpdate(prev, sil, now) {
return sil.Id, s.setSilence(sil, now, false)
return s.setSilence(sil, now, false)
}
if getState(prev, s.nowUTC()) != types.SilenceStateExpired {
// We cannot update the silence, expire the old one.
if err := s.expire(prev.Id); err != nil {
return "", fmt.Errorf("expire previous silence: %w", err)
return fmt.Errorf("expire previous silence: %w", err)
}
}
}
@ -627,13 +627,13 @@ func (s *Silences) Set(sil *pb.Silence) (string, error) {
// If we got here it's either a new silence or a replacing one.
if s.limits.MaxSilences != nil {
if m := s.limits.MaxSilences(); m > 0 && len(s.st)+1 > m {
return "", fmt.Errorf("exceeded maximum number of silences: %d (limit: %d)", len(s.st), m)
return fmt.Errorf("exceeded maximum number of silences: %d (limit: %d)", len(s.st), m)
}
}
uid, err := uuid.NewV4()
if err != nil {
return "", fmt.Errorf("generate uuid: %w", err)
return fmt.Errorf("generate uuid: %w", err)
}
sil.Id = uid.String()
@ -641,11 +641,7 @@ func (s *Silences) Set(sil *pb.Silence) (string, error) {
sil.StartsAt = now
}
if err = s.setSilence(sil, now, false); err != nil {
return "", err
}
return sil.Id, nil
return s.setSilence(sil, now, false)
}
// canUpdate returns true if silence a can be updated to b without

8
silence/silence_bench_test.go
View File

@ -58,8 +58,7 @@ func benchmarkMutes(b *testing.B, n int) {
var silenceIDs []string
for i := 0; i < n; i++ {
var silenceID string
silenceID, err = silences.Set(&silencepb.Silence{
s := &silencepb.Silence{
Matchers: []*silencepb.Matcher{{
Type: silencepb.Matcher_EQUAL,
Name: "foo",
@ -67,9 +66,10 @@ func benchmarkMutes(b *testing.B, n int) {
}},
StartsAt: now,
EndsAt: now.Add(time.Minute),
})
}
require.NoError(b, silences.Set(s))
require.NoError(b, err)
silenceIDs = append(silenceIDs, silenceID)
silenceIDs = append(silenceIDs, s.Id)
}
require.Len(b, silenceIDs, n)

129
silence/silence_test.go
View File

@ -321,14 +321,13 @@ func TestSilenceSet(t *testing.T) {
StartsAt: start1.Add(2 * time.Minute),
EndsAt: start1.Add(5 * time.Minute),
}
id1, err := s.Set(sil1)
require.NoError(t, err)
require.NotEqual(t, "", id1)
require.NoError(t, s.Set(sil1))
require.NotEqual(t, "", sil1.Id)
want := state{
id1: &pb.MeshSilence{
sil1.Id: &pb.MeshSilence{
Silence: &pb.Silence{
Id: id1,
Id: sil1.Id,
Matchers: []*pb.Matcher{{Name: "a", Pattern: "b"}},
StartsAt: start1.Add(2 * time.Minute),
EndsAt: start1.Add(5 * time.Minute),
@ -347,15 +346,14 @@ func TestSilenceSet(t *testing.T) {
Matchers: []*pb.Matcher{{Name: "a", Pattern: "b"}},
EndsAt: start2.Add(1 * time.Minute),
}
id2, err := s.Set(sil2)
require.NoError(t, err)
require.NotEqual(t, "", id2)
require.NoError(t, s.Set(sil2))
require.NotEqual(t, "", sil2.Id)
want = state{
id1: want[id1],
id2: &pb.MeshSilence{
sil1.Id: want[sil1.Id],
sil2.Id: &pb.MeshSilence{
Silence: &pb.Silence{
Id: id2,
Id: sil2.Id,
Matchers: []*pb.Matcher{{Name: "a", Pattern: "b"}},
StartsAt: start2,
EndsAt: start2.Add(1 * time.Minute),
@ -373,15 +371,14 @@ func TestSilenceSet(t *testing.T) {
sil3 := cloneSilence(sil2)
sil3.EndsAt = start3.Add(100 * time.Minute)
id3, err := s.Set(sil3)
require.NoError(t, err)
require.Equal(t, id2, id3)
require.NoError(t, s.Set(sil3))
require.Equal(t, sil2.Id, sil3.Id)
want = state{
id1: want[id1],
id2: &pb.MeshSilence{
sil1.Id: want[sil1.Id],
sil2.Id: &pb.MeshSilence{
Silence: &pb.Silence{
Id: id2,
Id: sil2.Id,
Matchers: []*pb.Matcher{{Name: "a", Pattern: "b"}},
StartsAt: start2,
EndsAt: start3.Add(100 * time.Minute),
@ -399,16 +396,15 @@ func TestSilenceSet(t *testing.T) {
sil4 := cloneSilence(sil3)
sil4.Matchers = []*pb.Matcher{{Name: "a", Pattern: "c"}}
id4, err := s.Set(sil4)
require.NoError(t, err)
require.NoError(t, s.Set(sil4))
// This new silence gets a new id.
require.NotEqual(t, id2, id4)
require.NotEqual(t, sil2.Id, sil4.Id)
want = state{
id1: want[id1],
id2: &pb.MeshSilence{
sil1.Id: want[sil1.Id],
sil2.Id: &pb.MeshSilence{
Silence: &pb.Silence{
Id: id2,
Id: sil2.Id,
Matchers: []*pb.Matcher{{Name: "a", Pattern: "b"}},
StartsAt: start2,
EndsAt: start4, // Expired
@ -416,9 +412,9 @@ func TestSilenceSet(t *testing.T) {
},
ExpiresAt: start4.Add(s.retention),
},
id4: &pb.MeshSilence{
sil4.Id: &pb.MeshSilence{
Silence: &pb.Silence{
Id: id4,
Id: sil4.Id,
Matchers: []*pb.Matcher{{Name: "a", Pattern: "c"}},
StartsAt: start4,
EndsAt: start3.Add(100 * time.Minute),
@ -437,17 +433,16 @@ func TestSilenceSet(t *testing.T) {
sil5.StartsAt = start1
sil5.EndsAt = start1.Add(5 * time.Minute)
id5, err := s.Set(sil5)
require.NoError(t, err)
require.NotEqual(t, id2, id4)
require.NoError(t, s.Set(sil5))
require.NotEqual(t, sil2.Id, sil5.Id)
want = state{
id1: want[id1],
id2: want[id2],
id4: want[id4],
id5: &pb.MeshSilence{
sil1.Id: want[sil1.Id],
sil2.Id: want[sil2.Id],
sil4.Id: want[sil4.Id],
sil5.Id: &pb.MeshSilence{
Silence: &pb.Silence{
Id: id5,
Id: sil5.Id,
Matchers: []*pb.Matcher{{Name: "a", Pattern: "b"}},
StartsAt: start5, // New silences have their start time set to "now" when created.
EndsAt: start1.Add(5 * time.Minute),
@ -474,9 +469,8 @@ func TestSilenceLimits(t *testing.T) {
StartsAt: time.Now(),
EndsAt: time.Now().Add(5 * time.Minute),
}
id1, err := s.Set(sil1)
require.NoError(t, err)
require.NotEqual(t, "", id1)
require.NoError(t, s.Set(sil1))
require.NotEqual(t, "", sil1.Id)
// Insert sil2 should fail because maximum number of silences
// has been exceeded.
@ -485,27 +479,24 @@ func TestSilenceLimits(t *testing.T) {
StartsAt: time.Now(),
EndsAt: time.Now().Add(5 * time.Minute),
}
id2, err := s.Set(sil2)
require.EqualError(t, err, "exceeded maximum number of silences: 1 (limit: 1)")
require.Equal(t, "", id2)
require.EqualError(t, s.Set(sil2), "exceeded maximum number of silences: 1 (limit: 1)")
require.Equal(t, "", sil2.Id)
// Expire sil1 and run the GC. This should allow sil2 to be
// inserted.
require.NoError(t, s.Expire(id1))
require.NoError(t, s.Expire(sil1.Id))
n, err := s.GC()
require.NoError(t, err)
require.Equal(t, 1, n)
id2, err = s.Set(sil2)
require.NoError(t, err)
require.NotEqual(t, "", id2)
require.NoError(t, s.Set(sil2))
require.NotEqual(t, "", sil2.Id)
// Should be able to update sil2 without hitting the limit.
_, err = s.Set(sil2)
require.NoError(t, err)
require.NoError(t, s.Set(sil2))
// Expire sil2.
require.NoError(t, s.Expire(id2))
require.NoError(t, s.Expire(sil2.Id))
n, err = s.GC()
require.NoError(t, err)
require.Equal(t, 1, n)
@ -527,12 +518,13 @@ func TestSilenceLimits(t *testing.T) {
StartsAt: time.Now(),
EndsAt: time.Now().Add(5 * time.Minute),
}
id3, err := s.Set(sil3)
err = s.Set(sil3)
require.Error(t, err)
// Do not check the exact size as it can change between consecutive runs
// due to padding.
require.Contains(t, err.Error(), "silence exceeded maximum size")
require.Equal(t, "", id3)
// TODO: Once we fix https://github.com/prometheus/alertmanager/issues/3878 this should be require.Equal.
require.NotEqual(t, "", sil3.Id)
}
func TestSilenceNoLimits(t *testing.T) {
@ -548,9 +540,8 @@ func TestSilenceNoLimits(t *testing.T) {
EndsAt: time.Now().Add(5 * time.Minute),
Comment: strings.Repeat("c", 2<<9),
}
id, err := s.Set(sil)
require.NoError(t, err)
require.NotEqual(t, "", id)
require.NoError(t, s.Set(sil))
require.NotEqual(t, "", sil.Id)
}
func TestSetActiveSilence(t *testing.T) {
@ -571,7 +562,7 @@ func TestSetActiveSilence(t *testing.T) {
StartsAt: startsAt,
EndsAt: endsAt,
}
id1, _ := s.Set(sil1)
require.NoError(t, s.Set(sil1))
// Update silence with 2 extra nanoseconds so the "seconds" part should not change
@ -579,20 +570,19 @@ func TestSetActiveSilence(t *testing.T) {
newEndsAt := endsAt.Add(2 * time.Minute)
sil2 := cloneSilence(sil1)
sil2.Id = id1
sil2.Id = sil1.Id
sil2.StartsAt = newStartsAt
sil2.EndsAt = newEndsAt
clock.Add(time.Minute)
now = s.nowUTC()
id2, err := s.Set(sil2)
require.NoError(t, err)
require.Equal(t, id1, id2)
require.NoError(t, s.Set(sil2))
require.Equal(t, sil1.Id, sil2.Id)
want := state{
id2: &pb.MeshSilence{
sil2.Id: &pb.MeshSilence{
Silence: &pb.Silence{
Id: id1,
Id: sil1.Id,
Matchers: []*pb.Matcher{{Name: "a", Pattern: "b"}},
StartsAt: newStartsAt,
EndsAt: newEndsAt,
@ -624,8 +614,7 @@ func TestSilencesSetFail(t *testing.T) {
},
}
for _, c := range cases {
_, err := s.Set(c.s)
checkErr(t, c.err, err)
checkErr(t, c.err, s.Set(c.s))
}
}
@ -1190,22 +1179,22 @@ func TestSilencer(t *testing.T) {
require.False(t, s.Mutes(model.LabelSet{"foo": "bar"}), "expected alert not silenced without any silences")
_, err = ss.Set(&pb.Silence{
sil1 := &pb.Silence{
Matchers: []*pb.Matcher{{Name: "foo", Pattern: "baz"}},
StartsAt: now.Add(-time.Hour),
EndsAt: now.Add(5 * time.Minute),
})
require.NoError(t, err)
}
require.NoError(t, ss.Set(sil1))
require.False(t, s.Mutes(model.LabelSet{"foo": "bar"}), "expected alert not silenced by non-matching silence")
id, err := ss.Set(&pb.Silence{
sil2 := &pb.Silence{
Matchers: []*pb.Matcher{{Name: "foo", Pattern: "bar"}},
StartsAt: now.Add(-time.Hour),
EndsAt: now.Add(5 * time.Minute),
})
require.NoError(t, err)
require.NotEmpty(t, id)
}
require.NoError(t, ss.Set(sil2))
require.NotEmpty(t, sil2.Id)
require.True(t, s.Mutes(model.LabelSet{"foo": "bar"}), "expected alert silenced by matching silence")
@ -1216,8 +1205,8 @@ func TestSilencer(t *testing.T) {
require.False(t, s.Mutes(model.LabelSet{"foo": "bar"}), "expected alert not silenced by expired silence")
// Update silence to start in the future.
_, err = ss.Set(&pb.Silence{
Id: id,
err = ss.Set(&pb.Silence{
Id: sil2.Id,
Matchers: []*pb.Matcher{{Name: "foo", Pattern: "bar"}},
StartsAt: now.Add(time.Hour),
EndsAt: now.Add(3 * time.Hour),
@ -1233,7 +1222,7 @@ func TestSilencer(t *testing.T) {
// Exposes issue #2426.
require.True(t, s.Mutes(model.LabelSet{"foo": "bar"}), "expected alert silenced by activated silence")
_, err = ss.Set(&pb.Silence{
err = ss.Set(&pb.Silence{
Matchers: []*pb.Matcher{{Name: "foo", Pattern: "b..", Type: pb.Matcher_REGEXP}},
StartsAt: now.Add(time.Hour),
EndsAt: now.Add(3 * time.Hour),