Remove Id return from silences.Set(*pb.Silence)

This commit removes the Id from the method silences.Set(*pb.Silence)
as it is redundant. The Id is still set even when creating a silence
fails. This will be fixed in a later change.

Signed-off-by: George Robinson <[email protected]>

api/v2/api.go

@@ -662,8 +662,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())
@@ -672,7 +671,7 @@ func (api *API) postSilencesHandler(params silence_ops.PostSilencesParams) middl
 	}
 
 	return silence_ops.NewPostSilencesOK().WithPayload(&silence_ops.PostSilencesOKBody{
-		SilenceID: sid,
+		SilenceID: sil.Id,
 	})
 }
 

api/v2/api_test.go

@@ -159,18 +159,16 @@ 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},
 		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))
+	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,18 +221,16 @@ 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},
 		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))
+	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,

notify/notify_test.go

@@ -717,11 +717,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)
 	}
 
@@ -798,7 +798,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)
 	}
 

silence/silence.go

@@ -621,7 +621,7 @@ func (s *Silences) Upsert(sil *pb.Silence) (string, error) {
 
 // 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()
 	return s.set(sil)
@@ -632,43 +632,39 @@ func (s *Silences) set(sil *pb.Silence) (string, error) {
 	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)
 			}
 		}
 	}
 
 	// 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()
 
 	if sil.StartsAt.Before(now) {
 		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

silence/silence_bench_test.go

@@ -58,18 +58,18 @@ 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",
 				Pattern: "bar",
 			}},
 			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)