Boot-test runServeCmd end to end (serve.go ~7% to ~64% coverage) (#48261)
Adds an end-to-end boot test for `runServeCmd`, the main server entry point. This is the coverage milestone for #33370: `serve.go` goes from ~7% to ~64%, and `runServeCmd` itself from 0% to ~62%. The earlier PRs on this issue (#44929, #45343, #45583, #46166, #46421, #46517, #46742, #46830, #46893, #47151, #47562, #47891) extracted testable pieces out of `runServeCmd`, but the function itself stayed at 0% — it blocks on an OS signal and wires the entire server together, so the only way to cover it is to actually boot it. This PR does that. `TestRunServeCmd` (gated behind `MYSQL_TEST` + `REDIS_TEST`) boots the full server against a real migrated test MySQL and Redis, waits for `/healthz`, then cancels the command context to trigger a graceful shutdown. It covers two paths: - **Full boot with Apple MDM enabled** — a 32-byte server private key brings up the Apple MDM protocol services and the host-identity / conditional-access SCEP setup, so the boot exercises the MDM startup path as well as the core wiring, cron schedules, and HTTP server. - **Fail-fast on bad config** — an invalid Redis host-cache configuration (enabled with a non-positive TTL) aborts startup through `initFatal` and returns rather than serving, covering the Redis-init error path and the nil-pool guard. Beyond coverage, this doubles as a regression net for the ongoing `runServeCmd` slicing: a future change that breaks startup now fails this test instead of reaching a release. **One production change**, in `runServeCmd`'s shutdown `select`: it now also watches `cmd.Context().Done()`. This is inert in production — the root command runs via `Execute()` (not `ExecuteContext()`), so `cmd.Context()` is `context.Background()` and never cancels. Only the test runs the command with a cancelable context, which is how it shuts the server down without sending a real signal (a `SIGTERM` would kill the test binary). A couple of notes for reviewers: - The test uses `os.Setenv` (not `t.Setenv`) because the MySQL test helper marks the test parallel; the boot scenarios run as serial subtests so the process-global config env doesn't race. - `runServeCmd` registers metrics with the process-global Prometheus registry, which can only happen once per process, so there is a single full boot here; the error-path scenario fails before that registration. - The test DB is loaded from a schema dump that doesn't mark every data migration as applied, so the boot runs with `FLEET_UPGRADES_ALLOW_MISSING_MIGRATIONS=1`. It adds ~2s to the `cmd/fleet` (`main`) test bundle, which is well off the CI critical path. **Related issue:** Refs #33370 # Checklist for submitter - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually (verified locally: boots to /healthz, graceful shutdown, ~64% serve.go coverage) - Changes file: not applicable — internal test coverage with no user-visible behavior change <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved server shutdown handling to stop cleanly when the running command’s context is canceled, not only on OS signals. * Added stronger startup validation to fail fast for invalid Redis host-cache configuration (e.g., non-positive TTL). * **Tests** * Added an end-to-end test that boots the server against real MySQL/Redis, verifies graceful startup/shutdown, and confirms fast-fail behavior for misconfiguration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Rajendra kadam
@@ -1037,6 +1037,11 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev
|
||||
select {
|
||||
case <-sig:
|
||||
case <-dbFatalCh:
|
||||
// cmd.Context() is context.Background() in production (the root command
|
||||
// is run via Execute, not ExecuteContext), so this case never fires
|
||||
// there. Tests run the command with a cancelable context to trigger a
|
||||
// graceful shutdown without sending an OS signal.
|
||||
case <-cmd.Context().Done():
|
||||
}
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
|
||||
defer cancel()
|
||||
|
||||
@@ -0,0 +1,181 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net"
|
||||
"net/http"
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/fleetdm/fleet/v4/pkg/fleethttp"
|
||||
"github.com/fleetdm/fleet/v4/server/config"
|
||||
testing_utils "github.com/fleetdm/fleet/v4/server/platform/mysql/testing_utils"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func freeLocalAddr(t *testing.T) string {
|
||||
l, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
require.NoError(t, err)
|
||||
addr := l.Addr().String()
|
||||
require.NoError(t, l.Close())
|
||||
return addr
|
||||
}
|
||||
|
||||
// fatalRecorder swaps the package-level initFatal var to collect calls instead
|
||||
// of terminating the test binary, restoring the original on cleanup. This is
|
||||
// safe because the boot test does not run in parallel.
|
||||
type fatalRecorder struct{ calls []string }
|
||||
|
||||
func installFatalRecorder(t *testing.T) *fatalRecorder {
|
||||
r := &fatalRecorder{}
|
||||
orig := initFatal
|
||||
initFatal = func(err error, msg string) {
|
||||
r.calls = append(r.calls, msg+": "+err.Error())
|
||||
}
|
||||
t.Cleanup(func() { initFatal = orig })
|
||||
return r
|
||||
}
|
||||
|
||||
func (r *fatalRecorder) contains(substr string) bool {
|
||||
for _, c := range r.calls {
|
||||
if strings.Contains(c, substr) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// configureBootEnv points the server's config at the given migrated test
|
||||
// database and the test Redis (on the given Redis logical database), on a free
|
||||
// local port, with TLS off. It returns the server's address.
|
||||
func configureBootEnv(t *testing.T, dbName string, redisDB int) string {
|
||||
serverAddr := freeLocalAddr(t)
|
||||
redisAddr := os.Getenv("REDIS_TEST_ADDRESS")
|
||||
if redisAddr == "" {
|
||||
redisAddr = "localhost:6379"
|
||||
}
|
||||
|
||||
t.Setenv("FLEET_MYSQL_ADDRESS", testing_utils.TestAddress)
|
||||
t.Setenv("FLEET_MYSQL_USERNAME", testing_utils.TestUsername)
|
||||
t.Setenv("FLEET_MYSQL_PASSWORD", testing_utils.TestPassword)
|
||||
t.Setenv("FLEET_MYSQL_DATABASE", dbName)
|
||||
t.Setenv("FLEET_REDIS_ADDRESS", redisAddr)
|
||||
t.Setenv("FLEET_REDIS_DATABASE", strconv.Itoa(redisDB))
|
||||
t.Setenv("FLEET_SERVER_ADDRESS", serverAddr)
|
||||
t.Setenv("FLEET_SERVER_TLS", "false")
|
||||
// The test schema is loaded from a dump that does not mark every data
|
||||
// migration as applied, so allow the server to boot past the migration
|
||||
// status check (the schema is functionally complete for a boot test).
|
||||
t.Setenv("FLEET_UPGRADES_ALLOW_MISSING_MIGRATIONS", "1")
|
||||
|
||||
return serverAddr
|
||||
}
|
||||
|
||||
// runServe runs the serve command with a cancelable context and any extra
|
||||
// command-line flags, returning a channel that receives the command's exit
|
||||
// error when runServeCmd returns.
|
||||
func runServe(ctx context.Context, extraArgs ...string) <-chan error {
|
||||
rootCmd := createRootCmd()
|
||||
configManager := config.NewManager(rootCmd)
|
||||
rootCmd.AddCommand(createServeCmd(configManager))
|
||||
rootCmd.SetArgs(append([]string{"serve", "--dev_license"}, extraArgs...))
|
||||
|
||||
done := make(chan error, 1)
|
||||
go func() { done <- rootCmd.ExecuteContext(ctx) }()
|
||||
return done
|
||||
}
|
||||
|
||||
func waitHealthy(t *testing.T, serverAddr string) bool {
|
||||
client := fleethttp.NewClient(fleethttp.WithTimeout(2 * time.Second))
|
||||
return assert.Eventually(t, func() bool {
|
||||
resp, err := client.Get("http://" + serverAddr + "/healthz") //nolint:gosec
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
return resp.StatusCode == http.StatusOK
|
||||
}, 60*time.Second, 250*time.Millisecond)
|
||||
}
|
||||
|
||||
func waitShutdown(t *testing.T, done <-chan error) {
|
||||
select {
|
||||
case err := <-done:
|
||||
require.NoError(t, err)
|
||||
case <-time.After(60 * time.Second):
|
||||
t.Fatal("server did not shut down within 60s of context cancellation")
|
||||
}
|
||||
}
|
||||
|
||||
// TestRunServeCmd boots the full server via runServeCmd against a real (migrated)
|
||||
// test MySQL and Redis and exercises the entire startup path end to end.
|
||||
//
|
||||
// The test does not run in parallel: it swaps the package-global initFatal and
|
||||
// sets process-wide config env, and the scenarios share one database via serial
|
||||
// subtests.
|
||||
func TestRunServeCmd(t *testing.T) {
|
||||
if os.Getenv("MYSQL_TEST") == "" || os.Getenv("REDIS_TEST") == "" {
|
||||
t.Skip("requires MYSQL_TEST=1 and REDIS_TEST=1")
|
||||
}
|
||||
|
||||
const dbName = "fleet_serve_boot_test"
|
||||
// Load the schema directly (rather than via CreateMySQLDS) so this test is
|
||||
// not marked parallel — it mutates process-global state (config env and the
|
||||
// initFatal var) that must not race with other package tests.
|
||||
testing_utils.LoadDefaultSchema(t, dbName, &testing_utils.DatastoreTestOptions{})
|
||||
|
||||
// Boots the full server and shuts it down gracefully on context
|
||||
// cancellation. A server private key is set so the boot also brings up the
|
||||
// Apple MDM protocol services and the host-identity / conditional-access
|
||||
// SCEP setup, exercising the MDM-enabled startup path.
|
||||
//
|
||||
// NOTE: runServeCmd registers metrics collectors with the process-global
|
||||
// Prometheus registry, which can only happen once per process, so this is
|
||||
// the single full boot in this package. Error-path scenarios below must fail
|
||||
// before that registration.
|
||||
t.Run("boots with Apple MDM enabled and shuts down gracefully", func(t *testing.T) {
|
||||
rec := installFatalRecorder(t)
|
||||
serverAddr := configureBootEnv(t, dbName, 12)
|
||||
t.Setenv("FLEET_SERVER_PRIVATE_KEY", strings.Repeat("a", 32))
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
done := runServe(ctx)
|
||||
// Always tear the server down, even if an assertion below aborts.
|
||||
defer func() {
|
||||
cancel()
|
||||
waitShutdown(t, done)
|
||||
}()
|
||||
|
||||
healthy := waitHealthy(t, serverAddr)
|
||||
require.Emptyf(t, rec.calls, "initFatal was called during boot: %v", rec.calls)
|
||||
require.True(t, healthy, "server did not become healthy")
|
||||
})
|
||||
|
||||
// An invalid Redis host-cache configuration (enabled with a non-positive
|
||||
// TTL) must make the server fail fast through initFatal and return rather
|
||||
// than start serving, exercising the Redis-init error path and the nil-pool
|
||||
// guard in runServeCmd.
|
||||
t.Run("refuses to boot on invalid host-cache config", func(t *testing.T) {
|
||||
rec := installFatalRecorder(t)
|
||||
configureBootEnv(t, dbName, 13)
|
||||
t.Setenv("FLEET_REDIS_HOST_CACHE_ENABLED", "true")
|
||||
t.Setenv("FLEET_REDIS_HOST_CACHE_TTL", "0")
|
||||
|
||||
// The server fails fast on the invalid config and returns on its own, so
|
||||
// no manual cancellation is needed; t.Context() is canceled at cleanup.
|
||||
done := runServe(t.Context())
|
||||
|
||||
select {
|
||||
case <-done:
|
||||
case <-time.After(60 * time.Second):
|
||||
t.Fatal("server did not return after invalid host-cache config")
|
||||
}
|
||||
|
||||
require.NotEmpty(t, rec.calls, "expected initFatal for invalid host-cache config")
|
||||
assert.Truef(t, rec.contains("host_cache_ttl must be > 0"),
|
||||
"expected a host-cache validation failure, got: %v", rec.calls)
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user