Skip to content

Commit 6968ad0

Browse files
committed
Support reloading MCP servers from config file on SIGHUP signal
1 parent 3789a33 commit 6968ad0

File tree

12 files changed

+660
-95
lines changed

12 files changed

+660
-95
lines changed

cmd/add_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func TestAddCmd_RegistryFails(t *testing.T) {
142142
cmdopts.WithRegistryBuilder(&fakeBuilder{err: errors.New("registry error")}),
143143
)
144144
require.NoError(t, err)
145-
145+
cmdObj.SetOut(io.Discard)
146146
cmdObj.SetArgs([]string{"server1"})
147147
err = cmdObj.Execute()
148148
require.Error(t, err)

cmd/config/daemon/validate_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ type mockValidateConfigLoader struct {
1919
err error
2020
}
2121

22-
func (m *mockValidateConfigLoader) Load(path string) (config.Modifier, error) {
22+
func (m *mockValidateConfigLoader) Load(_ string) (config.Modifier, error) {
2323
if m.err != nil {
2424
return nil, m.err
2525
}
@@ -159,7 +159,7 @@ func TestValidateCmd_ConfigLoadError(t *testing.T) {
159159

160160
// Assertions
161161
require.Error(t, err)
162-
require.Contains(t, err.Error(), "failed to load config")
162+
require.EqualError(t, err, "mock config load error")
163163
}
164164

165165
func TestValidateCmd_MultipleValidationErrors(t *testing.T) {

cmd/config/export/export.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func (c *Cmd) run(cmd *cobra.Command, _ []string) error {
101101
func (c *Cmd) handleExport() error {
102102
cfg, err := c.cfgLoader.Load(flags.ConfigFile)
103103
if err != nil {
104-
return fmt.Errorf("failed to load config: %w", err)
104+
return err
105105
}
106106

107107
rtCtx, err := c.ctxLoader.Load(flags.RuntimeFile)

cmd/config/export/export_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package export
22

33
import (
4+
"io"
45
"os"
56
"path/filepath"
67
"strings"
@@ -115,6 +116,9 @@ func TestExportCommand_Integration(t *testing.T) {
115116
exportCmd, err := NewCmd(&cmd.BaseCmd{})
116117
require.NoError(t, err)
117118

119+
// Mute the terminal output.
120+
exportCmd.SetOut(io.Discard)
121+
118122
// Set command-specific flags
119123
require.NoError(t, exportCmd.Flags().Set("context-output", contextOutput))
120124
require.NoError(t, exportCmd.Flags().Set("contract-output", contractOutput))

cmd/daemon.go

Lines changed: 113 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ func newDaemonCobraCmd(daemonCmd *DaemonCmd) *cobra.Command {
282282

283283
cobraCommand.MarkFlagsMutuallyExclusive("dev", flagAddr)
284284

285-
// Note: Additional CORS validation required to check CORS flags are present alongside --cors-enable.
285+
// NOTE: Additional CORS validation required to check CORS flags are present alongside --cors-enable.
286286
cobraCommand.MarkFlagsRequiredTogether(flagCORSEnable, flagCORSOrigin)
287287

288288
return cobraCommand
@@ -315,10 +315,16 @@ func (c *DaemonCmd) run(cmd *cobra.Command, _ []string) error {
315315
return err
316316
}
317317

318+
// Load the new configuration.
319+
cfg, err := c.LoadConfig(c.cfgLoader)
320+
if err != nil {
321+
return fmt.Errorf("%w: %w", config.ErrConfigLoadFailed, err)
322+
}
323+
318324
// Load configuration layers (config file, then flag overrides).
319-
warnings, err := c.loadConfigurationLayers(logger, cmd)
325+
warnings, err := c.loadConfigurationLayers(logger, cmd, cfg)
320326
if err != nil {
321-
return fmt.Errorf("failed to load configuration: %w", err)
327+
return err
322328
}
323329

324330
if c.dev && len(warnings) > 0 {
@@ -342,10 +348,14 @@ func (c *DaemonCmd) run(cmd *cobra.Command, _ []string) error {
342348
return err
343349
}
344350

345-
// Load runtime servers from config and context.
346-
runtimeServers, err := c.loadRuntimeServers()
351+
execCtx, err := c.ctxLoader.Load(flags.RuntimeFile)
347352
if err != nil {
348-
return fmt.Errorf("error loading runtime servers: %w", err)
353+
return fmt.Errorf("failed to load runtime context: %w", err)
354+
}
355+
356+
runtimeServers, err := runtime.AggregateConfigs(cfg, execCtx)
357+
if err != nil {
358+
return fmt.Errorf("failed to aggregate configs: %w", err)
349359
}
350360

351361
deps, err := daemon.NewDependencies(logger, addr, runtimeServers)
@@ -368,16 +378,22 @@ func (c *DaemonCmd) run(cmd *cobra.Command, _ []string) error {
368378
return fmt.Errorf("failed to create mcpd daemon instance: %w", err)
369379
}
370380

371-
daemonCtx, daemonCtxCancel := signal.NotifyContext(
372-
context.Background(),
373-
os.Interrupt,
374-
syscall.SIGTERM, syscall.SIGINT,
375-
)
376-
defer daemonCtxCancel()
381+
// Create signal contexts for shutdown and reload.
382+
shutdownCtx, shutdownCancel := context.WithCancel(context.Background())
383+
defer shutdownCancel()
384+
385+
// Setup signal handling.
386+
sigChan := make(chan os.Signal, 1)
387+
signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM, syscall.SIGINT, syscall.SIGHUP)
388+
defer signal.Stop(sigChan)
389+
390+
// Create reload channel for SIGHUP handling.
391+
reloadChan := make(chan struct{}, 1)
392+
defer close(reloadChan) // Ensure channel is closed on function exit
377393

378394
runErr := make(chan error, 1)
379395
go func() {
380-
if err := d.StartAndManage(daemonCtx); err != nil && !errors.Is(err, context.Canceled) {
396+
if err := d.StartAndManage(shutdownCtx); err != nil && !errors.Is(err, context.Canceled) {
381397
runErr <- err
382398
}
383399
close(runErr)
@@ -387,15 +403,36 @@ func (c *DaemonCmd) run(cmd *cobra.Command, _ []string) error {
387403
c.printDevBanner(cmd.OutOrStdout(), logger, addr)
388404
}
389405

390-
select {
391-
case <-daemonCtx.Done():
392-
logger.Info("Shutting down daemon...")
393-
err := <-runErr // Wait for cleanup and deferred logging
394-
logger.Info("Shutdown complete")
395-
return err // Graceful Ctrl+C / SIGTERM
396-
case err := <-runErr:
397-
logger.Error("daemon exited with error", "error", err)
398-
return err // Propagate daemon failure
406+
// Start signal handling in background.
407+
go c.handleSignals(logger, sigChan, reloadChan, shutdownCancel)
408+
409+
// Start the daemon's main loop which responds to reloads, shutdowns and startup errors.
410+
for {
411+
select {
412+
case <-reloadChan:
413+
logger.Info("Reloading servers...")
414+
if err := c.reloadServers(shutdownCtx, d); err != nil {
415+
logger.Error("Failed to reload servers, exiting to prevent inconsistent state", "error", err)
416+
// Signal shutdown to exit cleanly.
417+
shutdownCancel()
418+
return fmt.Errorf("configuration reload failed with unrecoverable error: %w", err)
419+
}
420+
421+
logger.Info("Configuration reloaded successfully")
422+
case <-shutdownCtx.Done():
423+
logger.Info("Shutting down daemon...")
424+
err := <-runErr // Wait for cleanup and deferred logging
425+
logger.Info("Shutdown complete")
426+
427+
return err // Graceful shutdown
428+
case err := <-runErr:
429+
if err != nil {
430+
logger.Error("daemon exited with error", "error", err)
431+
return err // Propagate daemon failure
432+
}
433+
434+
return nil
435+
}
399436
}
400437
}
401438

@@ -428,19 +465,17 @@ func formatValue(value any) string {
428465
// It follows the precedence order: flags > config file > defaults.
429466
// CLI flags override config file values when explicitly set.
430467
// Returns warnings for each flag override and any error encountered.
431-
func (c *DaemonCmd) loadConfigurationLayers(logger hclog.Logger, cmd *cobra.Command) ([]string, error) {
432-
cfgModifier, err := c.cfgLoader.Load(flags.ConfigFile)
433-
if err != nil {
434-
return nil, err
435-
}
436-
437-
cfg, ok := cfgModifier.(*config.Config)
438-
if !ok {
439-
return nil, fmt.Errorf("config file contains invalid configuration structure")
468+
func (c *DaemonCmd) loadConfigurationLayers(
469+
logger hclog.Logger,
470+
cmd *cobra.Command,
471+
cfg *config.Config,
472+
) ([]string, error) {
473+
if cfg == nil {
474+
return nil, fmt.Errorf("config data not present, cannot apply configuration layers")
440475
}
441476

477+
// No daemon config section - flags and defaults will be used.
442478
if cfg.Daemon == nil {
443-
// No daemon config section - flags and defaults will be used.
444479
return nil, nil
445480
}
446481

@@ -728,24 +763,62 @@ func (c *DaemonCmd) loadConfigCORS(cors *config.CORSConfigSection, logger hclog.
728763
return warnings
729764
}
730765

731-
// loadRuntimeServers loads the configuration and aggregates it with runtime context to produce runtime servers.
732-
func (c *DaemonCmd) loadRuntimeServers() ([]runtime.Server, error) {
733-
cfgModifier, err := c.cfgLoader.Load(flags.ConfigFile)
766+
// handleSignals processes OS signals for daemon lifecycle management.
767+
// This function is intended to be called in a dedicated goroutine.
768+
//
769+
// SIGHUP signals trigger configuration reloads via reloadChan.
770+
// Termination signals (SIGTERM, SIGINT, os.Interrupt) trigger graceful shutdown via shutdownCancel.
771+
// The function runs until a shutdown signal is received or sigChan is closed.
772+
// Non-blocking sends to reloadChan prevent duplicate reload requests.
773+
func (c *DaemonCmd) handleSignals(
774+
logger hclog.Logger,
775+
sigChan <-chan os.Signal,
776+
reloadChan chan<- struct{},
777+
shutdownCancel context.CancelFunc,
778+
) {
779+
for sig := range sigChan {
780+
switch sig {
781+
case syscall.SIGHUP:
782+
logger.Info("Received SIGHUP, triggering config reload")
783+
select {
784+
case reloadChan <- struct{}{}:
785+
// Reload signal sent.
786+
default:
787+
// Reload already pending, skip.
788+
logger.Warn("Config reload already in progress, skipping")
789+
}
790+
case os.Interrupt, syscall.SIGTERM, syscall.SIGINT:
791+
logger.Info("Received shutdown signal", "signal", sig)
792+
shutdownCancel()
793+
return
794+
}
795+
}
796+
}
797+
798+
// reloadServers reloads server configuration from config files.
799+
// This method only reloads runtime servers; daemon config changes require a restart.
800+
func (c *DaemonCmd) reloadServers(ctx context.Context, d *daemon.Daemon) error {
801+
cfg, err := c.LoadConfig(c.cfgLoader)
734802
if err != nil {
735-
return nil, fmt.Errorf("failed to load config: %w", err)
803+
return fmt.Errorf("%w: %w", config.ErrConfigLoadFailed, err)
736804
}
737805

738806
execCtx, err := c.ctxLoader.Load(flags.RuntimeFile)
739807
if err != nil {
740-
return nil, fmt.Errorf("failed to load runtime context: %w", err)
808+
return fmt.Errorf("failed to load runtime context: %w", err)
741809
}
742810

743-
servers, err := runtime.AggregateConfigs(cfgModifier, execCtx)
811+
newServers, err := runtime.AggregateConfigs(cfg, execCtx)
744812
if err != nil {
745-
return nil, fmt.Errorf("failed to aggregate configs: %w", err)
813+
return fmt.Errorf("failed to aggregate configs: %w", err)
814+
}
815+
816+
// Reload the servers in the daemon.
817+
if err := d.ReloadServers(ctx, newServers); err != nil {
818+
return fmt.Errorf("failed to reload servers: %w", err)
746819
}
747820

748-
return servers, nil
821+
return nil
749822
}
750823

751824
// validateFlags validates the command flags and their relationships.

0 commit comments

Comments
 (0)