From afafc0d73b85296549656bc99da6863d7a2bc091 Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Tue, 13 Feb 2024 16:39:57 +0100 Subject: [PATCH 1/2] alerts: add pagination to alerts endpoint and add another endpoint to dismiss all alerts at once --- alerts/alerts.go | 32 +++++++++++++++++++++++++++- bus/bus.go | 23 +++++++++++++++----- bus/client/alerts.go | 18 ++++++++++++++-- internal/testing/cluster_test.go | 36 +++++++++++++++++++++++++++++++- 4 files changed, 100 insertions(+), 9 deletions(-) diff --git a/alerts/alerts.go b/alerts/alerts.go index 4d6463fa2..c76898c8d 100644 --- a/alerts/alerts.go +++ b/alerts/alerts.go @@ -37,6 +37,7 @@ type ( Alerter interface { RegisterAlert(_ context.Context, a Alert) error DismissAlerts(_ context.Context, ids ...types.Hash256) error + DismissAllAlerts(_ context.Context) error } // Severity indicates the severity of an alert. @@ -63,6 +64,11 @@ type ( alerts map[types.Hash256]Alert webhookBroadcaster webhooks.Broadcaster } + + AlertsOpts struct { + Offset uint64 + Limit uint64 + } ) // String implements the fmt.Stringer interface. @@ -130,6 +136,17 @@ func (m *Manager) RegisterAlert(ctx context.Context, alert Alert) error { }) } +// DismissAllAlerts implements the Alerter interface. +func (m *Manager) DismissAllAlerts(ctx context.Context) error { + m.mu.Lock() + toDismiss := make([]types.Hash256, 0, len(m.alerts)) + for alertID := range m.alerts { + toDismiss = append(toDismiss, alertID) + } + m.mu.Unlock() + return m.DismissAlerts(ctx, toDismiss...) +} + // DismissAlerts implements the Alerter interface. func (m *Manager) DismissAlerts(ctx context.Context, ids ...types.Hash256) error { var dismissed []types.Hash256 @@ -159,10 +176,14 @@ func (m *Manager) DismissAlerts(ctx context.Context, ids ...types.Hash256) error } // Active returns the host's active alerts. -func (m *Manager) Active() []Alert { +func (m *Manager) Active(offset, limit uint64) []Alert { m.mu.Lock() defer m.mu.Unlock() + if offset >= uint64(len(m.alerts)) { + return nil + } + alerts := make([]Alert, 0, len(m.alerts)) for _, a := range m.alerts { alerts = append(alerts, a) @@ -170,6 +191,10 @@ func (m *Manager) Active() []Alert { sort.Slice(alerts, func(i, j int) bool { return alerts[i].Timestamp.After(alerts[j].Timestamp) }) + alerts = alerts[offset:] + if limit < uint64(len(alerts)) { + alerts = alerts[:limit] + } return alerts } @@ -213,6 +238,11 @@ func (a *originAlerter) RegisterAlert(ctx context.Context, alert Alert) error { return a.alerter.RegisterAlert(ctx, alert) } +// DismissAllAlerts implements the Alerter interface. +func (a *originAlerter) DismissAllAlerts(ctx context.Context) error { + return a.alerter.DismissAllAlerts(ctx) +} + // DismissAlerts implements the Alerter interface. func (a *originAlerter) DismissAlerts(ctx context.Context, ids ...types.Hash256) error { return a.alerter.DismissAlerts(ctx, ids...) diff --git a/bus/bus.go b/bus/bus.go index d11550595..82a89826f 100644 --- a/bus/bus.go +++ b/bus/bus.go @@ -245,9 +245,10 @@ func (b *bus) Handler() http.Handler { "POST /account/:id/requiressync": b.accountsRequiresSyncHandlerPOST, "POST /account/:id/resetdrift": b.accountsResetDriftHandlerPOST, - "GET /alerts": b.handleGETAlerts, - "POST /alerts/dismiss": b.handlePOSTAlertsDismiss, - "POST /alerts/register": b.handlePOSTAlertsRegister, + "GET /alerts": b.handleGETAlerts, + "POST /alerts/dismiss": b.handlePOSTAlertsDismiss, + "POST /alerts/dismissall": b.handlePOSTAlertsDismissAll, + "POST /alerts/register": b.handlePOSTAlertsRegister, "GET /autopilots": b.autopilotsListHandlerGET, "GET /autopilot/:id": b.autopilotsHandlerGET, @@ -1711,8 +1712,16 @@ func (b *bus) gougingParams(ctx context.Context) (api.GougingParams, error) { }, nil } -func (b *bus) handleGETAlerts(c jape.Context) { - c.Encode(b.alertMgr.Active()) +func (b *bus) handleGETAlerts(jc jape.Context) { + var offset, limit uint64 + if jc.DecodeForm("offset", &offset) != nil { + return + } else if jc.DecodeForm("limit", &limit) != nil { + return + } else if limit == 0 { + limit = math.MaxUint64 + } + jc.Encode(b.alertMgr.Active(offset, limit)) } func (b *bus) handlePOSTAlertsDismiss(jc jape.Context) { @@ -1723,6 +1732,10 @@ func (b *bus) handlePOSTAlertsDismiss(jc jape.Context) { jc.Check("failed to dismiss alerts", b.alertMgr.DismissAlerts(jc.Request.Context(), ids...)) } +func (b *bus) handlePOSTAlertsDismissAll(jc jape.Context) { + jc.Check("failed to dismiss alerts", b.alertMgr.DismissAllAlerts(jc.Request.Context())) +} + func (b *bus) handlePOSTAlertsRegister(jc jape.Context) { var alert alerts.Alert if jc.Decode(&alert) != nil { diff --git a/bus/client/alerts.go b/bus/client/alerts.go index 6af68c78d..ab1d7f094 100644 --- a/bus/client/alerts.go +++ b/bus/client/alerts.go @@ -2,17 +2,31 @@ package client import ( "context" + "fmt" + "net/url" "go.sia.tech/core/types" "go.sia.tech/renterd/alerts" ) // Alerts fetches the active alerts from the bus. -func (c *Client) Alerts() (alerts []alerts.Alert, err error) { - err = c.c.GET("/alerts", &alerts) +func (c *Client) Alerts(opts alerts.AlertsOpts) (alerts []alerts.Alert, err error) { + values := url.Values{} + if opts.Offset > 0 { + values.Set("offset", fmt.Sprint(opts.Offset)) + } + if opts.Limit != 0 { + values.Set("limit", fmt.Sprint(opts.Limit)) + } + err = c.c.GET("/alerts?"+values.Encode(), &alerts) return } +// DismissAllAlerts dimisses all alerts. +func (c *Client) DismissAllAlerts(ctx context.Context) error { + return c.c.WithContext(ctx).POST("/alerts/dismissall", nil, nil) +} + // DismissAlerts dimisses the alerts with the given IDs. func (c *Client) DismissAlerts(ctx context.Context, ids ...types.Hash256) error { return c.c.WithContext(ctx).POST("/alerts/dismiss", ids, nil) diff --git a/internal/testing/cluster_test.go b/internal/testing/cluster_test.go index b0de2946e..cd26b6519 100644 --- a/internal/testing/cluster_test.go +++ b/internal/testing/cluster_test.go @@ -1915,7 +1915,7 @@ func TestAlerts(t *testing.T) { tt.OK(b.RegisterAlert(context.Background(), alert)) findAlert := func(id types.Hash256) *alerts.Alert { t.Helper() - alerts, err := b.Alerts() + alerts, err := b.Alerts(alerts.AlertsOpts{}) tt.OK(err) for _, alert := range alerts { if alert.ID == id { @@ -1938,6 +1938,40 @@ func TestAlerts(t *testing.T) { if foundAlert != nil { t.Fatal("alert found") } + + // register 2 alerts + alert2 := alert + alert2.ID = frand.Entropy256() + alert2.Timestamp = time.Now().Add(time.Second) + tt.OK(b.RegisterAlert(context.Background(), alert)) + tt.OK(b.RegisterAlert(context.Background(), alert2)) + if foundAlert := findAlert(alert.ID); foundAlert == nil { + t.Fatal("alert not found") + } else if foundAlert := findAlert(alert2.ID); foundAlert == nil { + t.Fatal("alert not found") + } + + // try to find with offset = 1 + foundAlerts, err := b.Alerts(alerts.AlertsOpts{Offset: 1}) + tt.OK(err) + if len(foundAlerts) != 1 || foundAlerts[0].ID != alert.ID { + t.Fatal("wrong alert") + } + + // try to find with limit = 1 + foundAlerts, err = b.Alerts(alerts.AlertsOpts{Limit: 1}) + tt.OK(err) + if len(foundAlerts) != 1 || foundAlerts[0].ID != alert2.ID { + t.Fatal("wrong alert") + } + + // dismiss all + tt.OK(b.DismissAllAlerts(context.Background())) + foundAlerts, err = b.Alerts(alerts.AlertsOpts{}) + tt.OK(err) + if len(foundAlerts) != 0 { + t.Fatal("expected 0 alerts", len(foundAlerts)) + } } func TestMultipartUploads(t *testing.T) { From b22e96e8987da028ec75cc3a0ccf27d815d50ae4 Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Wed, 14 Feb 2024 13:10:29 +0100 Subject: [PATCH 2/2] bus: address comments --- alerts/alerts.go | 12 +++++++----- bus/bus.go | 7 ++++--- bus/client/alerts.go | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/alerts/alerts.go b/alerts/alerts.go index c76898c8d..424196d4f 100644 --- a/alerts/alerts.go +++ b/alerts/alerts.go @@ -66,8 +66,8 @@ type ( } AlertsOpts struct { - Offset uint64 - Limit uint64 + Offset int + Limit int } ) @@ -176,12 +176,14 @@ func (m *Manager) DismissAlerts(ctx context.Context, ids ...types.Hash256) error } // Active returns the host's active alerts. -func (m *Manager) Active(offset, limit uint64) []Alert { +func (m *Manager) Active(offset, limit int) []Alert { m.mu.Lock() defer m.mu.Unlock() - if offset >= uint64(len(m.alerts)) { + if offset >= len(m.alerts) { return nil + } else if limit == -1 { + limit = len(m.alerts) } alerts := make([]Alert, 0, len(m.alerts)) @@ -192,7 +194,7 @@ func (m *Manager) Active(offset, limit uint64) []Alert { return alerts[i].Timestamp.After(alerts[j].Timestamp) }) alerts = alerts[offset:] - if limit < uint64(len(alerts)) { + if limit < len(alerts) { alerts = alerts[:limit] } return alerts diff --git a/bus/bus.go b/bus/bus.go index 82a89826f..4342e493b 100644 --- a/bus/bus.go +++ b/bus/bus.go @@ -1713,13 +1713,14 @@ func (b *bus) gougingParams(ctx context.Context) (api.GougingParams, error) { } func (b *bus) handleGETAlerts(jc jape.Context) { - var offset, limit uint64 + offset, limit := 0, -1 if jc.DecodeForm("offset", &offset) != nil { return } else if jc.DecodeForm("limit", &limit) != nil { return - } else if limit == 0 { - limit = math.MaxUint64 + } else if offset < 0 { + jc.Error(errors.New("offset must be non-negative"), http.StatusBadRequest) + return } jc.Encode(b.alertMgr.Active(offset, limit)) } diff --git a/bus/client/alerts.go b/bus/client/alerts.go index ab1d7f094..6151db598 100644 --- a/bus/client/alerts.go +++ b/bus/client/alerts.go @@ -12,7 +12,7 @@ import ( // Alerts fetches the active alerts from the bus. func (c *Client) Alerts(opts alerts.AlertsOpts) (alerts []alerts.Alert, err error) { values := url.Values{} - if opts.Offset > 0 { + if opts.Offset != 0 { values.Set("offset", fmt.Sprint(opts.Offset)) } if opts.Limit != 0 {