fix: Add pre-checks to full-refresh endpoint to avoid misleading success responses (#622)

Author: bogdan-ts

engine/internal/retrieval/retrieval.go

@@ -76,6 +76,12 @@ type Scheduler struct {
 	Spec cron.Schedule
 }
 
+var (
+	ErrRefreshInProgress = errors.New("The data refresh/snapshot is currently in progress. Skip a new data refresh iteration")
+	ErrRefreshPending    = errors.New("Data retrieving suspended because Retrieval state is pending")
+	ErrNoAvailablePool   = errors.New("Pool to perform full refresh not found. Skip refreshing")
+)
+
 // New creates a new data retrieval.
 func New(cfg *dblabCfg.Config, engineProps *global.EngineProps, docker *client.Client, pm *pool.Manager, tm *telemetry.Agent,
 	runner runners.Runner) (*Retrieval, error) {
@@ -572,20 +578,20 @@ func (r *Retrieval) refreshFunc(ctx context.Context) func() {
 
 // FullRefresh performs full refresh for an unused storage pool and makes it active.
 func (r *Retrieval) FullRefresh(ctx context.Context) error {
-	if r.State.Status == models.Refreshing || r.State.Status == models.Snapshotting {
-		alert := telemetry.Alert{
-			Level:   models.RefreshSkipped,
-			Message: "The data refresh/snapshot is currently in progress. Skip a new data refresh iteration",
-		}
-		r.State.addAlert(alert)
-		r.tm.SendEvent(ctx, telemetry.AlertEvent, alert)
-		log.Msg(alert.Message)
-
-		return nil
-	}
+	if err := r.CanStartRefresh(); err != nil {
+		switch {
+		case errors.Is(err, ErrRefreshInProgress):
+			alert := telemetry.Alert{
+				Level:   models.RefreshSkipped,
+				Message: err.Error(),
+			}
+			r.State.addAlert(alert)
+			r.tm.SendEvent(ctx, telemetry.AlertEvent, alert)
+			log.Msg(alert.Message)
 
-	if r.State.Status == models.Pending {
-		log.Msg("Data retrieving suspended because Retrieval state is pending")
+		case errors.Is(err, ErrRefreshPending):
+			log.Msg(err.Error())
+		}
 
 		return nil
 	}
@@ -597,21 +603,22 @@ func (r *Retrieval) FullRefresh(ctx context.Context) error {
 
 	runCtx, cancel := context.WithCancel(ctx)
 	r.ctxCancel = cancel
-	elementToUpdate := r.poolManager.GetPoolToUpdate()
 
-	if elementToUpdate == nil || elementToUpdate.Value == nil {
+	if err := r.HasAvailablePool(); err != nil {
 		alert := telemetry.Alert{
 			Level:   models.RefreshSkipped,
-			Message: "Pool to perform full refresh not found. Skip refreshing",
+			Message: err.Error(),
 		}
 		r.State.addAlert(alert)
 		r.tm.SendEvent(ctx, telemetry.AlertEvent, alert)
-		log.Msg(alert.Message + ". Hint: Check that there is at least one pool that does not have clones running. " +
+		log.Msg(err.Error() + ". Hint: Check that there is at least one pool that does not have clones running. " +
 			"Refresh can be performed only to a pool without clones.")
 
 		return nil
 	}
 
+	elementToUpdate := r.poolManager.GetPoolToUpdate()
+
 	poolToUpdate, err := r.poolManager.GetFSManager(elementToUpdate.Value.(string))
 	if err != nil {
 		return errors.Wrap(err, "failed to get FSManager")
@@ -775,3 +782,24 @@ func (r *Retrieval) reportContainerSyncStatus(ctx context.Context, containerID s
 
 	return value, nil
 }
+
+func (r *Retrieval) CanStartRefresh() error {
+	if r.State.Status == models.Refreshing || r.State.Status == models.Snapshotting {
+		return ErrRefreshInProgress
+	}
+
+	if r.State.Status == models.Pending {
+		return ErrRefreshPending
+	}
+
+	return nil
+}
+
+func (r *Retrieval) HasAvailablePool() error {
+	element := r.poolManager.GetPoolToUpdate()
+	if element == nil || element.Value == nil {
+		return ErrNoAvailablePool
+	}
+
+	return nil
+}

engine/internal/srv/routes.go

@@ -952,6 +952,16 @@ func (s *Server) healthCheck(w http.ResponseWriter, _ *http.Request) {
 }
 
 func (s *Server) refresh(w http.ResponseWriter, r *http.Request) {
+	if err := s.Retrieval.CanStartRefresh(); err != nil {
+		api.SendBadRequestError(w, r, err.Error())
+		return
+	}
+
+	if err := s.Retrieval.HasAvailablePool(); err != nil {
+		api.SendBadRequestError(w, r, err.Error())
+		return
+	}
+
 	go func() {
 		if err := s.Retrieval.FullRefresh(context.Background()); err != nil {
 			log.Err("failed to initiate full refresh", err)
@@ -963,6 +973,5 @@ func (s *Server) refresh(w http.ResponseWriter, r *http.Request) {
 		Message: "Full refresh started",
 	}); err != nil {
 		api.SendError(w, r, err)
-		return
 	}
 }