fix: make the predastore readiness gate a real readiness probe (#685)

* fix: make the predastore readiness gate a real readiness probe

checkPredastoreReady only did a TCP dial + close, so a listening socket
satisfied it well before predastore could serve TLS. In CI this passed
in 170 microseconds, 0.72s after predastore opened its listener, and
recovery started against a backend that wasn't serving yet — predastore
logged the probe itself as a TLS handshake EOF.

Replace the dial with an HTTPS GET against the configured predastore
endpoint on a package-level client (hoisted out of the 2s poll loop),
with a per-request 3s context timeout. Any HTTP response counts as
ready, including 403 from this deliberately unsigned request, which is
exactly what an unauthenticated S3 GET returns. Only a transport or TLS
failure means not-ready.

This narrows the race window and removes the spurious TLS-handshake-EOF
log lines predastore emits on every daemon start, but it does not close
the window on its own: predastore can serve TLS before its storage
backend answers reads correctly. The load-bearing fix is the viperblock
VBState read retry landing separately.

* style: trim the predastore readiness comments to three lines

spinifex/daemon/daemon.go

@@ -2085,17 +2085,36 @@ func (d *Daemon) checkViperblockReady() bool {
 	return d.natsConn.IsConnected()
 }
 
-// checkPredastoreReady checks if predastore is reachable via TCP.
+// predastoreReadinessClient is hoisted to package scope so the 2s readiness
+// poll loop doesn't build a fresh transport every call. Verification is off
+// because the probe authenticates nothing and reads no data.
+var predastoreReadinessClient = &http.Client{
+	Transport: &http.Transport{
+		TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // readiness probe only, no data exchanged
+	},
+}
+
+// checkPredastoreReady reports whether predastore is serving HTTPS, not just
+// listening. Any response counts as ready, including the 401/403 an S3
+// endpoint returns for this deliberately unsigned request.
 func (d *Daemon) checkPredastoreReady() bool {
 	host := admin.DialTarget(d.config.Predastore.Host)
 	if host == "" {
 		return true // no predastore configured, skip check
 	}
-	conn, err := net.DialTimeout("tcp", host, 3*time.Second)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	defer cancel()
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://"+host+"/", nil)
 	if err != nil {
 		return false
 	}
-	_ = conn.Close()
+	resp, err := predastoreReadinessClient.Do(req)
+	if err != nil {
+		return false
+	}
+	defer resp.Body.Close()
 	return true
 }
 

spinifex/daemon/predastore_ready_test.go

@@ -0,0 +1,52 @@
+package daemon
+
+import (
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"github.com/mulgadc/spinifex/spinifex/config"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestCheckPredastoreReady_HTTPSServing asserts that any HTTP response from
+// predastore's endpoint counts as ready, including 403 — the response an S3
+// endpoint gives this deliberately unsigned probe request. The prior TCP-dial
+// probe treated a bare listening socket as ready and raced predastore's TLS
+// handshake; this proves the replacement actually waits for HTTP.
+func TestCheckPredastoreReady_HTTPSServing(t *testing.T) {
+	srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusForbidden)
+	}))
+	t.Cleanup(srv.Close)
+
+	u, err := url.Parse(srv.URL)
+	require.NoError(t, err)
+
+	d := &Daemon{config: &config.Config{Predastore: config.PredastoreConfig{Host: u.Host}}}
+	assert.True(t, d.checkPredastoreReady())
+}
+
+// TestCheckPredastoreReady_ClosedPort asserts a port nothing is listening on
+// is reported not-ready, rather than panicking or failing open.
+func TestCheckPredastoreReady_ClosedPort(t *testing.T) {
+	// Bind then immediately close to get a real, currently-unused port.
+	l, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	addr := l.Addr().String()
+	require.NoError(t, l.Close())
+
+	d := &Daemon{config: &config.Config{Predastore: config.PredastoreConfig{Host: addr}}}
+	assert.False(t, d.checkPredastoreReady())
+}
+
+// TestCheckPredastoreReady_NoPredastoreConfigured asserts the short-circuit:
+// an empty Predastore.Host means no predastore runs on this node, and the
+// check must return ready without dialling anything.
+func TestCheckPredastoreReady_NoPredastoreConfigured(t *testing.T) {
+	d := &Daemon{config: &config.Config{}}
+	assert.True(t, d.checkPredastoreReady())
+}