Skip to content

Commit 88dfc87

Browse files
committed
fix: prevent orphan VMM via socket tiebreaker + status one-shot default
Closes #48 and #49 — both were triggers for orphan cocoon/CH processes observed on cocoonset-gke-private (8 alive CH/FC without DB entries, plus 4-day-old cocoon-CLI watchers from prior vk-cocoon restarts). #49: vm rm --force could clean the DB but leave CH running when the pidfile/cmdline check returned a false negative (stale pidfile, recycled PID, manual rundir tampering). Add Backend.IsAPISocketLive as a pidfile-independent tiebreaker (utils.CheckSocket) and call it in DeleteAll after the stop step. Fires unconditionally — AF_UNIX has no TIME_WAIT and TerminateProcess only returns nil after full pid reap, so a successful stop guarantees the listener is gone; firing also catches the pidfd_linux.go's own VerifyProcessCmdline false-negative path that otherwise returns (true, nil) without sending a signal. #48: cocoon vm status defaulted to a 5s polling loop. When invoked from a shell that disconnected without SIGHUP propagation (sudo wrapper, bash -c pipeline, tmux killed without huponexit), the loop survived indefinitely — observed 21-day-old orphans on prod. Add --watch flag; default is one-shot snapshot via statusOnce. --event still implies streaming events. statusOnce + List share a renderVMList helper so the JSON/table/empty-list branch lives in one place.
1 parent 0324e80 commit 88dfc87

5 files changed

Lines changed: 170 additions & 16 deletions

File tree

cmd/vm/commands.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,13 @@ func Command(h Actions) *cobra.Command {
165165

166166
statusCmd := &cobra.Command{
167167
Use: "status [VM...]",
168-
Short: "Watch VM status in real time",
168+
Short: "Show VM status; --watch for refresh loop, --event for streaming",
169169
RunE: h.Status,
170170
}
171-
statusCmd.Flags().IntP("interval", "n", 5, "poll interval in seconds") //nolint:mnd
172-
statusCmd.Flags().Bool("event", false, "event stream mode (append changes instead of refreshing)")
173-
statusCmd.Flags().String("format", "", "output format: json (event mode only)")
171+
statusCmd.Flags().IntP("interval", "n", 5, "poll interval in seconds (only with --watch or --event)") //nolint:mnd
172+
statusCmd.Flags().BoolP("watch", "w", false, "refresh-loop mode (full-screen redraw each tick); omit for one-shot snapshot")
173+
statusCmd.Flags().Bool("event", false, "event stream mode (append changes instead of refreshing); implies polling")
174+
statusCmd.Flags().String("format", "", "output format: json (one-shot + event modes; --watch always renders a table)")
174175

175176
vmCmd.AddCommand(
176177
createCmd,

cmd/vm/status.go

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,16 +56,23 @@ func (h Handler) List(cmd *cobra.Command, _ []string) error {
5656
if err != nil {
5757
return fmt.Errorf("list: %w", err)
5858
}
59+
sortVMs(vms)
60+
format, _ := cmd.Flags().GetString("format")
61+
return renderVMList(vms, format)
62+
}
63+
64+
// renderVMList emits vms as JSON or table; "No VMs found." for empty in table mode.
65+
func renderVMList(vms []*types.VM, format string) error {
66+
if format == "json" {
67+
return cmdcore.OutputJSON(vms)
68+
}
5969
if len(vms) == 0 {
6070
fmt.Println("No VMs found.")
6171
return nil
6272
}
63-
64-
sortVMs(vms)
65-
66-
return cmdcore.OutputFormatted(cmd, vms, func(w *tabwriter.Writer) {
67-
printVMTable(w, vms)
68-
})
73+
w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0)
74+
printVMTable(w, vms)
75+
return w.Flush()
6976
}
7077

7178
func (h Handler) Status(cmd *cobra.Command, args []string) error {
@@ -79,19 +86,25 @@ func (h Handler) Status(cmd *cobra.Command, args []string) error {
7986
interval = 5 //nolint:mnd
8087
}
8188
eventMode, _ := cmd.Flags().GetBool("event")
89+
watchMode, _ := cmd.Flags().GetBool("watch")
90+
if eventMode && watchMode {
91+
return fmt.Errorf("--event and --watch are mutually exclusive")
92+
}
93+
format, _ := cmd.Flags().GetString("format")
8294

8395
hypers, hyperErr := cmdcore.InitAllHypervisors(conf)
8496
if hyperErr != nil {
8597
return hyperErr
8698
}
8799

88-
watchCh := mergeWatchChannels(ctx, hypers)
100+
if !eventMode && !watchMode {
101+
return statusOnce(ctx, hypers, args, format)
102+
}
89103

104+
watchCh := mergeWatchChannels(ctx, hypers)
90105
ticker := time.NewTicker(time.Duration(interval) * time.Second)
91106
defer ticker.Stop()
92107

93-
format, _ := cmd.Flags().GetString("format")
94-
95108
if eventMode {
96109
if format == "json" {
97110
statusEventLoopJSON(ctx, hypers, args, watchCh, ticker.C)
@@ -105,6 +118,17 @@ func (h Handler) Status(cmd *cobra.Command, args []string) error {
105118
return nil
106119
}
107120

121+
// statusOnce prints a single snapshot then returns; propagates ListAllVMs error (loop callers swallow).
122+
func statusOnce(ctx context.Context, hypers []hypervisor.Hypervisor, filters []string, format string) error {
123+
vms, err := cmdcore.ListAllVMs(ctx, hypers)
124+
if err != nil {
125+
return fmt.Errorf("status: %w", err)
126+
}
127+
vms = applyFilters(vms, filters)
128+
sortVMs(vms)
129+
return renderVMList(vms, format)
130+
}
131+
108132
func mergeWatchChannels(ctx context.Context, hypers []hypervisor.Hypervisor) <-chan struct{} {
109133
var channels []<-chan struct{}
110134
for _, h := range hypers {
@@ -266,23 +290,28 @@ func printEventRow(w *tabwriter.Writer, event string, snap vmSnapshot) {
266290
snap.ip, snap.image)
267291
}
268292

293+
// listAndFilter swallows backend errors with a warn so a transient hiccup can't break the polling tick; one-shot callers must use cmdcore.ListAllVMs directly.
269294
func listAndFilter(ctx context.Context, hypers []hypervisor.Hypervisor, filters []string) []*types.VM {
270295
vms, err := cmdcore.ListAllVMs(ctx, hypers)
271296
if err != nil {
272297
log.WithFunc("cmd.vm.listAndFilter").Warnf(ctx, "list: %v", err)
273298
return nil
274299
}
275300
sortVMs(vms)
301+
return applyFilters(vms, filters)
302+
}
303+
304+
func applyFilters(vms []*types.VM, filters []string) []*types.VM {
276305
if len(filters) == 0 {
277306
return vms
278307
}
279-
var result []*types.VM
308+
out := make([]*types.VM, 0, len(vms))
280309
for _, vm := range vms {
281310
if matchesFilter(vm, filters) {
282-
result = append(result, vm)
311+
out = append(out, vm)
283312
}
284313
}
285-
return result
314+
return out
286315
}
287316

288317
func matchesFilter(vm *types.VM, filters []string) bool {

cmd/vm/status_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,31 @@
11
package vm
22

33
import (
4+
"io"
5+
"os"
6+
"strings"
47
"testing"
58
"time"
69

710
"github.com/cocoonstack/cocoon/types"
811
)
912

13+
// captureStdout swaps os.Stdout for a pipe, runs fn, returns what fn wrote.
14+
func captureStdout(t *testing.T, fn func()) string {
15+
t.Helper()
16+
orig := os.Stdout
17+
r, w, err := os.Pipe()
18+
if err != nil {
19+
t.Fatalf("pipe: %v", err)
20+
}
21+
os.Stdout = w
22+
fn()
23+
_ = w.Close()
24+
os.Stdout = orig
25+
buf, _ := io.ReadAll(r)
26+
return string(buf)
27+
}
28+
1029
func TestMatchesFilter(t *testing.T) {
1130
vm := &types.VM{
1231
ID: "abcdef123456",
@@ -85,3 +104,83 @@ func TestVMIPsAndSort(t *testing.T) {
85104
t.Fatalf("takeSnapshot() = %+v", snap)
86105
}
87106
}
107+
108+
func TestRenderVMList(t *testing.T) {
109+
vm := &types.VM{
110+
ID: "abc",
111+
Config: types.VMConfig{Name: "demo", Config: types.Config{CPU: 1, Memory: 1 << 30, Image: "img"}},
112+
CreatedAt: time.Now(),
113+
}
114+
115+
tests := []struct {
116+
name string
117+
vms []*types.VM
118+
format string
119+
want string
120+
notWant string
121+
}{
122+
{name: "empty table → No VMs found", vms: nil, format: "", want: "No VMs found."},
123+
{name: "empty json → null", vms: nil, format: "json", want: "null"},
124+
{name: "table with vm contains name", vms: []*types.VM{vm}, format: "", want: "demo"},
125+
{name: "json with vm contains id", vms: []*types.VM{vm}, format: "json", want: `"id": "abc"`},
126+
{name: "table mode skips json marker", vms: []*types.VM{vm}, format: "", notWant: `"id":`},
127+
}
128+
129+
for _, tt := range tests {
130+
t.Run(tt.name, func(t *testing.T) {
131+
out := captureStdout(t, func() {
132+
if err := renderVMList(tt.vms, tt.format); err != nil {
133+
t.Fatalf("renderVMList: %v", err)
134+
}
135+
})
136+
if tt.want != "" && !strings.Contains(out, tt.want) {
137+
t.Errorf("output %q does not contain %q", out, tt.want)
138+
}
139+
if tt.notWant != "" && strings.Contains(out, tt.notWant) {
140+
t.Errorf("output %q unexpectedly contains %q", out, tt.notWant)
141+
}
142+
})
143+
}
144+
}
145+
146+
func TestApplyFilters(t *testing.T) {
147+
vms := []*types.VM{
148+
{ID: "abcdef123456", Config: types.VMConfig{Name: "alpha"}},
149+
{ID: "beadbeef0000", Config: types.VMConfig{Name: "beta"}},
150+
}
151+
tests := []struct {
152+
name string
153+
filters []string
154+
wantIDs []string
155+
}{
156+
{name: "no filter returns all", filters: nil, wantIDs: []string{"abcdef123456", "beadbeef0000"}},
157+
{name: "exact name", filters: []string{"alpha"}, wantIDs: []string{"abcdef123456"}},
158+
{name: "id prefix", filters: []string{"bead"}, wantIDs: []string{"beadbeef0000"}},
159+
{name: "multi filter union", filters: []string{"alpha", "bead"}, wantIDs: []string{"abcdef123456", "beadbeef0000"}},
160+
{name: "no match", filters: []string{"zzz"}, wantIDs: nil},
161+
}
162+
for _, tt := range tests {
163+
t.Run(tt.name, func(t *testing.T) {
164+
got := applyFilters(vms, tt.filters)
165+
gotIDs := make([]string, 0, len(got))
166+
for _, vm := range got {
167+
gotIDs = append(gotIDs, vm.ID)
168+
}
169+
if !equalStrings(gotIDs, tt.wantIDs) {
170+
t.Errorf("applyFilters(%v) = %v, want %v", tt.filters, gotIDs, tt.wantIDs)
171+
}
172+
})
173+
}
174+
}
175+
176+
func equalStrings(a, b []string) bool {
177+
if len(a) != len(b) {
178+
return false
179+
}
180+
for i := range a {
181+
if a[i] != b[i] {
182+
return false
183+
}
184+
}
185+
return true
186+
}

hypervisor/state.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"errors"
66
"fmt"
77
"io/fs"
8+
"net"
9+
"syscall"
810
"time"
911

1012
"github.com/projecteru2/core/log"
@@ -13,6 +15,8 @@ import (
1315
"github.com/cocoonstack/cocoon/utils"
1416
)
1517

18+
const socketProbeTimeout = 2 * time.Second
19+
1620
// WithRunningVM calls fn if rec still points to a live VM process.
1721
func (b *Backend) WithRunningVM(ctx context.Context, rec *VMRecord, fn func(pid int) error) error {
1822
pid, pidErr := utils.ReadPIDFile(b.PIDFilePath(rec.RunDir))
@@ -25,6 +29,23 @@ func (b *Backend) WithRunningVM(ctx context.Context, rec *VMRecord, fn func(pid
2529
return fn(pid)
2630
}
2731

32+
// IsAPISocketLive is a pidfile-independent liveness tiebreaker; honors ctx cancellation, fails closed on uncertain dial errors.
33+
func (b *Backend) IsAPISocketLive(ctx context.Context, rec *VMRecord) bool {
34+
sock := SocketPath(rec.RunDir)
35+
dialCtx, cancel := context.WithTimeout(ctx, socketProbeTimeout)
36+
defer cancel()
37+
conn, err := (&net.Dialer{}).DialContext(dialCtx, "unix", sock)
38+
if err == nil {
39+
_ = conn.Close()
40+
return true
41+
}
42+
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ECONNREFUSED) {
43+
return false
44+
}
45+
log.WithFunc(b.Typ+".IsAPISocketLive").Warnf(ctx, "uncertain socket probe %s: %v — failing closed", sock, err)
46+
return true
47+
}
48+
2849
// WithPausedVM pauses, runs fn, resumes; eager resume on success promotes its error, deferred resume on fn-error only logs.
2950
func (b *Backend) WithPausedVM(ctx context.Context, rec *VMRecord, pause, resume, fn func() error) error {
3051
return b.WithRunningVM(ctx, rec, func(_ int) error {

hypervisor/stop.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ func (b *Backend) DeleteAll(ctx context.Context, refs []string, force bool, stop
6464
}); runningErr != nil && !errors.Is(runningErr, ErrNotRunning) {
6565
return fmt.Errorf("stop before delete: %w", runningErr)
6666
}
67+
// Socket probe runs regardless of stop outcome: AF_UNIX has no TIME_WAIT so a real stop guarantees no listener; firing always also catches false-negative pidfile/cmdline shortcuts.
68+
if b.IsAPISocketLive(ctx, &rec) {
69+
return fmt.Errorf("refuse delete: api socket %s still responsive (suspected orphan vmm; kill the vmm process then retry)", SocketPath(rec.RunDir))
70+
}
6771
if rmErr := RemoveVMDirs(rec.RunDir, rec.LogDir); rmErr != nil {
6872
return fmt.Errorf("cleanup VM dirs: %w", rmErr)
6973
}

0 commit comments

Comments
 (0)