Commit fb2b232

Eric Bower  ·  2026-08-09 11:36:50 -0400 EDT
parent 2064538
fix(gc): wasn't killing zmx sessions properly
2 files changed,  +70, -28
+42, -20
 1@@ -666,9 +666,6 @@ func getDomain(eventType string) string {
 2 	if eventType == "local" {
 3 		return "local"
 4 	}
 5-	if eventType == "test-ci" || strings.HasPrefix(eventType, "test-") {
 6-		return eventType
 7-	}
 8 	return "ci"
 9 }
10 
11@@ -1919,28 +1916,29 @@ func cancelJobSessions(prefix string) {
12 	}
13 }
14 
15-// runGC kills all ci. zmx sessions older than 3 hours, regardless of status.
16-func runGC(cfg *Cfg) error {
17-	log := cfg.Logger.With("cmd", "gc")
18-	log.Debug("running garbage collection")
19-
20-	listOutput, err := exec.Command("zmx", "list").CombinedOutput()
21-	if err != nil {
22-		return fmt.Errorf("zmx list: %w", err)
23-	}
24-
25-	sessions := parseZMXList(string(listOutput))
26-
27-	cutoff := time.Now().Add(-3 * time.Hour).Unix()
28-
29+// findSessionsForGC identifies finished sessions or sessions older than 3 hours matching given prefixes.
30+func findSessionsForGC(sessions []SessionInfo, prefixes []string, now time.Time) []string {
31+	cutoff := now.Add(-3 * time.Hour).Unix()
32 	var toKill []string
33 	for _, s := range sessions {
34-		if !strings.HasPrefix(s.Name, "ci.") && !strings.HasPrefix(s.Name, "local.") {
35+		matched := false
36+		for _, prefix := range prefixes {
37+			if strings.HasPrefix(s.Name, prefix) {
38+				matched = true
39+				break
40+			}
41+		}
42+		if !matched {
43+			continue
44+		}
45+
46+		if s.Ended != "" {
47+			toKill = append(toKill, s.Name)
48 			continue
49 		}
50 
51 		if s.Created == "" {
52-			continue // skip sessions with no creation time
53+			continue
54 		}
55 
56 		var created int64
57@@ -1949,10 +1947,34 @@ func runGC(cfg *Cfg) error {
58 		}
59 
60 		if created < cutoff {
61-			log.Debug("session expired, scheduling for gc", "session", s.Name, "created", s.Created)
62 			toKill = append(toKill, s.Name)
63 		}
64 	}
65+	return toKill
66+}
67+
68+// runGC kills finished zmx sessions or sessions older than 3 hours.
69+func runGC(cfg *Cfg) error {
70+	log := cfg.Logger.With("cmd", "gc")
71+	log.Debug("running garbage collection")
72+
73+	listOutput, err := exec.Command("zmx", "list").CombinedOutput()
74+	if err != nil {
75+		return fmt.Errorf("zmx list: %w", err)
76+	}
77+
78+	sessions := parseZMXList(string(listOutput))
79+
80+	prefixes := []string{"ci.", "local."}
81+	if cfg != nil && cfg.SessionPrefix != "" {
82+		p := cfg.SessionPrefix
83+		if !strings.HasSuffix(p, ".") {
84+			p += "."
85+		}
86+		prefixes = append(prefixes, p)
87+	}
88+
89+	toKill := findSessionsForGC(sessions, prefixes, time.Now())
90 
91 	if len(toKill) == 0 {
92 		log.Debug("no sessions to garbage collect")
+28, -8
 1@@ -48,10 +48,10 @@ zmx run step2 echo "hello from step2"
 2 	ctx, cancel := context.WithCancel(context.Background())
 3 	testJobID := fmt.Sprintf("j-%d", time.Now().UnixNano()%100000000)
 4 	t.Cleanup(func() {
 5-		_ = exec.Command("zmx", "kill", "-f", fmt.Sprintf("test-ci.test-repo.%s", testJobID)).Run()
 6+		_ = exec.Command("zmx", "kill", "-f", fmt.Sprintf("local.test-repo.%s", testJobID)).Run()
 7 	})
 8 	event := Event{
 9-		Type:      "test-ci",
10+		Type:      "local",
11 		Name:      "test-repo",
12 		JobID:     testJobID,
13 		Workspace: workspaceDir,
14@@ -68,7 +68,7 @@ zmx run step2 echo "hello from step2"
15 		NewWorkspace:    defaultWorkspaceFactory,
16 		StatusOutput:    statusBuf,
17 		IncludeRunning:  true,
18-		SessionPrefix:   "test-ci.",
19+		SessionPrefix:   "local.",
20 	}
21 
22 	// 3. Run the runner (fire-and-forget, exits quickly)
23@@ -185,8 +185,8 @@ zmx run step2 echo "hello from step2"
24 	if finalPayload == nil {
25 		t.Fatal("no final payload")
26 	}
27-	// Filter to only sessions with our job prefix to avoid picking up unrelated test-ci.* sessions
28-	expectedPrefix := fmt.Sprintf("test-ci.%s.%s.", finalPayload.Name, finalPayload.JobID)
29+	// Filter to only sessions with our job prefix to avoid picking up unrelated local.* sessions
30+	expectedPrefix := fmt.Sprintf("local.%s.%s.", finalPayload.Name, finalPayload.JobID)
31 	sessions := make([]SessionInfo, 0, len(finalPayload.Sessions))
32 	for _, s := range finalPayload.Sessions {
33 		if strings.HasPrefix(s.Name, expectedPrefix) {
34@@ -559,7 +559,6 @@ func TestGetDomain(t *testing.T) {
35 		{"ci", "ci"},
36 		{"", "ci"},
37 		{"local", "local"},
38-		{"test-ci", "test-ci"},
39 	}
40 
41 	for _, tt := range tests {
42@@ -639,6 +638,27 @@ func findRunningJobsFromOutput(output, name string) ([]string, []SessionInfo) {
43 	return runners, sessions
44 }
45 
46+func TestFindSessionsForGC(t *testing.T) {
47+	now := time.Unix(1786289700, 0)
48+	twoHoursAgo := fmt.Sprintf("%d", now.Add(-2*time.Hour).Unix())
49+	fourHoursAgo := fmt.Sprintf("%d", now.Add(-4*time.Hour).Unix())
50+
51+	sessions := []SessionInfo{
52+		{Name: "ci.repo.active.runner", Created: twoHoursAgo, Ended: ""},
53+		{Name: "ci.repo.finished.runner", Created: twoHoursAgo, Ended: "1786289600"},
54+		{Name: "ci.repo.expired.runner", Created: fourHoursAgo, Ended: ""},
55+		{Name: "other.session", Created: fourHoursAgo, Ended: ""},
56+	}
57+
58+	toKill := findSessionsForGC(sessions, []string{"ci.", "local."}, now)
59+	if len(toKill) != 2 {
60+		t.Fatalf("expected 2 sessions for GC, got %d: %v", len(toKill), toKill)
61+	}
62+	if toKill[0] != "ci.repo.finished.runner" || toKill[1] != "ci.repo.expired.runner" {
63+		t.Errorf("unexpected sessions for GC: %v", toKill)
64+	}
65+}
66+
67 func TestKillSessionsEmpty(t *testing.T) {
68 	// Should not error with empty list
69 	if err := killSessions(nil); err != nil {
70@@ -795,7 +815,7 @@ func TestWaitAndReport_Cancellation(t *testing.T) {
71 	}
72 
73 	jobID := fmt.Sprintf("canceltest-%d", time.Now().UnixNano())
74-	prefix := "test-local.testrepo." + jobID + "."
75+	prefix := "local.testrepo." + jobID + "."
76 	runnerSession := prefix + "runner"
77 
78 	t.Cleanup(func() {
79@@ -812,7 +832,7 @@ func TestWaitAndReport_Cancellation(t *testing.T) {
80 		cancel()
81 	}()
82 
83-	err := waitAndReport(cfg, nil, "testrepo", jobID, "test-local")
84+	err := waitAndReport(cfg, nil, "testrepo", jobID, "local")
85 	if err == nil {
86 		t.Error("expected error when waitAndReport is cancelled, got nil")
87 	}