Only report resize errors with no change in outcome

sapcc · Mar 26, 2020 · bf2c1db · bf2c1db
1 parent a9e13bc
commit bf2c1db
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 30 deletions.
diff --git a/internal/api/errors.go b/internal/api/errors.go
@@ -121,8 +121,8 @@ func (h handler) GetAssetScrapeErrors(w http.ResponseWriter, r *http.Request) {
 
 	for _, res := range dbResources {
 		var dbAssets []db.Asset
-		_, err := h.DB.Select(&dbAssets,
-			`SELECT * FROM assets
+		_, err := h.DB.Select(&dbAssets, `
+			SELECT * FROM assets
 			 WHERE scrape_error_message != '' AND resource_id = $1
 			 ORDER BY id
 			`, res.ID)
@@ -164,23 +164,27 @@ func (h handler) GetAssetResizeErrors(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	var result struct {
-		AssetResizeErrors []AssetError `json:"asset_resize_errors"`
-	}
-
 	var dbResources []db.Resource
 	_, err := h.DB.Select(&dbResources,
 		`SELECT * FROM resources ORDER BY id`)
 	if respondwith.ErrorText(w, err) {
 		return
 	}
 
+	assetResizeErrs := []AssetError{}
 	for _, res := range dbResources {
 		var ops []db.FinishedOperation
+		//We only care about assets that are still problematic.
+		//So we want to skip "errored" operations where a more recent operation
+		//on the same asset finished as "succeeded", "cancelled", or "failed".
 		_, err := h.DB.Select(&ops, `
-			SELECT o.* FROM finished_operations o
-				JOIN assets a ON a.id = o.asset_id
-			 WHERE a.resource_id = $1
+			WITH latest_finished_operations AS (
+				SELECT DISTINCT ON (asset_id) o.* FROM finished_operations o
+					JOIN assets a ON a.id = o.asset_id
+				 WHERE a.resource_id = $1
+				 ORDER BY o.asset_id, o.finished_at DESC
+			)
+			SELECT * FROM latest_finished_operations WHERE outcome = 'errored'
 		`, res.ID)
 		if respondwith.ErrorText(w, err) {
 			return
@@ -199,24 +203,23 @@ func (h handler) GetAssetResizeErrors(w http.ResponseWriter, r *http.Request) {
 		}
 
 		for _, o := range ops {
-			if o.Outcome == db.OperationOutcomeErrored {
-				//We are only interested in the status errored.
-				result.AssetResizeErrors = append(result.AssetResizeErrors,
-					AssetError{
-						AssetUUID:   assetUUIDs[o.AssetID],
-						ProjectUUID: projectID,
-						DomainUUID:  res.DomainUUID,
-						AssetType:   string(res.AssetType),
-						OldSize:     o.OldSize,
-						NewSize:     o.NewSize,
-						Finished: &Checked{
-							AtUnix:       o.FinishedAt.Unix(),
-							ErrorMessage: o.ErrorMessage,
-						},
-					})
-			}
+			assetResizeErrs = append(assetResizeErrs,
+				AssetError{
+					AssetUUID:   assetUUIDs[o.AssetID],
+					ProjectUUID: projectID,
+					DomainUUID:  res.DomainUUID,
+					AssetType:   string(res.AssetType),
+					OldSize:     o.OldSize,
+					NewSize:     o.NewSize,
+					Finished: &Checked{
+						AtUnix:       o.FinishedAt.Unix(),
+						ErrorMessage: o.ErrorMessage,
+					},
+				})
 		}
 	}
 
-	respondwith.JSON(w, http.StatusOK, result)
+	respondwith.JSON(w, http.StatusOK, struct {
+		AssetScrapeErrors []AssetError `json:"asset_resize_errors,keepempty"`
+	}{assetResizeErrs})
 }
diff --git a/internal/api/errors_test.go b/internal/api/errors_test.go
@@ -21,6 +21,7 @@ package api
 import (
 	"net/http"
 	"testing"
+	"time"
 
 	"github.com/sapcc/castellum/internal/db"
 	"github.com/sapcc/castellum/internal/test"
@@ -84,7 +85,6 @@ func TestGetAssetScrapeErrors(baseT *testing.T) {
 		}.Check(t.T, hh)
 		mv.Allow("cluster:access")
 
-		//happy path
 		assert.HTTPRequest{
 			Method:       "GET",
 			Path:         "/v1/admin/asset-scrape-errors",
@@ -120,8 +120,8 @@ func TestGetAssetResizeErrors(baseT *testing.T) {
 		}.Check(t.T, hh)
 		mv.Allow("cluster:access")
 
-		//happy path
-		assert.HTTPRequest{
+		//check that the "errored" resize operation is rendered properly
+		req := assert.HTTPRequest{
 			Method:       "GET",
 			Path:         "/v1/admin/asset-resize-errors",
 			ExpectStatus: http.StatusOK,
@@ -141,6 +141,26 @@ func TestGetAssetResizeErrors(baseT *testing.T) {
 					},
 				},
 			},
-		}.Check(t.T, hh)
+		}
+		req.Check(t.T, hh)
+
+		//add a new operation on the same asset that results with outcome
+		//"succeeded" and check that we get an empty list
+		t.Must(h.DB.Insert(&db.FinishedOperation{
+			AssetID:      1,
+			Reason:       db.OperationReasonCritical,
+			Outcome:      db.OperationOutcomeSucceeded,
+			OldSize:      1024,
+			NewSize:      1025,
+			UsagePercent: 97,
+			CreatedAt:    time.Unix(70, 0).UTC(),
+			ConfirmedAt:  p2time(time.Unix(71, 0).UTC()),
+			GreenlitAt:   p2time(time.Unix(71, 0).UTC()),
+			FinishedAt:   time.Unix(73, 0).UTC(),
+		}))
+		req.ExpectBody = assert.JSONObject{
+			"asset_resize_errors": []assert.JSONObject{},
+		}
+		req.Check(t.T, hh)
 	})
 }