Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions control/contract_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,9 @@ func (fakeEnvironmentRepository) UpdateEnvironment(context.Context, control.Work
func (fakeEnvironmentRepository) DeleteEnvironment(context.Context, control.WorkspaceID, control.EnvironmentID) error {
return nil
}
func (fakeEnvironmentRepository) DeleteEnvironmentUnlessReferenced(context.Context, control.WorkspaceID, control.EnvironmentID, []control.SessionState) error {
return nil
}
func (fakeEnvironmentRepository) CountSessionsByEnvironment(context.Context, control.WorkspaceID, control.EnvironmentID, []control.SessionState) (int, error) {
return 0, nil
}
Expand Down
4 changes: 4 additions & 0 deletions control/ports.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ type SessionRepository interface {
// ListEnvironments → Store.ListEnvironments
// UpdateEnvironment → Store.UpdateEnvironment
// DeleteEnvironment → Store.DeleteEnvironment
// DeleteEnvironmentUnlessReferenced -> Store.DeleteEnvironmentUnlessReferenced
// CountSessionsByEnvironment → Store.CountSessionsByEnvironment
// SetEnvironmentSnapshot → Store.SetEnvironmentSnapshot
type EnvironmentRepository interface {
Expand All @@ -122,6 +123,9 @@ type EnvironmentRepository interface {
ListEnvironments(ctx context.Context, ws WorkspaceID, q EnvironmentQuery) ([]Environment, string, error)
UpdateEnvironment(ctx context.Context, ws WorkspaceID, e Environment) (Environment, error)
DeleteEnvironment(ctx context.Context, ws WorkspaceID, id EnvironmentID) error
// DeleteEnvironmentUnlessReferenced atomically deletes envID unless a
// session in states still references it: ErrConflict then, ErrNotFound if envID is already gone.
DeleteEnvironmentUnlessReferenced(ctx context.Context, ws WorkspaceID, id EnvironmentID, states []SessionState) error
// CountSessionsByEnvironment counts sessions on envID whose state is in
// states; an empty states counts every session on the environment.
CountSessionsByEnvironment(ctx context.Context, ws WorkspaceID, envID EnvironmentID, states []SessionState) (int, error)
Expand Down
17 changes: 6 additions & 11 deletions controlapp/environments.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,8 @@ func (s *EnvironmentService) UpdateEnvironment(ctx context.Context, scope contro
}

// DeleteEnvironment reads and authorizes the environment, then refuses the
// delete while any non-terminal session still references it. A repository
// ErrNotFound remains ErrNotFound even if another workspace holds the same
// opaque ID.
// delete while any non-terminal session still references it, in one
// repository call. ErrNotFound persists even under another workspace's ID.
func (s *EnvironmentService) DeleteEnvironment(ctx context.Context, scope control.Scope, cmd control.DeleteEnvironment) error {
if err := scope.Validate(); err != nil {
return control.ErrInvalid
Expand All @@ -253,18 +252,14 @@ func (s *EnvironmentService) DeleteEnvironment(ctx context.Context, scope contro
return control.ErrDenied
}

n, err := s.environments.CountSessionsByEnvironment(ctx, scope.WorkspaceID, cmd.ID, control.NonTerminal)
if err != nil {
return control.ErrUnavailable
}
if n != 0 {
return control.ErrConflict
}
return s.uow.Run(ctx, func(ctx context.Context) error {
if err := s.environments.DeleteEnvironment(ctx, scope.WorkspaceID, cmd.ID); err != nil {
if err := s.environments.DeleteEnvironmentUnlessReferenced(ctx, scope.WorkspaceID, cmd.ID, control.NonTerminal); err != nil {
if errors.Is(err, control.ErrNotFound) {
return control.ErrNotFound
}
if errors.Is(err, control.ErrConflict) {
return control.ErrConflict
}
return control.ErrUnavailable
}
return recordEvent(ctx, s.ids, s.events, s.clock, scope, control.ActionDelete,
Expand Down
4 changes: 2 additions & 2 deletions controlapp/environments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,8 @@ func TestDeleteEnvironmentGuard(t *testing.T) {
if err := f.svc.DeleteEnvironment(context.Background(), sessionTestScope(), control.DeleteEnvironment{ID: "env_example"}); !errors.Is(err, control.ErrConflict) {
t.Fatalf("delete with live session: got %v, want ErrConflict", err)
}
if f.log.has("environments:delete") {
t.Fatalf("delete reached the repository despite a live session: %v", f.log.snapshot())
if _, ok := f.repo.rows["env_example"]; !ok {
t.Fatalf("environment removed despite a live session")
}

// No live session deletes and records an event.
Expand Down
3 changes: 3 additions & 0 deletions controlapp/fleet_external_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ func (fleetExtEnvironments) UpdateEnvironment(context.Context, control.Workspace
func (fleetExtEnvironments) DeleteEnvironment(context.Context, control.WorkspaceID, control.EnvironmentID) error {
return control.ErrUnsupported
}
func (fleetExtEnvironments) DeleteEnvironmentUnlessReferenced(context.Context, control.WorkspaceID, control.EnvironmentID, []control.SessionState) error {
return control.ErrUnsupported
}
func (fleetExtEnvironments) CountSessionsByEnvironment(context.Context, control.WorkspaceID, control.EnvironmentID, []control.SessionState) (int, error) {
return 0, nil
}
Expand Down
3 changes: 3 additions & 0 deletions controlapp/fleet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,9 @@ func (f *fleetFakeEnvironments) UpdateEnvironment(context.Context, control.Works
func (f *fleetFakeEnvironments) DeleteEnvironment(context.Context, control.WorkspaceID, control.EnvironmentID) error {
return control.ErrUnsupported
}
func (f *fleetFakeEnvironments) DeleteEnvironmentUnlessReferenced(context.Context, control.WorkspaceID, control.EnvironmentID, []control.SessionState) error {
return control.ErrUnsupported
}
func (f *fleetFakeEnvironments) CountSessionsByEnvironment(context.Context, control.WorkspaceID, control.EnvironmentID, []control.SessionState) (int, error) {
return 0, nil
}
Expand Down
27 changes: 27 additions & 0 deletions controlapp/repotest/repotest.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ func cases() []suiteCase {
{"E6 guarded snapshot and its capability", caseEnvironmentSnapshot},
{"E7 count sessions by environment", caseCountSessionsByEnvironment},
{"E8 an empty workspace is invalid on every environment method", caseEnvironmentEmptyWorkspace},
{"E9 delete-unless-referenced guards, then deletes, then reports gone", caseDeleteEnvironmentUnlessReferenced},

{"F1 runner round trip and order", caseRunnerRoundTrip},
{"F2 runners are isolated by pool", caseRunnerPoolIsolation},
Expand Down Expand Up @@ -1097,6 +1098,32 @@ func caseCountSessionsByEnvironment(t *testing.T, s Stores) {
}
}

// caseDeleteEnvironmentUnlessReferenced (E9) pins the guard's contract:
// refuse while a session in the given states still references the
// environment, delete once none does, report ErrNotFound once it is gone.
func caseDeleteEnvironmentUnlessReferenced(t *testing.T, s Stores) {
ctx := context.Background()

env := mustCreateEnv(t, s, Alpha, fixtureEnvironment("env_seq", "dev"))
mustCreate(t, s, Alpha, control.Session{
ID: "sess_seq", CreatorID: "act_a", State: control.StateQueued, PoolID: PoolA, EnvironmentID: env.ID})
if err := s.Environments.DeleteEnvironmentUnlessReferenced(ctx, Alpha, env.ID, control.NonTerminal); !errors.Is(err, control.ErrConflict) {
t.Fatalf("delete with a live session: err = %v, want ErrConflict", err)
}
if _, err := s.Environments.GetEnvironment(ctx, Alpha, env.ID); err != nil {
t.Fatalf("environment removed despite a live session: %v", err)
}
if err := s.Sessions.Transition(ctx, Alpha, "sess_seq", control.NonTerminal, control.StateDestroyed, control.TransitionOpts{}); err != nil {
t.Fatal(err)
}
if err := s.Environments.DeleteEnvironmentUnlessReferenced(ctx, Alpha, env.ID, control.NonTerminal); err != nil {
t.Fatalf("delete once the only session is terminal: %v", err)
}
if err := s.Environments.DeleteEnvironmentUnlessReferenced(ctx, Alpha, env.ID, control.NonTerminal); !errors.Is(err, control.ErrNotFound) {
t.Fatalf("delete an already-gone environment: err = %v, want ErrNotFound", err)
}
}

// caseEnvironmentEmptyWorkspace (E8) pins that no environment method accepts
// an unscoped call.
func caseEnvironmentEmptyWorkspace(t *testing.T, s Stores) {
Expand Down
3 changes: 3 additions & 0 deletions controlapp/sessions_external_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ func (sessionExtEnvironmentRepo) UpdateEnvironment(context.Context, control.Work
func (sessionExtEnvironmentRepo) DeleteEnvironment(context.Context, control.WorkspaceID, control.EnvironmentID) error {
return nil
}
func (sessionExtEnvironmentRepo) DeleteEnvironmentUnlessReferenced(context.Context, control.WorkspaceID, control.EnvironmentID, []control.SessionState) error {
return nil
}
func (sessionExtEnvironmentRepo) CountSessionsByEnvironment(context.Context, control.WorkspaceID, control.EnvironmentID, []control.SessionState) (int, error) {
return 0, nil
}
Expand Down
20 changes: 20 additions & 0 deletions controlapp/sessions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,26 @@ func (r *sessionStubEnvironmentRepo) CountSessionsByEnvironment(ctx context.Cont
return r.liveSessionCount, nil
}

func (r *sessionStubEnvironmentRepo) DeleteEnvironmentUnlessReferenced(ctx context.Context, ws control.WorkspaceID, id control.EnvironmentID, states []control.SessionState) error {
r.log.add("environments:delete-unless-referenced")
r.lastCountStates = states
if r.countErr != nil {
return r.countErr
}
if r.liveSessionCount != 0 {
return control.ErrConflict
}
if r.deleteErr != nil {
return r.deleteErr
}
e, ok := r.rows[id]
if !ok || e.WorkspaceID != ws {
return control.ErrNotFound
}
delete(r.rows, id)
return nil
}

func (r *sessionStubEnvironmentRepo) SetEnvironmentSnapshot(ctx context.Context, ws control.WorkspaceID, envID control.EnvironmentID, expectHash, ref string, runnerID control.RunnerID) error {
r.log.add("environments:set-snapshot")
return nil
Expand Down
5 changes: 5 additions & 0 deletions controlapp/uow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ func (r *uowEnvRepo) DeleteEnvironment(ctx context.Context, ws control.Workspace
return r.sessionStubEnvironmentRepo.DeleteEnvironment(ctx, ws, id)
}

func (r *uowEnvRepo) DeleteEnvironmentUnlessReferenced(ctx context.Context, ws control.WorkspaceID, id control.EnvironmentID, states []control.SessionState) error {
r.deleteDepth = unitDepth(ctx)
return r.sessionStubEnvironmentRepo.DeleteEnvironmentUnlessReferenced(ctx, ws, id, states)
}

// uowRecorder is the event recorder reporting the depth each record ran at.
type uowRecorder struct {
*sessionStubEventRecorder
Expand Down
74 changes: 74 additions & 0 deletions internal/controld/environment_delete_race_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// internal/controld/environment_delete_race_test.go
package controld

import (
"context"
"net/http"
"testing"

"github.com/tokencanopy/rainier/control"
)

// raceCreateSessionStore wraps MemStore to seed a session against an
// environment the moment its own delete call runs, the latest point a
// concurrent CreateSession could still land.
type raceCreateSessionStore struct {
MemStore
envID control.EnvironmentID
triggered bool
}

func (r *raceCreateSessionStore) Environments() control.EnvironmentRepository {
return raceCreateSessionEnvironments{EnvironmentRepository: r.MemStore.Environments(), owner: r}
}

type raceCreateSessionEnvironments struct {
control.EnvironmentRepository
owner *raceCreateSessionStore
}

func (r raceCreateSessionEnvironments) seedRacer(ctx context.Context, ws control.WorkspaceID, id control.EnvironmentID) {
if o := r.owner; !o.triggered && id == o.envID {
o.triggered = true
if _, err := o.MemStore.Sessions().CreateSession(ctx, ws, control.Session{
ID: "sess_racer", CreatorID: "usr_test", State: control.StateQueued,
PoolID: installPool, EnvironmentID: id,
}); err != nil {
panic("raceCreateSessionStore: seeding the racer session: " + err.Error())
}
}
}

func (r raceCreateSessionEnvironments) DeleteEnvironment(ctx context.Context, ws control.WorkspaceID, id control.EnvironmentID) error {
r.seedRacer(ctx, ws, id)
return r.EnvironmentRepository.DeleteEnvironment(ctx, ws, id)
}

func (r raceCreateSessionEnvironments) DeleteEnvironmentUnlessReferenced(ctx context.Context, ws control.WorkspaceID, id control.EnvironmentID, states []control.SessionState) error {
r.seedRacer(ctx, ws, id)
return r.EnvironmentRepository.DeleteEnvironmentUnlessReferenced(ctx, ws, id, states)
}

// TestDeleteEnvironmentSeesASessionCreatedRightBeforeItsOwnDelete pins a
// session created immediately before the delete's own repository call:
// the guard must still see it, whichever delete path is live.
func TestDeleteEnvironmentSeesASessionCreatedRightBeforeItsOwnDelete(t *testing.T) {
race := &raceCreateSessionStore{MemStore: NewMemStore()}
_, ts := newTestControldOver(t, race)
_, adminTok := loginUser(t, race, "root", "admin")
created := createEnv(t, ts, adminTok, envCreateBody("dev", nil))
race.envID = control.EnvironmentID(created.ID)

resp := doRequest(t, ts, http.MethodDelete, "/v0/environments/"+created.ID, adminTok, nil, nil)
raw := readBody(t, resp)
if !race.triggered {
t.Fatalf("the racer session was never seeded; this test proves nothing")
}
if resp.StatusCode != http.StatusConflict {
t.Fatalf("status = %d, want 409 (sess_racer was created immediately before the delete); body=%s", resp.StatusCode, raw)
}
if _, err := race.Environments().GetEnvironment(context.Background(), installWorkspace,
control.EnvironmentID(created.ID)); err != nil {
t.Errorf("environment removed despite the racer session: %v", err)
}
}
27 changes: 27 additions & 0 deletions internal/controld/memstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,33 @@ func (r memEnvironments) DeleteEnvironment(ctx context.Context, ws control.Works
return nil
}

// DeleteEnvironmentUnlessReferenced holds m.mu across the whole check and
// delete, one critical section instead of the two lock acquisitions
// CountSessionsByEnvironment and DeleteEnvironment used separately.
func (r memEnvironments) DeleteEnvironmentUnlessReferenced(ctx context.Context, ws control.WorkspaceID, id control.EnvironmentID, states []control.SessionState) error {
if ws == "" {
return control.ErrInvalid
}
m := r.m
m.mu.Lock()
defer m.mu.Unlock()
key := environmentKey{ws, id}
if _, ok := m.environments[key]; !ok {
return control.ErrNotFound
}
for k, s := range m.sessions {
if k.ws != ws || s.EnvironmentID != id {
continue
}
if len(states) == 0 || slices.Contains(states, s.State) {
return control.ErrConflict
}
}
delete(m.environments, key)
delete(m.snapshots, key)
return nil
}

func (r memEnvironments) CountSessionsByEnvironment(ctx context.Context, ws control.WorkspaceID, envID control.EnvironmentID, states []control.SessionState) (int, error) {
if ws == "" {
return 0, control.ErrInvalid
Expand Down
38 changes: 38 additions & 0 deletions internal/controld/pgstore/environments.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,44 @@ func (r pgEnvironments) DeleteEnvironment(ctx context.Context, ws control.Worksp
return nil
}

// DeleteEnvironmentUnlessReferenced folds the check into the DELETE's own
// WHERE clause, so the decision and the write are one statement instead of
// the separate CountSessionsByEnvironment-then-DeleteEnvironment pair.
func (r pgEnvironments) DeleteEnvironmentUnlessReferenced(ctx context.Context, ws control.WorkspaceID, id control.EnvironmentID, states []control.SessionState) error {
if ws == "" {
return control.ErrInvalid
}
sql := `DELETE FROM environments WHERE workspace_id = $1 AND id = $2 AND NOT EXISTS (
SELECT 1 FROM sessions WHERE workspace_id = $1 AND environment_id = $2`
args := []any{string(ws), string(id)}
if len(states) > 0 {
strs := make([]string, len(states))
for i, st := range states {
strs[i] = string(st)
}
args = append(args, strs)
sql += ` AND state = ANY($3)`
}
sql += `)`
ct, err := r.s.q(ctx).Exec(ctx, sql, args...)
if err != nil {
return unavailable("delete environment unless referenced", err)
}
if ct.RowsAffected() > 0 {
return nil
}
// Zero rows means either the environment is already gone or it is still
// referenced; disambiguate with a read, the same stale-vs-not-found shape
// SetEnvironmentSnapshot already uses.
if _, err := r.GetEnvironment(ctx, ws, id); err != nil {
if errors.Is(err, control.ErrNotFound) {
return control.ErrNotFound
}
return unavailable("delete environment unless referenced", err)
}
return control.ErrConflict
}

func (r pgEnvironments) CountSessionsByEnvironment(ctx context.Context, ws control.WorkspaceID, envID control.EnvironmentID, states []control.SessionState) (int, error) {
if ws == "" {
return 0, control.ErrInvalid
Expand Down
Loading