fix: properly handle shutdown signal for admin dashboard

cmd/argocd/commands/admin/dashboard.go

@@ -1,7 +1,10 @@
 package admin
 
 import (
+	"context"
 	"fmt"
+	"os/signal"
+	"syscall"
 
 	"github.com/spf13/cobra"
 	"k8s.io/client-go/tools/clientcmd"
@@ -15,22 +18,55 @@ import (
 	"github.com/argoproj/argo-cd/v3/util/errors"
 )
 
+// DashboardConfig holds the configuration for starting the dashboard
+type DashboardConfig struct {
+	Port         int
+	Address      string
+	ClientOpts   *argocdclient.ClientOptions
+	ClientConfig clientcmd.ClientConfig
+	Context      string
+}
+
+type dashboard struct {
+	startLocalServer func(ctx context.Context, clientOpts *argocdclient.ClientOptions, contextName string, port *int, address *string, clientConfig clientcmd.ClientConfig) (func(), error)
+}
+
+// NewDashboard initializes a new dashboard with default dependencies
+func NewDashboard() *dashboard {
+	return &dashboard{
+		startLocalServer: headless.MaybeStartLocalServer,
+	}
+}
+
+// Run runs the dashboard and blocks until context is done
+func (ds *dashboard) Run(ctx context.Context, config *DashboardConfig) error {
+	ctx, stop := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM)
+	defer stop()
+	config.ClientOpts.Core = true
+	println("starting dashboard")
+	shutDownFunc, err := ds.startLocalServer(ctx, config.ClientOpts, config.Context, &config.Port, &config.Address, config.ClientConfig)
+	if err != nil {
+		return fmt.Errorf("could not start dashboard: %w", err)
+	}
+	fmt.Printf("Argo CD UI is available at http://%s:%d\n", config.Address, config.Port)
+	<-ctx.Done()
+	stop() // unregister the signal handler as soon as we receive a signal
+	println("signal received, shutting down dashboard")
+	if shutDownFunc != nil {
+		shutDownFunc()
+	}
+	println("clean shutdown")
+	return nil
+}
+
 func NewDashboardCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command {
-	var (
-		port         int
-		address      string
-		clientConfig clientcmd.ClientConfig
-	)
+	config := &DashboardConfig{ClientOpts: clientOpts}
 	cmd := &cobra.Command{
 		Use:   "dashboard",
 		Short: "Starts Argo CD Web UI locally",
 		Run: func(cmd *cobra.Command, _ []string) {
-			ctx := cmd.Context()
-
-			clientOpts.Core = true
-			errors.CheckError(headless.MaybeStartLocalServer(ctx, clientOpts, initialize.RetrieveContextIfChanged(cmd.Flag("context")), &port, &address, clientConfig))
-			println(fmt.Sprintf("Argo CD UI is available at http://%s:%d", address, port))
-			<-ctx.Done()
+			config.Context = initialize.RetrieveContextIfChanged(cmd.Flag("context"))
+			errors.CheckError(NewDashboard().Run(cmd.Context(), config))
 		},
 		Example: `# Start the Argo CD Web UI locally on the default port and address
 $ argocd admin dashboard
@@ -42,8 +78,8 @@ $ argocd admin dashboard --port 8080 --address 127.0.0.1
 $ argocd admin dashboard --redis-compress gzip
   `,
 	}
-	clientConfig = cli.AddKubectlFlagsToSet(cmd.Flags())
-	cmd.Flags().IntVar(&port, "port", common.DefaultPortAPIServer, "Listen on given port")
-	cmd.Flags().StringVar(&address, "address", common.DefaultAddressAdminDashboard, "Listen on given address")
+	config.ClientConfig = cli.AddKubectlFlagsToSet(cmd.Flags())
+	cmd.Flags().IntVar(&config.Port, "port", common.DefaultPortAPIServer, "Listen on given port")
+	cmd.Flags().StringVar(&config.Address, "address", common.DefaultAddressAdminDashboard, "Listen on given address")
 	return cmd
 }

cmd/argocd/commands/admin/dashboard_test.go

@@ -0,0 +1,50 @@
+package admin
+
+import (
+	"context"
+	"os"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"k8s.io/client-go/tools/clientcmd"
+
+	"github.com/argoproj/argo-cd/v3/pkg/apiclient"
+)
+
+func TestRun_SignalHandling_GracefulShutdown(t *testing.T) {
+	stopCalled := false
+	d := &dashboard{
+		startLocalServer: func(_ context.Context, opts *apiclient.ClientOptions, _ string, _ *int, _ *string, _ clientcmd.ClientConfig) (func(), error) {
+			return func() {
+				stopCalled = true
+				require.True(t, opts.Core, "Core client option should be set to true")
+			}, nil
+		},
+	}
+
+	var err error
+	doneCh := make(chan struct{})
+	go func() {
+		err = d.Run(t.Context(), &DashboardConfig{ClientOpts: &apiclient.ClientOptions{}})
+		close(doneCh)
+	}()
+
+	// Allow some time for the dashboard to register the signal handler
+	time.Sleep(50 * time.Millisecond)
+
+	proc, err := os.FindProcess(os.Getpid())
+	require.NoErrorf(t, err, "failed to find process: %v", err)
+	err = proc.Signal(syscall.SIGINT)
+	require.NoErrorf(t, err, "failed to send SIGINT: %v", err)
+
+	select {
+	case <-doneCh:
+		require.NoError(t, err)
+	case <-time.After(500 * time.Millisecond):
+		t.Fatal("timeout: dashboard.Run did not exit after SIGINT")
+	}
+
+	require.True(t, stopCalled)
+}

cmd/argocd/commands/headless/headless.go

@@ -177,7 +177,7 @@ func testAPI(ctx context.Context, clientOpts *apiclient.ClientOptions) error {
 //
 // If the clientOpts enables core mode, but the local config does not have core mode enabled, this function will
 // not start the local server.
-func MaybeStartLocalServer(ctx context.Context, clientOpts *apiclient.ClientOptions, ctxStr string, port *int, address *string, clientConfig clientcmd.ClientConfig) error {
+func MaybeStartLocalServer(ctx context.Context, clientOpts *apiclient.ClientOptions, ctxStr string, port *int, address *string, clientConfig clientcmd.ClientConfig) (func(), error) {
 	if clientConfig == nil {
 		flags := pflag.NewFlagSet("tmp", pflag.ContinueOnError)
 		clientConfig = cli.AddKubectlFlagsToSet(flags)
@@ -187,20 +187,20 @@ func MaybeStartLocalServer(ctx context.Context, clientOpts *apiclient.ClientOpti
 		// Core mode is enabled on client options. Check the local config to see if we should start the API server.
 		localCfg, err := localconfig.ReadLocalConfig(clientOpts.ConfigPath)
 		if err != nil {
-			return fmt.Errorf("error reading local config: %w", err)
+			return nil, fmt.Errorf("error reading local config: %w", err)
 		}
 		if localCfg != nil {
 			configCtx, err := localCfg.ResolveContext(clientOpts.Context)
 			if err != nil {
-				return fmt.Errorf("error resolving context: %w", err)
+				return nil, fmt.Errorf("error resolving context: %w", err)
 			}
 			// There was a local config file, so determine whether core mode is enabled per the config file.
 			startInProcessAPI = configCtx.Server.Core
 		}
 	}
 	// If we're in core mode, start the API server on the fly.
 	if !startInProcessAPI {
-		return nil
+		return nil, nil
 	}
 
 	// get rid of logging error handler
@@ -215,55 +215,55 @@ func MaybeStartLocalServer(ctx context.Context, clientOpts *apiclient.ClientOpti
 		addr := *address + ":0"
 		ln, err := net.Listen("tcp", addr)
 		if err != nil {
-			return fmt.Errorf("failed to listen on %q: %w", addr, err)
+			return nil, fmt.Errorf("failed to listen on %q: %w", addr, err)
 		}
 		port = &ln.Addr().(*net.TCPAddr).Port
 		utilio.Close(ln)
 	}
 
 	restConfig, err := clientConfig.ClientConfig()
 	if err != nil {
-		return fmt.Errorf("error creating client config: %w", err)
+		return nil, fmt.Errorf("error creating client config: %w", err)
 	}
 	appClientset, err := appclientset.NewForConfig(restConfig)
 	if err != nil {
-		return fmt.Errorf("error creating app clientset: %w", err)
+		return nil, fmt.Errorf("error creating app clientset: %w", err)
 	}
 	kubeClientset, err := kubernetes.NewForConfig(restConfig)
 	if err != nil {
-		return fmt.Errorf("error creating kubernetes clientset: %w", err)
+		return nil, fmt.Errorf("error creating kubernetes clientset: %w", err)
 	}
 
 	dynamicClientset, err := dynamic.NewForConfig(restConfig)
 	if err != nil {
-		return fmt.Errorf("error creating kubernetes dynamic clientset: %w", err)
+		return nil, fmt.Errorf("error creating kubernetes dynamic clientset: %w", err)
 	}
 
 	scheme := runtime.NewScheme()
 	err = v1alpha1.AddToScheme(scheme)
 	if err != nil {
-		return fmt.Errorf("error adding argo resources to scheme: %w", err)
+		return nil, fmt.Errorf("error adding argo resources to scheme: %w", err)
 	}
 	err = corev1.AddToScheme(scheme)
 	if err != nil {
-		return fmt.Errorf("error adding corev1 resources to scheme: %w", err)
+		return nil, fmt.Errorf("error adding corev1 resources to scheme: %w", err)
 	}
 	controllerClientset, err := client.New(restConfig, client.Options{
 		Scheme: scheme,
 	})
 	if err != nil {
-		return fmt.Errorf("error creating kubernetes controller clientset: %w", err)
+		return nil, fmt.Errorf("error creating kubernetes controller clientset: %w", err)
 	}
 	controllerClientset = client.NewDryRunClient(controllerClientset)
 
 	namespace, _, err := clientConfig.Namespace()
 	if err != nil {
-		return fmt.Errorf("error getting namespace: %w", err)
+		return nil, fmt.Errorf("error getting namespace: %w", err)
 	}
 
 	mr, err := miniredis.Run()
 	if err != nil {
-		return fmt.Errorf("error running miniredis: %w", err)
+		return nil, fmt.Errorf("error running miniredis: %w", err)
 	}
 	redisOptions := &redis.Options{Addr: mr.Addr()}
 	if err = common.SetOptionalRedisPasswordFromKubeConfig(ctx, kubeClientset, namespace, redisOptions); err != nil {
@@ -291,7 +291,7 @@ func MaybeStartLocalServer(ctx context.Context, clientOpts *apiclient.ClientOpti
 
 	lns, err := srv.Listen()
 	if err != nil {
-		return fmt.Errorf("failed to listen: %w", err)
+		return nil, fmt.Errorf("failed to listen: %w", err)
 	}
 	go srv.Run(ctx, lns)
 	clientOpts.ServerAddr = fmt.Sprintf("%s:%d", *address, *port)
@@ -309,9 +309,9 @@ func MaybeStartLocalServer(ctx context.Context, clientOpts *apiclient.ClientOpti
 		time.Sleep(time.Second)
 	}
 	if err != nil {
-		return fmt.Errorf("all retries failed: %w", err)
+		return nil, fmt.Errorf("all retries failed: %w", err)
 	}
-	return nil
+	return srv.Shutdown, nil
 }
 
 // NewClientOrDie creates a new API client from a set of config options, or fails fatally if the new client creation fails.
@@ -321,7 +321,7 @@ func NewClientOrDie(opts *apiclient.ClientOptions, c *cobra.Command) apiclient.C
 	ctxStr := initialize.RetrieveContextIfChanged(c.Flag("context"))
 	// If we're in core mode, start the API server on the fly and configure the client `opts` to use it.
 	// If we're not in core mode, this function call will do nothing.
-	err := MaybeStartLocalServer(ctx, opts, ctxStr, nil, nil, nil)
+	_, err := MaybeStartLocalServer(ctx, opts, ctxStr, nil, nil, nil)
 	if err != nil {
 		log.Fatal(err)
 	}

server/server.go

@@ -203,7 +203,7 @@ type ArgoCDServer struct {
 	configMapInformer  cache.SharedIndexInformer
 	serviceSet         *ArgoCDServiceSet
 	extensionManager   *extension.Manager
-	shutdown           func()
+	Shutdown           func()
 	terminateRequested atomic.Bool
 	available          atomic.Bool
 }
@@ -385,7 +385,7 @@ func NewServer(ctx context.Context, opts ArgoCDServerOpts, appsetOpts Applicatio
 		secretInformer:     secretInformer,
 		configMapInformer:  configMapInformer,
 		extensionManager:   em,
-		shutdown:           noopShutdown,
+		Shutdown:           noopShutdown,
 		stopCh:             make(chan os.Signal, 1),
 	}
 
@@ -560,7 +560,7 @@ func (server *ArgoCDServer) Run(ctx context.Context, listeners *Listeners) {
 		if r := recover(); r != nil {
 			log.WithField("trace", string(debug.Stack())).Error("Recovered from panic: ", r)
 			server.terminateRequested.Store(true)
-			server.shutdown()
+			server.Shutdown()
 		}
 	}()
 
@@ -727,7 +727,7 @@ func (server *ArgoCDServer) Run(ctx context.Context, listeners *Listeners) {
 			log.Warn("Graceful shutdown timeout. Exiting...")
 		}
 	}
-	server.shutdown = shutdownFunc
+	server.Shutdown = shutdownFunc
 	signal.Notify(server.stopCh, os.Interrupt, syscall.SIGINT, syscall.SIGTERM)
 	server.available.Store(true)
 
@@ -738,11 +738,11 @@ func (server *ArgoCDServer) Run(ctx context.Context, listeners *Listeners) {
 		if signal != gracefulRestartSignal {
 			server.terminateRequested.Store(true)
 		}
-		server.shutdown()
+		server.Shutdown()
 	case <-ctx.Done():
 		log.Infof("API Server: %s", ctx.Err())
 		server.terminateRequested.Store(true)
-		server.shutdown()
+		server.Shutdown()
 	}
 }