mirror of
https://github.com/gravitational/teleport.git
synced 2026-09-19 01:58:44 +08:00
dc01dd9fdc
* Detect EC2 join failures and surface via readyz * Replace readyz join check with systemd log capture * Add JoinFailureTimeout constant The installer's readyz polling timeout is referenced by both the autodiscover installer and the SSM waiter. A shared constant avoids drift between the two values and keeps the user-facing status message in sync with the actual wait duration. * Replace two-attempt join check with poll loop The join health check now polls the readyz endpoint in a loop until the node reports ready or a five-minute timeout elapses. Previously the check ran at most twice with a fixed delay of thirty seconds, which missed nodes that needed longer to join and could not distinguish a slow start from a permanent failure. On timeout the error includes a best-effort systemd service snapshot gathered via systemctl show. This replaces the earlier systemctl is-active gate that blocked readyz from being checked at all when the service was in a non-active or transient state. The readyz function returns a simple (ready, error) pair instead of the earlier four-value tuple, and per-attempt timeouts that hit deadline-exceeded are treated as transient so that polling continues. A clockwork.Clock field enables deterministic time control in tests. * Align SSM waiter timeout with poll timeout The SSM command waiter used a hardcoded 100-second wait that was inherited from the v1 SDK default. The installer now polls readyz for up to five minutes, so the SSM waiter must outlast that to avoid reporting a stale in-progress state as the final outcome. The new timeout is the join-failure timeout plus a ten-minute pad. * Update docs for readyz poll loop errors Error patterns, status messages, and the example event code now reflect the polling-based join health check. The event code moves from TDS01E to TDS00W, and the user task description no longer assumes an IAM join handshake rejection as the sole failure mode. * Trim SSM stdout/stderr in audit events SSMRun audit events can carry unbounded stdout/stderr from command invocations, risking the 64 KB stream message limit. Cap each field at 24 000 characters (matching the per-field cap in the event schema) by keeping only the trailing portion and aligning the cut to a full-line boundary so diagnostics stay readable. * Simplify output. Co-authored-by: Roman Tkachenko <roman@goteleport.com> * Expand RedactFlagArgs doc comment * Add IsConnectionError to utils package Move the socket-level connection check from the installer package into the shared utils package. The readyz checker needs this to distinguish transient socket errors from application failures, and the existing helpers in utils already cover related network error classification. * Remove install-script binary-exists guard The shell installer no longer skips the download when a Teleport binary already exists on disk. Previously the guard would bypass reinstallation, but the installer is now expected to always run so that package-managed upgrades and configuration changes take effect. * Remove readiness handler tests from lib/service These tests covered the HTTP readyz handler that is no longer exercised through the service-level test surface. Readyz check behaviour is now validated through the installer readyz checker tests, making these redundant. * Simplify join-failure error output The error writer no longer unpacks trace message slices or strips standalone sentinel lines before printing. Errors now flow through as plain strings, which is sufficient because the upstream assembly was simplified to produce one coherent error message. * Extract readyz checker and diagnostics helpers Split readyz check logic and systemd diagnostics out of the large autodiscover file into dedicated files. The readyz checker becomes a standalone struct with its own per-attempt timeout, connection error classification, and test suite. Diagnostics helpers such as journal capture, service state gathering, and the systemctl runner move into a diagnostics file alongside ErrJoinFailure and related constants. The poll loop in checkJoinHealth is restructured to perform an immediate first check before entering the timer loop, and error assembly is inlined instead of threading through append helpers. Tests now use a fake clock and goroutines to exercise the timer driven polling flow deterministically. * Move JoinFailureTimeout to installer package Relocate JoinFailureTimeout from installstatus/exitcodes.go into installer/autodiscover.go where the readyz polling logic that consumes it lives. The exit-code String() method now returns a generic timeout message; ssm_install.go appends the concrete duration for the join-failure case. * Reuse poll timer in readyz health check loop The readyz poll loop allocated a new timer on every iteration and checked readiness only after waiting. The loop now runs the first check immediately before entering the poll cycle and reuses a single timer via Reset, reducing per-iteration allocations. The ready state drives the loop condition directly, moving the success return after the loop so the function ends with the diagnostic-failure path at the bottom. * Replace binary mocks with function overrides Join-health timeout tests relied on bintest process mocks for both systemctl and journalctl, requiring filesystem setup, path wiring, and post-run invocation assertions for every test case. Diagnostics and journal capture now use optional function-level overrides that are injected through the installer config, with thin wrapper methods that fall through to the real implementation when no override is set. Previously each test needed six to eight lines of mock orchestration that obscured the actual test logic. * Update lib/srv/server/installer/autodiscover_test.go Scope context to test. Co-authored-by: Chris Thach <chris.thach@goteleport.com> * Update lib/srv/server/installer/autodiscover_test.go Scope context to test. Co-authored-by: Chris Thach <chris.thach@goteleport.com> * Update lib/srv/server/installer/autodiscover_test.go Scope context to test. Co-authored-by: Chris Thach <chris.thach@goteleport.com> * Update lib/srv/server/installer/autodiscover_test.go nstaller/autodiscover_test.go Co-authored-by: Chris Thach <chris.thach@goteleport.com> * Simplify arg redaction loop to use range Co-authored-by: Chris Thach <chris.thach@goteleport.com> * Simplify override fields to defaults in config Co-authored-by: Tiago Silva <tiago.silva@goteleport.com> * Add JoinFailureError structured error type Replace the ErrJoinFailure sentinel with a JoinFailureError struct that carries message, service diagnostics, journal output, and last readyz error as separate fields. Previously callers had to parse a concatenated string to extract individual diagnostic sections, making downstream formatting and programmatic inspection fragile. * Surface unexpected readyz errors to caller Return unexpected errors from the readyz check instead of swallowing them and returning false. The polling loop in checkJoinHealth keeps retrying on these errors but now records the last one so it appears in the timeout diagnostics. Previously a transient HTTP or socket error was silently discarded, making it impossible to distinguish "not ready yet" from "endpoint unreachable." * Replace clock and overrides with function fields Remove the clockwork dependency from the join-health check and use the standard time package directly. Promote journalOverride and the diagnosticsOverride to first-class diagnostics and journal function fields initialized in checkAndSetDefaults. Build a JoinFailureError on timeout instead of concatenating string parts. Tests switch from fakeClock and BlockUntilContext to synctest.Test with time.Sleep, removing the need for manual clock advancing. * Use JoinFailureError in CLI error output Switch the CLI join-failure path from errors.Is with the old sentinel to errors.As with JoinFailureError. The writer function now accepts the typed error and prints each diagnostic field on its own line. Previously the error was printed as a single opaque string, making it harder to scan in SSM console output. * Normalize dash prefix in flag redaction Add a lookupRedactor helper that maps both single-dash flags to their double-dash redactor registration and vice versa, so callers need not register both variants. Previously a -token=secret argument passed through unredacted because only --token was registered in the redactor map. * Improve trace msg Co-authored-by: Gavin Frazar <gavin.frazar@goteleport.com> * Minor refactor Co-authored-by: Gavin Frazar <gavin.frazar@goteleport.com> * Minor refactor Co-authored-by: Gavin Frazar <gavin.frazar@goteleport.com> * Minor refactor Co-authored-by: Gavin Frazar <gavin.frazar@goteleport.com> * Minor refactor Co-authored-by: Gavin Frazar <gavin.frazar@goteleport.com> * Minor refactor Co-authored-by: Gavin Frazar <gavin.frazar@goteleport.com> * Minor log improvement Co-authored-by: Gavin Frazar <gavin.frazar@goteleport.com> * Remove redundant check Co-authored-by: Gavin Frazar <gavin.frazar@goteleport.com> * Quote command+args Co-authored-by: Gavin Frazar <gavin.frazar@goteleport.com> * Remove token-redaction utility and its usage Log node-configure arguments directly instead of redacting the join token. The token is a short-lived secret issued by the auth server for EC2 auto-discovery; it expires before log entries are typically reviewed, so the redaction layer added complexity without meaningful security benefit. Previously every call to the node-configure step copied the argument slice through a redactor map and a flag-parsing loop. Replace hardcoded five-second sleeps in health-check tests with the installer's readyzPollInterval field so the tests stay correct when the interval changes. Remove a stray closing brace left over from a prior edit. * Improve diagnostics error messages and comments Add a comment explaining the systemctl-show output format that the property parser expects. Quote "systemctl show" in the log message for the invocation-ID retrieval failure so it renders as a distinct command name rather than bare words. Switch the infrastructure error path to use the %q format verb with the full command object so the message includes the complete binary path instead of manually joining the argument list. * Use server clock for EC2 installation SyncTime Set SyncTime to the discovery server's clock instead of the SSM run event timestamp. Previously the field carried the event time, which reflects when SSM dispatched the command rather than when the server recorded the installation result, causing user-task entries to show stale timestamps that did not advance on re-evaluation. * Expand trimToRecentTail godoc comment * Bound diagnostics collection after join timeout Diagnostics and journal capture now run under a dedicated five-second timeout derived from the parent context. Previously they inherited the caller's context with no independent deadline, so a hung systemctl or journalctl call could block indefinitely after the readyz poll already determined the node failed to join. Journal capture errors are downgraded from a hard return to a warning log entry. A transient journalctl failure was masking the underlying join-failure error, causing the caller to receive a journal-capture error instead of the structured JoinFailureError that carries the diagnostics payload. * Document waitTimeoutPad tradeoff and TODO * Replace systemctl subprocess with D-Bus API Diagnostics collection now queries systemd properties through the native D-Bus connection rather than shelling out to systemctl show and parsing stdout. The D-Bus interface needs fully-qualified unit names, so the unit-name builder appends a ".service" suffix to each returned value. This eliminates the systemctl binary-path dependency during diagnostics and avoids subprocess-parsing fragility around exit-code versus infrastructure error disambiguation. * Fix go mod * Bump how many journal lines we output * Extract systemd client into lib/systemd package The systemd D-Bus property lookups, invocation ID retrieval, and the journalctl capture now live in a reusable lib/systemd package that exposes a Client with ReadServiceState, InvocationID, and CaptureJournal methods. Previously these helpers lived as unexported methods on the AutoDiscoverNodeInstaller type, so any other call site that needed a systemd state snapshot had to either duplicate the logic or import the installer package just to reach them. Moving them to a dedicated package keeps the shared integration layer free of installer-specific policy and makes the code reusable by unrelated callers. The installer diagnostics file retains only installer-facing types and the enableAndRestartTeleportService logic. JoinFailureError doc comments no longer hardcode the 5m0s timeout in the example, since the timeout is now configurable and changes across releases. Two clarifying comments on runSystemctlCommand document why context errors are preferred over exec errors, and why ExitError branches include the exit code and the captured stdout and stderr streams in the wrapped error message. The installer-side test file drops the helpers' tests because they now live alongside the package they cover in lib/systemd. The remaining TestJoinFailureErrorString case derives its expected message from JoinFailureTimeout rather than hardcoding a duration literal. * Bound join-health polling and diagnostics collection The join-health check now drives polling with retryutils.NewRetryV2 against a dedicated pollCtx bounded by readyzPollTimeout, and the diagnostics collection step races the diagnostics and journal goroutines against diagCtx using channels and a select loop. Previously the poll loop used a hand-rolled timer with a remaining-duration calculation, and the diagnostics step waited on a sync.WaitGroup, so a stuck diagnostics or journal implementation blocked the join failure report indefinitely even after the parent context was canceled. With the new structure, context cancellation and the diagnostic timeout both unblock the join failure path, and the journal output field now carries an explanatory message when collection times out or is canceled instead of being silently empty. JoinFailureTimeout is reduced from five to three minutes to surface join failures sooner in SSM invocation results, where the full five minute wait previously delayed operator feedback without improving the odds of recovery. LastError is no longer populated when the error from readyzChecker originates from the poll context already expiring, so the join failure message reflects the actual join outcome rather than a context cancellation observed only after the poll deadline. The installer now delegates diagnostics collection and journal capture to a teleportsystemd.Client constructed from the installer config, rather than calling unexported systemd helpers on the installer itself. The local dbusConn interface and its sole usage are removed in favor of teleportsystemd.NewConnFunc, which is now the test seam for injecting fake D-Bus connections into the diagnostics client. The autodiscover tests cover the new behaviors: a stuck diagnostics or journal implementation does not block join failure reporting when the parent context is canceled, a poll timeout does not populate LastError, and a diagnostics timeout produces a human-readable journal substitute. Existing tests switch from context.Background() to t.Context() so they inherit the per-test deadline configured by the testing package. The EC2 troubleshooting docs and ssm_install_test fixtures stop using a hardcoded five-minute duration and instead derive it from exported installer.JoinFailureTimeout, so they track future timeout changes without manual edits across docs, tests, and error message snippets. * Use t.Context in readyz checker tests * Reduce join-failure timeout default from 3m to 2m --------- Co-authored-by: Roman Tkachenko <roman@goteleport.com> Co-authored-by: Chris Thach <chris.thach@goteleport.com> Co-authored-by: Tiago Silva <tiago.silva@goteleport.com> Co-authored-by: Gavin Frazar <gavin.frazar@goteleport.com>