Improve Teleport reload behavior

assets/aws/files/system/teleport-acm.service

@@ -11,6 +11,6 @@ Restart=always
 RestartSec=5
 RuntimeDirectory=teleport
 ExecStart=/usr/local/bin/teleport start --config=/etc/teleport.yaml --diag-addr=127.0.0.1:3000 --pid-file=/run/teleport/teleport.pid
-ExecReload=/bin/kill -HUP $MAINPID
+ExecReload=pkill -HUP -L -F /run/teleport/teleport.pid
 PIDFile=/run/teleport/teleport.pid
-LimitNOFILE=524288
+LimitNOFILE=524288

assets/aws/files/system/teleport-auth.service

@@ -11,9 +11,9 @@ Restart=always
 RestartSec=5
 RuntimeDirectory=teleport
 ExecStart=/usr/local/bin/teleport start --config=/etc/teleport.yaml --diag-addr=127.0.0.1:3000 --pid-file=/run/teleport/teleport.pid
-ExecReload=/bin/kill -HUP $MAINPID
+ExecReload=pkill -HUP -L -F /run/teleport/teleport.pid
 PIDFile=/run/teleport/teleport.pid
 LimitNOFILE=524288
 
 [Install]
-WantedBy=multi-user.target
+WantedBy=multi-user.target

assets/aws/files/system/teleport-node.service

@@ -12,9 +12,9 @@ RestartSec=5
 RuntimeDirectory=teleport
 ExecStartPre=/usr/local/bin/teleport-ssm-get-token
 ExecStart=/usr/local/bin/teleport start --config=/etc/teleport.yaml --diag-addr=127.0.0.1:3000 --pid-file=/run/teleport/teleport.pid
-ExecReload=/bin/kill -HUP $MAINPID
+ExecReload=pkill -HUP -L -F /run/teleport/teleport.pid
 PIDFile=/run/teleport/teleport.pid
 LimitNOFILE=524288
 
 [Install]
-WantedBy=multi-user.target
+WantedBy=multi-user.target

assets/aws/files/system/teleport-proxy-acm.service

@@ -13,9 +13,9 @@ RuntimeDirectory=teleport
 EnvironmentFile=/etc/teleport.d/conf
 ExecStartPre=/usr/local/bin/teleport-ssm-get-token
 ExecStart=/usr/local/bin/teleport start --config=/etc/teleport.yaml --diag-addr=127.0.0.1:3000 --pid-file=/run/teleport/teleport.pid
-ExecReload=/bin/kill -HUP $MAINPID
+ExecReload=pkill -HUP -L -F /run/teleport/teleport.pid
 PIDFile=/run/teleport/teleport.pid
 LimitNOFILE=524288
 
 [Install]
-WantedBy=multi-user.target
+WantedBy=multi-user.target

assets/aws/files/system/teleport-proxy.service

@@ -14,9 +14,9 @@ EnvironmentFile=/etc/teleport.d/conf
 ExecStartPre=/usr/local/bin/teleport-ssm-get-token
 ExecStartPre=/bin/aws s3 sync s3://${TELEPORT_S3_BUCKET}/live/${TELEPORT_DOMAIN_NAME} /var/lib/teleport
 ExecStart=/usr/local/bin/teleport start --config=/etc/teleport.yaml --diag-addr=127.0.0.1:3000 --pid-file=/run/teleport/teleport.pid
-ExecReload=/bin/kill -HUP $MAINPID
+ExecReload=pkill -HUP -L -F /run/teleport/teleport.pid
 PIDFile=/run/teleport/teleport.pid
 LimitNOFILE=524288
 
 [Install]
-WantedBy=multi-user.target
+WantedBy=multi-user.target

assets/aws/files/system/teleport.service

@@ -12,9 +12,9 @@ RestartSec=5
 RuntimeDirectory=teleport
 ExecStartPre=/usr/local/bin/teleport-all-pre-start
 ExecStart=/usr/local/bin/teleport start --config=/etc/teleport.yaml --diag-addr=127.0.0.1:3000 --pid-file=/run/teleport/teleport.pid
-ExecReload=/bin/kill -HUP $MAINPID
+ExecReload=pkill -HUP -L -F /run/teleport/teleport.pid
 PIDFile=/run/teleport/teleport.pid
 LimitNOFILE=524288
 
 [Install]
-WantedBy=multi-user.target
+WantedBy=multi-user.target

examples/systemd/fips/teleport.service

@@ -7,7 +7,7 @@ Type=simple
 Restart=on-failure
 EnvironmentFile=-/etc/default/teleport
 ExecStart=/usr/local/bin/teleport start --config /etc/teleport.yaml --fips --pid-file=/run/teleport.pid
-ExecReload=/bin/kill -HUP $MAINPID
+ExecReload=pkill -HUP -L -F /run/teleport.pid
 PIDFile=/run/teleport.pid
 LimitNOFILE=524288
 

examples/systemd/production/auth/teleport.service

@@ -11,9 +11,9 @@ Restart=on-failure
 # --roles='proxy,auth,node' is the default value
 # if none is set
 ExecStart=/usr/local/bin/teleport start --roles=auth --config=/etc/teleport.yaml --diag-addr=127.0.0.1:3000 --pid-file=/run/teleport.pid
-ExecReload=/bin/kill -HUP $MAINPID
+ExecReload=pkill -HUP -L -F /run/teleport.pid
 PIDFile=/run/teleport.pid
 LimitNOFILE=524288
 
 [Install]
-WantedBy=multi-user.target
+WantedBy=multi-user.target

examples/systemd/production/node/teleport.service

@@ -11,7 +11,7 @@ Restart=on-failure
 # --roles='proxy,auth,node' is the default value
 # if none is set
 ExecStart=/usr/local/bin/teleport start --roles=node --config=/etc/teleport.yaml --pid-file=/run/teleport.pid
-ExecReload=/bin/kill -HUP $MAINPID
+ExecReload=pkill -HUP -L -F /run/teleport.pid
 PIDFile=/run/teleport.pid
 LimitNOFILE=524288
 

examples/systemd/production/proxy/teleport.service

@@ -11,9 +11,9 @@ Restart=on-failure
 # --roles='proxy,auth,node' is the default value
 # if none is set
 ExecStart=/usr/local/bin/teleport start --roles=proxy --config=/etc/teleport.yaml --pid-file=/run/teleport.pid
-ExecReload=/bin/kill -HUP $MAINPID
+ExecReload=pkill -HUP -L -F /run/teleport.pid
 PIDFile=/run/teleport.pid
 LimitNOFILE=524288
 
 [Install]
-WantedBy=multi-user.target
+WantedBy=multi-user.target

examples/systemd/teleport.service

@@ -7,7 +7,7 @@ Type=simple
 Restart=on-failure
 EnvironmentFile=-/etc/default/teleport
 ExecStart=/usr/local/bin/teleport start --config /etc/teleport.yaml --pid-file=/run/teleport.pid
-ExecReload=/bin/kill -HUP $MAINPID
+ExecReload=pkill -HUP -L -F /run/teleport.pid
 PIDFile=/run/teleport.pid
 LimitNOFILE=524288
 

go.mod

@@ -99,6 +99,7 @@ require (
 	github.com/google/go-containerregistry v0.19.0
 	github.com/google/go-querystring v1.1.0
 	github.com/google/go-tpm-tools v0.4.2
+	github.com/google/renameio/v2 v2.0.0
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
 	github.com/google/uuid v1.6.0
 	github.com/googleapis/gax-go/v2 v2.12.0
@@ -340,7 +341,6 @@ require (
 	github.com/google/go-tpm v0.9.0 // indirect
 	github.com/google/go-tspi v0.3.0 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
-	github.com/google/renameio/v2 v2.0.0 // indirect
 	github.com/google/s2a-go v0.1.7 // indirect
 	github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect
 	github.com/gorilla/handlers v1.5.2 // indirect

lib/config/systemd.go

@@ -47,12 +47,13 @@ Type=simple
 Restart=on-failure
 EnvironmentFile=-{{ .EnvironmentFile }}
 ExecStart={{ .TeleportInstallationFile }} start --config {{ .TeleportConfigPath }} --pid-file={{ .PIDFile }}
-ExecReload=/bin/kill -HUP $MAINPID
+ExecReload=pkill -HUP -L -F "{{ .PIDFile }}"
 PIDFile={{ .PIDFile }}
 LimitNOFILE={{ .FileDescriptorLimit }}
 
 [Install]
-WantedBy=multi-user.target`))
+WantedBy=multi-user.target
+`))
 
 // SystemdFlags specifies configuration parameters for a systemd unit file.
 type SystemdFlags struct {

lib/config/testdata/TestWriteSystemdUnitFile.golden

@@ -7,9 +7,9 @@ Type=simple
 Restart=on-failure
 EnvironmentFile=-/custom/env/dir/teleport
 ExecStart=/custom/install/dir/teleport start --config /etc/teleport.yaml --pid-file=/custom/pid/dir/teleport.pid
-ExecReload=/bin/kill -HUP $MAINPID
+ExecReload=pkill -HUP -L -F "/custom/pid/dir/teleport.pid"
 PIDFile=/custom/pid/dir/teleport.pid
 LimitNOFILE=16384
 
 [Install]
-WantedBy=multi-user.target
+WantedBy=multi-user.target

lib/defaults/defaults.go

@@ -315,10 +315,6 @@ const (
 	// LowResPollingPeriod is a default low resolution polling period
 	LowResPollingPeriod = 600 * time.Second
 
-	// HighResReportingPeriod is a high resolution polling reporting
-	// period used in services
-	HighResReportingPeriod = 10 * time.Second
-
 	// SessionControlTimeout is the maximum amount of time a controlled session
 	// may persist after contact with the auth server is lost (sessctl semaphore
 	// leases are refreshed at a rate of ~1/2 this duration).

lib/service/service.go

@@ -46,6 +46,7 @@ import (
 	"time"
 
 	awscredentials "github.com/aws/aws-sdk-go/aws/credentials"
+	"github.com/google/renameio/v2"
 	"github.com/google/uuid"
 	"github.com/gravitational/roundtrip"
 	"github.com/gravitational/trace"
@@ -57,6 +58,7 @@ import (
 	"golang.org/x/crypto/acme"
 	"golang.org/x/crypto/acme/autocert"
 	"golang.org/x/crypto/ssh"
+	"golang.org/x/sys/unix"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/keepalive"
@@ -389,10 +391,9 @@ type TeleportProcess struct {
 	// closed all the listeners)
 	listenersClosed bool
 
-	// forkedPIDs is a collection of a teleport processes forked
-	// during restart used to collect their status in case if the
-	// child process crashed.
-	forkedPIDs []int
+	// forkedTeleportCount is the count of forked Teleport child processes
+	// currently active, as spawned by SIGHUP or SIGUSR2.
+	forkedTeleportCount atomic.Int32
 
 	// storage is a server local storage
 	storage *auth.ProcessStorage
@@ -738,6 +739,9 @@ func waitAndReload(ctx context.Context, cfg servicecfg.Config, srv Process, newT
 	}
 	newCfg := cfg
 	newCfg.FileDescriptors = fileDescriptors
+	// our PID hasn't changed as we reload in-process, and if we're no longer
+	// the "main" Teleport process we don't want to overwrite the PID file
+	newCfg.PIDFile = ""
 	newSrv, err := newTeleport(&newCfg)
 	if err != nil {
 		warnOnErr(srv.Close(), cfg.Log)
@@ -1252,13 +1256,8 @@ func NewTeleport(cfg *servicecfg.Config) (*TeleportProcess, error) {
 
 	// create the new pid file only after started successfully
 	if cfg.PIDFile != "" {
-		f, err := os.OpenFile(cfg.PIDFile, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o666)
-		if err != nil {
-			return nil, trace.ConvertSystemError(err)
-		}
-		_, err = fmt.Fprintf(f, "%v", os.Getpid())
-		if err = trace.NewAggregate(err, f.Close()); err != nil {
-			return nil, trace.Wrap(err)
+		if err := createLockedPIDFile(cfg.PIDFile); err != nil {
+			return nil, trace.Wrap(err, "creating pidfile")
 		}
 	}
 
@@ -5528,7 +5527,7 @@ func (process *TeleportProcess) StartShutdown(ctx context.Context) context.Conte
 	warnOnErr(process.stopListeners(), process.log)
 
 	// populate context values
-	if len(process.getForkedPIDs()) > 0 {
+	if process.forkedTeleportCount.Load() > 0 {
 		ctx = services.ProcessForkedContext(ctx)
 	}
 
@@ -6101,3 +6100,35 @@ func (process *TeleportProcess) newExternalAuditStorageConfigurator() (*external
 	statusService := local.NewStatusService(process.backend)
 	return externalauditstorage.NewConfigurator(process.ExitContext(), ecaSvc, integrationSvc, statusService)
 }
+
+// createLockedPIDFile creates a PID file in the path specified by pidFile
+// containing the current PID, atomically swapping it in the final place and
+// leaving it with an exclusive advisory lock that will get released when the
+// process ends, for the benefit of "pkill -L".
+func createLockedPIDFile(pidFile string) error {
+	pending, err := renameio.NewPendingFile(pidFile, renameio.WithPermissions(0o644))
+	if err != nil {
+		return trace.ConvertSystemError(err)
+	}
+	defer pending.Cleanup()
+	if _, err := fmt.Fprintf(pending, "%v\n", os.Getpid()); err != nil {
+		return trace.ConvertSystemError(err)
+	}
+
+	const minimumDupFD = 3 // skip stdio
+	locker, err := unix.FcntlInt(pending.Fd(), unix.F_DUPFD_CLOEXEC, minimumDupFD)
+	runtime.KeepAlive(pending)
+	if err != nil {
+		return trace.ConvertSystemError(err)
+	}
+	if err := unix.Flock(locker, unix.LOCK_EX|unix.LOCK_NB); err != nil {
+		_ = unix.Close(locker)
+		return trace.ConvertSystemError(err)
+	}
+	// deliberately leak the fd to hold the lock until the process dies
+
+	if err := pending.CloseAtomicallyReplace(); err != nil {
+		return trace.ConvertSystemError(err)
+	}
+	return nil
+}