Skip to content

Commit cfd6038

Browse files
committed
fix(reaper): forkExec error no such file or directory & prometheus exit error
This is the fix described in #178 but with minimal changes applied, since we want to limit the scope of changes of an individual PR.
1 parent 8fd022b commit cfd6038

File tree

4 files changed

+216
-3
lines changed

4 files changed

+216
-3
lines changed

cronexpr/cronexpr/go.mod

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
module github.com/aptible/supercronic/cronexpr/cronexpr
22

3-
go 1.18
3+
go 1.24.6
4+
5+
toolchain go1.24.7
46

57
replace github.com/aptible/supercronic => ../../
68

main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func main() {
123123
forkExec()
124124
return
125125
}
126-
126+
127127
logrus.Warn("process reaping disabled, not pid 1")
128128
}
129129
crontabFileName := flag.Args()[0]

reaper.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ func forkExec() {
1717
return
1818
}
1919

20+
exe, err := os.Executable()
21+
if err != nil {
22+
logrus.Fatalf("Failed to get executable %s", err.Error())
23+
}
24+
2025
pattrs := &syscall.ProcAttr{
2126
Dir: pwd,
2227
Env: os.Environ(),
@@ -28,7 +33,7 @@ func forkExec() {
2833
}
2934
args := make([]string, 0, len(os.Args)+1)
3035
// disable reaping for supercronic, avoid no sense warning
31-
args = append(args, os.Args[0], "-no-reap")
36+
args = append(args, exe, "-no-reap")
3237
args = append(args, os.Args[1:]...)
3338

3439
pid, err := syscall.ForkExec(args[0], args, pattrs)

reaper_test.go

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
package main
2+
3+
import (
4+
"os"
5+
"os/exec"
6+
"path/filepath"
7+
"runtime"
8+
"strings"
9+
"syscall"
10+
"testing"
11+
)
12+
13+
func TestForkExecUsesExecutablePath(t *testing.T) {
14+
if os.Getpid() == 1 {
15+
t.Skip("Cannot test forkExec as pid 1")
16+
}
17+
18+
testExe := createTestExecutable(t)
19+
defer os.Remove(testExe)
20+
21+
originalArgs := os.Args
22+
defer func() {
23+
os.Args = originalArgs
24+
}()
25+
26+
// Simulate the problematic scenario: os.Args[0] is just the command name without path
27+
// This is what would happen when supercronic is called from a different directory
28+
os.Args = []string{"supercronic", "-no-reap", "-test", "/dev/null"}
29+
30+
tempDir := t.TempDir()
31+
originalWd, _ := os.Getwd()
32+
defer os.Chdir(originalWd)
33+
34+
err := os.Chdir(tempDir)
35+
if err != nil {
36+
t.Fatalf("Failed to change directory: %v", err)
37+
}
38+
39+
testForkExecBehavior(t)
40+
}
41+
42+
func TestOsExecutableVsOsArgs(t *testing.T) {
43+
exe, err := os.Executable()
44+
if err != nil {
45+
t.Fatalf("Failed to get executable: %v", err)
46+
}
47+
48+
if !filepath.IsAbs(exe) {
49+
t.Errorf("os.Executable() should return absolute path, got: %s", exe)
50+
}
51+
52+
arg0 := os.Args[0]
53+
54+
if !strings.Contains(exe, filepath.Base(arg0)) {
55+
t.Logf("os.Executable(): %s", exe)
56+
t.Logf("os.Args[0]: %s", arg0)
57+
}
58+
59+
if _, err := os.Stat(exe); err != nil {
60+
t.Errorf("os.Executable() returned non-existent file: %v", err)
61+
}
62+
}
63+
64+
func TestReaperForkExecFromRootDirectory(t *testing.T) {
65+
if runtime.GOOS == "windows" {
66+
t.Skip("This test is Unix-specific")
67+
}
68+
69+
if os.Getuid() != 0 {
70+
t.Skip("This test requires root privileges to change to / directory safely")
71+
}
72+
73+
testBinary := createTestExecutable(t)
74+
defer os.Remove(testBinary)
75+
76+
originalWd, _ := os.Getwd()
77+
defer os.Chdir(originalWd)
78+
79+
err := os.Chdir("/")
80+
if err != nil {
81+
t.Fatalf("Failed to change to root directory: %v", err)
82+
}
83+
84+
originalArgs := os.Args
85+
defer func() {
86+
os.Args = originalArgs
87+
}()
88+
89+
os.Args = []string{"supercronic", "-no-reap", "-test", "/dev/null"}
90+
91+
exe, err := os.Executable()
92+
if err != nil {
93+
t.Fatalf("Failed to get executable path: %v", err)
94+
}
95+
96+
if !filepath.IsAbs(exe) {
97+
t.Errorf("Expected absolute path, got: %s", exe)
98+
}
99+
100+
if _, err := os.Stat(exe); err != nil {
101+
t.Errorf("Executable path doesn't exist: %v", err)
102+
}
103+
104+
if filepath.Base(os.Args[0]) != filepath.Base(exe) {
105+
t.Logf("os.Args[0]: %s (would fail with old code)", os.Args[0])
106+
t.Logf("os.Executable(): %s (works with new code)", exe)
107+
}
108+
}
109+
110+
func createTestExecutable(t *testing.T) string {
111+
t.Helper()
112+
113+
// Create temporary Go source
114+
tempDir := t.TempDir()
115+
srcFile := filepath.Join(tempDir, "test_supercronic.go")
116+
117+
testSrc := `package main
118+
import (
119+
"flag"
120+
"fmt"
121+
"os"
122+
)
123+
func main() {
124+
test := flag.Bool("test", false, "test mode")
125+
noReap := flag.Bool("no-reap", false, "disable reaping")
126+
flag.Parse()
127+
128+
if *test {
129+
fmt.Println("test mode - would validate crontab")
130+
os.Exit(0)
131+
}
132+
133+
if *noReap {
134+
fmt.Println("reaping disabled")
135+
}
136+
137+
// Simulate supercronic behavior
138+
fmt.Println("supercronic test executable running")
139+
}`
140+
141+
err := os.WriteFile(srcFile, []byte(testSrc), 0644)
142+
if err != nil {
143+
t.Fatalf("Failed to write test source: %v", err)
144+
}
145+
146+
// Build the test executable
147+
exeFile := filepath.Join(tempDir, "test_supercronic")
148+
cmd := exec.Command("go", "build", "-o", exeFile, srcFile)
149+
if err := cmd.Run(); err != nil {
150+
t.Fatalf("Failed to build test executable: %v", err)
151+
}
152+
153+
return exeFile
154+
}
155+
156+
func testForkExecBehavior(t *testing.T) {
157+
exe, err := os.Executable()
158+
if err != nil {
159+
t.Fatalf("Failed to get executable (this is what the old bug was): %v", err)
160+
}
161+
162+
args := make([]string, 0, len(os.Args)+1)
163+
args = append(args, exe, "-no-reap") // This is the fix - use exe instead of os.Args[0]
164+
args = append(args, os.Args[1:]...)
165+
166+
if !filepath.IsAbs(args[0]) {
167+
t.Errorf("First argument should be absolute path, got: %s", args[0])
168+
}
169+
170+
if _, err := os.Stat(args[0]); err != nil {
171+
t.Errorf("Executable path in args[0] doesn't exist: %v", err)
172+
}
173+
174+
if err := syscall.Access(args[0], syscall.F_OK); err != nil {
175+
t.Errorf("syscall.Access failed on executable path: %v", err)
176+
}
177+
}
178+
179+
func TestForkExecIntegration(t *testing.T) {
180+
if os.Getpid() == 1 {
181+
t.Skip("Cannot test forkExec as pid 1 - would interfere with process reaping")
182+
}
183+
184+
originalArgs := os.Args
185+
defer func() {
186+
os.Args = originalArgs
187+
}()
188+
189+
os.Args = []string{"supercronic", "-test", "/dev/null"}
190+
191+
exe, err := os.Executable()
192+
if err != nil {
193+
t.Fatalf("os.Executable() failed: %v", err)
194+
}
195+
196+
args := make([]string, 0, len(os.Args)+1)
197+
args = append(args, exe, "-no-reap")
198+
args = append(args, os.Args[1:]...)
199+
200+
if _, err := os.Stat(args[0]); err != nil {
201+
t.Errorf("forkExec would fail because args[0] is not a valid executable: %v", err)
202+
}
203+
204+
t.Logf("forkExec would use executable: %s", args[0])
205+
t.Logf("forkExec would use args: %v", args)
206+
}

0 commit comments

Comments
 (0)