Unverified Commit 1d6c7300 authored by Minseong Choi's avatar Minseong Choi 💬
Browse files

fix(nano): drain in-flight logins on shutdown 

The installer and the config template tell the operator to run
systemctl restart felis-nano after editing the source list. nano had no
signal handling, so SIGTERM killed it mid-request: a login waiting on an
upstream had its connection reset, and Velocity disconnected that
player with "authentication servers are down". felis api already drains
on shutdown; nano did not.

nano now listens itself, serves until SIGINT or SIGTERM, then shuts the
server down gracefully with a 30-second limit. That outlasts the source
scan of any realistic list, at five seconds per source, and stays well
inside systemd's default 90-second stop timeout.

The new test holds a request inside the handler, cancels the serve
context, and checks that serveNano is still running 200 ms later, that
the held request then gets its answer, and that serveNano returns 0.
Replacing the graceful shutdown with Close fails it.
parent fa3eda52

cmd/felis/nano.go

+34 −2
Changes for cmd/felis/nano.go: 34 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -29,7 +29,12 @@ import ( 
	"flag"

	"fmt"

	"io"

	"net"

	"net/http"

	"os"

	"os/signal"

	"syscall"

	"time"



	"felis.lolicon.best/internal/api"

	"felis.lolicon.best/internal/config"
@@ -71,13 +76,40 @@ func cmdNano(args []string, stdout, stderr io.Writer) int { 
		fmt.Fprintf(stderr, "  [%d] %s -> %s\n", i+1, s.Tag, s.URL)

	}



	srv := newAPIServer(*listen, nanoHandler(cfg.AuthSources, stderr))

	if err := srv.ListenAndServe(); err != nil {

	ln, err := net.Listen("tcp", *listen)

	if err != nil {

		fmt.Fprintln(stderr, "felis nano:", err)

		return 1

	}

	ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)

	defer stop()

	return serveNano(ctx, newAPIServer(*listen, nanoHandler(cfg.AuthSources, stderr)), ln, stderr)

}



// nanoDrainTimeout outlasts the source scan of any realistic list (each source is given

// five seconds) and stays well inside systemd's default 90-second stop timeout.

const nanoDrainTimeout = 30 * time.Second



// serveNano serves until ctx ends, then drains. A restart, the documented way to pick up a

// config edit, sends SIGTERM; without the drain a login already waiting on an upstream has

// its connection reset, and Velocity tells that player the auth servers are down.

func serveNano(ctx context.Context, srv *http.Server, ln net.Listener, stderr io.Writer) int {

	errc := make(chan error, 1)

	go func() { errc <- srv.Serve(ln) }()

	select {

	case err := <-errc:

		fmt.Fprintln(stderr, "felis nano:", err)

		return 1

	case <-ctx.Done():

		shutdownCtx, cancel := context.WithTimeout(context.Background(), nanoDrainTimeout)

		defer cancel()

		if err := srv.Shutdown(shutdownCtx); err != nil {

			fmt.Fprintln(stderr, "felis nano: shutdown:", err)

			return 1

		}

		return 0

	}

}



// nanoHandler is what felis nano serves: the shared hasJoined handler, Mojang first, behind

// a request log.

cmd/felis/nano_test.go

+47 −0
Changes for cmd/felis/nano_test.go: 47 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -2,14 +2,61 @@ package main 


import (

	"bytes"

	"context"

	"io"

	"net"

	"net/http"

	"net/http/httptest"

	"strconv"

	"strings"

	"testing"

	"time"

	"unicode/utf8"

)



// A stop signal that lands while a login is waiting on an upstream must let that login

// finish: the request is answered, and serveNano returns only afterwards.

func TestNanoDrainsInFlightLoginOnShutdown(t *testing.T) {

	entered, release := make(chan struct{}), make(chan struct{})

	srv := newAPIServer("", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

		close(entered)

		<-release

		w.WriteHeader(http.StatusNoContent)

	}))

	ln, err := net.Listen("tcp", "127.0.0.1:0")

	if err != nil {

		t.Fatal(err)

	}

	ctx, stop := context.WithCancel(context.Background())

	done := make(chan int, 1)

	go func() { done <- serveNano(ctx, srv, ln, io.Discard) }()



	got := make(chan int, 1)

	go func() {

		resp, err := http.Get("http://" + ln.Addr().String() + "/session/minecraft/hasJoined")

		if err != nil {

			got <- -1

			return

		}

		resp.Body.Close()

		got <- resp.StatusCode

	}()

	<-entered

	stop()

	select {

	case <-done:

		t.Fatal("serveNano returned while a login was still in flight")

	case <-time.After(200 * time.Millisecond):

	}

	close(release)

	if code := <-got; code != http.StatusNoContent {

		t.Fatalf("in-flight login got %d, want its answer (204)", code)

	}

	if rc := <-done; rc != 0 {

		t.Fatalf("serveNano = %d after a clean drain, want 0", rc)

	}

}



// The request log prints text the caller chose. A bidi override must not reorder the line,

// an invalid byte must not make journald store the entry as a blob, and a huge query must

// not become a huge log line. serverId is left out so the handler answers without asking