fix: keep app running until rebuild succeeds (#897)
* fix: keep app running until rebuild succeeds Keep the previous app process alive while a new build is compiling, then stop it only after the build succeeds and before starting the replacement. Preserve stop_on_error behavior by stopping the existing process on failed pre-build or build steps when configured. Fixes #746 Also addresses #860. * test(runner): align rebuild test with retained app The PR changes rebuild behavior so the previous app keeps serving until a replacement build succeeds. The old rebuild test still expected a connection-refused gap, which made Ubuntu CI time out even though the new behavior was intentional. Validate the new contract by keeping the old server reachable during a delayed rebuild, then waiting for the rebuilt server response before shutdown. Also make the port helper check listen errors before reading the listener. Validation: go test ./runner -run '^TestRebuild$' -count=3 -v; go test ./...; make test-ci; ./hack/check.sh all
This commit is contained in:
committed by
GitHub
parent
7a43c0e5c3
commit
f896386121
+4
-4
@@ -427,9 +427,6 @@ func (e *Engine) start() {
|
||||
// No build is currently running
|
||||
}
|
||||
|
||||
// if current app is running, stop it
|
||||
e.stopBin()
|
||||
|
||||
go e.buildRun()
|
||||
}
|
||||
}
|
||||
@@ -526,6 +523,7 @@ func (e *Engine) buildRun() {
|
||||
if err = e.runPreCmd(); err != nil {
|
||||
e.runnerLog("failed to execute pre_cmd: %s", err.Error())
|
||||
if e.config.Build.StopOnError {
|
||||
e.stopBin()
|
||||
return
|
||||
}
|
||||
}
|
||||
@@ -536,6 +534,7 @@ func (e *Engine) buildRun() {
|
||||
// It only makes sense to run it if we stop on error. Otherwise when
|
||||
// running the binary again the error modal will be overwritten by
|
||||
// the reload.
|
||||
e.stopBin()
|
||||
if e.config.Proxy.Enabled {
|
||||
e.proxy.BuildFailed(BuildFailedMsg{
|
||||
Error: err.Error(),
|
||||
@@ -557,6 +556,8 @@ func (e *Engine) buildRun() {
|
||||
default:
|
||||
}
|
||||
|
||||
e.stopBin()
|
||||
|
||||
if err = e.runBin(); err != nil {
|
||||
e.runnerLog("failed to run, error: %s", err.Error())
|
||||
}
|
||||
@@ -746,7 +747,6 @@ func (e *Engine) runBin() error {
|
||||
e.proxy.Reload()
|
||||
}
|
||||
|
||||
e.stopBin()
|
||||
e.withLock(func() {
|
||||
e.binStopCh = killFunc(cmd, stdout, stderr, killCh, processExit)
|
||||
})
|
||||
|
||||
+164
-17
@@ -3,8 +3,10 @@ package runner
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"log"
|
||||
"net"
|
||||
"net/http"
|
||||
"os"
|
||||
"os/exec"
|
||||
"os/signal"
|
||||
@@ -216,12 +218,122 @@ func TestRunBin(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunBinDoesNotStopExistingBin(t *testing.T) {
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("uses POSIX sleep command")
|
||||
}
|
||||
|
||||
engine, err := NewEngine("", nil, true)
|
||||
require.NoError(t, err)
|
||||
engine.config.Log.Silent = true
|
||||
engine.config.Build.Entrypoint = entrypoint{}
|
||||
engine.config.Build.Bin = "sleep 1"
|
||||
|
||||
oldStopped := make(chan struct{})
|
||||
shutdown := make(chan chan int)
|
||||
engine.binStopCh = shutdown
|
||||
go func() {
|
||||
closer, ok := <-shutdown
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
close(closer)
|
||||
close(oldStopped)
|
||||
}()
|
||||
|
||||
err = engine.runBin()
|
||||
require.NoError(t, err)
|
||||
defer func() {
|
||||
engine.stopBin()
|
||||
close(shutdown)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-oldStopped:
|
||||
t.Fatal("runBin should not stop the previous binary; buildRun stops it after a successful build")
|
||||
default:
|
||||
}
|
||||
|
||||
err = waitForCondition(t, time.Second, func() bool {
|
||||
stillOld := false
|
||||
engine.withLock(func() {
|
||||
stillOld = engine.binStopCh == shutdown
|
||||
})
|
||||
return !stillOld
|
||||
}, "runBin to register the new binary stop channel")
|
||||
require.NoError(t, err)
|
||||
|
||||
select {
|
||||
case <-oldStopped:
|
||||
t.Fatal("runBin should not stop the previous binary; buildRun stops it after a successful build")
|
||||
default:
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildRunStopsExistingBinAfterSuccessfulBuild(t *testing.T) {
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("uses POSIX shell commands")
|
||||
}
|
||||
|
||||
engine, err := NewEngine("", nil, true)
|
||||
require.NoError(t, err)
|
||||
|
||||
engine.config.Log.Silent = true
|
||||
engine.config.Build.Cmd = "true"
|
||||
engine.config.Build.Entrypoint = entrypoint{}
|
||||
engine.config.Build.Bin = "sleep 1"
|
||||
|
||||
stopped := make(chan struct{})
|
||||
shutdown := make(chan chan int)
|
||||
engine.binStopCh = shutdown
|
||||
go func() {
|
||||
closer := <-shutdown
|
||||
close(closer)
|
||||
close(stopped)
|
||||
}()
|
||||
defer engine.stopBin()
|
||||
|
||||
engine.buildRun()
|
||||
|
||||
select {
|
||||
case <-stopped:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("expected successful build to stop the existing binary before running the new one")
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildRunStopsExistingBinWhenBuildFailsWithStopOnError(t *testing.T) {
|
||||
engine, err := NewEngine("", nil, true)
|
||||
require.NoError(t, err)
|
||||
|
||||
engine.config.Log.Silent = true
|
||||
engine.config.Build.Cmd = "exit 1"
|
||||
engine.config.Build.StopOnError = true
|
||||
|
||||
stopped := make(chan struct{})
|
||||
shutdown := make(chan chan int, 1)
|
||||
engine.binStopCh = shutdown
|
||||
go func() {
|
||||
closer := <-shutdown
|
||||
close(closer)
|
||||
close(stopped)
|
||||
}()
|
||||
|
||||
engine.buildRun()
|
||||
|
||||
select {
|
||||
case <-stopped:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("expected failed build with stop_on_error to stop the existing binary")
|
||||
}
|
||||
}
|
||||
|
||||
func GetPort() (int, func()) {
|
||||
l, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
port := l.Addr().(*net.TCPAddr).Port
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
port := l.Addr().(*net.TCPAddr).Port
|
||||
return port, func() {
|
||||
_ = l.Close()
|
||||
}
|
||||
@@ -242,6 +354,7 @@ func TestRebuild(t *testing.T) {
|
||||
chdir(t, tmpDir)
|
||||
engine, err := NewEngine("", nil, true)
|
||||
engine.config.Build.ExcludeUnchanged = true
|
||||
engine.config.Build.PreCmd = []string{"sleep 1"}
|
||||
if err != nil {
|
||||
t.Fatalf("Should not be fail: %s.", err)
|
||||
}
|
||||
@@ -262,22 +375,22 @@ func TestRebuild(t *testing.T) {
|
||||
|
||||
t.Logf("start change main.go")
|
||||
// change file of main.go
|
||||
// just append a new empty line to main.go
|
||||
file, err := os.OpenFile("main.go", os.O_APPEND|os.O_WRONLY, 0o644)
|
||||
err = writeGoCode(tmpDir, port, `http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
|
||||
_, _ = w.Write([]byte("rebuilt"))
|
||||
})`)
|
||||
if err != nil {
|
||||
t.Fatalf("Should not be fail: %s.", err)
|
||||
}
|
||||
defer file.Close()
|
||||
_, err = file.WriteString("\n")
|
||||
if err != nil {
|
||||
t.Fatalf("Should not be fail: %s.", err)
|
||||
|
||||
deadline := time.Now().Add(500 * time.Millisecond)
|
||||
for time.Now().Before(deadline) {
|
||||
if !checkPortHaveBeenUsed(port) {
|
||||
t.Fatal("previous binary should keep serving while rebuild is in progress")
|
||||
}
|
||||
time.Sleep(20 * time.Millisecond)
|
||||
}
|
||||
err = waitingPortConnectionRefused(t, port, time.Second*10)
|
||||
if err != nil {
|
||||
t.Fatalf("timeout: %s.", err)
|
||||
}
|
||||
t.Logf("connection refused")
|
||||
err = waitingPortReady(t, port, time.Second*10)
|
||||
|
||||
err = waitingHTTPBody(t, port, "rebuilt", time.Second*10)
|
||||
if err != nil {
|
||||
t.Fatalf("Should not be fail: %s.", err)
|
||||
}
|
||||
@@ -294,6 +407,32 @@ func TestRebuild(t *testing.T) {
|
||||
assert.True(t, checkPortConnectionRefused(port))
|
||||
}
|
||||
|
||||
func waitingHTTPBody(t *testing.T, port int, want string, timeout time.Duration) error {
|
||||
t.Helper()
|
||||
t.Logf("waiting port %d HTTP body %q", port, want)
|
||||
|
||||
client := http.Client{Timeout: 200 * time.Millisecond}
|
||||
deadline := time.Now().Add(timeout)
|
||||
ticker := time.NewTicker(20 * time.Millisecond)
|
||||
defer ticker.Stop()
|
||||
|
||||
for {
|
||||
resp, err := client.Get(fmt.Sprintf("http://127.0.0.1:%d/", port))
|
||||
if err == nil {
|
||||
body, readErr := io.ReadAll(resp.Body)
|
||||
_ = resp.Body.Close()
|
||||
if readErr == nil && string(body) == want {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
if time.Now().After(deadline) {
|
||||
return fmt.Errorf("timeout waiting for port %d HTTP body %q", port, want)
|
||||
}
|
||||
<-ticker.C
|
||||
}
|
||||
}
|
||||
|
||||
func waitingPortConnectionRefused(t *testing.T, port int, timeout time.Duration) error {
|
||||
t.Helper()
|
||||
t.Logf("waiting port %d connection refused", port)
|
||||
@@ -777,8 +916,7 @@ go 1.17
|
||||
return nil
|
||||
}
|
||||
|
||||
// generateGoCode generates golang code to tempdir
|
||||
func generateGoCode(dir string, port int) error {
|
||||
func writeGoCode(dir string, port int, setup string) error {
|
||||
code := fmt.Sprintf(`package main
|
||||
|
||||
import (
|
||||
@@ -787,9 +925,10 @@ import (
|
||||
)
|
||||
|
||||
func main() {
|
||||
%s
|
||||
log.Fatal(http.ListenAndServe("127.0.0.1:%v", nil))
|
||||
}
|
||||
`, port)
|
||||
`, setup, port)
|
||||
file, err := os.Create(dir + "/main.go")
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -802,13 +941,21 @@ func main() {
|
||||
if err := file.Close(); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// generateGoCode generates golang code to tempdir
|
||||
func generateGoCode(dir string, port int) error {
|
||||
if err := writeGoCode(dir, port, ""); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// generate go mod file
|
||||
mod := `module air.sample.com
|
||||
|
||||
go 1.17
|
||||
`
|
||||
file, err = os.Create(dir + "/go.mod")
|
||||
file, err := os.Create(dir + "/go.mod")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user