Added tests for Alerts

- tests for related cluster method, rest api, client method etc
- clean expired alerts everytime a new alerts come in

api/rest/client/methods.go

@@ -276,7 +276,7 @@ func (c *defaultClient) RecoverAll(ctx context.Context, local bool) ([]*api.Glob
 
 // Alerts returns things that are wrong with cluster.
 func (c *defaultClient) Alerts(ctx context.Context) (map[string]api.Alert, error) {
-	ctx, span := trace.StartSpan(ctx, "client/RecoverAll")
+	ctx, span := trace.StartSpan(ctx, "client/Alert")
 	defer span.End()
 
 	var alerts map[string]api.Alert

api/rest/client/methods_test.go

@@ -414,6 +414,29 @@ func TestRecoverAll(t *testing.T) {
 	testClients(t, api, testF)
 }
 
+func TestAlerts(t *testing.T) {
+	ctx := context.Background()
+	api := testAPI(t)
+	defer shutdown(api)
+
+	testF := func(t *testing.T, c Client) {
+		alerts, err := c.Alerts(ctx)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(alerts) != 1 {
+			t.Fatal("expected 1 alert")
+		}
+		pID2 := peer.IDB58Encode(test.PeerID2)
+		_, ok := alerts[pID2]
+		if !ok {
+			t.Errorf("expected an alert from %s", pID2)
+		}
+	}
+
+	testClients(t, api, testF)
+}
+
 func TestGetConnectGraph(t *testing.T) {
 	ctx := context.Background()
 	api := testAPI(t)

api/rest/restapi_test.go

@@ -849,6 +849,22 @@ func TestAPIMetricNamesEndpoint(t *testing.T) {
 	testBothEndpoints(t, tf)
 }
 
+func TestAPIAlertsEndpoint(t *testing.T) {
+	ctx := context.Background()
+	rest := testAPI(t)
+	defer rest.Shutdown(ctx)
+
+	tf := func(t *testing.T, url urlF) {
+		var resp map[string]api.Alert
+		makeGet(t, rest, url(rest)+"/alerts", &resp)
+		if len(resp) != 1 {
+			t.Error("expected one alert")
+		}
+	}
+
+	testBothEndpoints(t, tf)
+}
+
 func TestAPIStatusAllEndpoint(t *testing.T) {
 	ctx := context.Background()
 	rest := testAPI(t)

cluster.go

@@ -397,14 +397,14 @@ func (c *Cluster) Alerts() map[string]api.Alert {
 	alerts := make(map[string]api.Alert)
 
 	c.alertsMux.Lock()
-	defer c.alertsMux.Unlock()
-	for i, alert := range c.alerts {
-		if time.Now().Before(time.Unix(0, alert.Expiry)) {
-			alerts[i] = alert
-		} else {
-			delete(c.alerts, i)
+	{
+		for i, alert := range c.alerts {
+			if time.Now().Before(time.Unix(0, alert.Expiry)) {
+				alerts[i] = alert
+			}
 		}
 	}
+	c.alertsMux.Unlock()
 
 	return alerts
 }
@@ -418,6 +418,11 @@ func (c *Cluster) alertsHandler() {
 		case alrt := <-c.monitor.Alerts():
 			logger.Warningf("metric alert for %s: Peer: %s.", alrt.MetricName, alrt.Peer)
 			c.alertsMux.Lock()
+			for pID, alert := range c.alerts {
+				if time.Now().After(time.Unix(0, alert.Expiry)) {
+					delete(c.alerts, pID)
+				}
+			}
 			c.alerts[peer.IDB58Encode(alrt.Peer)] = *alrt
 			c.alertsMux.Unlock()
 

ipfscluster_test.go

@@ -416,19 +416,22 @@ func createClusters(t *testing.T) ([]*Cluster, []*test.IpfsMock) {
 }
 
 func shutdownClusters(t *testing.T, clusters []*Cluster, m []*test.IpfsMock) {
-	ctx := context.Background()
 	for i, c := range clusters {
-		err := c.Shutdown(ctx)
-		if err != nil {
-			t.Error(err)
-		}
-		c.dht.Close()
-		c.host.Close()
-		m[i].Close()
+		shutdownCluster(t, c, m[i])
 	}
 	os.RemoveAll(testsFolder)
 }
 
+func shutdownCluster(t *testing.T, c *Cluster, m *test.IpfsMock) {
+	err := c.Shutdown(context.Background())
+	if err != nil {
+		t.Error(err)
+	}
+	c.dht.Close()
+	c.host.Close()
+	m.Close()
+}
+
 func runF(t *testing.T, clusters []*Cluster, f func(*testing.T, *Cluster)) {
 	var wg sync.WaitGroup
 	for _, c := range clusters {
@@ -2065,3 +2068,18 @@ func TestClusterPinsWithExpiration(t *testing.T) {
 		t.Error("pin should not be part of the state")
 	}
 }
+
+func TestClusterAlerts(t *testing.T) {
+	clusters, mock := createClusters(t)
+	defer shutdownClusters(t, clusters, mock)
+
+	ttlDelay()
+	shutdownCluster(t, clusters[1], mock[1])
+	// config.MonitorPingInterval is kept at 1s
+	time.Sleep(2 * time.Second)
+
+	alerts := clusters[0].Alerts()
+	if len(alerts) != 1 {
+		t.Error("expected one alert")
+	}
+}

sharness/t0032-ctl-health.sh

@@ -25,6 +25,10 @@ test_expect_success IPFS,CLUSTER "list latest metrics logged by this peer" '
     ipfs-cluster-ctl health metrics freespace | grep -q -E "(^$pid \| freespace: [0-9]+ (G|M|K)B \| Expires in: [0-9]+ seconds from now)"
 '
 
+test_expect_success IPFS,CLUSTER "alerts must succeed" '
+    ipfs-cluster-ctl alerts
+'
+
 test_clean_ipfs
 test_clean_cluster
 

test/rpc_api_mock.go

@@ -349,6 +349,17 @@ func (mock *mockCluster) SendInformerMetric(ctx context.Context, in struct{}, ou
 	return nil
 }
 
+func (mock *mockCluster) Alerts(ctx context.Context, in struct{}, out *map[string]api.Alert) error {
+	*out = map[string]api.Alert{
+		peer.IDB58Encode(PeerID2): api.Alert{
+			Peer:       PeerID2,
+			MetricName: "ping",
+			Expiry:     time.Now().Add(30 * time.Second).UnixNano(),
+		},
+	}
+	return nil
+}
+
 /* Tracker methods */
 
 func (mock *mockPinTracker) Track(ctx context.Context, in *api.Pin, out *struct{}) error {