Merge pull request #431 from haoming29/imprv-test-shutdown

Provide ConfigureOriginAPI a cancel context that can be cancelled on shutdown

cmd/cache_serve.go

@@ -84,11 +84,11 @@ func serveCache( /*cmd*/ *cobra.Command /*args*/, []string) error {
 		config.CleanupTempResources()
 	}()
 
-	wg.Add(1)
 	err := xrootd.SetUpMonitoring(shutdownCtx, &wg)
 	if err != nil {
 		return err
 	}
+	wg.Add(1) // Add to wait group after SetUpMonitoring finishes to avoid deadlock
 
 	nsAds, err := getNSAdsFromDirector()
 	if err != nil {

cmd/director_serve.go

@@ -61,8 +61,8 @@ func serveDirector( /*cmd*/ *cobra.Command /*args*/, []string) error {
 	}
 	go director.PeriodicCacheReload()
 
-	wg.Add(1)
 	director.ConfigTTLCache(shutdownCtx, &wg)
+	wg.Add(1) // Add to wait group after ConfigTTLCache finishes to avoid deadlock
 
 	engine, err := web_ui.GetEngine()
 	if err != nil {

cmd/origin_serve.go

@@ -51,11 +51,11 @@ func serveOrigin( /*cmd*/ *cobra.Command /*args*/, []string) error {
 		config.CleanupTempResources()
 	}()
 
-	wg.Add(1)
 	err := xrootd.SetUpMonitoring(shutdownCtx, &wg)
 	if err != nil {
 		return err
 	}
+	wg.Add(1) // Add to wg afterward to ensure no error causes deadlock
 
 	originServer := &origin_ui.OriginServer{}
 	err = server_ui.CheckDefaults(originServer)
@@ -76,9 +76,10 @@ func serveOrigin( /*cmd*/ *cobra.Command /*args*/, []string) error {
 	}
 
 	// Set up the APIs unrelated to UI, which only contains director-based health test reporting endpoint for now
-	if err = origin_ui.ConfigureOriginAPI(engine); err != nil {
+	if err = origin_ui.ConfigureOriginAPI(engine, shutdownCtx, &wg); err != nil {
 		return err
 	}
+	wg.Add(1)
 	if err = server_ui.RegisterNamespaceWithRetry(); err != nil {
 		return err
 	}

director/cache_ads_test.go

@@ -212,8 +212,8 @@ func TestConfigCacheEviction(t *testing.T) {
 		// Start cache eviction
 		shutdownCtx, shutdownCancel := context.WithCancel(context.Background())
 		var wg sync.WaitGroup
-		wg.Add(1)
 		ConfigTTLCache(shutdownCtx, &wg)
+		wg.Add(1)
 		defer func() {
 			shutdownCancel()
 			wg.Wait()
@@ -275,8 +275,8 @@ func TestServerAdsCacheEviction(t *testing.T) {
 		// Start cache eviction
 		shutdownCtx, shutdownCancel := context.WithCancel(context.Background())
 		var wg sync.WaitGroup
-		wg.Add(1)
 		ConfigTTLCache(shutdownCtx, &wg)
+		wg.Add(1)
 		defer func() {
 			shutdownCancel()
 			wg.Wait()

director/origin_api_test.go

@@ -169,9 +169,8 @@ func TestNamespaceKeysCacheEviction(t *testing.T) {
 		// Start cache eviction
 		shutdownCtx, shutdownCancel := context.WithCancel(context.Background())
 		var wg sync.WaitGroup
-		wg.Add(1)
 		ConfigTTLCache(shutdownCtx, &wg)
-
+		wg.Add(1)
 		defer func() {
 			shutdownCancel()
 			wg.Wait()

metrics/xrootd_metrics.go

@@ -287,6 +287,7 @@ func ConfigureMonitoring(ctx context.Context, wg *sync.WaitGroup) (int, error) {
 		sessions.Stop()
 		userids.Stop()
 		transfers.Stop()
+		log.Infoln("Gracefully stopping metrics cache auto eviction...")
 	}()
 
 	go func() {

origin_ui/origin_api.go

@@ -19,12 +19,10 @@
 package origin_ui
 
 import (
+	"context"
 	"fmt"
-	"os"
-	"os/signal"
 	"strings"
 	"sync"
-	"syscall"
 	"time"
 
 	"github.com/gin-gonic/gin"
@@ -42,7 +40,6 @@ var (
 	// Duration to wait before timeout
 	// TODO: Do we want to make this a configurable value?
 	directorTimeoutDuration = 30 * time.Second
-	exitSignals             = make(chan os.Signal, 1)
 	exitLoop                = make(chan struct{})
 )
 
@@ -140,7 +137,7 @@ func directorTestResponse(ctx *gin.Context) {
 }
 
 // Configure API endpoints for origin that are not tied to UI
-func ConfigureOriginAPI(router *gin.Engine) error {
+func ConfigureOriginAPI(router *gin.Engine, ctx context.Context, wg *sync.WaitGroup) error {
 	if router == nil {
 		return errors.New("Origin configuration passed a nil pointer")
 	}
@@ -149,21 +146,19 @@ func ConfigureOriginAPI(router *gin.Engine) error {
 	// start the timer for the director test report timeout
 	resetDirectorTimeoutTimer()
 
-	// When program exits
-	signal.Notify(exitSignals, syscall.SIGINT, syscall.SIGTERM, syscall.SIGQUIT)
-
 	go func() {
 		// Gracefully stop the timer at the exit of the program
-		<-exitSignals
+		defer wg.Done()
+		<-ctx.Done()
 		timerMutex.Lock()
 		defer timerMutex.Unlock()
-		log.Infoln("Gracefully stopping the director-health test timeout timer...")
 		// Terminate the infinite loop to reset the timer
 		close(exitLoop)
 		if directorTimeoutTimer != nil {
 			directorTimeoutTimer.Stop()
 			directorTimeoutTimer = nil
 		}
+		log.Infoln("Gracefully stopping the director-health test timeout timer...")
 	}()
 
 	group := router.Group("/api/v1.0/origin-api")

xrootd/origin_test.go

@@ -84,7 +84,6 @@ func TestOrigin(t *testing.T) {
 
 	shutdownCtx, shutdownCancel := context.WithCancel(context.Background())
 	var wg sync.WaitGroup
-	wg.Add(1)
 
 	defer func() {
 		shutdownCancel()
@@ -93,6 +92,7 @@ func TestOrigin(t *testing.T) {
 
 	err = SetUpMonitoring(shutdownCtx, &wg)
 	require.NoError(t, err)
+	wg.Add(1)
 
 	configPath, err := ConfigXrootd(true)
 	require.NoError(t, err)