sneak/upaas
1
0
Fork 1
Code Issues 5 Pull Requests 2 Actions Packages Projects Releases 1 Wiki Activity

fix: clean up orphan resources on deploy cancellation (closes #89) #93

Merged
sneak merged 2 commits from fix/deploy-cancel-cleanup into main 2026-02-20 05:22:59 +01:00
Conversation 4 Commits 2 Files Changed 4
+7 -3
2 changed files with 7 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 95a690e805 - Show all commits
internal/service/deploy/deploy.go
+2 -1
View File
@@ -11,6 +11,7 @@ import (
"log/slog"
"os"
"path/filepath"
"strings"
"sync"
"time"
@@ -715,7 +716,7 @@ func (svc *Service) cleanupCancelledDeploy(
prefix := fmt.Sprintf("%d-", deployment.ID)
for _, entry := range entries {
if entry.IsDir() && len(entry.Name()) > len(prefix) && entry.Name()[:len(prefix)] == prefix {
if entry.IsDir() && strings.HasPrefix(entry.Name(), prefix) {
clawbot commented 2026-02-19 22:50:05 +01:00
Outdated
Review
Copy Link
Copy Source

Nit: entry.Name()[:len(prefix)] == prefix — prefer strings.HasPrefix(entry.Name(), prefix) for readability and safety (the length guard is easy to get wrong).

Nit: `entry.Name()[:len(prefix)] == prefix` — prefer `strings.HasPrefix(entry.Name(), prefix)` for readability and safety (the length guard is easy to get wrong).
dirPath := filepath.Join(buildDir, entry.Name())
removeErr := os.RemoveAll(dirPath)
internal/service/deploy/export_test.go
+5 -2
View File
@@ -6,6 +6,7 @@ import (
"log/slog"
"os"
"path/filepath"
"strings"
"git.eeqj.de/sneak/upaas/internal/config"
"git.eeqj.de/sneak/upaas/internal/docker"
@@ -47,7 +48,9 @@ func NewTestServiceWithConfig(log *slog.Logger, cfg *config.Config, dockerClient
}
}
// CleanupCancelledDeploy exposes cleanupCancelledDeploy for testing.
// CleanupCancelledDeploy exposes the build directory cleanup portion of
// cleanupCancelledDeploy for testing. It removes build directories matching
// the deployment ID prefix.
func (svc *Service) CleanupCancelledDeploy(
ctx context.Context,
clawbot commented 2026-02-19 22:50:05 +01:00
Outdated
Review
Copy Link
Copy Source

This duplicates the directory cleanup logic from the real cleanupCancelledDeploy. If the real implementation changes (e.g. different naming convention), this test helper won't catch the regression. Consider refactoring so the test exercises the actual code path.

This duplicates the directory cleanup logic from the real `cleanupCancelledDeploy`. If the real implementation changes (e.g. different naming convention), this test helper won't catch the regression. Consider refactoring so the test exercises the actual code path.
appName string,
@@ -66,7 +69,7 @@ func (svc *Service) CleanupCancelledDeploy(
prefix := fmt.Sprintf("%d-", deploymentID)
for _, entry := range entries {
if entry.IsDir() && len(entry.Name()) > len(prefix) && entry.Name()[:len(prefix)] == prefix {
if entry.IsDir() && strings.HasPrefix(entry.Name(), prefix) {
dirPath := filepath.Join(buildDir, entry.Name())
_ = os.RemoveAll(dirPath)
}