mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: fix failed workspaces continuously auto-deleting (#10069)
- Fixes an issue where workspaces that are eligible for auto-deletion are retried every tick (1 minute) even if the previous deletion transition failed. The updated logic only attempts to delete workspaces that previously failed once a day (24 hours since last attempt).
This commit is contained in:
@@ -24,7 +24,6 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/schedule"
|
||||
"github.com/coder/coder/v2/coderd/schedule/cron"
|
||||
"github.com/coder/coder/v2/coderd/wsbuilder"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
)
|
||||
|
||||
// Executor automatically starts or stops workspaces.
|
||||
@@ -310,7 +309,7 @@ func getNextTransition(
|
||||
// make it dormant.
|
||||
return "", database.BuildReasonAutolock, nil
|
||||
|
||||
case isEligibleForDelete(ws, templateSchedule, currentTick):
|
||||
case isEligibleForDelete(ws, templateSchedule, latestBuild, latestJob, currentTick):
|
||||
return database.WorkspaceTransitionDelete, database.BuildReasonAutodelete, nil
|
||||
default:
|
||||
return "", "", xerrors.Errorf("last transition not valid for autostart or autostop")
|
||||
@@ -320,7 +319,7 @@ func getNextTransition(
|
||||
// isEligibleForAutostart returns true if the workspace should be autostarted.
|
||||
func isEligibleForAutostart(ws database.Workspace, build database.WorkspaceBuild, job database.ProvisionerJob, templateSchedule schedule.TemplateScheduleOptions, currentTick time.Time) bool {
|
||||
// Don't attempt to autostart failed workspaces.
|
||||
if codersdk.ProvisionerJobStatus(job.JobStatus) == codersdk.ProvisionerJobFailed {
|
||||
if job.JobStatus == database.ProvisionerJobStatusFailed {
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -354,7 +353,7 @@ func isEligibleForAutostart(ws database.Workspace, build database.WorkspaceBuild
|
||||
|
||||
// isEligibleForAutostart returns true if the workspace should be autostopped.
|
||||
func isEligibleForAutostop(ws database.Workspace, build database.WorkspaceBuild, job database.ProvisionerJob, currentTick time.Time) bool {
|
||||
if codersdk.ProvisionerJobStatus(job.JobStatus) == codersdk.ProvisionerJobFailed {
|
||||
if job.JobStatus == database.ProvisionerJobStatusFailed {
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -381,13 +380,21 @@ func isEligibleForDormantStop(ws database.Workspace, templateSchedule schedule.T
|
||||
currentTick.Sub(ws.LastUsedAt) > templateSchedule.TimeTilDormant
|
||||
}
|
||||
|
||||
func isEligibleForDelete(ws database.Workspace, templateSchedule schedule.TemplateScheduleOptions, currentTick time.Time) bool {
|
||||
// Only attempt to delete dormant workspaces.
|
||||
return ws.DormantAt.Valid && ws.DeletingAt.Valid &&
|
||||
func isEligibleForDelete(ws database.Workspace, templateSchedule schedule.TemplateScheduleOptions, lastBuild database.WorkspaceBuild, lastJob database.ProvisionerJob, currentTick time.Time) bool {
|
||||
eligible := ws.DormantAt.Valid && ws.DeletingAt.Valid &&
|
||||
// Dormant workspaces should only be deleted if a time_til_dormant_autodelete value is specified.
|
||||
templateSchedule.TimeTilDormantAutoDelete > 0 &&
|
||||
// The workspace must breach the time_til_dormant_autodelete value.
|
||||
currentTick.After(ws.DeletingAt.Time)
|
||||
|
||||
// If the last delete job failed we should wait 24 hours before trying again.
|
||||
// Builds are resource-intensive so retrying every minute is not productive
|
||||
// and will hold compute hostage.
|
||||
if lastBuild.Transition == database.WorkspaceTransitionDelete && lastJob.JobStatus == database.ProvisionerJobStatusFailed {
|
||||
return eligible && lastJob.Finished() && currentTick.Sub(lastJob.FinishedAt()) > time.Hour*24
|
||||
}
|
||||
|
||||
return eligible
|
||||
}
|
||||
|
||||
// isEligibleForFailedStop returns true if the workspace is eligible to be stopped
|
||||
@@ -396,7 +403,7 @@ func isEligibleForFailedStop(build database.WorkspaceBuild, job database.Provisi
|
||||
// If the template has specified a failure TLL.
|
||||
return templateSchedule.FailureTTL > 0 &&
|
||||
// And the job resulted in failure.
|
||||
codersdk.ProvisionerJobStatus(job.JobStatus) == codersdk.ProvisionerJobFailed &&
|
||||
job.JobStatus == database.ProvisionerJobStatusFailed &&
|
||||
build.Transition == database.WorkspaceTransitionStart &&
|
||||
// And sufficient time has elapsed since the job has completed.
|
||||
job.CompletedAt.Valid &&
|
||||
|
||||
@@ -367,3 +367,19 @@ func ConvertWorkspaceRows(rows []GetWorkspacesRow) []Workspace {
|
||||
func (g Group) IsEveryone() bool {
|
||||
return g.ID == g.OrganizationID
|
||||
}
|
||||
|
||||
func (p ProvisionerJob) Finished() bool {
|
||||
return p.CanceledAt.Valid || p.CompletedAt.Valid
|
||||
}
|
||||
|
||||
func (p ProvisionerJob) FinishedAt() time.Time {
|
||||
if p.CompletedAt.Valid {
|
||||
return p.CompletedAt.Time
|
||||
}
|
||||
|
||||
if p.CanceledAt.Valid {
|
||||
return p.CanceledAt.Time
|
||||
}
|
||||
|
||||
return time.Time{}
|
||||
}
|
||||
|
||||
@@ -649,6 +649,93 @@ func TestWorkspaceAutobuild(t *testing.T) {
|
||||
stats = <-statsCh
|
||||
require.Len(t, stats.Transitions, 0)
|
||||
})
|
||||
|
||||
// Test that failing to auto-delete a workspace will only retry
|
||||
// once a day.
|
||||
t.Run("FailedDeleteRetryDaily", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
ticker = make(chan time.Time)
|
||||
statCh = make(chan autobuild.Stats)
|
||||
transitionTTL = time.Minute
|
||||
ctx = testutil.Context(t, testutil.WaitMedium)
|
||||
)
|
||||
|
||||
client, user := coderdenttest.New(t, &coderdenttest.Options{
|
||||
Options: &coderdtest.Options{
|
||||
AutobuildTicker: ticker,
|
||||
IncludeProvisionerDaemon: true,
|
||||
AutobuildStats: statCh,
|
||||
TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()),
|
||||
},
|
||||
LicenseOptions: &coderdenttest.LicenseOptions{
|
||||
Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1},
|
||||
},
|
||||
})
|
||||
|
||||
// Create a template version that passes to get a functioning workspace.
|
||||
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
|
||||
Parse: echo.ParseComplete,
|
||||
ProvisionPlan: echo.PlanComplete,
|
||||
ProvisionApply: echo.ApplyComplete,
|
||||
})
|
||||
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
|
||||
|
||||
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
|
||||
|
||||
ws := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
|
||||
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID)
|
||||
|
||||
// Create a new version that will fail when we try to delete a workspace.
|
||||
version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
|
||||
Parse: echo.ParseComplete,
|
||||
ProvisionPlan: echo.PlanComplete,
|
||||
ProvisionApply: echo.ApplyFailed,
|
||||
}, func(ctvr *codersdk.CreateTemplateVersionRequest) {
|
||||
ctvr.TemplateID = template.ID
|
||||
})
|
||||
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
|
||||
|
||||
// Try to delete the workspace. This simulates a "failed" autodelete.
|
||||
build, err := client.CreateWorkspaceBuild(ctx, ws.ID, codersdk.CreateWorkspaceBuildRequest{
|
||||
Transition: codersdk.WorkspaceTransitionDelete,
|
||||
TemplateVersionID: version.ID,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
build = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, build.ID)
|
||||
require.NotEmpty(t, build.Job.Error)
|
||||
|
||||
// Update our workspace to be dormant so that it qualifies for auto-deletion.
|
||||
err = client.UpdateWorkspaceDormancy(ctx, ws.ID, codersdk.UpdateWorkspaceDormancy{
|
||||
Dormant: true,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Enable auto-deletion for the template.
|
||||
_, err = client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{
|
||||
TimeTilDormantAutoDeleteMillis: transitionTTL.Milliseconds(),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
ws = coderdtest.MustWorkspace(t, client, ws.ID)
|
||||
require.NotNil(t, ws.DeletingAt)
|
||||
|
||||
// Simulate ticking an hour after the workspace is expected to be deleted.
|
||||
// Under normal circumstances this should result in a transition but
|
||||
// since our last build resulted in failure it should be skipped.
|
||||
ticker <- build.Job.CompletedAt.Add(time.Hour)
|
||||
stats := <-statCh
|
||||
require.Len(t, stats.Transitions, 0)
|
||||
|
||||
// Simulate ticking a day after the workspace was last attempted to
|
||||
// be deleted. This should result in an attempt.
|
||||
ticker <- build.Job.CompletedAt.Add(time.Hour * 25)
|
||||
stats = <-statCh
|
||||
require.Len(t, stats.Transitions, 1)
|
||||
require.Equal(t, database.WorkspaceTransitionDelete, stats.Transitions[ws.ID])
|
||||
})
|
||||
}
|
||||
|
||||
func TestWorkspacesFiltering(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user