fix(edgestacks): reset stale status on edge group reassignment BE-13141 (#3275)

api/http/handler/edgegroups/edgegroup_update.go

@@ -182,6 +182,16 @@ func (handler *Handler) updateEndpointStacks(tx dataservices.DataStoreTx, endpoi
 		edgeStackSet[edgeStackID] = true
 	}
 
+	for edgeStackID := range edgeStackSet {
+		if relation.EdgeStacks[edgeStackID] {
+			continue
+		}
+
+		if err := tx.EdgeStackStatus().Clear(edgeStackID, []portainer.EndpointID{endpoint.ID}); err != nil {
+			return err
+		}
+	}
+
 	relation.EdgeStacks = edgeStackSet
 
 	return tx.EndpointRelation().UpdateEndpointRelation(endpoint.ID, relation)

api/http/handler/edgegroups/edgegroup_update_test.go

@@ -1,12 +1,15 @@
 package edgegroups
 
 import (
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 
 	portainer "github.com/portainer/portainer/api"
+	"github.com/portainer/portainer/api/dataservices"
 	"github.com/portainer/portainer/api/datastore"
 	"github.com/portainer/portainer/api/internal/testhelpers"
 	"github.com/portainer/portainer/api/roar"
@@ -15,6 +18,26 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+var errForcedClearFailure = errors.New("forced clear failure")
+
+// errClearEdgeStackStatusService forces Clear to fail so that error-propagation
+// paths in updateEndpointStacks can be exercised without a live DB failure.
+type errClearEdgeStackStatusService struct {
+	dataservices.EdgeStackStatusService
+}
+
+func (errClearEdgeStackStatusService) Clear(portainer.EdgeStackID, []portainer.EndpointID) error {
+	return errForcedClearFailure
+}
+
+type errClearDataStoreTx struct {
+	dataservices.DataStoreTx
+}
+
+func (errClearDataStoreTx) EdgeStackStatus() dataservices.EdgeStackStatusService {
+	return errClearEdgeStackStatusService{}
+}
+
 func TestEdgeGroupUpdateHandler(t *testing.T) {
 	t.Parallel()
 	handler, store := newHandlerWithEdgeEndpoints(t)
@@ -44,6 +67,92 @@ func TestEdgeGroupUpdateHandler(t *testing.T) {
 	require.ElementsMatch(t, []portainer.EndpointID{1, 2, 3}, responseGroup.Endpoints)
 }
 
+// TestEdgeGroupUpdateClearsStaleStatusOnEndpointReassignment reproduces
+// BE-13141: an endpoint newly added to an Edge group that is related to an
+// Edge stack must not keep showing the status left over from a previous
+// deployment to that stack.
+func TestEdgeGroupUpdateClearsStaleStatusOnEndpointReassignment(t *testing.T) {
+	t.Parallel()
+	handler, store := newHandlerWithEdgeEndpoints(t)
+
+	err := store.EdgeGroup().Create(&portainer.EdgeGroup{
+		ID:          1,
+		Name:        "Test Edge Group",
+		EndpointIDs: roar.FromSlice([]portainer.EndpointID{2, 3}),
+	})
+	require.NoError(t, err)
+
+	edgeStack := portainer.EdgeStack{
+		ID:             1,
+		Name:           "test-edge-stack",
+		CreationDate:   time.Now().Unix(),
+		EdgeGroups:     []portainer.EdgeGroupID{1},
+		ProjectPath:    "/project/path",
+		EntryPoint:     "entrypoint",
+		ManifestPath:   "/manifest/path",
+		DeploymentType: portainer.EdgeStackDeploymentKubernetes,
+	}
+	err = store.EdgeStack().Create(edgeStack.ID, &edgeStack)
+	require.NoError(t, err)
+
+	staleStatus := &portainer.EdgeStackStatusForEnv{
+		EndpointID: 1,
+		Status: []portainer.EdgeStackDeploymentStatus{
+			{Time: 1, Type: portainer.EdgeStackStatusError, Error: "boom"},
+		},
+	}
+
+	err = store.EdgeStackStatus().Create(edgeStack.ID, 1, staleStatus)
+	require.NoError(t, err)
+
+	rr := httptest.NewRecorder()
+
+	req := httptest.NewRequest(
+		http.MethodPut,
+		"/edge_groups/1",
+		strings.NewReader(`{"Endpoints": [1, 2, 3]}`),
+	)
+
+	handler.ServeHTTP(rr, req)
+	require.Equal(t, http.StatusOK, rr.Result().StatusCode)
+
+	status, err := store.EdgeStackStatus().Read(edgeStack.ID, 1)
+	require.NoError(t, err)
+	require.Empty(t, status.Status)
+}
+
+// TestUpdateEndpointStacksPropagatesClearError ensures a failure to clear the
+// stale Edge stack status aborts the relation update instead of being
+// silently ignored.
+func TestUpdateEndpointStacksPropagatesClearError(t *testing.T) {
+	t.Parallel()
+	handler, store := newHandlerWithEdgeEndpoints(t)
+
+	endpoint, err := store.Endpoint().Endpoint(1)
+	require.NoError(t, err)
+
+	edgeGroup := portainer.EdgeGroup{
+		ID:          1,
+		Name:        "Test Edge Group",
+		EndpointIDs: roar.FromSlice([]portainer.EndpointID{endpoint.ID}),
+	}
+	err = store.EdgeGroup().Create(&edgeGroup)
+	require.NoError(t, err)
+
+	edgeStack := portainer.EdgeStack{
+		ID:         1,
+		Name:       "test-edge-stack",
+		EdgeGroups: []portainer.EdgeGroupID{edgeGroup.ID},
+	}
+	err = store.EdgeStack().Create(edgeStack.ID, &edgeStack)
+	require.NoError(t, err)
+
+	err = store.UpdateTx(func(tx dataservices.DataStoreTx) error {
+		return handler.updateEndpointStacks(errClearDataStoreTx{DataStoreTx: tx}, endpoint, []portainer.EdgeGroup{edgeGroup}, []portainer.EdgeStack{edgeStack})
+	})
+	require.ErrorIs(t, err, errForcedClearFailure)
+}
+
 func TestEdgeGroupUpdatePanic(t *testing.T) {
 	t.Parallel()
 	_, store := datastore.MustNewTestStore(t, false, true)

api/http/handler/edgestacks/edgestack_update.go

@@ -158,6 +158,10 @@ func (handler *Handler) handleChangeEdgeGroups(tx dataservices.DataStoreTx, edge
 		if err := tx.EndpointRelation().AddEndpointRelationsForEdgeStack(relatedEnvironmentsToAdd.Keys(), edgeStack); err != nil {
 			return nil, nil, errors.WithMessage(err, "Unable to add edge stack relations to the database")
 		}
+
+		if err := tx.EdgeStackStatus().Clear(edgeStack.ID, relatedEnvironmentsToAdd.Keys()); err != nil {
+			return nil, nil, errors.WithMessage(err, "Unable to clear edge stack status for the newly related environments")
+		}
 	}
 
 	return newRelatedEnvironmentIDs, relatedEnvironmentsToAdd, nil

api/http/handler/edgestacks/edgestack_update_test.go

@@ -2,6 +2,7 @@ package edgestacks
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -9,12 +10,34 @@ import (
 	"testing"
 
 	portainer "github.com/portainer/portainer/api"
+	"github.com/portainer/portainer/api/dataservices"
+	"github.com/portainer/portainer/api/internal/edge"
 	"github.com/portainer/portainer/api/roar"
 
 	"github.com/segmentio/encoding/json"
 	"github.com/stretchr/testify/require"
 )
 
+var errForcedClearFailure = errors.New("forced clear failure")
+
+// errClearEdgeStackStatusService forces Clear to fail so that error-propagation
+// paths in handleChangeEdgeGroups can be exercised without a live DB failure.
+type errClearEdgeStackStatusService struct {
+	dataservices.EdgeStackStatusService
+}
+
+func (errClearEdgeStackStatusService) Clear(portainer.EdgeStackID, []portainer.EndpointID) error {
+	return errForcedClearFailure
+}
+
+type errClearDataStoreTx struct {
+	dataservices.DataStoreTx
+}
+
+func (errClearDataStoreTx) EdgeStackStatus() dataservices.EdgeStackStatusService {
+	return errClearEdgeStackStatusService{}
+}
+
 // Update
 func TestUpdateAndInspect(t *testing.T) {
 	t.Parallel()
@@ -103,6 +126,91 @@ func TestUpdateAndInspect(t *testing.T) {
 	}
 }
 
+// TestUpdateEdgeGroupsClearsStaleStatusOnReassignment reproduces BE-13141: an
+// endpoint that is removed from an edge stack's edge groups and later
+// re-added must not keep showing the status from its previous deployment.
+func TestUpdateEdgeGroupsClearsStaleStatusOnReassignment(t *testing.T) {
+	t.Parallel()
+	handler, rawAPIKey := setupHandler(t)
+
+	endpoint := createEndpoint(t, handler.DataStore)
+	edgeStack := createEdgeStack(t, handler.DataStore, endpoint.ID)
+
+	staleStatus := &portainer.EdgeStackStatusForEnv{
+		EndpointID: endpoint.ID,
+		Status: []portainer.EdgeStackDeploymentStatus{
+			{Time: 1, Type: portainer.EdgeStackStatusError, Error: "boom"},
+		},
+	}
+
+	err := handler.DataStore.EdgeStackStatus().Create(edgeStack.ID, endpoint.ID, staleStatus)
+	require.NoError(t, err)
+
+	emptyEdgeGroup := portainer.EdgeGroup{
+		ID:   2,
+		Name: "EdgeGroup 2",
+	}
+
+	err = handler.DataStore.EdgeGroup().Create(&emptyEdgeGroup)
+	require.NoError(t, err)
+
+	// Reassign the stack away from the endpoint's group.
+	updateEdgeStackRequest(t, handler, rawAPIKey, edgeStack.ID, updateEdgeStackPayload{
+		StackFileContent: "update-test",
+		EdgeGroups:       []portainer.EdgeGroupID{emptyEdgeGroup.ID},
+		DeploymentType:   portainer.EdgeStackDeploymentCompose,
+	})
+
+	// Re-assign the stack back to the endpoint's original group.
+	updateEdgeStackRequest(t, handler, rawAPIKey, edgeStack.ID, updateEdgeStackPayload{
+		StackFileContent: "update-test",
+		EdgeGroups:       edgeStack.EdgeGroups,
+		DeploymentType:   portainer.EdgeStackDeploymentCompose,
+	})
+
+	status, err := handler.DataStore.EdgeStackStatus().Read(edgeStack.ID, endpoint.ID)
+	require.NoError(t, err)
+	require.Empty(t, status.Status)
+}
+
+// TestHandleChangeEdgeGroupsPropagatesClearError ensures a failure to clear
+// the stale Edge stack status aborts the edge groups change instead of being
+// silently ignored.
+func TestHandleChangeEdgeGroupsPropagatesClearError(t *testing.T) {
+	t.Parallel()
+	handler, _ := setupHandler(t)
+
+	endpoint := createEndpoint(t, handler.DataStore)
+	edgeStack := createEdgeStack(t, handler.DataStore, endpoint.ID)
+
+	err := handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error {
+		relationConfig, err := edge.FetchEndpointRelationsConfig(tx)
+		require.NoError(t, err)
+
+		_, _, err = handler.handleChangeEdgeGroups(errClearDataStoreTx{DataStoreTx: tx}, &edgeStack, edgeStack.EdgeGroups, nil, relationConfig)
+
+		return err
+	})
+	require.ErrorIs(t, err, errForcedClearFailure)
+}
+
+func updateEdgeStackRequest(t *testing.T, handler *Handler, rawAPIKey string, edgeStackID portainer.EdgeStackID, payload updateEdgeStackPayload) {
+	t.Helper()
+
+	jsonPayload, err := json.Marshal(payload)
+	require.NoError(t, err)
+
+	r := bytes.NewBuffer(jsonPayload)
+	req, err := http.NewRequest(http.MethodPut, fmt.Sprintf("/edge_stacks/%d", edgeStackID), r)
+	require.NoError(t, err)
+
+	req.Header.Add("x-api-key", rawAPIKey)
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	require.Equal(t, http.StatusOK, rec.Code)
+}
+
 func TestUpdateWithInvalidEdgeGroups(t *testing.T) {
 	t.Parallel()
 	handler, rawAPIKey := setupHandler(t)