From e08d15f221945540546d7d12a55adc58e16f843b Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:20:22 -0300 Subject: [PATCH] fix(edgestacks): reset stale status on edge group reassignment BE-13141 (#3275) --- .../handler/edgegroups/edgegroup_update.go | 10 ++ .../edgegroups/edgegroup_update_test.go | 109 ++++++++++++++++++ .../handler/edgestacks/edgestack_update.go | 4 + .../edgestacks/edgestack_update_test.go | 108 +++++++++++++++++ 4 files changed, 231 insertions(+) diff --git a/api/http/handler/edgegroups/edgegroup_update.go b/api/http/handler/edgegroups/edgegroup_update.go index c2d84975ef..a6a2dd6259 100644 --- a/api/http/handler/edgegroups/edgegroup_update.go +++ b/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) diff --git a/api/http/handler/edgegroups/edgegroup_update_test.go b/api/http/handler/edgegroups/edgegroup_update_test.go index b14ed317fe..a26ad0c44c 100644 --- a/api/http/handler/edgegroups/edgegroup_update_test.go +++ b/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) diff --git a/api/http/handler/edgestacks/edgestack_update.go b/api/http/handler/edgestacks/edgestack_update.go index efb4d6d202..5341709ad6 100644 --- a/api/http/handler/edgestacks/edgestack_update.go +++ b/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 diff --git a/api/http/handler/edgestacks/edgestack_update_test.go b/api/http/handler/edgestacks/edgestack_update_test.go index 1665f39ac1..3dcd2e6e0f 100644 --- a/api/http/handler/edgestacks/edgestack_update_test.go +++ b/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)