Add readyz socket endpoint to tbot for use by teleport-update (#60477)

* PID and readyz changes for tbot

* remove teleport-update changes

* tidy

* tidy

* godoc

* linting, typos

* lint

* shared cli args + cli tests

* lint

* missing .

* flag docs

* pid example

* Apply suggestion from @strideynet

Co-authored-by: Noah Stride <[email protected]>

* windows fix

* windows fix for merge queue

* missing errors on windows

---------

Co-authored-by: Noah Stride <[email protected]>

Author: sclevine

docs/pages/reference/cli/tbot.mdx

@@ -291,6 +291,7 @@ specific section for details when using a YAML config file or legacy output.
 | `--registration-secret` | An optional joining secret to use on first join with the `bound_keypair` join method. This can also be provided via the `TBOT_REGISTRATION_SECRET` environment variable. |
 | `--registration-secret-path` | An optional path to a file containing a joining secret to use on first join with the `bound_keypair` join method. |
 | `--static-key-path`     | An optional path to a file containing a static private key for use with the `bound_keypair` join method. A base64-encoded key can also be provided via the `TBOT_BOUND_KEYPAIR_STATIC_KEY` environment variable. |
+| `--pid-file`            | Full path to the PID file. By default no PID file will be created. |
 
 ## tbot start legacy
 
@@ -321,6 +322,7 @@ another dedicated mode instead.
 | `--join-method`      | Method to use to join the cluster. Can be `token`, `azure`, `circleci`, `gcp`, `github`, `gitlab` or `iam`.                                                                     |
 | `--oneshot`          | If set, quit after the first renewal.                                                                                                                                           |
 | `--log-format`       | Controls the format of output logs. Can be `json` or `text`. Defaults to `text`.                                                                                                |
+| `--pid-file`         | Full path to the PID file. By default no PID file will be created.                                                                                                              |
 
 ### Examples
 <Tabs>

docs/pages/reference/machine-workload-identity/machine-id/diagnostics-service.mdx

@@ -72,7 +72,8 @@ Content-Type: application/json
       "status": "unhealthy",
       "reason": "access denied to perform action \"read\" on \"workload_identity\""
     }
-  }
+  },
+  "pid": 42344
 }
 ```
 

lib/service/service.go

@@ -59,7 +59,6 @@ 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"
@@ -181,6 +180,7 @@ import (
 	"github.com/gravitational/teleport/lib/utils"
 	"github.com/gravitational/teleport/lib/utils/cert"
 	logutils "github.com/gravitational/teleport/lib/utils/log"
+	procutils "github.com/gravitational/teleport/lib/utils/process"
 	"github.com/gravitational/teleport/lib/versioncontrol/endpoint"
 	uw "github.com/gravitational/teleport/lib/versioncontrol/upgradewindow"
 	"github.com/gravitational/teleport/lib/web"
@@ -1648,7 +1648,7 @@ func NewTeleport(cfg *servicecfg.Config) (_ *TeleportProcess, err error) {
 
 	// create the new pid file only after started successfully
 	if cfg.PIDFile != "" {
-		if err := createLockedPIDFile(cfg.PIDFile); err != nil {
+		if err := procutils.CreateLockedPIDFile(cfg.PIDFile); err != nil {
 			return nil, trace.Wrap(err, "creating pidfile")
 		}
 	}
@@ -7380,35 +7380,3 @@ func (process *TeleportProcess) newExternalAuditStorageConfigurator() (*external
 	statusService := local.NewStatusService(process.backend)
 	return externalauditstorage.NewConfigurator(process.ExitContext(), easSvc, 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
-}

lib/service/signals.go

@@ -294,7 +294,7 @@ func (process *TeleportProcess) createListener(typ ListenerType, address string)
 		return nil, trace.BadParameter("listening is blocked")
 	}
 
-	// When the process exists, the socket files are left behind (to cover
+	// When the process exits, the socket files are left behind (to cover
 	// forking scenarios). To guarantee there won't be errors like "address
 	// already in use", delete the file before starting the listener.
 	if typ.Network() == "unix" {
@@ -318,7 +318,7 @@ func (process *TeleportProcess) createListener(typ ListenerType, address string)
 
 	// The default behavior for unix listeners is to delete the file when the
 	// listener closes (unlinking). However, if the process forks, the file
-	// descriptor will be gone when its parent process exists, causing the new
+	// descriptor will be gone when its parent process exits, causing the new
 	// listener to have no socket file.
 	if unixListener, ok := listener.(*net.UnixListener); ok {
 		unixListener.SetUnlinkOnClose(false)

lib/tbot/cli/start_legacy.go

@@ -132,6 +132,13 @@ type LegacyCommand struct {
 	// If not set, no diagnostics listener is created.
 	DiagAddr string
 
+	// DiagSocketForUpdater specifies the diagnostics http service address that
+	// should be exposed to the updater via UNIX domain socket.
+	DiagSocketForUpdater string
+
+	// PIDFile is the path to the PID file. If not set, no PID file will be created.
+	PIDFile string
+
 	oneshotSetByUser bool
 }
 
@@ -158,6 +165,8 @@ func NewLegacyCommand(parentCmd *kingpin.CmdClause, action MutatorAction, mode C
 	c.cmd.Flag("join-method", "Method to use to join the cluster. "+joinMethodList).EnumVar(&c.JoinMethod, onboarding.SupportedJoinMethods...)
 	c.cmd.Flag("oneshot", "If set, quit after the first renewal.").IsSetByUser(&c.oneshotSetByUser).BoolVar(&c.Oneshot)
 	c.cmd.Flag("diag-addr", "If set and the bot is in debug mode, a diagnostics service will listen on specified address.").StringVar(&c.DiagAddr)
+	c.cmd.Flag("diag-socket-for-updater", "If set, run the diagnostics service on the specified socket path for teleport-update to consume.").Hidden().StringVar(&c.DiagSocketForUpdater)
+	c.cmd.Flag("pid-file", "Full path to the PID file. By default no PID file will be created.").StringVar(&c.PIDFile)
 
 	return c
 }
@@ -273,5 +282,8 @@ func (c *LegacyCommand) ApplyConfig(cfg *config.BotConfig, l *slog.Logger) error
 		cfg.DiagAddr = c.DiagAddr
 	}
 
+	cfg.DiagSocketForUpdater = c.DiagSocketForUpdater
+	cfg.PIDFile = c.PIDFile
+
 	return nil
 }

lib/tbot/cli/start_legacy_test.go

@@ -48,6 +48,8 @@ func TestLegacyCommand(t *testing.T) {
 				"--data-dir=/foo",
 				"--destination-dir=/bar",
 				"--auth-server=example.com:3024",
+				"--pid-file=/run/tbot.pid",
+				"--diag-socket-for-updater=/var/lib/teleport/bot/debug.sock",
 			},
 			assertConfig: func(t *testing.T, cfg *config.BotConfig) {
 				token, err := cfg.Onboarding.Token()
@@ -61,6 +63,8 @@ func TestLegacyCommand(t *testing.T) {
 				require.True(t, cfg.Oneshot)
 				require.Equal(t, "0.0.0.0:8080", cfg.DiagAddr)
 				require.Equal(t, "example.com:3024", cfg.AuthServer)
+				require.Equal(t, "/run/tbot.pid", cfg.PIDFile)
+				require.Equal(t, "/var/lib/teleport/bot/debug.sock", cfg.DiagSocketForUpdater)
 
 				dir, ok := cfg.Storage.Destination.(*destination.Directory)
 				require.True(t, ok)

lib/tbot/cli/start_shared.go

@@ -119,8 +119,10 @@ type sharedStartArgs struct {
 	StaticKeyPath          string
 	Keypair                string
 
-	Oneshot  bool
-	DiagAddr string
+	Oneshot              bool
+	DiagAddr             string
+	DiagSocketForUpdater string
+	PIDFile              string
 
 	oneshotSetByUser bool
 }
@@ -146,6 +148,8 @@ func newSharedStartArgs(cmd *kingpin.CmdClause) *sharedStartArgs {
 	cmd.Flag("registration-secret-path", "For bound keypair joining, specifies a file containing a registration secret for use at first join.").StringVar(&args.RegistrationSecretPath)
 	cmd.Flag("static-key-path", "For bound keypair joining, specifies a path to a static key.").StringVar(&args.StaticKeyPath)
 	cmd.Flag("join-uri", "An optional URI with joining and authentication parameters. Individual flags for proxy, join method, token, etc may be used instead.").StringVar(&args.JoiningURI)
+	cmd.Flag("diag-socket-for-updater", "If set, run the diagnostics service on the specified socket path for teleport-update to consume.").Hidden().StringVar(&args.DiagSocketForUpdater)
+	cmd.Flag("pid-file", "Full path to the PID file. By default no PID file will be created.").StringVar(&args.PIDFile)
 
 	return args
 }
@@ -264,6 +268,9 @@ func (s *sharedStartArgs) ApplyConfig(cfg *config.BotConfig, l *slog.Logger) err
 		cfg.Onboarding.BoundKeypair.StaticPrivateKeyPath = s.StaticKeyPath
 	}
 
+	cfg.DiagSocketForUpdater = s.DiagSocketForUpdater
+	cfg.PIDFile = s.PIDFile
+
 	return nil
 }
 

lib/tbot/cli/start_shared_test.go

@@ -44,6 +44,8 @@ func TestSharedStartArgs(t *testing.T) {
 		"--diag-addr=0.0.0.0:8080",
 		"--storage=file:///foo/bar",
 		"--proxy-server=example.teleport.sh:443",
+		"--pid-file=/run/tbot.pid",
+		"--diag-socket-for-updater=/var/lib/teleport/bot/debug.sock",
 	})
 	require.NoError(t, err)
 
@@ -56,6 +58,8 @@ func TestSharedStartArgs(t *testing.T) {
 	require.Equal(t, "0.0.0.0:8080", args.DiagAddr)
 	require.Equal(t, "file:///foo/bar", args.Storage)
 	require.Equal(t, "example.teleport.sh:443", args.ProxyServer)
+	require.Equal(t, "/run/tbot.pid", args.PIDFile)
+	require.Equal(t, "/var/lib/teleport/bot/debug.sock", args.DiagSocketForUpdater)
 
 	// Convert these args to a BotConfig.
 	cfg, err := LoadConfigWithMutators(&GlobalArgs{}, args)

lib/tbot/config/config.go

@@ -99,6 +99,13 @@ type BotConfig struct {
 	// If not set, no diagnostics listener is created.
 	DiagAddr string `yaml:"diag_addr,omitempty"`
 
+	// DiagSocketForUpdater specifies the path to the diagnostics http service socket that
+	// should be exposed to the updater.
+	DiagSocketForUpdater string `yaml:"-"`
+
+	// PIDFile is the path to the PID file that should be created by the bot.
+	PIDFile string `yaml:"-"`
+
 	// ReloadCh allows a channel to be injected into the bot to trigger a
 	// renewal.
 	ReloadCh <-chan struct{} `yaml:"-"`

tool/tbot/systemd.tmpl renamed to lib/tbot/config/systemd/systemd.tmpl

@@ -10,7 +10,7 @@ Group={{ .Group }}
 Restart=always
 RestartSec=5
 Environment="TELEPORT_ANONYMOUS_TELEMETRY={{ if .AnonymousTelemetry }}1{{ else }}0{{ end }}"
-ExecStart={{ .TBotPath }} start -c {{ .ConfigPath }}
+ExecStart={{ .TBotPath }} start -c {{ .ConfigPath }}{{ with .DiagSocketForUpdater }} --diag-socket-for-updater={{ . }}{{ end }} --pid-file=/run/{{ .UnitName }}.pid
 ExecReload=/bin/kill -HUP $MAINPID
 PIDFile=/run/{{ .UnitName }}.pid
 LimitNOFILE=524288