From 805ea0fe65b6079240e02def99610334b6963638 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Tue, 23 Jun 2026 22:47:27 +0000 Subject: [PATCH 01/16] agent: add preflight check design --- designs/agent-preflight.md | 470 +++++++++++++++++++++++++++++++++++++ 1 file changed, 470 insertions(+) create mode 100644 designs/agent-preflight.md diff --git a/designs/agent-preflight.md b/designs/agent-preflight.md new file mode 100644 index 000000000..b3ff929a8 --- /dev/null +++ b/designs/agent-preflight.md @@ -0,0 +1,470 @@ +# Agent Preflight + +`unbounded-agent preflight` validates that a host and agent configuration are +ready for node bootstrap before the agent mutates host state or joins the +cluster. The command is owned by the node agent because only the agent runs on +the target host with direct access to systemd, systemd-nspawn, kernel state, +network reachability, artifact mirrors, and GPU devices. + +This design intentionally focuses on the standalone preflight command. + +## Goals + +- Provide an agent-native command that can run on the node before bootstrap. +- Reuse existing agent config loading and goal-state resolution behavior where + possible. +- Detect hard bootstrap blockers before host mutation begins. +- Report warnings and fatal errors in a familiar kubeadm-style format. +- Allow selected fatal checks to be downgraded to warnings for break-glass + scenarios. +- Support machine-readable output for automation. +- Keep the command non-mutating. + +## Non-goals + +- Do not add offline deployment flags to the preflight command itself. +- Do not perform package installation, rootfs provisioning, image pulls, or + other durable host changes. +- Do not solve the full offline deployment config model in this document. + +## Kubeadm reference + +Kubeadm provides a useful model for this experience. Its preflight +implementation uses small check units with a shared interface: + +```go +type Checker interface { + Check() (warnings, errorList []error) + Name() string +} +``` + +Kubeadm runs these checks before cluster initialization or join work, prints +warnings and errors separately, fails on errors by default, and lets users make +specific checks non-fatal with `--ignore-preflight-errors`: + +```bash +kubeadm init phase preflight --config kubeadm-config.yaml +kubeadm init --ignore-preflight-errors=Swap,SystemVerification +kubeadm init --ignore-preflight-errors=all +``` + +The user-facing output is simple and recognizable: + +```text +[preflight] Running pre-flight checks + [WARNING Swap]: swap is enabled + [ERROR IsPrivilegedUser]: user is not running as root +[preflight] Some fatal errors occurred: +... +[preflight] If you know what you are doing, you can make a check non-fatal with `--ignore-preflight-errors=...` +``` + +`unbounded-agent preflight` should follow the same semantics where they fit: + +- Checks have stable names. +- Checks return warnings and errors separately. +- Errors are fatal by default. +- Ignored errors are downgraded to warnings. +- `all` ignores all preflight errors. +- Text output is optimized for humans. +- Structured output is available for automation. + +## Command UX + +The primary command is: + +```bash +unbounded-agent preflight +``` + +The command should load configuration through the existing agent config loading +behavior. + +Selected checks can be made non-fatal: + +```bash +unbounded-agent preflight --ignore-preflight-errors=swap-active +unbounded-agent preflight --ignore-preflight-errors=nvidia-runtime,nvidia-driver-libraries +unbounded-agent preflight --ignore-preflight-errors=all +``` + +Warnings do not fail the command by default. Operators can choose to fail on any +warning: + +```bash +unbounded-agent preflight --fail-on-warnings +``` + +Automation can request JSON output: + +```bash +unbounded-agent preflight --output json > preflight-report.json +``` + +Initial flags: + +| Flag | Meaning | +|---|---| +| `--ignore-preflight-errors` | Comma-separated check names whose errors should be reported as warnings. The special value `all` ignores all errors. | +| `--fail-on-warnings` | Exit non-zero when any warning is returned, even if there are no fatal errors. | +| `--output` | Output format. Supported values: `text`, `json`. Default: `text`. | + +The command should not accept deployment-shaping flags such as `--offline`, +`--gpu`, or mirror URLs. Preflight validates the loaded agent config; it does +not define the deployment config. + +## Command semantics + +`unbounded-agent preflight` should: + +1. Initialize command logging. +2. Load agent config. +3. Apply existing config normalization. +4. Resolve machine goal state far enough to know expected rootfs, downloads, + kubelet settings, and host integrations. +5. Build the applicable check list from the config and resolved goal state. +6. Run checks. +7. Print warnings and errors. +8. Exit `0` when no fatal errors remain after ignore rules and + `--fail-on-warnings` is not set. +9. Exit non-zero when one or more fatal errors remain, or when warnings are + present and `--fail-on-warnings` is set. +10. Avoid durable host mutation. + +The command should be safe to run repeatedly before bootstrap and after failed +bootstrap attempts. + +## Check model + +The preflight framework should use a result-oriented checker interface so it can +report successful checks, warnings, ignored errors, and fatal errors through the +same model: + +```go +type Checker interface { + Name() string + Check(ctx context.Context) []Result +} + +type Result struct { + Name string + Target string + Severity Severity // ok, warning, error + Message string + Ignored bool +} +``` + +The runner is responsible for applying `--ignore-preflight-errors`, formatting +results, and returning a fatal error when required. An ignored error is reported +as a warning with `Ignored: true`: + +```text +[WARNING swap-active]: swap is enabled +``` + +If `--fail-on-warnings` is set, any warning causes the command to exit non-zero, +including warnings created by ignored errors. + +Severity should follow a simple policy: + +- Return a warning when bootstrap can safely remediate the condition without + external input. For example, active swap can be a warning when bootstrap will + disable it. +- Return an error when bootstrap cannot proceed or remediation requires external + input. For example, missing host packages with unreachable package sources is + an error. +- Return an error when continuing would risk joining the node with incorrect + identity, credentials, rootfs, runtime, or GPU behavior. + +### Package ownership + +The reusable preflight framework should live in: + +```text +pkg/agent/preflight +``` + +This package owns the common report model, severity handling, +`--ignore-preflight-errors` behavior, `--fail-on-warnings` behavior, and runner +logic: + +```go +func Run(ctx context.Context, checks []Checker, opts Options) Report +``` + +The check implementation should be reusable outside the agent command so +external callers can run the same checks and consume the same report model. The +command package should only handle CLI flags, config loading, invoking the +preflight package, and formatting command output. + +Concrete checker constructors should live near the phase they validate. The +phase package owns the bootstrap behavior, so it should also own the +non-mutating checks that predict whether that behavior will succeed. + +Examples: + +```text +pkg/agent/phases/host + InstallPackages(...) + CheckHostPackages(...) + CheckHostOSConfiguration(...) + +pkg/agent/phases/rootfs + Provision(...) + CheckRootFSProvisioning(...) + CheckKubernetesArtifacts(...) + CheckCRIArtifacts(...) + CheckCNIArtifacts(...) + +pkg/agent/phases/nodestart + StartNode(...) + CheckNSpawnRuntime(...) +``` + +The agent command composes these phase-owned checkers after loading config and +resolving goal state: + +```go +checks := []preflight.Checker{ + preflight.AgentConfig(cfg), + host.CheckHostPackages(log), + host.CheckHostOSConfiguration(log), + rootfs.CheckOCIImageReachable(rootFSGoalState), + rootfs.CheckKubernetesArtifacts(rootFSGoalState), + rootfs.CheckCRIArtifacts(rootFSGoalState), + rootfs.CheckCNIArtifacts(rootFSGoalState), + nodestart.CheckNSpawnRuntime(nodeStartGoalState), +} + +report := preflight.Run(ctx, checks, opts) +``` + +Shared implementation should be factored below both the mutating task and the +checker. For example, artifact preflight should use the same URL resolution, +download, decompression, and verification helpers used by rootfs provisioning, +but it should not call the mutating rootfs task itself. + +Checks may use temporary files or temporary directories when they need to reuse +the same download, decompression, verification, or registry code paths as +bootstrap. Temporary state must be cleaned up before the check returns and must +not change durable host state. + +### Relationship to phases.Task + +The existing bootstrap executor is built around `phases.Task`: + +```go +type Task interface { + Name() string + Do(ctx context.Context) error +} +``` + +The current task model is a good execution model for bootstrap phases, but it is +not the right interface to run preflight checks directly: + +- `Do` returns only one fatal error, while preflight needs warnings and errors. +- `phases.Serial` stops on the first error, while preflight should collect all + applicable findings before returning. +- `phases.Parallel` cancels remaining tasks on the first error, which is useful + for bootstrap but loses diagnostic information for preflight. +- Existing phase tasks are intentionally mutating. For example, + `host.InstallPackages` installs packages, `host.ConfigureOS` writes sysctl + config and runs `sysctl --system`, `host.ConfigureNFTables` writes and starts + a systemd unit, `host.DisableSwap` runs `swapoff`, rootfs tasks download and + install binaries, and nodestart tasks start the nspawn machine and services. +- The preflight command must be non-mutating. + +Preflight should therefore define its own checker interface and runner. It can +still reuse the same ideas as `phases.Task`: stable names, serial/parallel +composition, elapsed-time logging when useful, and small units with explicit +dependencies. + +### Phase-aligned organization + +Preflight checks should be grouped around the same conceptual phases and task +names used by bootstrap. This keeps the output actionable because a failed +check points at the bootstrap step that would fail later. + +For example: + +```text +[ERROR host-packages]: missing required packages and package sources are unreachable +[WARNING swap-active]: swap is active and bootstrap will disable it +[ERROR oci-image-reachable]: failed to resolve rootfs image manifest (target: rootfs image) +``` + +The check name should remain stable and ignoreable, but it does not need to be +derived from a task name. Each checker can return an ad-hoc name that best +describes the condition it validates. Check names should use kebab-case so they +are easy to read in CLI output and pass to `--ignore-preflight-errors`. +Phase/task grouping can be represented in code structure, comments, or optional +metadata for JSON output without becoming part of the check name. + +Examples: + +```text +agent-config +host-packages +host-package-sources +swap-active +oci-image-reachable +kubernetes-artifacts +cri-artifacts +cni-artifacts +nvidia-driver-libraries +``` + +The ignore flag should accept stable check names: + +```bash +unbounded-agent preflight --ignore-preflight-errors=swap-active +unbounded-agent preflight --ignore-preflight-errors=nvidia-driver-libraries +``` + +The minimum requirement is exact check-name matching plus `all`, matching +kubeadm's mental model. + +## Text output + +Default output should be kubeadm-like: + +```text +[preflight] Running unbounded-agent pre-flight checks + [WARNING swap-active]: swap is enabled and will be disabled during bootstrap + [ERROR host-packages]: missing required packages and package sources are unreachable + [ERROR oci-image-reachable]: failed to resolve rootfs image manifest (target: rootfs image) +[preflight] Some fatal errors occurred: + [ERROR host-packages]: missing required packages and package sources are unreachable + [ERROR oci-image-reachable]: failed to resolve rootfs image manifest (target: rootfs image) +[preflight] If you know what you are doing, you can make a check non-fatal with `--ignore-preflight-errors=...` +``` + +Warnings should be printed as they are discovered. Fatal errors may be buffered +and summarized at the end, matching kubeadm's behavior. + +Preflight output must not print raw configured values such as URLs, image +references, tokens, certificate data, file contents, or credential-bearing +strings. Reports should include only the logical target being checked, such as +`rootfs image`, `kubernetes artifacts`, `cluster API server`, or +`bootstrap credential`. + +Checkers must sanitize or wrap errors from lower-level libraries before adding +them to a report. Download, registry, TLS, file, and package-manager errors may +include raw URLs, image references, paths, or credentials; those values must not +be copied into `Result.Message`. + +## JSON output + +JSON output should contain every check result, including successful checks, +warnings, ignored errors, and fatal errors: + +```json +{ + "status": "failed", + "checks": [ + { + "name": "agent-config", + "severity": "ok", + "message": "agent config is valid", + "target": "agent config", + "ignored": false + }, + { + "name": "host-packages", + "severity": "error", + "message": "missing required packages and package sources are unreachable", + "target": "host packages", + "ignored": false + }, + { + "name": "swap-active", + "severity": "warning", + "message": "swap is enabled and will be disabled during bootstrap", + "target": "host swap", + "ignored": false + } + ] +} +``` + +The schema should stay intentionally small. It should include `ignored` for each +check result so automation can distinguish normal warnings from ignored errors: + +```json +{ + "name": "swap-active", + "severity": "warning", + "message": "swap is enabled and will be disabled during bootstrap", + "target": "host swap", + "ignored": false +} +``` + +Possible future fields include `category`, `suggestion`, and +`documentationURL`. The same redaction rule applies to JSON output: include +logical targets, not raw config values. + +## Check set + +The check set should prioritize conditions that directly predict bootstrap +failure. Checks should be outcome-oriented: report whether the host, config, +artifacts, and credentials are ready for bootstrap instead of exposing every +low-level helper command as a separate check. + +Config check: + +| Check | Purpose | +|---|---| +| `agent-config` | Validate the loaded agent config and return config errors for missing required fields, invalid values, inconsistent settings, unsupported Kubernetes versions, missing OCI rootfs image, invalid download source templates, or invalid kubelet auth configuration. | + +Host phase checks: + +| Check | Purpose | +|---|---| +| `is-privileged-user` | Ensure the command is running as root. | +| `host-packages` | Validate a supported package manager exists and required host packages are installed. If packages are missing, validate whether they appear installable without mutating package-manager state. In offline or blocked-network environments this should fail when required packages are missing and cannot be installed. | +| `host-package-sources` | Validate package source reachability with non-mutating probes when package installation would be required. Skip or pass when all required packages are already installed. | +| `host-os-configuration` | Validate host OS configuration can be applied: sysctl config path writable, relevant kernel parameters acceptable or settable, and systemd unit paths writable. | +| `nspawn-runtime` | Validate the host systemd environment can manage nspawn machines using installed host capabilities. | +| `docker-active` | Warn if Docker is active and bootstrap will disable or avoid it. | +| `swap-active` | Warn when swap is enabled if bootstrap will disable it. | +| `disk-space` | Validate enough space exists for rootfs and component downloads. | +| `cgroups` | Validate cgroup support expected by kubelet/containerd. | +| `api-server-reachable` | Validate the configured Kubernetes API server is reachable from the host. | +| `cluster-credentials` | Validate the cluster CA data and configured bootstrap credential are present and parseable for kubelet registration. | +| `node-identity` | Validate node name resolution using the same order as agent config normalization: explicit `NodeName`, host hostname, then `MachineName`. The resolved value must be compatible with kubelet registration. | + +Rootfs provisioning checks: + +| Check | Purpose | +|---|---| +| `machine-dir` | Validate the target machine directory state is compatible with bootstrap. | +| `oci-image-reference` | Validate the rootfs image reference parses. | +| `oci-image-reachable` | Validate the configured rootfs image manifest can be resolved without pulling layers. | +| `rootfs-provisioning` | Validate rootfs provisioning prerequisites are available from installed host packages and host-side nspawn config paths are writable. | +| `kubernetes-artifacts` | Validate kubelet/kubectl/kube-proxy artifacts and checksums using the same download and verification calls used by rootfs provisioning, without installing files. | +| `cri-artifacts` | Validate containerd, runc, and crictl artifacts using the same download/decompression or download calls used by rootfs provisioning, without installing files. | +| `cni-artifacts` | Validate CNI plugin artifacts using the same download/decompression calls used by rootfs provisioning, without installing files. | +| `rootfs-parent-writable` | Validate the parent directory for rootfs creation can be created or written. | + +GPU checks: + +| Check | Purpose | +|---|---| +| `gpu-config` | Validate GPU policy/config once a GPU config block exists. | +| `nvidia-devices` | Validate expected NVIDIA device files are present. | +| `nvidia-driver-libraries` | Validate expected NVIDIA host libraries are discoverable. | +| `nvidia-runtime` | Validate NVIDIA runtime and CDI generation prerequisites when required. | + +Offline checks should be added after the preflight command when the agent has an +explicit offline policy in config. They are omitted when no offline policy is +configured: + +| Check | Purpose | +|---|---| +| `offline-no-upstream-urls` | Fail if offline mode resolves any artifact to an upstream default. | +| `allowed-hosts` | Validate all resolved URLs target configured allowed hosts. | +| `mirror-reachability` | Validate configured mirrors are reachable from the host. | From a05bca53f543240d228bb95869697ca0e47f994e Mon Sep 17 00:00:00 2001 From: Baichao He Date: Tue, 23 Jun 2026 22:56:25 +0000 Subject: [PATCH 02/16] agent: refine preflight design --- designs/agent-preflight.md | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/designs/agent-preflight.md b/designs/agent-preflight.md index b3ff929a8..94a423629 100644 --- a/designs/agent-preflight.md +++ b/designs/agent-preflight.md @@ -251,37 +251,6 @@ the same download, decompression, verification, or registry code paths as bootstrap. Temporary state must be cleaned up before the check returns and must not change durable host state. -### Relationship to phases.Task - -The existing bootstrap executor is built around `phases.Task`: - -```go -type Task interface { - Name() string - Do(ctx context.Context) error -} -``` - -The current task model is a good execution model for bootstrap phases, but it is -not the right interface to run preflight checks directly: - -- `Do` returns only one fatal error, while preflight needs warnings and errors. -- `phases.Serial` stops on the first error, while preflight should collect all - applicable findings before returning. -- `phases.Parallel` cancels remaining tasks on the first error, which is useful - for bootstrap but loses diagnostic information for preflight. -- Existing phase tasks are intentionally mutating. For example, - `host.InstallPackages` installs packages, `host.ConfigureOS` writes sysctl - config and runs `sysctl --system`, `host.ConfigureNFTables` writes and starts - a systemd unit, `host.DisableSwap` runs `swapoff`, rootfs tasks download and - install binaries, and nodestart tasks start the nspawn machine and services. -- The preflight command must be non-mutating. - -Preflight should therefore define its own checker interface and runner. It can -still reuse the same ideas as `phases.Task`: stable names, serial/parallel -composition, elapsed-time logging when useful, and small units with explicit -dependencies. - ### Phase-aligned organization Preflight checks should be grouped around the same conceptual phases and task From cb7fd9ade5d547dc8bba920ffb83f72e59b78f74 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Wed, 24 Jun 2026 00:25:03 +0000 Subject: [PATCH 03/16] agent: implement preflight command --- cmd/agent/internal/cmd/cmd.go | 1 + cmd/agent/internal/cmd/preflight.go | 156 ++++++++++++++ cmd/agent/internal/cmd/preflight_test.go | 86 ++++++++ pkg/agent/config/config.go | 40 ++++ pkg/agent/config/config_test.go | 71 +++++++ .../phases/host/preflight_agent_config.go | 37 ++++ .../host/preflight_agent_config_test.go | 50 +++++ .../host/preflight_cluster_credentials.go | 56 +++++ .../preflight_cluster_credentials_test.go | 50 +++++ .../phases/nodestart/preflight_api_server.go | 65 ++++++ .../nodestart/preflight_api_server_test.go | 58 ++++++ .../phases/rootfs/preflight_goal_state.go | 50 +++++ .../rootfs/preflight_goal_state_test.go | 51 +++++ pkg/agent/preflight/preflight.go | 191 ++++++++++++++++++ pkg/agent/preflight/preflight_test.go | 61 ++++++ 15 files changed, 1023 insertions(+) create mode 100644 cmd/agent/internal/cmd/preflight.go create mode 100644 cmd/agent/internal/cmd/preflight_test.go create mode 100644 pkg/agent/phases/host/preflight_agent_config.go create mode 100644 pkg/agent/phases/host/preflight_agent_config_test.go create mode 100644 pkg/agent/phases/host/preflight_cluster_credentials.go create mode 100644 pkg/agent/phases/host/preflight_cluster_credentials_test.go create mode 100644 pkg/agent/phases/nodestart/preflight_api_server.go create mode 100644 pkg/agent/phases/nodestart/preflight_api_server_test.go create mode 100644 pkg/agent/phases/rootfs/preflight_goal_state.go create mode 100644 pkg/agent/phases/rootfs/preflight_goal_state_test.go create mode 100644 pkg/agent/preflight/preflight.go create mode 100644 pkg/agent/preflight/preflight_test.go diff --git a/cmd/agent/internal/cmd/cmd.go b/cmd/agent/internal/cmd/cmd.go index 075d1b60d..c3cd57e34 100644 --- a/cmd/agent/internal/cmd/cmd.go +++ b/cmd/agent/internal/cmd/cmd.go @@ -26,6 +26,7 @@ func Run() { root.AddCommand( newCmdStart(cmdCtx), + newCmdPreflight(cmdCtx), newCmdDaemon(cmdCtx), newCmdReset(cmdCtx), newCmdVersion(), diff --git a/cmd/agent/internal/cmd/preflight.go b/cmd/agent/internal/cmd/preflight.go new file mode 100644 index 000000000..39b4c48fe --- /dev/null +++ b/cmd/agent/internal/cmd/preflight.go @@ -0,0 +1,156 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package cmd + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + "strings" + + "github.com/spf13/cobra" + + "github.com/Azure/unbounded/internal/provision" + "github.com/Azure/unbounded/pkg/agent/phases/host" + "github.com/Azure/unbounded/pkg/agent/phases/nodestart" + "github.com/Azure/unbounded/pkg/agent/phases/rootfs" + "github.com/Azure/unbounded/pkg/agent/preflight" +) + +type preflightHandler struct { + cmdCtx *CommandContext + configPath string + ignorePreflightErrors []string + failOnWarnings bool + output string + writer io.Writer +} + +func newCmdPreflight(cmdCtx *CommandContext) *cobra.Command { + handler := &preflightHandler{cmdCtx: cmdCtx, writer: os.Stdout} + + cmd := &cobra.Command{ + Use: "preflight", + Short: "Run non-mutating preflight checks", + Long: "Run non-mutating preflight checks for the host and agent configuration before node bootstrap.", + RunE: func(cmd *cobra.Command, _ []string) error { + return handler.execute(cmd.Context()) + }, + } + + cmd.Flags().StringVar(&handler.configPath, "config", "", "Path to agent config file") + cmd.Flags().StringSliceVar(&handler.ignorePreflightErrors, "ignore-preflight-errors", nil, "Comma-separated preflight check names whose errors should be reported as warnings") + cmd.Flags().BoolVar(&handler.failOnWarnings, "fail-on-warnings", false, "Fail when any preflight warning is returned") + cmd.Flags().StringVar(&handler.output, "output", "text", "Output format: text or json") + + return cmd +} + +func (h *preflightHandler) execute(ctx context.Context) error { + h.cmdCtx.Setup() + + if h.configPath != "" { + oldConfigPath := os.Getenv(configFileEnv) + defer os.Setenv(configFileEnv, oldConfigPath) //nolint:errcheck // best effort restore + + if err := os.Setenv(configFileEnv, h.configPath); err != nil { + return err + } + } + + cfg, err := loadConfig(h.cmdCtx.Logger) + if err != nil { + return err + } + + checks := []preflight.Checker{ + host.CheckAgentConfig(&cfg.AgentConfig), + host.CheckClusterCredentials(&cfg.AgentConfig, cfg.Attest != nil), + nodestart.CheckAPIServerReachable(cfg.Kubelet.ApiServer), + rootfs.CheckGoalState(h.cmdCtx.Logger, &cfg.AgentConfig, provision.ResolveDownloadOverrides(cfg.Downloads)), + } + + opts := preflight.Options{ + IgnoreErrors: h.ignorePreflightErrors, + FailOnWarnings: h.failOnWarnings, + } + report := preflight.Run(ctx, checks, opts) + + switch strings.ToLower(h.output) { + case "", "text": + if err := writePreflightText(h.writer, report); err != nil { + return err + } + case "json": + enc := json.NewEncoder(h.writer) + enc.SetIndent("", " ") + + if err := enc.Encode(report); err != nil { + return err + } + default: + return fmt.Errorf("unsupported output format %q", h.output) + } + + return report.Err(h.failOnWarnings) +} + +func writePreflightText(w io.Writer, report preflight.Report) error { + if _, err := fmt.Fprintln(w, "[preflight] Running unbounded-agent pre-flight checks"); err != nil { + return err + } + + var errors []preflight.Result + + for _, result := range report.Checks { + switch result.Severity { + case preflight.SeverityError: + errors = append(errors, result) + case preflight.SeverityWarning: + if _, err := fmt.Fprintf(w, "\t[WARNING %s]: %s", result.Name, result.Message); err != nil { + return err + } + + if result.Target != "" { + if _, err := fmt.Fprintf(w, " (target: %s)", result.Target); err != nil { + return err + } + } + + if _, err := fmt.Fprintln(w); err != nil { + return err + } + } + } + + if len(errors) == 0 { + return nil + } + + if _, err := fmt.Fprintln(w, "[preflight] Some fatal errors occurred:"); err != nil { + return err + } + + for _, result := range errors { + if _, err := fmt.Fprintf(w, "\t[ERROR %s]: %s", result.Name, result.Message); err != nil { + return err + } + + if result.Target != "" { + if _, err := fmt.Fprintf(w, " (target: %s)", result.Target); err != nil { + return err + } + } + + if _, err := fmt.Fprintln(w); err != nil { + return err + } + } + + _, err := fmt.Fprintln(w, "[preflight] If you know what you are doing, you can make a check non-fatal with `--ignore-preflight-errors=...`") + + return err +} diff --git a/cmd/agent/internal/cmd/preflight_test.go b/cmd/agent/internal/cmd/preflight_test.go new file mode 100644 index 000000000..02abc1e57 --- /dev/null +++ b/cmd/agent/internal/cmd/preflight_test.go @@ -0,0 +1,86 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Azure/unbounded/internal/provision" +) + +func preflightConfig(apiServer string) provision.UnboundedAgentConfig { + cfg := sampleConfig() + cfg.Kubelet.ApiServer = apiServer + cfg.OCIImage = "registry.example.com/unbounded/rootfs:v1" + + return cfg +} + +func TestNewCmdPreflight(t *testing.T) { + cmd := newCmdPreflight(&CommandContext{LogFormat: "text"}) + + assert.Equal(t, "preflight", cmd.Use) + assert.NotNil(t, cmd.Flags().Lookup("config")) + assert.NotNil(t, cmd.Flags().Lookup("ignore-preflight-errors")) + assert.NotNil(t, cmd.Flags().Lookup("fail-on-warnings")) + assert.NotNil(t, cmd.Flags().Lookup("output")) +} + +func TestPreflightJSONOutput(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(srv.Close) + + path := writeConfigFile(t, preflightConfig(srv.URL)) + + var out bytes.Buffer + + h := &preflightHandler{ + cmdCtx: &CommandContext{LogFormat: "text"}, + configPath: path, + ignorePreflightErrors: []string{"api-server-reachable"}, + output: "json", + writer: &out, + } + + require.NoError(t, h.execute(context.Background())) + + var report struct { + Status string `json:"status"` + Checks []struct { + Name string `json:"name"` + Ignored bool `json:"ignored"` + } `json:"checks"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &report)) + assert.Equal(t, "ok", report.Status) + assert.NotEmpty(t, report.Checks) +} + +func TestPreflightTextOutputError(t *testing.T) { + path := writeConfigFile(t, preflightConfig("https://127.0.0.1:1")) + + var out bytes.Buffer + + h := &preflightHandler{ + cmdCtx: &CommandContext{LogFormat: "text"}, + configPath: path, + output: "text", + writer: &out, + } + + err := h.execute(context.Background()) + require.Error(t, err) + assert.Contains(t, out.String(), "[ERROR api-server-reachable]") + assert.NotContains(t, out.String(), "127.0.0.1") +} diff --git a/pkg/agent/config/config.go b/pkg/agent/config/config.go index 56f025761..14a46c71d 100644 --- a/pkg/agent/config/config.go +++ b/pkg/agent/config/config.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "maps" + "net/url" "os" "slices" "strings" @@ -111,6 +112,45 @@ func (a *AgentConfig) DeepCopy() *AgentConfig { return &out } +// Validate checks that required agent configuration fields are present and +// internally consistent. Kubelet auth is validated when present; callers that +// require a bootstrap credential should enforce that separately because some +// flows fill the credential later through attestation. +func (a *AgentConfig) Validate() error { + if a == nil { + return fmt.Errorf("agent config is nil") + } + + var errs []error + if strings.TrimSpace(a.MachineName) == "" { + errs = append(errs, fmt.Errorf("MachineName is required")) + } + + if nodeName := strings.TrimSpace(a.NodeName); nodeName == "" { + errs = append(errs, fmt.Errorf("NodeName is required")) + } else if !isValidNodeName(nodeName) { + errs = append(errs, fmt.Errorf("NodeName is not a valid Kubernetes node name")) + } + + if strings.TrimSpace(a.Cluster.ClusterDNS) == "" { + errs = append(errs, fmt.Errorf("Cluster.ClusterDNS is required")) + } + + apiServer := strings.TrimSpace(a.Kubelet.ApiServer) + if apiServer == "" { + errs = append(errs, fmt.Errorf("Kubelet.ApiServer is required")) + } else if u, err := url.Parse(apiServer); err != nil || u.Scheme == "" || u.Host == "" { + errs = append(errs, fmt.Errorf("Kubelet.ApiServer is invalid")) + } + + // Kubelet auth is intentionally not validated here. Some consumers provide + // credentials later through product-specific flows such as attestation, and + // that context is outside the shared AgentConfig. Callers that require a + // static bootstrap credential should validate Kubelet.Auth separately. + + return errors.Join(errs...) +} + // AgentClusterConfig holds the cluster-level values the agent needs to // join the Kubernetes control plane. type AgentClusterConfig struct { diff --git a/pkg/agent/config/config_test.go b/pkg/agent/config/config_test.go index f734050dd..a1c0f9077 100644 --- a/pkg/agent/config/config_test.go +++ b/pkg/agent/config/config_test.go @@ -103,6 +103,77 @@ func TestCRIConfig_JSONRoundTrip(t *testing.T) { assert.Equal(t, "1.6.0", decoded.CNI.PluginVersion) } +func TestAgentConfig_Validate(t *testing.T) { + t.Parallel() + + valid := func() *AgentConfig { + return &AgentConfig{ + MachineName: "machine-1", + NodeName: "node-1", + Cluster: AgentClusterConfig{ + CaCertBase64: "Y2E=", + ClusterDNS: "10.0.0.10", + Version: "1.34.0", + }, + Kubelet: AgentKubeletConfig{ + ApiServer: "https://api.example.com:443", + Auth: KubeletAuthInfo{ + BootstrapToken: "abc123.secret456", + }, + }, + } + } + + tests := []struct { + name string + mutate func(*AgentConfig) + wantErr string + }{ + { + name: "valid", + }, + { + name: "missing machine name", + mutate: func(cfg *AgentConfig) { + cfg.MachineName = "" + }, + wantErr: "MachineName", + }, + { + name: "invalid node name", + mutate: func(cfg *AgentConfig) { + cfg.NodeName = "Invalid_Node" + }, + wantErr: "NodeName", + }, + { + name: "missing auth allowed", + mutate: func(cfg *AgentConfig) { + cfg.Kubelet.Auth = KubeletAuthInfo{} + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + cfg := valid() + if tt.mutate != nil { + tt.mutate(cfg) + } + + err := cfg.Validate() + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + return + } + + require.NoError(t, err) + }) + } +} + func TestCRIConfig_OmittedWhenEmpty(t *testing.T) { t.Parallel() diff --git a/pkg/agent/phases/host/preflight_agent_config.go b/pkg/agent/phases/host/preflight_agent_config.go new file mode 100644 index 000000000..cf9af95c0 --- /dev/null +++ b/pkg/agent/phases/host/preflight_agent_config.go @@ -0,0 +1,37 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package host + +import ( + "context" + + "github.com/Azure/unbounded/pkg/agent/config" + "github.com/Azure/unbounded/pkg/agent/preflight" +) + +// CheckAgentConfigName is the stable name for the agent config validation check. +const CheckAgentConfigName = "agent-config" + +type agentConfigChecker struct { + config *config.AgentConfig +} + +// CheckAgentConfig returns a checker that validates the shared agent config +// shape. Product-specific credential requirements are validated by separate +// checks. +func CheckAgentConfig(cfg *config.AgentConfig) preflight.Checker { + return agentConfigChecker{config: cfg} +} + +// Name returns the stable check name used in reports and ignore rules. +func (c agentConfigChecker) Name() string { return CheckAgentConfigName } + +// Check validates the shared agent config without mutating it. +func (c agentConfigChecker) Check(context.Context) []preflight.Result { + if err := c.config.Validate(); err != nil { + return preflight.ResultsError(CheckAgentConfigName, "agent config", "agent config is invalid") + } + + return preflight.ResultsOK(CheckAgentConfigName, "agent config", "agent config is valid") +} diff --git a/pkg/agent/phases/host/preflight_agent_config_test.go b/pkg/agent/phases/host/preflight_agent_config_test.go new file mode 100644 index 000000000..6475a61d1 --- /dev/null +++ b/pkg/agent/phases/host/preflight_agent_config_test.go @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package host + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/Azure/unbounded/pkg/agent/config" + "github.com/Azure/unbounded/pkg/agent/preflight" +) + +func validPreflightConfig() *config.AgentConfig { + return &config.AgentConfig{ + MachineName: "machine-1", + NodeName: "node-1", + Cluster: config.AgentClusterConfig{ + CaCertBase64: "Y2E=", + ClusterDNS: "10.0.0.10", + Version: "1.34.0", + }, + Kubelet: config.AgentKubeletConfig{ + ApiServer: "https://api.example.com:443", + Auth: config.KubeletAuthInfo{ + BootstrapToken: "abc123.secret456", + }, + }, + } +} + +func TestCheckAgentConfigValid(t *testing.T) { + results := CheckAgentConfig(validPreflightConfig()).Check(context.Background()) + + assert.Equal(t, preflight.ResultsOK(CheckAgentConfigName, "agent config", "agent config is valid"), results) +} + +func TestCheckAgentConfigInvalid(t *testing.T) { + cfg := validPreflightConfig() + cfg.MachineName = "" + + results := CheckAgentConfig(cfg).Check(context.Background()) + + assert.Equal(t, preflight.SeverityError, results[0].Severity) + assert.Equal(t, CheckAgentConfigName, results[0].Name) + assert.Equal(t, "agent config", results[0].Target) + assert.Equal(t, "agent config is invalid", results[0].Message) +} diff --git a/pkg/agent/phases/host/preflight_cluster_credentials.go b/pkg/agent/phases/host/preflight_cluster_credentials.go new file mode 100644 index 000000000..663131b4e --- /dev/null +++ b/pkg/agent/phases/host/preflight_cluster_credentials.go @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package host + +import ( + "context" + "encoding/base64" + "strings" + + "github.com/Azure/unbounded/pkg/agent/config" + "github.com/Azure/unbounded/pkg/agent/preflight" +) + +// CheckClusterCredentialsName is the stable name for cluster credential validation. +const CheckClusterCredentialsName = "cluster-credentials" + +type clusterCredentialsChecker struct { + config *config.AgentConfig + attestationConfigured bool +} + +// CheckClusterCredentials returns a checker that validates cluster CA data and +// the bootstrap credential. When attestationConfigured is true, missing kubelet +// auth is allowed because attestation can provide the credential later. +func CheckClusterCredentials(cfg *config.AgentConfig, attestationConfigured bool) preflight.Checker { + return clusterCredentialsChecker{config: cfg, attestationConfigured: attestationConfigured} +} + +// Name returns the stable check name used in reports and ignore rules. +func (c clusterCredentialsChecker) Name() string { return CheckClusterCredentialsName } + +// Check validates cluster credential inputs without printing credential values. +func (c clusterCredentialsChecker) Check(context.Context) []preflight.Result { + if c.config == nil { + return preflight.ResultsError(CheckClusterCredentialsName, "cluster credentials", "agent config is missing") + } + + var errs []string + if _, err := base64.StdEncoding.DecodeString(c.config.Cluster.CaCertBase64); err != nil { + errs = append(errs, "cluster CA data is invalid") + } + + auth := c.config.Kubelet.Auth + if !c.attestationConfigured { + if err := auth.Validate(); err != nil { + errs = append(errs, "bootstrap credential is invalid") + } + } + + if len(errs) > 0 { + return preflight.ResultsError(CheckClusterCredentialsName, "cluster credentials", strings.Join(errs, "; ")) + } + + return preflight.ResultsOK(CheckClusterCredentialsName, "cluster credentials", "cluster credentials are valid") +} diff --git a/pkg/agent/phases/host/preflight_cluster_credentials_test.go b/pkg/agent/phases/host/preflight_cluster_credentials_test.go new file mode 100644 index 000000000..d1e4cb9ab --- /dev/null +++ b/pkg/agent/phases/host/preflight_cluster_credentials_test.go @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package host + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/Azure/unbounded/pkg/agent/preflight" +) + +func TestCheckClusterCredentialsValid(t *testing.T) { + results := CheckClusterCredentials(validPreflightConfig(), false).Check(context.Background()) + + assert.Equal(t, preflight.ResultsOK(CheckClusterCredentialsName, "cluster credentials", "cluster credentials are valid"), results) +} + +func TestCheckClusterCredentialsAllowsAttestation(t *testing.T) { + cfg := validPreflightConfig() + cfg.Kubelet.Auth.BootstrapToken = "" + + results := CheckClusterCredentials(cfg, true).Check(context.Background()) + + assert.Equal(t, preflight.SeverityOK, results[0].Severity) +} + +func TestCheckClusterCredentialsRequiresAuthWhenNoAttestation(t *testing.T) { + cfg := validPreflightConfig() + cfg.Kubelet.Auth.BootstrapToken = "" + + results := CheckClusterCredentials(cfg, false).Check(context.Background()) + + assert.Equal(t, preflight.SeverityError, results[0].Severity) + assert.Equal(t, CheckClusterCredentialsName, results[0].Name) + assert.Equal(t, "cluster credentials", results[0].Target) + assert.Equal(t, "bootstrap credential is invalid", results[0].Message) +} + +func TestCheckClusterCredentialsInvalidCA(t *testing.T) { + cfg := validPreflightConfig() + cfg.Cluster.CaCertBase64 = "not-base64" + + results := CheckClusterCredentials(cfg, false).Check(context.Background()) + + assert.Equal(t, preflight.SeverityError, results[0].Severity) + assert.Contains(t, results[0].Message, "cluster CA data is invalid") +} diff --git a/pkg/agent/phases/nodestart/preflight_api_server.go b/pkg/agent/phases/nodestart/preflight_api_server.go new file mode 100644 index 000000000..08907f0ec --- /dev/null +++ b/pkg/agent/phases/nodestart/preflight_api_server.go @@ -0,0 +1,65 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package nodestart + +import ( + "context" + "fmt" + "net/http" + "net/url" + "strings" + "time" + + "github.com/Azure/unbounded/pkg/agent/preflight" +) + +// CheckAPIServerReachableName is the stable name for API server reachability. +const CheckAPIServerReachableName = "api-server-reachable" + +type apiServerReachableChecker struct { + url string + httpClient *http.Client +} + +// CheckAPIServerReachable returns a non-mutating checker that validates the +// configured Kubernetes API server can be reached from the host. The checker +// redacts the configured endpoint from result messages. +func CheckAPIServerReachable(apiServer string) preflight.Checker { + return apiServerReachableChecker{url: apiServer} +} + +func (c apiServerReachableChecker) Name() string { return CheckAPIServerReachableName } + +func (c apiServerReachableChecker) Check(ctx context.Context) []preflight.Result { + if strings.TrimSpace(c.url) == "" { + return preflight.ResultsError(CheckAPIServerReachableName, "cluster API server", "API server is required") + } + + parsed, err := url.Parse(c.url) + if err != nil || parsed.Scheme == "" || parsed.Host == "" { + return preflight.ResultsError(CheckAPIServerReachableName, "cluster API server", "API server endpoint is invalid") + } + + client := c.httpClient + if client == nil { + client = &http.Client{Timeout: 10 * time.Second} + } + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, strings.TrimRight(c.url, "/")+"/readyz", http.NoBody) + if err != nil { + return preflight.ResultsError(CheckAPIServerReachableName, "cluster API server", "API server request could not be created") + } + + resp, err := client.Do(req) + if err != nil { + return preflight.ResultsError(CheckAPIServerReachableName, "cluster API server", "API server is not reachable") + } + defer resp.Body.Close() //nolint:errcheck // best effort close + + if resp.StatusCode >= http.StatusInternalServerError { + return preflight.ResultsError(CheckAPIServerReachableName, "cluster API server", fmt.Sprintf("API server returned status %d", resp.StatusCode)) + } + + return preflight.ResultsOK(CheckAPIServerReachableName, "cluster API server", "API server is reachable") +} diff --git a/pkg/agent/phases/nodestart/preflight_api_server_test.go b/pkg/agent/phases/nodestart/preflight_api_server_test.go new file mode 100644 index 000000000..040999c87 --- /dev/null +++ b/pkg/agent/phases/nodestart/preflight_api_server_test.go @@ -0,0 +1,58 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package nodestart + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/Azure/unbounded/pkg/agent/preflight" +) + +func TestCheckAPIServerReachableOK(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/readyz", r.URL.Path) + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(srv.Close) + + results := CheckAPIServerReachable(srv.URL).Check(context.Background()) + + assert.Equal(t, preflight.ResultsOK(CheckAPIServerReachableName, "cluster API server", "API server is reachable"), results) +} + +func TestCheckAPIServerReachableInvalidEndpoint(t *testing.T) { + results := CheckAPIServerReachable("://bad").Check(context.Background()) + + assert.Equal(t, preflight.SeverityError, results[0].Severity) + assert.Equal(t, CheckAPIServerReachableName, results[0].Name) + assert.Equal(t, "cluster API server", results[0].Target) + assert.Equal(t, "API server endpoint is invalid", results[0].Message) +} + +func TestCheckAPIServerReachableRequestFailureIsRedacted(t *testing.T) { + const endpoint = "https://127.0.0.1:1" + + results := CheckAPIServerReachable(endpoint).Check(context.Background()) + + assert.Equal(t, preflight.SeverityError, results[0].Severity) + assert.Equal(t, "API server is not reachable", results[0].Message) + assert.NotContains(t, results[0].Message, endpoint) +} + +func TestCheckAPIServerReachableServerError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + t.Cleanup(srv.Close) + + results := CheckAPIServerReachable(srv.URL).Check(context.Background()) + + assert.Equal(t, preflight.SeverityError, results[0].Severity) + assert.Equal(t, "API server returned status 500", results[0].Message) +} diff --git a/pkg/agent/phases/rootfs/preflight_goal_state.go b/pkg/agent/phases/rootfs/preflight_goal_state.go new file mode 100644 index 000000000..b3e4a15f5 --- /dev/null +++ b/pkg/agent/phases/rootfs/preflight_goal_state.go @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package rootfs + +import ( + "context" + "log/slog" + + "github.com/Azure/unbounded/pkg/agent/config" + "github.com/Azure/unbounded/pkg/agent/goalstates" + "github.com/Azure/unbounded/pkg/agent/preflight" +) + +const ( + // CheckGoalStateName is the stable name for machine goal-state resolution. + CheckGoalStateName = "goal-state" + // CheckOCIImageReachableName is the stable name for rootfs image validation. + CheckOCIImageReachableName = "oci-image-reachable" +) + +type goalStateChecker struct { + log *slog.Logger + config *config.AgentConfig + downloads *goalstates.DownloadOverrides +} + +// CheckGoalState returns a checker that validates the agent config can be +// resolved into a machine goal state and that an OCI rootfs image is selected. +func CheckGoalState(log *slog.Logger, cfg *config.AgentConfig, downloads *goalstates.DownloadOverrides) preflight.Checker { + return goalStateChecker{log: log, config: cfg, downloads: downloads} +} + +func (c goalStateChecker) Name() string { return CheckGoalStateName } + +func (c goalStateChecker) Check(context.Context) []preflight.Result { + gs, err := goalstates.ResolveMachine(c.log, c.config, goalstates.NSpawnMachineKube1, c.downloads) + if err != nil { + return preflight.ResultsError(CheckGoalStateName, "goal state", "goal state could not be resolved") + } + + if gs.RootFS.OCIImage == "" { + // TODO: replace this with an OCI manifest reachability check that uses + // the same registry parsing and plain-HTTP handling as OCI rootfs + // provisioning, without pulling image layers. + return preflight.ResultsError(CheckOCIImageReachableName, "rootfs image", "OCI rootfs image is required") + } + + return preflight.ResultsOK(CheckGoalStateName, "goal state", "goal state resolved") +} diff --git a/pkg/agent/phases/rootfs/preflight_goal_state_test.go b/pkg/agent/phases/rootfs/preflight_goal_state_test.go new file mode 100644 index 000000000..911cd85e9 --- /dev/null +++ b/pkg/agent/phases/rootfs/preflight_goal_state_test.go @@ -0,0 +1,51 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package rootfs + +import ( + "context" + "log/slog" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/Azure/unbounded/pkg/agent/config" + "github.com/Azure/unbounded/pkg/agent/preflight" +) + +func validGoalStateConfig() *config.AgentConfig { + return &config.AgentConfig{ + MachineName: "machine-1", + NodeName: "node-1", + Cluster: config.AgentClusterConfig{ + CaCertBase64: "Y2E=", + ClusterDNS: "10.0.0.10", + Version: "1.34.0", + }, + Kubelet: config.AgentKubeletConfig{ + ApiServer: "https://api.example.com:443", + Auth: config.KubeletAuthInfo{ + BootstrapToken: "abc123.secret456", + }, + }, + OCIImage: "registry.example.com/unbounded/rootfs:v1", + } +} + +func TestCheckGoalStateOK(t *testing.T) { + results := CheckGoalState(slog.New(slog.DiscardHandler), validGoalStateConfig(), nil).Check(context.Background()) + + assert.Equal(t, []preflight.Result{preflight.OK(CheckGoalStateName, "goal state", "goal state resolved")}, results) +} + +func TestCheckGoalStateResolveError(t *testing.T) { + cfg := validGoalStateConfig() + cfg.Cluster.CaCertBase64 = "not-base64" + + results := CheckGoalState(slog.New(slog.DiscardHandler), cfg, nil).Check(context.Background()) + + assert.Equal(t, preflight.SeverityError, results[0].Severity) + assert.Equal(t, CheckGoalStateName, results[0].Name) + assert.Equal(t, "goal state could not be resolved", results[0].Message) +} diff --git a/pkg/agent/preflight/preflight.go b/pkg/agent/preflight/preflight.go new file mode 100644 index 000000000..08b71ec36 --- /dev/null +++ b/pkg/agent/preflight/preflight.go @@ -0,0 +1,191 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// Package preflight provides reusable non-mutating checks for validating a host +// and agent configuration before bootstrap. +package preflight + +import ( + "context" + "fmt" + "slices" + "strings" +) + +type Severity string + +const ( + // SeverityOK indicates the check completed successfully. + SeverityOK Severity = "ok" + // SeverityWarning indicates the check found a condition that bootstrap can + // usually remediate or that does not necessarily block bootstrap. + SeverityWarning Severity = "warning" + // SeverityError indicates the check found a fatal condition that should block + // bootstrap unless explicitly ignored. + SeverityError Severity = "error" +) + +// Checker is a non-mutating preflight validation unit. +type Checker interface { + Name() string + Check(ctx context.Context) []Result +} + +// Result describes one preflight check outcome. Message and Target must not +// include raw configured values such as URLs, tokens, image references, or file +// contents. +type Result struct { + Name string `json:"name"` + Target string `json:"target"` + Severity Severity `json:"severity"` + Message string `json:"message"` + Ignored bool `json:"ignored"` +} + +// Options controls preflight result handling. +type Options struct { + IgnoreErrors []string + FailOnWarnings bool +} + +// Report is the complete preflight output for both text and JSON consumers. +type Report struct { + Status string `json:"status"` + Checks []Result `json:"checks"` + Summary Summary `json:"summary"` +} + +// Summary contains aggregate result counts after ignore handling is applied. +type Summary struct { + OK int `json:"ok"` + Warnings int `json:"warnings"` + Errors int `json:"errors"` +} + +// Run executes all checks, applies ignore rules, and returns a complete report. +func Run(ctx context.Context, checks []Checker, opts Options) Report { + ignored := ignoreSet(opts.IgnoreErrors) + results := make([]Result, 0, len(checks)) + + for _, check := range checks { + for _, result := range check.Check(ctx) { + if result.Name == "" { + result.Name = check.Name() + } + + if result.Target == "" { + result.Target = result.Name + } + + if result.Severity == "" { + result.Severity = SeverityOK + } + + if result.Severity == SeverityError && ignored(result.Name) { + result.Severity = SeverityWarning + result.Ignored = true + } + + results = append(results, result) + } + } + + return buildReport(results, opts.FailOnWarnings) +} + +// OK returns a successful check result. +func OK(name, target, message string) Result { + return Result{Name: name, Target: target, Severity: SeverityOK, Message: message} +} + +// Warning returns a warning check result. +func Warning(name, target, message string) Result { + return Result{Name: name, Target: target, Severity: SeverityWarning, Message: message} +} + +// Error returns a fatal check result. +func Error(name, target, message string) Result { + return Result{Name: name, Target: target, Severity: SeverityError, Message: message} +} + +// Results returns a result slice for concise checker returns and test fixtures. +func Results(results ...Result) []Result { + return results +} + +// ResultsOK returns a single successful check result as a slice. +func ResultsOK(name, target, message string) []Result { + return Results(OK(name, target, message)) +} + +// ResultsWarning returns a single warning check result as a slice. +func ResultsWarning(name, target, message string) []Result { + return Results(Warning(name, target, message)) +} + +// ResultsError returns a single fatal check result as a slice. +func ResultsError(name, target, message string) []Result { + return Results(Error(name, target, message)) +} + +// HasErrors reports whether any fatal errors remain after ignore handling. +func (r Report) HasErrors() bool { + return r.Summary.Errors > 0 +} + +// HasWarnings reports whether the report contains any warnings. +func (r Report) HasWarnings() bool { + return r.Summary.Warnings > 0 +} + +// Err converts the report status into a command error. +func (r Report) Err(failOnWarnings bool) error { + if r.HasErrors() { + return fmt.Errorf("preflight checks failed") + } + + if failOnWarnings && r.HasWarnings() { + return fmt.Errorf("preflight checks returned warnings") + } + + return nil +} + +func buildReport(results []Result, failOnWarnings bool) Report { + summary := Summary{} + + for _, result := range results { + switch result.Severity { + case SeverityError: + summary.Errors++ + case SeverityWarning: + summary.Warnings++ + default: + summary.OK++ + } + } + + status := "ok" + if summary.Errors > 0 || (failOnWarnings && summary.Warnings > 0) { + status = "failed" + } + + return Report{Status: status, Checks: results, Summary: summary} +} + +func ignoreSet(values []string) func(string) bool { + normalized := make([]string, 0, len(values)) + for _, value := range values { + for _, part := range strings.Split(value, ",") { + part = strings.ToLower(strings.TrimSpace(part)) + if part != "" { + normalized = append(normalized, part) + } + } + } + + return func(name string) bool { + name = strings.ToLower(strings.TrimSpace(name)) + return slices.Contains(normalized, "all") || slices.Contains(normalized, name) + } +} diff --git a/pkg/agent/preflight/preflight_test.go b/pkg/agent/preflight/preflight_test.go new file mode 100644 index 000000000..0713badb7 --- /dev/null +++ b/pkg/agent/preflight/preflight_test.go @@ -0,0 +1,61 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package preflight + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +type fakeChecker struct { + name string + results []Result +} + +func (f fakeChecker) Name() string { return f.name } + +func (f fakeChecker) Check(context.Context) []Result { return f.results } + +func TestRunIncludesAllResults(t *testing.T) { + report := Run(context.Background(), []Checker{ + fakeChecker{results: []Result{OK("agent-config", "agent config", "valid")}}, + fakeChecker{results: []Result{Warning("swap-active", "host swap", "enabled")}}, + fakeChecker{results: []Result{Error("host-packages", "host packages", "missing")}}, + }, Options{}) + + assert.Equal(t, "failed", report.Status) + assert.Equal(t, Summary{OK: 1, Warnings: 1, Errors: 1}, report.Summary) + assert.Len(t, report.Checks, 3) +} + +func TestRunDowngradesIgnoredErrors(t *testing.T) { + report := Run(context.Background(), []Checker{ + fakeChecker{results: []Result{Error("host-packages", "host packages", "missing")}}, + }, Options{IgnoreErrors: []string{"host-packages"}}) + + assert.Equal(t, "ok", report.Status) + assert.Equal(t, Summary{Warnings: 1}, report.Summary) + assert.True(t, report.Checks[0].Ignored) + assert.Equal(t, SeverityWarning, report.Checks[0].Severity) +} + +func TestRunFailOnWarnings(t *testing.T) { + report := Run(context.Background(), []Checker{ + fakeChecker{results: []Result{Warning("swap-active", "host swap", "enabled")}}, + }, Options{FailOnWarnings: true}) + + assert.Equal(t, "failed", report.Status) + assert.Error(t, report.Err(true)) +} + +func TestRunIgnoreAll(t *testing.T) { + report := Run(context.Background(), []Checker{ + fakeChecker{results: []Result{Error("host-packages", "host packages", "missing")}}, + }, Options{IgnoreErrors: []string{"all"}}) + + assert.Equal(t, "ok", report.Status) + assert.True(t, report.Checks[0].Ignored) +} From 39ba64c63aaab38f06e5379c03268334884ddd7b Mon Sep 17 00:00:00 2001 From: Baichao He Date: Wed, 24 Jun 2026 00:31:47 +0000 Subject: [PATCH 04/16] agent: run preflight in e2e bootstrap --- hack/agent/e2e-kind/e2e.py | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/hack/agent/e2e-kind/e2e.py b/hack/agent/e2e-kind/e2e.py index 36223e9f5..68abe55d7 100755 --- a/hack/agent/e2e-kind/e2e.py +++ b/hack/agent/e2e-kind/e2e.py @@ -213,6 +213,10 @@ def scp_cmd(src: str, dst: str) -> subprocess.CompletedProcess[str]: return run(["scp", *SSH_OPTS, src, dst]) +def scp_from_vm(src: str, dst: Path) -> subprocess.CompletedProcess[str]: + return run(["scp", *SSH_OPTS, f"{SSH_TARGET}:{src}", str(dst)]) + + def kubectl(args: list[str], **kw: Any) -> subprocess.CompletedProcess[str]: return run([KUBECTL, *args], **kw) @@ -1639,6 +1643,21 @@ def _run_agent_inner(agent_url: str, node_config: NodeConfig) -> None: bootstrap_script_path.chmod(0o600) log(f"Bootstrap script written to {bootstrap_script_path}") + bootstrap_preflight_script = bootstrap_script.replace( + 'echo "Running unbounded-agent start..."\n"${AGENT_BIN}" start ${_START_ARGS}', + textwrap.dedent("""\ + echo "Running unbounded-agent preflight..." + "${AGENT_BIN}" preflight ${_START_ARGS} --output text | tee /tmp/unbounded-agent-preflight.txt + "${AGENT_BIN}" preflight ${_START_ARGS} --output json > /tmp/unbounded-agent-preflight.json + echo "Running unbounded-agent start..." + "${AGENT_BIN}" start ${_START_ARGS}"""), + ) + if bootstrap_preflight_script == bootstrap_script: + die("failed to inject unbounded-agent preflight into bootstrap script") + + bootstrap_script = bootstrap_preflight_script + bootstrap_script_path.write_text(bootstrap_script) + # Wait for cloud-init and verify connectivity log("Waiting for cloud-init to complete on VM...") subprocess.run(["ssh", *SSH_OPTS, SSH_TARGET, "sudo cloud-init status --wait"], @@ -1664,6 +1683,10 @@ def _run_agent_inner(agent_url: str, node_config: NodeConfig) -> None: f"sudo {env_prefix} /tmp/bootstrap.sh", ]) + log("Copying preflight reports from VM...") + scp_from_vm("/tmp/unbounded-agent-preflight.txt", VM_DIR / "unbounded-agent-preflight.txt") + scp_from_vm("/tmp/unbounded-agent-preflight.json", VM_DIR / "unbounded-agent-preflight.json") + # --------------------------------------------------------------------------- # wait-for-node @@ -2872,6 +2895,20 @@ def _collect_one_vm_logs(logs_dir: Path, vm_name: str, vm_ip: str, vm_dir: Path, serial_log = vm_dir / f"{vm_name}.log" if serial_log.exists(): shutil.copyfile(serial_log, logs_dir / f"{prefix}vm-serial.log") + for name in ("unbounded-agent-preflight.txt", "unbounded-agent-preflight.json"): + src = vm_dir / name + if src.exists(): + shutil.copyfile(src, logs_dir / f"{prefix}{name}") + + for name in ("unbounded-agent-preflight.txt", "unbounded-agent-preflight.json"): + result = subprocess.run( + ["scp", *ssh_opts, f"{ssh_target}:/tmp/{name}", str(logs_dir / f"{prefix}{name}")], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + ) + if result.returncode == 0: + diag(f"Collected {name} from VM") ssh_opts = [ "-o", "StrictHostKeyChecking=no", From 76b02c190a9abca1243bf48164111866964eba08 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Wed, 24 Jun 2026 00:39:47 +0000 Subject: [PATCH 05/16] agent: use cluster ca in preflight api check --- cmd/agent/internal/cmd/preflight.go | 8 +++++- hack/agent/e2e-kind/e2e.py | 17 +++++------ .../phases/nodestart/preflight_api_server.go | 28 +++++++++++++++++-- .../nodestart/preflight_api_server_test.go | 8 +++--- 4 files changed, 45 insertions(+), 16 deletions(-) diff --git a/cmd/agent/internal/cmd/preflight.go b/cmd/agent/internal/cmd/preflight.go index 39b4c48fe..8e9596364 100644 --- a/cmd/agent/internal/cmd/preflight.go +++ b/cmd/agent/internal/cmd/preflight.go @@ -5,6 +5,7 @@ package cmd import ( "context" + "encoding/base64" "encoding/json" "fmt" "io" @@ -66,10 +67,15 @@ func (h *preflightHandler) execute(ctx context.Context) error { return err } + caCertData, err := base64.StdEncoding.DecodeString(cfg.Cluster.CaCertBase64) + if err != nil { + caCertData = nil + } + checks := []preflight.Checker{ host.CheckAgentConfig(&cfg.AgentConfig), host.CheckClusterCredentials(&cfg.AgentConfig, cfg.Attest != nil), - nodestart.CheckAPIServerReachable(cfg.Kubelet.ApiServer), + nodestart.CheckAPIServerReachable(cfg.Kubelet.ApiServer, caCertData), rootfs.CheckGoalState(h.cmdCtx.Logger, &cfg.AgentConfig, provision.ResolveDownloadOverrides(cfg.Downloads)), } diff --git a/hack/agent/e2e-kind/e2e.py b/hack/agent/e2e-kind/e2e.py index 68abe55d7..6714d47e4 100755 --- a/hack/agent/e2e-kind/e2e.py +++ b/hack/agent/e2e-kind/e2e.py @@ -2895,6 +2895,15 @@ def _collect_one_vm_logs(logs_dir: Path, vm_name: str, vm_ip: str, vm_dir: Path, serial_log = vm_dir / f"{vm_name}.log" if serial_log.exists(): shutil.copyfile(serial_log, logs_dir / f"{prefix}vm-serial.log") + + ssh_opts = [ + "-o", "StrictHostKeyChecking=no", + "-o", "UserKnownHostsFile=/dev/null", + "-o", "ConnectTimeout=5", + "-i", str(vm_dir / "ssh" / "id_ed25519"), + ] + ssh_target = f"{VM_SSH_USER}@{vm_ip}" + for name in ("unbounded-agent-preflight.txt", "unbounded-agent-preflight.json"): src = vm_dir / name if src.exists(): @@ -2910,14 +2919,6 @@ def _collect_one_vm_logs(logs_dir: Path, vm_name: str, vm_ip: str, vm_dir: Path, if result.returncode == 0: diag(f"Collected {name} from VM") - ssh_opts = [ - "-o", "StrictHostKeyChecking=no", - "-o", "UserKnownHostsFile=/dev/null", - "-o", "ConnectTimeout=5", - "-i", str(vm_dir / "ssh" / "id_ed25519"), - ] - ssh_target = f"{VM_SSH_USER}@{vm_ip}" - def ssh_log(name: str, command: str) -> None: _write_command_log(logs_dir / f"{prefix}{name}", ["ssh", *ssh_opts, ssh_target, command]) diff --git a/pkg/agent/phases/nodestart/preflight_api_server.go b/pkg/agent/phases/nodestart/preflight_api_server.go index 08907f0ec..2ece7ed48 100644 --- a/pkg/agent/phases/nodestart/preflight_api_server.go +++ b/pkg/agent/phases/nodestart/preflight_api_server.go @@ -5,6 +5,8 @@ package nodestart import ( "context" + "crypto/tls" + "crypto/x509" "fmt" "net/http" "net/url" @@ -19,14 +21,15 @@ const CheckAPIServerReachableName = "api-server-reachable" type apiServerReachableChecker struct { url string + caCertData []byte httpClient *http.Client } // CheckAPIServerReachable returns a non-mutating checker that validates the // configured Kubernetes API server can be reached from the host. The checker // redacts the configured endpoint from result messages. -func CheckAPIServerReachable(apiServer string) preflight.Checker { - return apiServerReachableChecker{url: apiServer} +func CheckAPIServerReachable(apiServer string, caCertData []byte) preflight.Checker { + return apiServerReachableChecker{url: apiServer, caCertData: caCertData} } func (c apiServerReachableChecker) Name() string { return CheckAPIServerReachableName } @@ -43,7 +46,7 @@ func (c apiServerReachableChecker) Check(ctx context.Context) []preflight.Result client := c.httpClient if client == nil { - client = &http.Client{Timeout: 10 * time.Second} + client = c.httpClientWithCA() } req, err := http.NewRequestWithContext(ctx, http.MethodGet, strings.TrimRight(c.url, "/")+"/readyz", http.NoBody) @@ -63,3 +66,22 @@ func (c apiServerReachableChecker) Check(ctx context.Context) []preflight.Result return preflight.ResultsOK(CheckAPIServerReachableName, "cluster API server", "API server is reachable") } + +func (c apiServerReachableChecker) httpClientWithCA() *http.Client { + transport := &http.Transport{} + if defaultTransport, ok := http.DefaultTransport.(*http.Transport); ok { + transport = defaultTransport.Clone() + } + + if len(c.caCertData) > 0 { + pool, err := x509.SystemCertPool() + if err != nil { + pool = x509.NewCertPool() + } + + pool.AppendCertsFromPEM(c.caCertData) + transport.TLSClientConfig = &tls.Config{RootCAs: pool} //nolint:gosec // uses configured root CAs. + } + + return &http.Client{Timeout: 10 * time.Second, Transport: transport} +} diff --git a/pkg/agent/phases/nodestart/preflight_api_server_test.go b/pkg/agent/phases/nodestart/preflight_api_server_test.go index 040999c87..25c19ccff 100644 --- a/pkg/agent/phases/nodestart/preflight_api_server_test.go +++ b/pkg/agent/phases/nodestart/preflight_api_server_test.go @@ -21,13 +21,13 @@ func TestCheckAPIServerReachableOK(t *testing.T) { })) t.Cleanup(srv.Close) - results := CheckAPIServerReachable(srv.URL).Check(context.Background()) + results := CheckAPIServerReachable(srv.URL, nil).Check(context.Background()) assert.Equal(t, preflight.ResultsOK(CheckAPIServerReachableName, "cluster API server", "API server is reachable"), results) } func TestCheckAPIServerReachableInvalidEndpoint(t *testing.T) { - results := CheckAPIServerReachable("://bad").Check(context.Background()) + results := CheckAPIServerReachable("://bad", nil).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Equal(t, CheckAPIServerReachableName, results[0].Name) @@ -38,7 +38,7 @@ func TestCheckAPIServerReachableInvalidEndpoint(t *testing.T) { func TestCheckAPIServerReachableRequestFailureIsRedacted(t *testing.T) { const endpoint = "https://127.0.0.1:1" - results := CheckAPIServerReachable(endpoint).Check(context.Background()) + results := CheckAPIServerReachable(endpoint, nil).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Equal(t, "API server is not reachable", results[0].Message) @@ -51,7 +51,7 @@ func TestCheckAPIServerReachableServerError(t *testing.T) { })) t.Cleanup(srv.Close) - results := CheckAPIServerReachable(srv.URL).Check(context.Background()) + results := CheckAPIServerReachable(srv.URL, nil).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Equal(t, "API server returned status 500", results[0].Message) From 58a525b1d6be4a1fb5f3a99a29c26e06474a2fdb Mon Sep 17 00:00:00 2001 From: Baichao He Date: Wed, 24 Jun 2026 01:20:29 +0000 Subject: [PATCH 06/16] agent: add host preflight checks --- cmd/agent/internal/cmd/preflight.go | 10 + cmd/agent/internal/cmd/preflight_test.go | 2 +- pkg/agent/phases/host/preflight_host.go | 280 +++++++++++++++++++ pkg/agent/phases/host/preflight_host_test.go | 158 +++++++++++ 4 files changed, 449 insertions(+), 1 deletion(-) create mode 100644 pkg/agent/phases/host/preflight_host.go create mode 100644 pkg/agent/phases/host/preflight_host_test.go diff --git a/cmd/agent/internal/cmd/preflight.go b/cmd/agent/internal/cmd/preflight.go index 8e9596364..925e76db9 100644 --- a/cmd/agent/internal/cmd/preflight.go +++ b/cmd/agent/internal/cmd/preflight.go @@ -73,8 +73,18 @@ func (h *preflightHandler) execute(ctx context.Context) error { } checks := []preflight.Checker{ + host.CheckIsPrivilegedUser(), host.CheckAgentConfig(&cfg.AgentConfig), host.CheckClusterCredentials(&cfg.AgentConfig, cfg.Attest != nil), + host.CheckHostPackages(h.cmdCtx.Logger), + host.CheckHostPackageSources(h.cmdCtx.Logger), + host.CheckHostOSConfiguration(), + host.CheckNSpawnRuntime(), + host.CheckDockerActive(h.cmdCtx.Logger), + host.CheckSwapActive(), + host.CheckDiskSpace(), + host.CheckCgroups(), + host.CheckNodeIdentity(&cfg.AgentConfig), nodestart.CheckAPIServerReachable(cfg.Kubelet.ApiServer, caCertData), rootfs.CheckGoalState(h.cmdCtx.Logger, &cfg.AgentConfig, provision.ResolveDownloadOverrides(cfg.Downloads)), } diff --git a/cmd/agent/internal/cmd/preflight_test.go b/cmd/agent/internal/cmd/preflight_test.go index 02abc1e57..bb557939e 100644 --- a/cmd/agent/internal/cmd/preflight_test.go +++ b/cmd/agent/internal/cmd/preflight_test.go @@ -48,7 +48,7 @@ func TestPreflightJSONOutput(t *testing.T) { h := &preflightHandler{ cmdCtx: &CommandContext{LogFormat: "text"}, configPath: path, - ignorePreflightErrors: []string{"api-server-reachable"}, + ignorePreflightErrors: []string{"all"}, output: "json", writer: &out, } diff --git a/pkg/agent/phases/host/preflight_host.go b/pkg/agent/phases/host/preflight_host.go new file mode 100644 index 000000000..ff307a301 --- /dev/null +++ b/pkg/agent/phases/host/preflight_host.go @@ -0,0 +1,280 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package host + +import ( + "bufio" + "context" + "io/fs" + "log/slog" + "os" + "os/exec" + "path/filepath" + "strings" + "syscall" + + "github.com/Azure/unbounded/internal/executil" + "github.com/Azure/unbounded/pkg/agent/config" + "github.com/Azure/unbounded/pkg/agent/goalstates" + "github.com/Azure/unbounded/pkg/agent/preflight" +) + +const ( + CheckIsPrivilegedUserName = "is-privileged-user" + CheckHostPackagesName = "host-packages" + CheckHostPackageSourcesName = "host-package-sources" + CheckHostOSConfigurationName = "host-os-configuration" + CheckNSpawnRuntimeName = "nspawn-runtime" + CheckDockerActiveName = "docker-active" + CheckSwapActiveName = "swap-active" + CheckDiskSpaceName = "disk-space" + CheckCgroupsName = "cgroups" + CheckNodeIdentityName = "node-identity" + + minFreeDiskBytes = 8 * 1024 * 1024 * 1024 +) + +type hostCheckDeps struct { + lookupPath func(string) (string, error) + uid func() int + statfs func(string, *syscall.Statfs_t) error + readFile func(string) ([]byte, error) + stat func(string) (fs.FileInfo, error) + writeProbe func(string) error + outputCmd func(context.Context, *slog.Logger, string, ...string) (string, error) +} + +func defaultHostCheckDeps() hostCheckDeps { + return hostCheckDeps{ + lookupPath: exec.LookPath, + uid: os.Geteuid, + statfs: syscall.Statfs, + readFile: os.ReadFile, + stat: os.Stat, + writeProbe: probeWritableDir, + outputCmd: executil.OutputCmd, + } +} + +type simpleHostChecker struct { + name string + check func(context.Context) []preflight.Result +} + +func (c simpleHostChecker) Name() string { return c.name } + +func (c simpleHostChecker) Check(ctx context.Context) []preflight.Result { return c.check(ctx) } + +func CheckIsPrivilegedUser() preflight.Checker { + return checkIsPrivilegedUser(defaultHostCheckDeps()) +} + +func checkIsPrivilegedUser(deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: CheckIsPrivilegedUserName, check: func(context.Context) []preflight.Result { + if deps.uid() != 0 { + return preflight.ResultsError(CheckIsPrivilegedUserName, "host user", "preflight must run as root") + } + + return preflight.ResultsOK(CheckIsPrivilegedUserName, "host user", "preflight is running as root") + }} +} + +func CheckHostPackages(log *slog.Logger) preflight.Checker { + return checkHostPackages(log, defaultHostCheckDeps()) +} + +func checkHostPackages(log *slog.Logger, deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: CheckHostPackagesName, check: func(ctx context.Context) []preflight.Result { + pm, err := detectHostPackageManager(deps.lookupPath) + if err != nil { + return preflight.ResultsError(CheckHostPackagesName, "host packages", "supported host package manager is required") + } + + var missing []string + + for _, pkg := range pm.requiredPackages { + if !pm.installed(ctx, log, pkg) { + missing = append(missing, pkg) + } + } + + if len(missing) > 0 { + return preflight.ResultsError(CheckHostPackagesName, "host packages", "required host packages are missing") + } + + return preflight.ResultsOK(CheckHostPackagesName, "host packages", "required host packages are installed") + }} +} + +func CheckHostPackageSources(log *slog.Logger) preflight.Checker { + return checkHostPackageSources(log, defaultHostCheckDeps()) +} + +func checkHostPackageSources(log *slog.Logger, deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: CheckHostPackageSourcesName, check: func(ctx context.Context) []preflight.Result { + pm, err := detectHostPackageManager(deps.lookupPath) + if err != nil { + return preflight.ResultsError(CheckHostPackageSourcesName, "host package sources", "supported host package manager is required") + } + + for _, pkg := range pm.requiredPackages { + if !pm.installed(ctx, log, pkg) { + return preflight.ResultsWarning(CheckHostPackageSourcesName, "host package sources", "package sources may be required for missing host packages") + } + } + + return preflight.ResultsOK(CheckHostPackageSourcesName, "host package sources", "package source access is not required") + }} +} + +func CheckHostOSConfiguration() preflight.Checker { + return checkHostOSConfiguration(defaultHostCheckDeps()) +} + +func checkHostOSConfiguration(deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: CheckHostOSConfigurationName, check: func(context.Context) []preflight.Result { + if err := deps.writeProbe(filepath.Dir(hostSysctlPath)); err != nil { + return preflight.ResultsError(CheckHostOSConfigurationName, "host OS configuration", "host OS configuration paths are not writable") + } + + if err := deps.writeProbe(goalstates.SystemdSystemDir); err != nil { + return preflight.ResultsError(CheckHostOSConfigurationName, "host OS configuration", "systemd unit directory is not writable") + } + + return preflight.ResultsOK(CheckHostOSConfigurationName, "host OS configuration", "host OS configuration can be applied") + }} +} + +func CheckNSpawnRuntime() preflight.Checker { + return checkNSpawnRuntime(defaultHostCheckDeps()) +} + +func checkNSpawnRuntime(deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: CheckNSpawnRuntimeName, check: func(context.Context) []preflight.Result { + for _, binary := range []string{"systemctl", "machinectl", "systemd-nspawn"} { + if _, err := deps.lookupPath(binary); err != nil { + return preflight.ResultsError(CheckNSpawnRuntimeName, "nspawn runtime", "nspawn runtime tools are required") + } + } + + if _, err := deps.stat("/run/systemd/system"); err != nil { + return preflight.ResultsError(CheckNSpawnRuntimeName, "nspawn runtime", "systemd runtime is required") + } + + return preflight.ResultsOK(CheckNSpawnRuntimeName, "nspawn runtime", "nspawn runtime is available") + }} +} + +func CheckDockerActive(log *slog.Logger) preflight.Checker { + return checkDockerActive(log, defaultHostCheckDeps()) +} + +func checkDockerActive(log *slog.Logger, deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: CheckDockerActiveName, check: func(ctx context.Context) []preflight.Result { + out, err := deps.outputCmd(ctx, log, "systemctl", "is-active", dockerServiceUnit) + if err == nil && strings.TrimSpace(out) == "active" { + return preflight.ResultsWarning(CheckDockerActiveName, "docker service", "Docker is active and bootstrap will disable it") + } + + return preflight.ResultsOK(CheckDockerActiveName, "docker service", "Docker is not active") + }} +} + +func CheckSwapActive() preflight.Checker { + return checkSwapActive(defaultHostCheckDeps()) +} + +func checkSwapActive(deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: CheckSwapActiveName, check: func(context.Context) []preflight.Result { + active, err := swapActive(deps.readFile) + if err != nil { + return preflight.ResultsWarning(CheckSwapActiveName, "host swap", "swap state could not be determined") + } + + if active { + return preflight.ResultsWarning(CheckSwapActiveName, "host swap", "swap is enabled and bootstrap will disable it") + } + + return preflight.ResultsOK(CheckSwapActiveName, "host swap", "swap is not active") + }} +} + +func CheckDiskSpace() preflight.Checker { + return checkDiskSpace(defaultHostCheckDeps()) +} + +func checkDiskSpace(deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: CheckDiskSpaceName, check: func(context.Context) []preflight.Result { + var stat syscall.Statfs_t + if err := deps.statfs("/var/lib", &stat); err != nil { + return preflight.ResultsError(CheckDiskSpaceName, "host disk", "available disk space could not be determined") + } + + free := stat.Bavail * uint64(stat.Bsize) + if free < minFreeDiskBytes { + return preflight.ResultsError(CheckDiskSpaceName, "host disk", "available disk space is below the minimum") + } + + return preflight.ResultsOK(CheckDiskSpaceName, "host disk", "sufficient disk space is available") + }} +} + +func CheckCgroups() preflight.Checker { + return checkCgroups(defaultHostCheckDeps()) +} + +func checkCgroups(deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: CheckCgroupsName, check: func(context.Context) []preflight.Result { + if _, err := deps.stat("/sys/fs/cgroup"); err != nil { + return preflight.ResultsError(CheckCgroupsName, "host cgroups", "cgroup filesystem is required") + } + + return preflight.ResultsOK(CheckCgroupsName, "host cgroups", "cgroup filesystem is available") + }} +} + +func CheckNodeIdentity(cfg *config.AgentConfig) preflight.Checker { + return simpleHostChecker{name: CheckNodeIdentityName, check: func(context.Context) []preflight.Result { + if cfg == nil || strings.TrimSpace(cfg.NodeName) == "" { + return preflight.ResultsError(CheckNodeIdentityName, "node identity", "node name could not be resolved") + } + + return preflight.ResultsOK(CheckNodeIdentityName, "node identity", "node name is resolved") + }} +} + +func probeWritableDir(dir string) error { + f, err := os.CreateTemp(dir, ".unbounded-preflight-*") + if err != nil { + return err + } + + name := f.Name() + if err := f.Close(); err != nil { + os.Remove(name) //nolint:errcheck // best effort cleanup after close failure. + return err + } + + return os.Remove(name) +} + +func swapActive(readFile func(string) ([]byte, error)) (bool, error) { + data, err := readFile("/proc/swaps") + if err != nil { + return false, err + } + + scanner := bufio.NewScanner(strings.NewReader(string(data))) + for scanner.Scan() { + if strings.HasPrefix(scanner.Text(), "Filename") { + continue + } + + if strings.TrimSpace(scanner.Text()) != "" { + return true, nil + } + } + + return false, scanner.Err() +} diff --git a/pkg/agent/phases/host/preflight_host_test.go b/pkg/agent/phases/host/preflight_host_test.go new file mode 100644 index 000000000..bedd6bbb3 --- /dev/null +++ b/pkg/agent/phases/host/preflight_host_test.go @@ -0,0 +1,158 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package host + +import ( + "context" + "errors" + "io/fs" + "log/slog" + "os/exec" + "syscall" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/Azure/unbounded/pkg/agent/config" + "github.com/Azure/unbounded/pkg/agent/preflight" +) + +func TestCheckIsPrivilegedUser(t *testing.T) { + results := checkIsPrivilegedUser(hostCheckDeps{uid: func() int { return 0 }}).Check(context.Background()) + assert.Equal(t, preflight.SeverityOK, results[0].Severity) + + results = checkIsPrivilegedUser(hostCheckDeps{uid: func() int { return 1000 }}).Check(context.Background()) + assert.Equal(t, preflight.SeverityError, results[0].Severity) +} + +func TestCheckHostPackagesMissingPackageManager(t *testing.T) { + deps := defaultHostCheckDeps() + deps.lookupPath = lookupPathWith(nil) + + results := checkHostPackages(slog.New(slog.DiscardHandler), deps).Check(context.Background()) + + assert.Equal(t, preflight.SeverityError, results[0].Severity) +} + +func TestCheckHostOSConfiguration(t *testing.T) { + deps := defaultHostCheckDeps() + deps.writeProbe = func(string) error { return nil } + + results := checkHostOSConfiguration(deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityOK, results[0].Severity) + + deps.writeProbe = func(string) error { return errors.New("denied") } + results = checkHostOSConfiguration(deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityError, results[0].Severity) +} + +func TestCheckNSpawnRuntime(t *testing.T) { + deps := defaultHostCheckDeps() + deps.lookupPath = lookupPathWith(map[string]bool{ + "systemctl": true, + "machinectl": true, + "systemd-nspawn": true, + }) + deps.stat = func(string) (fs.FileInfo, error) { return nil, nil } + + results := checkNSpawnRuntime(deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityOK, results[0].Severity) + + deps.lookupPath = lookupPathWith(map[string]bool{"systemctl": true}) + results = checkNSpawnRuntime(deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityError, results[0].Severity) +} + +func TestCheckDockerActive(t *testing.T) { + deps := defaultHostCheckDeps() + deps.outputCmd = outputWith("active\n", nil) + + results := checkDockerActive(slog.New(slog.DiscardHandler), deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityWarning, results[0].Severity) + + deps.outputCmd = outputWith("inactive\n", nil) + results = checkDockerActive(slog.New(slog.DiscardHandler), deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityOK, results[0].Severity) +} + +func TestCheckSwapActive(t *testing.T) { + deps := defaultHostCheckDeps() + deps.readFile = readFileString("Filename\tType\tSize\tUsed\tPriority\n", nil) + + results := checkSwapActive(deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityOK, results[0].Severity) + + deps.readFile = readFileString("Filename\tType\tSize\tUsed\tPriority\n/swapfile file 1024 0 -2\n", nil) + results = checkSwapActive(deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityWarning, results[0].Severity) +} + +func TestCheckDiskSpace(t *testing.T) { + deps := defaultHostCheckDeps() + deps.statfs = statfsWithFreeBytes(minFreeDiskBytes) + + results := checkDiskSpace(deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityOK, results[0].Severity) + + deps.statfs = statfsWithFreeBytes(1) + results = checkDiskSpace(deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityError, results[0].Severity) +} + +func TestCheckCgroups(t *testing.T) { + deps := defaultHostCheckDeps() + deps.stat = statExists() + + results := checkCgroups(deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityOK, results[0].Severity) + + deps.stat = statMissing() + results = checkCgroups(deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityError, results[0].Severity) +} + +func TestCheckNodeIdentity(t *testing.T) { + results := CheckNodeIdentity(&config.AgentConfig{NodeName: "node-1"}).Check(context.Background()) + assert.Equal(t, preflight.SeverityOK, results[0].Severity) + + results = CheckNodeIdentity(&config.AgentConfig{}).Check(context.Background()) + assert.Equal(t, preflight.SeverityError, results[0].Severity) +} + +func statfsWithFreeBytes(bytes uint64) func(string, *syscall.Statfs_t) error { + return func(_ string, stat *syscall.Statfs_t) error { + stat.Bsize = 1 + stat.Bavail = bytes + + return nil + } +} + +func statExists() func(string) (fs.FileInfo, error) { + return func(string) (fs.FileInfo, error) { return nil, nil } +} + +func statMissing() func(string) (fs.FileInfo, error) { + return func(string) (fs.FileInfo, error) { return nil, errors.New("missing") } +} + +func lookupPathWith(paths map[string]bool) func(string) (string, error) { + return func(name string) (string, error) { + if paths[name] { + return "/usr/bin/" + name, nil + } + + return "", exec.ErrNotFound + } +} + +func outputWith(value string, err error) func(context.Context, *slog.Logger, string, ...string) (string, error) { + return func(context.Context, *slog.Logger, string, ...string) (string, error) { + return value, err + } +} + +func readFileString(value string, err error) func(string) ([]byte, error) { + return func(string) ([]byte, error) { return []byte(value), err } +} From 03f0e4fd6b489f80b3ab2224b00941dad669a6f8 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Wed, 24 Jun 2026 02:07:14 +0000 Subject: [PATCH 07/16] agent: refine preflight host checks --- cmd/agent/internal/cmd/preflight.go | 52 +++-- cmd/agent/internal/cmd/preflight_test.go | 22 ++ designs/agent-preflight.md | 23 +- .../phases/host/preflight_agent_config.go | 27 ++- .../host/preflight_agent_config_test.go | 9 +- .../host/preflight_cluster_credentials.go | 23 +- .../preflight_cluster_credentials_test.go | 13 +- pkg/agent/phases/host/preflight_host.go | 202 ++++++++++-------- pkg/agent/phases/host/preflight_host_test.go | 56 +++-- .../phases/nodestart/preflight_api_server.go | 31 ++- .../nodestart/preflight_api_server_test.go | 13 +- .../phases/rootfs/preflight_goal_state.go | 14 +- .../rootfs/preflight_goal_state_test.go | 4 +- pkg/agent/preflight/preflight.go | 18 +- pkg/agent/preflight/preflight_test.go | 51 +++++ 15 files changed, 354 insertions(+), 204 deletions(-) diff --git a/cmd/agent/internal/cmd/preflight.go b/cmd/agent/internal/cmd/preflight.go index 925e76db9..101ed0f5f 100644 --- a/cmd/agent/internal/cmd/preflight.go +++ b/cmd/agent/internal/cmd/preflight.go @@ -73,19 +73,17 @@ func (h *preflightHandler) execute(ctx context.Context) error { } checks := []preflight.Checker{ - host.CheckIsPrivilegedUser(), - host.CheckAgentConfig(&cfg.AgentConfig), - host.CheckClusterCredentials(&cfg.AgentConfig, cfg.Attest != nil), + host.CheckIsPrivilegedUser(h.cmdCtx.Logger), + host.CheckAgentConfig(h.cmdCtx.Logger, &cfg.AgentConfig), + host.CheckClusterCredentials(h.cmdCtx.Logger, &cfg.AgentConfig, cfg.Attest != nil), host.CheckHostPackages(h.cmdCtx.Logger), - host.CheckHostPackageSources(h.cmdCtx.Logger), - host.CheckHostOSConfiguration(), - host.CheckNSpawnRuntime(), + host.CheckHostOSConfiguration(h.cmdCtx.Logger), + host.CheckNSpawnRuntime(h.cmdCtx.Logger), host.CheckDockerActive(h.cmdCtx.Logger), - host.CheckSwapActive(), - host.CheckDiskSpace(), - host.CheckCgroups(), - host.CheckNodeIdentity(&cfg.AgentConfig), - nodestart.CheckAPIServerReachable(cfg.Kubelet.ApiServer, caCertData), + host.CheckSwapActive(h.cmdCtx.Logger), + host.CheckDiskSpace(h.cmdCtx.Logger), + host.CheckCgroups(h.cmdCtx.Logger), + nodestart.CheckAPIServerReachable(h.cmdCtx.Logger, cfg.Kubelet.ApiServer, caCertData), rootfs.CheckGoalState(h.cmdCtx.Logger, &cfg.AgentConfig, provision.ResolveDownloadOverrides(cfg.Downloads)), } @@ -123,20 +121,14 @@ func writePreflightText(w io.Writer, report preflight.Report) error { for _, result := range report.Checks { switch result.Severity { + case preflight.SeverityOK: + if err := writePreflightResult(w, "OK", result); err != nil { + return err + } case preflight.SeverityError: errors = append(errors, result) case preflight.SeverityWarning: - if _, err := fmt.Fprintf(w, "\t[WARNING %s]: %s", result.Name, result.Message); err != nil { - return err - } - - if result.Target != "" { - if _, err := fmt.Fprintf(w, " (target: %s)", result.Target); err != nil { - return err - } - } - - if _, err := fmt.Fprintln(w); err != nil { + if err := writePreflightResult(w, "WARNING", result); err != nil { return err } } @@ -170,3 +162,19 @@ func writePreflightText(w io.Writer, report preflight.Report) error { return err } + +func writePreflightResult(w io.Writer, status string, result preflight.Result) error { + if _, err := fmt.Fprintf(w, "\t[%s %s]: %s", status, result.Name, result.Message); err != nil { + return err + } + + if result.Target != "" { + if _, err := fmt.Fprintf(w, " (target: %s)", result.Target); err != nil { + return err + } + } + + _, err := fmt.Fprintln(w) + + return err +} diff --git a/cmd/agent/internal/cmd/preflight_test.go b/cmd/agent/internal/cmd/preflight_test.go index bb557939e..1451d3b08 100644 --- a/cmd/agent/internal/cmd/preflight_test.go +++ b/cmd/agent/internal/cmd/preflight_test.go @@ -84,3 +84,25 @@ func TestPreflightTextOutputError(t *testing.T) { assert.Contains(t, out.String(), "[ERROR api-server-reachable]") assert.NotContains(t, out.String(), "127.0.0.1") } + +func TestPreflightTextOutputIncludesOK(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(srv.Close) + + path := writeConfigFile(t, preflightConfig(srv.URL)) + + var out bytes.Buffer + + h := &preflightHandler{ + cmdCtx: &CommandContext{LogFormat: "text"}, + configPath: path, + ignorePreflightErrors: []string{"all"}, + output: "text", + writer: &out, + } + + require.NoError(t, h.execute(context.Background())) + assert.Contains(t, out.String(), "[OK agent-config]") +} diff --git a/designs/agent-preflight.md b/designs/agent-preflight.md index 94a423629..2bd02680d 100644 --- a/designs/agent-preflight.md +++ b/designs/agent-preflight.md @@ -172,9 +172,12 @@ Severity should follow a simple policy: - Return a warning when bootstrap can safely remediate the condition without external input. For example, active swap can be a warning when bootstrap will disable it. +- Return a warning when bootstrap can remediate the condition by installing or + reconfiguring host state. For example, missing required host packages can be a + warning when package source access is allowed. - Return an error when bootstrap cannot proceed or remediation requires external - input. For example, missing host packages with unreachable package sources is - an error. + input. For example, missing required host packages should become an error in + offline mode. - Return an error when continuing would risk joining the node with incorrect identity, credentials, rootfs, runtime, or GPU behavior. @@ -260,7 +263,7 @@ check points at the bootstrap step that would fail later. For example: ```text -[ERROR host-packages]: missing required packages and package sources are unreachable +[WARNING host-packages]: required host packages are missing and may be installed by bootstrap: systemd-container, curl [WARNING swap-active]: swap is active and bootstrap will disable it [ERROR oci-image-reachable]: failed to resolve rootfs image manifest (target: rootfs image) ``` @@ -277,7 +280,6 @@ Examples: ```text agent-config host-packages -host-package-sources swap-active oci-image-reachable kubernetes-artifacts @@ -303,10 +305,9 @@ Default output should be kubeadm-like: ```text [preflight] Running unbounded-agent pre-flight checks [WARNING swap-active]: swap is enabled and will be disabled during bootstrap - [ERROR host-packages]: missing required packages and package sources are unreachable + [WARNING host-packages]: required host packages are missing and may be installed by bootstrap: systemd-container, curl [ERROR oci-image-reachable]: failed to resolve rootfs image manifest (target: rootfs image) [preflight] Some fatal errors occurred: - [ERROR host-packages]: missing required packages and package sources are unreachable [ERROR oci-image-reachable]: failed to resolve rootfs image manifest (target: rootfs image) [preflight] If you know what you are doing, you can make a check non-fatal with `--ignore-preflight-errors=...` ``` @@ -343,8 +344,8 @@ warnings, ignored errors, and fatal errors: }, { "name": "host-packages", - "severity": "error", - "message": "missing required packages and package sources are unreachable", + "severity": "warning", + "message": "required host packages are missing and may be installed by bootstrap: systemd-container, curl", "target": "host packages", "ignored": false }, @@ -394,17 +395,15 @@ Host phase checks: | Check | Purpose | |---|---| | `is-privileged-user` | Ensure the command is running as root. | -| `host-packages` | Validate a supported package manager exists and required host packages are installed. If packages are missing, validate whether they appear installable without mutating package-manager state. In offline or blocked-network environments this should fail when required packages are missing and cannot be installed. | -| `host-package-sources` | Validate package source reachability with non-mutating probes when package installation would be required. Skip or pass when all required packages are already installed. | +| `host-packages` | Validate a supported package manager exists and required host packages are installed. Missing packages are reported by name as warnings when bootstrap can install them. They should become errors in offline mode. | | `host-os-configuration` | Validate host OS configuration can be applied: sysctl config path writable, relevant kernel parameters acceptable or settable, and systemd unit paths writable. | -| `nspawn-runtime` | Validate the host systemd environment can manage nspawn machines using installed host capabilities. | +| `nspawn-runtime` | Validate the host systemd environment can manage nspawn machines using installed host capabilities. Missing tools are warnings when bootstrap can install them. They should become errors in offline mode. | | `docker-active` | Warn if Docker is active and bootstrap will disable or avoid it. | | `swap-active` | Warn when swap is enabled if bootstrap will disable it. | | `disk-space` | Validate enough space exists for rootfs and component downloads. | | `cgroups` | Validate cgroup support expected by kubelet/containerd. | | `api-server-reachable` | Validate the configured Kubernetes API server is reachable from the host. | | `cluster-credentials` | Validate the cluster CA data and configured bootstrap credential are present and parseable for kubelet registration. | -| `node-identity` | Validate node name resolution using the same order as agent config normalization: explicit `NodeName`, host hostname, then `MachineName`. The resolved value must be compatible with kubelet registration. | Rootfs provisioning checks: diff --git a/pkg/agent/phases/host/preflight_agent_config.go b/pkg/agent/phases/host/preflight_agent_config.go index cf9af95c0..948487ca2 100644 --- a/pkg/agent/phases/host/preflight_agent_config.go +++ b/pkg/agent/phases/host/preflight_agent_config.go @@ -5,33 +5,40 @@ package host import ( "context" + "log/slog" "github.com/Azure/unbounded/pkg/agent/config" "github.com/Azure/unbounded/pkg/agent/preflight" ) -// CheckAgentConfigName is the stable name for the agent config validation check. -const CheckAgentConfigName = "agent-config" +const checkAgentConfigName = "agent-config" type agentConfigChecker struct { + log *slog.Logger config *config.AgentConfig } -// CheckAgentConfig returns a checker that validates the shared agent config -// shape. Product-specific credential requirements are validated by separate -// checks. -func CheckAgentConfig(cfg *config.AgentConfig) preflight.Checker { - return agentConfigChecker{config: cfg} +// CheckAgentConfig verifies the shared agent config has been normalized and is +// internally consistent. Product-specific credential requirements are validated +// by separate checks. +func CheckAgentConfig(log *slog.Logger, cfg *config.AgentConfig) preflight.Checker { + return agentConfigChecker{log: log, config: cfg} } // Name returns the stable check name used in reports and ignore rules. -func (c agentConfigChecker) Name() string { return CheckAgentConfigName } +func (c agentConfigChecker) Name() string { return checkAgentConfigName } // Check validates the shared agent config without mutating it. func (c agentConfigChecker) Check(context.Context) []preflight.Result { + c.log.Debug("checking agent config") + if err := c.config.Validate(); err != nil { - return preflight.ResultsError(CheckAgentConfigName, "agent config", "agent config is invalid") + c.log.Debug("agent config validation failed") + + return preflight.ResultsError(checkAgentConfigName, "agent config", "agent config is invalid") } - return preflight.ResultsOK(CheckAgentConfigName, "agent config", "agent config is valid") + c.log.Debug("agent config validation passed") + + return preflight.ResultsOK(checkAgentConfigName, "agent config", "agent config is valid") } diff --git a/pkg/agent/phases/host/preflight_agent_config_test.go b/pkg/agent/phases/host/preflight_agent_config_test.go index 6475a61d1..91cff5973 100644 --- a/pkg/agent/phases/host/preflight_agent_config_test.go +++ b/pkg/agent/phases/host/preflight_agent_config_test.go @@ -5,6 +5,7 @@ package host import ( "context" + "log/slog" "testing" "github.com/stretchr/testify/assert" @@ -32,19 +33,19 @@ func validPreflightConfig() *config.AgentConfig { } func TestCheckAgentConfigValid(t *testing.T) { - results := CheckAgentConfig(validPreflightConfig()).Check(context.Background()) + results := CheckAgentConfig(slog.New(slog.DiscardHandler), validPreflightConfig()).Check(context.Background()) - assert.Equal(t, preflight.ResultsOK(CheckAgentConfigName, "agent config", "agent config is valid"), results) + assert.Equal(t, preflight.ResultsOK(checkAgentConfigName, "agent config", "agent config is valid"), results) } func TestCheckAgentConfigInvalid(t *testing.T) { cfg := validPreflightConfig() cfg.MachineName = "" - results := CheckAgentConfig(cfg).Check(context.Background()) + results := CheckAgentConfig(slog.New(slog.DiscardHandler), cfg).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) - assert.Equal(t, CheckAgentConfigName, results[0].Name) + assert.Equal(t, checkAgentConfigName, results[0].Name) assert.Equal(t, "agent config", results[0].Target) assert.Equal(t, "agent config is invalid", results[0].Message) } diff --git a/pkg/agent/phases/host/preflight_cluster_credentials.go b/pkg/agent/phases/host/preflight_cluster_credentials.go index 663131b4e..e995bb544 100644 --- a/pkg/agent/phases/host/preflight_cluster_credentials.go +++ b/pkg/agent/phases/host/preflight_cluster_credentials.go @@ -6,16 +6,17 @@ package host import ( "context" "encoding/base64" + "log/slog" "strings" "github.com/Azure/unbounded/pkg/agent/config" "github.com/Azure/unbounded/pkg/agent/preflight" ) -// CheckClusterCredentialsName is the stable name for cluster credential validation. -const CheckClusterCredentialsName = "cluster-credentials" +const checkClusterCredentialsName = "cluster-credentials" type clusterCredentialsChecker struct { + log *slog.Logger config *config.AgentConfig attestationConfigured bool } @@ -23,17 +24,19 @@ type clusterCredentialsChecker struct { // CheckClusterCredentials returns a checker that validates cluster CA data and // the bootstrap credential. When attestationConfigured is true, missing kubelet // auth is allowed because attestation can provide the credential later. -func CheckClusterCredentials(cfg *config.AgentConfig, attestationConfigured bool) preflight.Checker { - return clusterCredentialsChecker{config: cfg, attestationConfigured: attestationConfigured} +func CheckClusterCredentials(log *slog.Logger, cfg *config.AgentConfig, attestationConfigured bool) preflight.Checker { + return clusterCredentialsChecker{log: log, config: cfg, attestationConfigured: attestationConfigured} } // Name returns the stable check name used in reports and ignore rules. -func (c clusterCredentialsChecker) Name() string { return CheckClusterCredentialsName } +func (c clusterCredentialsChecker) Name() string { return checkClusterCredentialsName } // Check validates cluster credential inputs without printing credential values. func (c clusterCredentialsChecker) Check(context.Context) []preflight.Result { + c.log.Debug("checking cluster credentials", "attestationConfigured", c.attestationConfigured) + if c.config == nil { - return preflight.ResultsError(CheckClusterCredentialsName, "cluster credentials", "agent config is missing") + return preflight.ResultsError(checkClusterCredentialsName, "cluster credentials", "agent config is missing") } var errs []string @@ -49,8 +52,12 @@ func (c clusterCredentialsChecker) Check(context.Context) []preflight.Result { } if len(errs) > 0 { - return preflight.ResultsError(CheckClusterCredentialsName, "cluster credentials", strings.Join(errs, "; ")) + c.log.Debug("cluster credential validation failed", "errors", len(errs)) + + return preflight.ResultsError(checkClusterCredentialsName, "cluster credentials", strings.Join(errs, "; ")) } - return preflight.ResultsOK(CheckClusterCredentialsName, "cluster credentials", "cluster credentials are valid") + c.log.Debug("cluster credential validation passed") + + return preflight.ResultsOK(checkClusterCredentialsName, "cluster credentials", "cluster credentials are valid") } diff --git a/pkg/agent/phases/host/preflight_cluster_credentials_test.go b/pkg/agent/phases/host/preflight_cluster_credentials_test.go index d1e4cb9ab..0a9f214f1 100644 --- a/pkg/agent/phases/host/preflight_cluster_credentials_test.go +++ b/pkg/agent/phases/host/preflight_cluster_credentials_test.go @@ -5,6 +5,7 @@ package host import ( "context" + "log/slog" "testing" "github.com/stretchr/testify/assert" @@ -13,16 +14,16 @@ import ( ) func TestCheckClusterCredentialsValid(t *testing.T) { - results := CheckClusterCredentials(validPreflightConfig(), false).Check(context.Background()) + results := CheckClusterCredentials(slog.New(slog.DiscardHandler), validPreflightConfig(), false).Check(context.Background()) - assert.Equal(t, preflight.ResultsOK(CheckClusterCredentialsName, "cluster credentials", "cluster credentials are valid"), results) + assert.Equal(t, preflight.ResultsOK(checkClusterCredentialsName, "cluster credentials", "cluster credentials are valid"), results) } func TestCheckClusterCredentialsAllowsAttestation(t *testing.T) { cfg := validPreflightConfig() cfg.Kubelet.Auth.BootstrapToken = "" - results := CheckClusterCredentials(cfg, true).Check(context.Background()) + results := CheckClusterCredentials(slog.New(slog.DiscardHandler), cfg, true).Check(context.Background()) assert.Equal(t, preflight.SeverityOK, results[0].Severity) } @@ -31,10 +32,10 @@ func TestCheckClusterCredentialsRequiresAuthWhenNoAttestation(t *testing.T) { cfg := validPreflightConfig() cfg.Kubelet.Auth.BootstrapToken = "" - results := CheckClusterCredentials(cfg, false).Check(context.Background()) + results := CheckClusterCredentials(slog.New(slog.DiscardHandler), cfg, false).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) - assert.Equal(t, CheckClusterCredentialsName, results[0].Name) + assert.Equal(t, checkClusterCredentialsName, results[0].Name) assert.Equal(t, "cluster credentials", results[0].Target) assert.Equal(t, "bootstrap credential is invalid", results[0].Message) } @@ -43,7 +44,7 @@ func TestCheckClusterCredentialsInvalidCA(t *testing.T) { cfg := validPreflightConfig() cfg.Cluster.CaCertBase64 = "not-base64" - results := CheckClusterCredentials(cfg, false).Check(context.Background()) + results := CheckClusterCredentials(slog.New(slog.DiscardHandler), cfg, false).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Contains(t, results[0].Message, "cluster CA data is invalid") diff --git a/pkg/agent/phases/host/preflight_host.go b/pkg/agent/phases/host/preflight_host.go index ff307a301..b75bb5777 100644 --- a/pkg/agent/phases/host/preflight_host.go +++ b/pkg/agent/phases/host/preflight_host.go @@ -6,6 +6,7 @@ package host import ( "bufio" "context" + "fmt" "io/fs" "log/slog" "os" @@ -15,22 +16,19 @@ import ( "syscall" "github.com/Azure/unbounded/internal/executil" - "github.com/Azure/unbounded/pkg/agent/config" "github.com/Azure/unbounded/pkg/agent/goalstates" "github.com/Azure/unbounded/pkg/agent/preflight" ) const ( - CheckIsPrivilegedUserName = "is-privileged-user" - CheckHostPackagesName = "host-packages" - CheckHostPackageSourcesName = "host-package-sources" - CheckHostOSConfigurationName = "host-os-configuration" - CheckNSpawnRuntimeName = "nspawn-runtime" - CheckDockerActiveName = "docker-active" - CheckSwapActiveName = "swap-active" - CheckDiskSpaceName = "disk-space" - CheckCgroupsName = "cgroups" - CheckNodeIdentityName = "node-identity" + checkIsPrivilegedUserName = "is-privileged-user" + checkHostPackagesName = "host-packages" + checkHostOSConfigurationName = "host-os-configuration" + checkNSpawnRuntimeName = "nspawn-runtime" + checkDockerActiveName = "docker-active" + checkSwapActiveName = "swap-active" + checkDiskSpaceName = "disk-space" + checkCgroupsName = "cgroups" minFreeDiskBytes = 8 * 1024 * 1024 * 1024 ) @@ -66,182 +64,206 @@ func (c simpleHostChecker) Name() string { return c.name } func (c simpleHostChecker) Check(ctx context.Context) []preflight.Result { return c.check(ctx) } -func CheckIsPrivilegedUser() preflight.Checker { - return checkIsPrivilegedUser(defaultHostCheckDeps()) +// CheckIsPrivilegedUser verifies preflight is running as root. +func CheckIsPrivilegedUser(log *slog.Logger) preflight.Checker { + return checkIsPrivilegedUser(log, defaultHostCheckDeps()) } -func checkIsPrivilegedUser(deps hostCheckDeps) preflight.Checker { - return simpleHostChecker{name: CheckIsPrivilegedUserName, check: func(context.Context) []preflight.Result { - if deps.uid() != 0 { - return preflight.ResultsError(CheckIsPrivilegedUserName, "host user", "preflight must run as root") +func checkIsPrivilegedUser(log *slog.Logger, deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: checkIsPrivilegedUserName, check: func(context.Context) []preflight.Result { + uid := deps.uid() + log.Debug("checking effective user", "uid", uid) + + if uid != 0 { + return preflight.ResultsError(checkIsPrivilegedUserName, "host user", "preflight must run as root") } - return preflight.ResultsOK(CheckIsPrivilegedUserName, "host user", "preflight is running as root") + return preflight.ResultsOK(checkIsPrivilegedUserName, "host user", "preflight is running as root") }} } +// CheckHostPackages verifies all required host packages are already installed. func CheckHostPackages(log *slog.Logger) preflight.Checker { return checkHostPackages(log, defaultHostCheckDeps()) } func checkHostPackages(log *slog.Logger, deps hostCheckDeps) preflight.Checker { - return simpleHostChecker{name: CheckHostPackagesName, check: func(ctx context.Context) []preflight.Result { + return simpleHostChecker{name: checkHostPackagesName, check: func(ctx context.Context) []preflight.Result { pm, err := detectHostPackageManager(deps.lookupPath) if err != nil { - return preflight.ResultsError(CheckHostPackagesName, "host packages", "supported host package manager is required") + log.Debug("host package manager detection failed") + + return preflight.ResultsError(checkHostPackagesName, "host packages", "supported host package manager is required: apt-get, tdnf, or dnf") } + log.Debug("detected host package manager", "packageManager", pm.name, "requiredPackages", strings.Join(pm.requiredPackages, ",")) + var missing []string for _, pkg := range pm.requiredPackages { if !pm.installed(ctx, log, pkg) { + // TODO: when offline mode is configured, missing required host + // packages should be reported as an error because bootstrap cannot + // rely on package source access to remediate them. missing = append(missing, pkg) } } if len(missing) > 0 { - return preflight.ResultsError(CheckHostPackagesName, "host packages", "required host packages are missing") - } - - return preflight.ResultsOK(CheckHostPackagesName, "host packages", "required host packages are installed") - }} -} + log.Debug("required host packages are missing", "packages", strings.Join(missing, ",")) -func CheckHostPackageSources(log *slog.Logger) preflight.Checker { - return checkHostPackageSources(log, defaultHostCheckDeps()) -} - -func checkHostPackageSources(log *slog.Logger, deps hostCheckDeps) preflight.Checker { - return simpleHostChecker{name: CheckHostPackageSourcesName, check: func(ctx context.Context) []preflight.Result { - pm, err := detectHostPackageManager(deps.lookupPath) - if err != nil { - return preflight.ResultsError(CheckHostPackageSourcesName, "host package sources", "supported host package manager is required") + // TODO: when offline mode is configured, missing required host + // packages should be reported as an error because bootstrap cannot + // rely on package source access to remediate them. + return preflight.ResultsWarning(checkHostPackagesName, "host packages", "required host packages are missing and may be installed by bootstrap: "+strings.Join(missing, ", ")) } - for _, pkg := range pm.requiredPackages { - if !pm.installed(ctx, log, pkg) { - return preflight.ResultsWarning(CheckHostPackageSourcesName, "host package sources", "package sources may be required for missing host packages") - } - } + log.Debug("required host packages are installed") - return preflight.ResultsOK(CheckHostPackageSourcesName, "host package sources", "package source access is not required") + return preflight.ResultsOK(checkHostPackagesName, "host packages", "required host packages are installed") }} } -func CheckHostOSConfiguration() preflight.Checker { - return checkHostOSConfiguration(defaultHostCheckDeps()) +// CheckHostOSConfiguration verifies host OS configuration paths are writable. +func CheckHostOSConfiguration(log *slog.Logger) preflight.Checker { + return checkHostOSConfiguration(log, defaultHostCheckDeps()) } -func checkHostOSConfiguration(deps hostCheckDeps) preflight.Checker { - return simpleHostChecker{name: CheckHostOSConfigurationName, check: func(context.Context) []preflight.Result { - if err := deps.writeProbe(filepath.Dir(hostSysctlPath)); err != nil { - return preflight.ResultsError(CheckHostOSConfigurationName, "host OS configuration", "host OS configuration paths are not writable") +func checkHostOSConfiguration(log *slog.Logger, deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: checkHostOSConfigurationName, check: func(context.Context) []preflight.Result { + sysctlDir := filepath.Dir(hostSysctlPath) + log.Debug("checking host OS configuration path", "path", sysctlDir) + + if err := deps.writeProbe(sysctlDir); err != nil { + return preflight.ResultsError(checkHostOSConfigurationName, "host OS configuration", "host OS configuration path is not writable: "+sysctlDir) } + log.Debug("checking systemd unit directory", "path", goalstates.SystemdSystemDir) + if err := deps.writeProbe(goalstates.SystemdSystemDir); err != nil { - return preflight.ResultsError(CheckHostOSConfigurationName, "host OS configuration", "systemd unit directory is not writable") + return preflight.ResultsError(checkHostOSConfigurationName, "host OS configuration", "systemd unit directory is not writable: "+goalstates.SystemdSystemDir) } - return preflight.ResultsOK(CheckHostOSConfigurationName, "host OS configuration", "host OS configuration can be applied") + return preflight.ResultsOK(checkHostOSConfigurationName, "host OS configuration", "host OS configuration can be applied") }} } -func CheckNSpawnRuntime() preflight.Checker { - return checkNSpawnRuntime(defaultHostCheckDeps()) +// CheckNSpawnRuntime verifies systemd-nspawn runtime tools are available. +func CheckNSpawnRuntime(log *slog.Logger) preflight.Checker { + return checkNSpawnRuntime(log, defaultHostCheckDeps()) } -func checkNSpawnRuntime(deps hostCheckDeps) preflight.Checker { - return simpleHostChecker{name: CheckNSpawnRuntimeName, check: func(context.Context) []preflight.Result { +func checkNSpawnRuntime(log *slog.Logger, deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: checkNSpawnRuntimeName, check: func(context.Context) []preflight.Result { for _, binary := range []string{"systemctl", "machinectl", "systemd-nspawn"} { + log.Debug("checking nspawn runtime tool", "binary", binary) + if _, err := deps.lookupPath(binary); err != nil { - return preflight.ResultsError(CheckNSpawnRuntimeName, "nspawn runtime", "nspawn runtime tools are required") + // TODO: when offline mode is configured, missing nspawn runtime + // tools should be reported as an error because bootstrap cannot rely + // on package installation to remediate them. + return preflight.ResultsWarning(checkNSpawnRuntimeName, "nspawn runtime", "nspawn runtime tool is missing and may be installed by bootstrap: "+binary) } } - if _, err := deps.stat("/run/systemd/system"); err != nil { - return preflight.ResultsError(CheckNSpawnRuntimeName, "nspawn runtime", "systemd runtime is required") + systemdRuntimePath := "/run/systemd/system" + log.Debug("checking systemd runtime path", "path", systemdRuntimePath) + + if _, err := deps.stat(systemdRuntimePath); err != nil { + return preflight.ResultsWarning(checkNSpawnRuntimeName, "nspawn runtime", "systemd runtime path is not currently available: "+systemdRuntimePath) } - return preflight.ResultsOK(CheckNSpawnRuntimeName, "nspawn runtime", "nspawn runtime is available") + return preflight.ResultsOK(checkNSpawnRuntimeName, "nspawn runtime", "nspawn runtime is available") }} } +// CheckDockerActive warns when Docker is active. func CheckDockerActive(log *slog.Logger) preflight.Checker { return checkDockerActive(log, defaultHostCheckDeps()) } func checkDockerActive(log *slog.Logger, deps hostCheckDeps) preflight.Checker { - return simpleHostChecker{name: CheckDockerActiveName, check: func(ctx context.Context) []preflight.Result { + return simpleHostChecker{name: checkDockerActiveName, check: func(ctx context.Context) []preflight.Result { out, err := deps.outputCmd(ctx, log, "systemctl", "is-active", dockerServiceUnit) + log.Debug("checked Docker unit state", "unit", dockerServiceUnit, "state", strings.TrimSpace(out), "error", err != nil) + if err == nil && strings.TrimSpace(out) == "active" { - return preflight.ResultsWarning(CheckDockerActiveName, "docker service", "Docker is active and bootstrap will disable it") + return preflight.ResultsWarning(checkDockerActiveName, "docker service", "Docker is active and bootstrap will disable it") } - return preflight.ResultsOK(CheckDockerActiveName, "docker service", "Docker is not active") + return preflight.ResultsOK(checkDockerActiveName, "docker service", "Docker is not active") }} } -func CheckSwapActive() preflight.Checker { - return checkSwapActive(defaultHostCheckDeps()) +// CheckSwapActive warns when host swap is active. +func CheckSwapActive(log *slog.Logger) preflight.Checker { + return checkSwapActive(log, defaultHostCheckDeps()) } -func checkSwapActive(deps hostCheckDeps) preflight.Checker { - return simpleHostChecker{name: CheckSwapActiveName, check: func(context.Context) []preflight.Result { +func checkSwapActive(log *slog.Logger, deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: checkSwapActiveName, check: func(context.Context) []preflight.Result { active, err := swapActive(deps.readFile) + log.Debug("checked host swap state", "active", active, "error", err != nil) + if err != nil { - return preflight.ResultsWarning(CheckSwapActiveName, "host swap", "swap state could not be determined") + return preflight.ResultsWarning(checkSwapActiveName, "host swap", "swap state could not be determined from /proc/swaps") } if active { - return preflight.ResultsWarning(CheckSwapActiveName, "host swap", "swap is enabled and bootstrap will disable it") + return preflight.ResultsWarning(checkSwapActiveName, "host swap", "swap is enabled and bootstrap will disable it") } - return preflight.ResultsOK(CheckSwapActiveName, "host swap", "swap is not active") + return preflight.ResultsOK(checkSwapActiveName, "host swap", "swap is not active") }} } -func CheckDiskSpace() preflight.Checker { - return checkDiskSpace(defaultHostCheckDeps()) +// CheckDiskSpace verifies enough free disk is available for bootstrap. +func CheckDiskSpace(log *slog.Logger) preflight.Checker { + return checkDiskSpace(log, defaultHostCheckDeps()) } -func checkDiskSpace(deps hostCheckDeps) preflight.Checker { - return simpleHostChecker{name: CheckDiskSpaceName, check: func(context.Context) []preflight.Result { +func checkDiskSpace(log *slog.Logger, deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: checkDiskSpaceName, check: func(context.Context) []preflight.Result { var stat syscall.Statfs_t - if err := deps.statfs("/var/lib", &stat); err != nil { - return preflight.ResultsError(CheckDiskSpaceName, "host disk", "available disk space could not be determined") + + diskPath := "/var/lib" + if err := deps.statfs(diskPath, &stat); err != nil { + log.Debug("failed to check disk space", "path", diskPath) + + return preflight.ResultsError(checkDiskSpaceName, "host disk", "available disk space could not be determined for "+diskPath) } free := stat.Bavail * uint64(stat.Bsize) + log.Debug("checked disk space", "path", diskPath, "freeGiB", gib(free), "requiredGiB", gib(minFreeDiskBytes)) + if free < minFreeDiskBytes { - return preflight.ResultsError(CheckDiskSpaceName, "host disk", "available disk space is below the minimum") + return preflight.ResultsError(checkDiskSpaceName, "host disk", fmt.Sprintf("available disk space is below the minimum for %s: current %.1f GiB, required %.1f GiB", diskPath, gib(free), gib(minFreeDiskBytes))) } - return preflight.ResultsOK(CheckDiskSpaceName, "host disk", "sufficient disk space is available") + return preflight.ResultsOK(checkDiskSpaceName, "host disk", "sufficient disk space is available") }} } -func CheckCgroups() preflight.Checker { - return checkCgroups(defaultHostCheckDeps()) +// CheckCgroups verifies the host cgroup filesystem is available. +func CheckCgroups(log *slog.Logger) preflight.Checker { + return checkCgroups(log, defaultHostCheckDeps()) } -func checkCgroups(deps hostCheckDeps) preflight.Checker { - return simpleHostChecker{name: CheckCgroupsName, check: func(context.Context) []preflight.Result { - if _, err := deps.stat("/sys/fs/cgroup"); err != nil { - return preflight.ResultsError(CheckCgroupsName, "host cgroups", "cgroup filesystem is required") +func checkCgroups(log *slog.Logger, deps hostCheckDeps) preflight.Checker { + return simpleHostChecker{name: checkCgroupsName, check: func(context.Context) []preflight.Result { + cgroupPath := "/sys/fs/cgroup" + log.Debug("checking cgroup filesystem", "path", cgroupPath) + + if _, err := deps.stat(cgroupPath); err != nil { + return preflight.ResultsError(checkCgroupsName, "host cgroups", "cgroup filesystem is required at "+cgroupPath) } - return preflight.ResultsOK(CheckCgroupsName, "host cgroups", "cgroup filesystem is available") + return preflight.ResultsOK(checkCgroupsName, "host cgroups", "cgroup filesystem is available") }} } -func CheckNodeIdentity(cfg *config.AgentConfig) preflight.Checker { - return simpleHostChecker{name: CheckNodeIdentityName, check: func(context.Context) []preflight.Result { - if cfg == nil || strings.TrimSpace(cfg.NodeName) == "" { - return preflight.ResultsError(CheckNodeIdentityName, "node identity", "node name could not be resolved") - } - - return preflight.ResultsOK(CheckNodeIdentityName, "node identity", "node name is resolved") - }} +func gib(bytes uint64) float64 { + return float64(bytes) / (1024 * 1024 * 1024) } func probeWritableDir(dir string) error { diff --git a/pkg/agent/phases/host/preflight_host_test.go b/pkg/agent/phases/host/preflight_host_test.go index bedd6bbb3..2ef3978c4 100644 --- a/pkg/agent/phases/host/preflight_host_test.go +++ b/pkg/agent/phases/host/preflight_host_test.go @@ -14,15 +14,14 @@ import ( "github.com/stretchr/testify/assert" - "github.com/Azure/unbounded/pkg/agent/config" "github.com/Azure/unbounded/pkg/agent/preflight" ) func TestCheckIsPrivilegedUser(t *testing.T) { - results := checkIsPrivilegedUser(hostCheckDeps{uid: func() int { return 0 }}).Check(context.Background()) + results := checkIsPrivilegedUser(slog.New(slog.DiscardHandler), hostCheckDeps{uid: func() int { return 0 }}).Check(context.Background()) assert.Equal(t, preflight.SeverityOK, results[0].Severity) - results = checkIsPrivilegedUser(hostCheckDeps{uid: func() int { return 1000 }}).Check(context.Background()) + results = checkIsPrivilegedUser(slog.New(slog.DiscardHandler), hostCheckDeps{uid: func() int { return 1000 }}).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) } @@ -33,18 +32,30 @@ func TestCheckHostPackagesMissingPackageManager(t *testing.T) { results := checkHostPackages(slog.New(slog.DiscardHandler), deps).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) + assert.Contains(t, results[0].Message, "apt-get") +} + +func TestCheckHostPackagesListsMissingPackages(t *testing.T) { + deps := defaultHostCheckDeps() + deps.lookupPath = lookupPathWith(map[string]bool{"apt-get": true}) + + results := checkHostPackages(slog.New(slog.DiscardHandler), deps).Check(context.Background()) + + assert.Equal(t, preflight.SeverityWarning, results[0].Severity) + assert.Contains(t, results[0].Message, "systemd-container") } func TestCheckHostOSConfiguration(t *testing.T) { deps := defaultHostCheckDeps() deps.writeProbe = func(string) error { return nil } - results := checkHostOSConfiguration(deps).Check(context.Background()) + results := checkHostOSConfiguration(slog.New(slog.DiscardHandler), deps).Check(context.Background()) assert.Equal(t, preflight.SeverityOK, results[0].Severity) deps.writeProbe = func(string) error { return errors.New("denied") } - results = checkHostOSConfiguration(deps).Check(context.Background()) + results = checkHostOSConfiguration(slog.New(slog.DiscardHandler), deps).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) + assert.Contains(t, results[0].Message, "/etc/sysctl.d") } func TestCheckNSpawnRuntime(t *testing.T) { @@ -56,12 +67,13 @@ func TestCheckNSpawnRuntime(t *testing.T) { }) deps.stat = func(string) (fs.FileInfo, error) { return nil, nil } - results := checkNSpawnRuntime(deps).Check(context.Background()) + results := checkNSpawnRuntime(slog.New(slog.DiscardHandler), deps).Check(context.Background()) assert.Equal(t, preflight.SeverityOK, results[0].Severity) deps.lookupPath = lookupPathWith(map[string]bool{"systemctl": true}) - results = checkNSpawnRuntime(deps).Check(context.Background()) - assert.Equal(t, preflight.SeverityError, results[0].Severity) + results = checkNSpawnRuntime(slog.New(slog.DiscardHandler), deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityWarning, results[0].Severity) + assert.Contains(t, results[0].Message, "machinectl") } func TestCheckDockerActive(t *testing.T) { @@ -80,44 +92,44 @@ func TestCheckSwapActive(t *testing.T) { deps := defaultHostCheckDeps() deps.readFile = readFileString("Filename\tType\tSize\tUsed\tPriority\n", nil) - results := checkSwapActive(deps).Check(context.Background()) + results := checkSwapActive(slog.New(slog.DiscardHandler), deps).Check(context.Background()) assert.Equal(t, preflight.SeverityOK, results[0].Severity) deps.readFile = readFileString("Filename\tType\tSize\tUsed\tPriority\n/swapfile file 1024 0 -2\n", nil) - results = checkSwapActive(deps).Check(context.Background()) + results = checkSwapActive(slog.New(slog.DiscardHandler), deps).Check(context.Background()) assert.Equal(t, preflight.SeverityWarning, results[0].Severity) + + deps.readFile = readFileString("", errors.New("missing")) + results = checkSwapActive(slog.New(slog.DiscardHandler), deps).Check(context.Background()) + assert.Contains(t, results[0].Message, "/proc/swaps") } func TestCheckDiskSpace(t *testing.T) { deps := defaultHostCheckDeps() deps.statfs = statfsWithFreeBytes(minFreeDiskBytes) - results := checkDiskSpace(deps).Check(context.Background()) + results := checkDiskSpace(slog.New(slog.DiscardHandler), deps).Check(context.Background()) assert.Equal(t, preflight.SeverityOK, results[0].Severity) deps.statfs = statfsWithFreeBytes(1) - results = checkDiskSpace(deps).Check(context.Background()) + results = checkDiskSpace(slog.New(slog.DiscardHandler), deps).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) + assert.Contains(t, results[0].Message, "/var/lib") + assert.Contains(t, results[0].Message, "current 0.0 GiB") + assert.Contains(t, results[0].Message, "required 8.0 GiB") } func TestCheckCgroups(t *testing.T) { deps := defaultHostCheckDeps() deps.stat = statExists() - results := checkCgroups(deps).Check(context.Background()) + results := checkCgroups(slog.New(slog.DiscardHandler), deps).Check(context.Background()) assert.Equal(t, preflight.SeverityOK, results[0].Severity) deps.stat = statMissing() - results = checkCgroups(deps).Check(context.Background()) - assert.Equal(t, preflight.SeverityError, results[0].Severity) -} - -func TestCheckNodeIdentity(t *testing.T) { - results := CheckNodeIdentity(&config.AgentConfig{NodeName: "node-1"}).Check(context.Background()) - assert.Equal(t, preflight.SeverityOK, results[0].Severity) - - results = CheckNodeIdentity(&config.AgentConfig{}).Check(context.Background()) + results = checkCgroups(slog.New(slog.DiscardHandler), deps).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) + assert.Contains(t, results[0].Message, "/sys/fs/cgroup") } func statfsWithFreeBytes(bytes uint64) func(string, *syscall.Statfs_t) error { diff --git a/pkg/agent/phases/nodestart/preflight_api_server.go b/pkg/agent/phases/nodestart/preflight_api_server.go index 2ece7ed48..0d11eb4cb 100644 --- a/pkg/agent/phases/nodestart/preflight_api_server.go +++ b/pkg/agent/phases/nodestart/preflight_api_server.go @@ -8,6 +8,7 @@ import ( "crypto/tls" "crypto/x509" "fmt" + "log/slog" "net/http" "net/url" "strings" @@ -16,10 +17,10 @@ import ( "github.com/Azure/unbounded/pkg/agent/preflight" ) -// CheckAPIServerReachableName is the stable name for API server reachability. -const CheckAPIServerReachableName = "api-server-reachable" +const checkAPIServerReachableName = "api-server-reachable" type apiServerReachableChecker struct { + log *slog.Logger url string caCertData []byte httpClient *http.Client @@ -28,20 +29,22 @@ type apiServerReachableChecker struct { // CheckAPIServerReachable returns a non-mutating checker that validates the // configured Kubernetes API server can be reached from the host. The checker // redacts the configured endpoint from result messages. -func CheckAPIServerReachable(apiServer string, caCertData []byte) preflight.Checker { - return apiServerReachableChecker{url: apiServer, caCertData: caCertData} +func CheckAPIServerReachable(log *slog.Logger, apiServer string, caCertData []byte) preflight.Checker { + return apiServerReachableChecker{log: log, url: apiServer, caCertData: caCertData} } -func (c apiServerReachableChecker) Name() string { return CheckAPIServerReachableName } +func (c apiServerReachableChecker) Name() string { return checkAPIServerReachableName } func (c apiServerReachableChecker) Check(ctx context.Context) []preflight.Result { + c.log.Debug("checking API server reachability", "target", "cluster API server", "caConfigured", len(c.caCertData) > 0) + if strings.TrimSpace(c.url) == "" { - return preflight.ResultsError(CheckAPIServerReachableName, "cluster API server", "API server is required") + return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", "API server is required") } parsed, err := url.Parse(c.url) if err != nil || parsed.Scheme == "" || parsed.Host == "" { - return preflight.ResultsError(CheckAPIServerReachableName, "cluster API server", "API server endpoint is invalid") + return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", "API server endpoint is invalid") } client := c.httpClient @@ -51,20 +54,26 @@ func (c apiServerReachableChecker) Check(ctx context.Context) []preflight.Result req, err := http.NewRequestWithContext(ctx, http.MethodGet, strings.TrimRight(c.url, "/")+"/readyz", http.NoBody) if err != nil { - return preflight.ResultsError(CheckAPIServerReachableName, "cluster API server", "API server request could not be created") + return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", "API server request could not be created") } resp, err := client.Do(req) if err != nil { - return preflight.ResultsError(CheckAPIServerReachableName, "cluster API server", "API server is not reachable") + c.log.Debug("API server reachability check failed", "target", "cluster API server") + + return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", "API server is not reachable") } defer resp.Body.Close() //nolint:errcheck // best effort close if resp.StatusCode >= http.StatusInternalServerError { - return preflight.ResultsError(CheckAPIServerReachableName, "cluster API server", fmt.Sprintf("API server returned status %d", resp.StatusCode)) + c.log.Debug("API server reachability check returned server error", "target", "cluster API server", "status", resp.StatusCode) + + return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", fmt.Sprintf("API server returned status %d", resp.StatusCode)) } - return preflight.ResultsOK(CheckAPIServerReachableName, "cluster API server", "API server is reachable") + c.log.Debug("API server reachability check passed", "target", "cluster API server", "status", resp.StatusCode) + + return preflight.ResultsOK(checkAPIServerReachableName, "cluster API server", "API server is reachable") } func (c apiServerReachableChecker) httpClientWithCA() *http.Client { diff --git a/pkg/agent/phases/nodestart/preflight_api_server_test.go b/pkg/agent/phases/nodestart/preflight_api_server_test.go index 25c19ccff..6bbf5b3f4 100644 --- a/pkg/agent/phases/nodestart/preflight_api_server_test.go +++ b/pkg/agent/phases/nodestart/preflight_api_server_test.go @@ -5,6 +5,7 @@ package nodestart import ( "context" + "log/slog" "net/http" "net/http/httptest" "testing" @@ -21,16 +22,16 @@ func TestCheckAPIServerReachableOK(t *testing.T) { })) t.Cleanup(srv.Close) - results := CheckAPIServerReachable(srv.URL, nil).Check(context.Background()) + results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), srv.URL, nil).Check(context.Background()) - assert.Equal(t, preflight.ResultsOK(CheckAPIServerReachableName, "cluster API server", "API server is reachable"), results) + assert.Equal(t, preflight.ResultsOK(checkAPIServerReachableName, "cluster API server", "API server is reachable"), results) } func TestCheckAPIServerReachableInvalidEndpoint(t *testing.T) { - results := CheckAPIServerReachable("://bad", nil).Check(context.Background()) + results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), "://bad", nil).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) - assert.Equal(t, CheckAPIServerReachableName, results[0].Name) + assert.Equal(t, checkAPIServerReachableName, results[0].Name) assert.Equal(t, "cluster API server", results[0].Target) assert.Equal(t, "API server endpoint is invalid", results[0].Message) } @@ -38,7 +39,7 @@ func TestCheckAPIServerReachableInvalidEndpoint(t *testing.T) { func TestCheckAPIServerReachableRequestFailureIsRedacted(t *testing.T) { const endpoint = "https://127.0.0.1:1" - results := CheckAPIServerReachable(endpoint, nil).Check(context.Background()) + results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), endpoint, nil).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Equal(t, "API server is not reachable", results[0].Message) @@ -51,7 +52,7 @@ func TestCheckAPIServerReachableServerError(t *testing.T) { })) t.Cleanup(srv.Close) - results := CheckAPIServerReachable(srv.URL, nil).Check(context.Background()) + results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), srv.URL, nil).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Equal(t, "API server returned status 500", results[0].Message) diff --git a/pkg/agent/phases/rootfs/preflight_goal_state.go b/pkg/agent/phases/rootfs/preflight_goal_state.go index b3e4a15f5..83c0d411a 100644 --- a/pkg/agent/phases/rootfs/preflight_goal_state.go +++ b/pkg/agent/phases/rootfs/preflight_goal_state.go @@ -13,10 +13,8 @@ import ( ) const ( - // CheckGoalStateName is the stable name for machine goal-state resolution. - CheckGoalStateName = "goal-state" - // CheckOCIImageReachableName is the stable name for rootfs image validation. - CheckOCIImageReachableName = "oci-image-reachable" + checkGoalStateName = "goal-state" + checkOCIImageReachableName = "oci-image-reachable" ) type goalStateChecker struct { @@ -31,20 +29,20 @@ func CheckGoalState(log *slog.Logger, cfg *config.AgentConfig, downloads *goalst return goalStateChecker{log: log, config: cfg, downloads: downloads} } -func (c goalStateChecker) Name() string { return CheckGoalStateName } +func (c goalStateChecker) Name() string { return checkGoalStateName } func (c goalStateChecker) Check(context.Context) []preflight.Result { gs, err := goalstates.ResolveMachine(c.log, c.config, goalstates.NSpawnMachineKube1, c.downloads) if err != nil { - return preflight.ResultsError(CheckGoalStateName, "goal state", "goal state could not be resolved") + return preflight.ResultsError(checkGoalStateName, "goal state", "goal state could not be resolved") } if gs.RootFS.OCIImage == "" { // TODO: replace this with an OCI manifest reachability check that uses // the same registry parsing and plain-HTTP handling as OCI rootfs // provisioning, without pulling image layers. - return preflight.ResultsError(CheckOCIImageReachableName, "rootfs image", "OCI rootfs image is required") + return preflight.ResultsError(checkOCIImageReachableName, "rootfs image", "OCI rootfs image is required but no image was selected") } - return preflight.ResultsOK(CheckGoalStateName, "goal state", "goal state resolved") + return preflight.ResultsOK(checkGoalStateName, "goal state", "goal state resolved") } diff --git a/pkg/agent/phases/rootfs/preflight_goal_state_test.go b/pkg/agent/phases/rootfs/preflight_goal_state_test.go index 911cd85e9..0842e349a 100644 --- a/pkg/agent/phases/rootfs/preflight_goal_state_test.go +++ b/pkg/agent/phases/rootfs/preflight_goal_state_test.go @@ -36,7 +36,7 @@ func validGoalStateConfig() *config.AgentConfig { func TestCheckGoalStateOK(t *testing.T) { results := CheckGoalState(slog.New(slog.DiscardHandler), validGoalStateConfig(), nil).Check(context.Background()) - assert.Equal(t, []preflight.Result{preflight.OK(CheckGoalStateName, "goal state", "goal state resolved")}, results) + assert.Equal(t, []preflight.Result{preflight.OK(checkGoalStateName, "goal state", "goal state resolved")}, results) } func TestCheckGoalStateResolveError(t *testing.T) { @@ -46,6 +46,6 @@ func TestCheckGoalStateResolveError(t *testing.T) { results := CheckGoalState(slog.New(slog.DiscardHandler), cfg, nil).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) - assert.Equal(t, CheckGoalStateName, results[0].Name) + assert.Equal(t, checkGoalStateName, results[0].Name) assert.Equal(t, "goal state could not be resolved", results[0].Message) } diff --git a/pkg/agent/preflight/preflight.go b/pkg/agent/preflight/preflight.go index 08b71ec36..f34110ec9 100644 --- a/pkg/agent/preflight/preflight.go +++ b/pkg/agent/preflight/preflight.go @@ -10,6 +10,7 @@ import ( "fmt" "slices" "strings" + "sync" ) type Severity string @@ -65,10 +66,21 @@ type Summary struct { // Run executes all checks, applies ignore rules, and returns a complete report. func Run(ctx context.Context, checks []Checker, opts Options) Report { ignored := ignoreSet(opts.IgnoreErrors) - results := make([]Result, 0, len(checks)) + checkResults := make([][]Result, len(checks)) + + var wg sync.WaitGroup + + for i, check := range checks { + wg.Go(func() { + checkResults[i] = check.Check(ctx) + }) + } + + wg.Wait() - for _, check := range checks { - for _, result := range check.Check(ctx) { + results := make([]Result, 0, len(checks)) + for i, check := range checks { + for _, result := range checkResults[i] { if result.Name == "" { result.Name = check.Name() } diff --git a/pkg/agent/preflight/preflight_test.go b/pkg/agent/preflight/preflight_test.go index 0713badb7..0ce04df4b 100644 --- a/pkg/agent/preflight/preflight_test.go +++ b/pkg/agent/preflight/preflight_test.go @@ -6,6 +6,7 @@ package preflight import ( "context" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -59,3 +60,53 @@ func TestRunIgnoreAll(t *testing.T) { assert.Equal(t, "ok", report.Status) assert.True(t, report.Checks[0].Ignored) } + +func TestRunPreservesInputOrderWhileRunningConcurrently(t *testing.T) { + release := make(chan struct{}) + started := make(chan string, 2) + + report := make(chan Report, 1) + + go func() { + report <- Run(context.Background(), []Checker{ + blockingChecker{name: "first", started: started, release: release}, + blockingChecker{name: "second", started: started, release: release}, + }, Options{}) + }() + + seen := map[string]bool{} + + for range 2 { + select { + case name := <-started: + seen[name] = true + case <-time.After(time.Second): + t.Fatal("checks did not start concurrently") + } + } + + assert.True(t, seen["first"]) + assert.True(t, seen["second"]) + + close(release) + + got := <-report + assert.Equal(t, "first", got.Checks[0].Name) + assert.Equal(t, "second", got.Checks[1].Name) +} + +type blockingChecker struct { + name string + started chan<- string + release <-chan struct{} +} + +func (b blockingChecker) Name() string { return b.name } + +func (b blockingChecker) Check(context.Context) []Result { + b.started <- b.name + + <-b.release + + return ResultsOK(b.name, b.name, b.name) +} From a00c63fde95893765df2df9e4a30bdbbb1abefc4 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Wed, 24 Jun 2026 05:15:02 +0000 Subject: [PATCH 08/16] agent: add local rootfs preflight checks --- cmd/agent/internal/cmd/preflight.go | 29 +++- designs/agent-preflight.md | 5 +- pkg/agent/internal/utilio/fs.go | 17 ++ pkg/agent/internal/utilio/fs_test.go | 18 ++ pkg/agent/phases/host/preflight_host.go | 109 ++++++++---- .../phases/rootfs/preflight_goal_state.go | 27 +-- .../rootfs/preflight_goal_state_test.go | 36 ++-- pkg/agent/phases/rootfs/preflight_local.go | 162 ++++++++++++++++++ .../phases/rootfs/preflight_local_test.go | 87 ++++++++++ 9 files changed, 419 insertions(+), 71 deletions(-) create mode 100644 pkg/agent/phases/rootfs/preflight_local.go create mode 100644 pkg/agent/phases/rootfs/preflight_local_test.go diff --git a/cmd/agent/internal/cmd/preflight.go b/cmd/agent/internal/cmd/preflight.go index 101ed0f5f..27fc5e164 100644 --- a/cmd/agent/internal/cmd/preflight.go +++ b/cmd/agent/internal/cmd/preflight.go @@ -15,6 +15,7 @@ import ( "github.com/spf13/cobra" "github.com/Azure/unbounded/internal/provision" + "github.com/Azure/unbounded/pkg/agent/goalstates" "github.com/Azure/unbounded/pkg/agent/phases/host" "github.com/Azure/unbounded/pkg/agent/phases/nodestart" "github.com/Azure/unbounded/pkg/agent/phases/rootfs" @@ -43,7 +44,12 @@ func newCmdPreflight(cmdCtx *CommandContext) *cobra.Command { } cmd.Flags().StringVar(&handler.configPath, "config", "", "Path to agent config file") - cmd.Flags().StringSliceVar(&handler.ignorePreflightErrors, "ignore-preflight-errors", nil, "Comma-separated preflight check names whose errors should be reported as warnings") + cmd.Flags().StringSliceVar( + &handler.ignorePreflightErrors, + "ignore-preflight-errors", + nil, + "Comma-separated preflight check names whose errors should be reported as warnings", + ) cmd.Flags().BoolVar(&handler.failOnWarnings, "fail-on-warnings", false, "Fail when any preflight warning is returned") cmd.Flags().StringVar(&handler.output, "output", "text", "Output format: text or json") @@ -72,6 +78,19 @@ func (h *preflightHandler) execute(ctx context.Context) error { caCertData = nil } + goalState, goalStateErr := goalstates.ResolveMachine( + h.cmdCtx.Logger, + &cfg.AgentConfig, + goalstates.NSpawnMachineKube1, + provision.ResolveDownloadOverrides(cfg.Downloads), + ) + + var rootFSGoalState *goalstates.RootFS + + if goalState != nil { + rootFSGoalState = goalState.RootFS + } + checks := []preflight.Checker{ host.CheckIsPrivilegedUser(h.cmdCtx.Logger), host.CheckAgentConfig(h.cmdCtx.Logger, &cfg.AgentConfig), @@ -84,7 +103,8 @@ func (h *preflightHandler) execute(ctx context.Context) error { host.CheckDiskSpace(h.cmdCtx.Logger), host.CheckCgroups(h.cmdCtx.Logger), nodestart.CheckAPIServerReachable(h.cmdCtx.Logger, cfg.Kubelet.ApiServer, caCertData), - rootfs.CheckGoalState(h.cmdCtx.Logger, &cfg.AgentConfig, provision.ResolveDownloadOverrides(cfg.Downloads)), + rootfs.CheckGoalState(h.cmdCtx.Logger, goalStateErr, rootFSGoalState), + rootfs.CheckNSpawnMachineProvisioning(h.cmdCtx.Logger, rootFSGoalState), } opts := preflight.Options{ @@ -158,7 +178,10 @@ func writePreflightText(w io.Writer, report preflight.Report) error { } } - _, err := fmt.Fprintln(w, "[preflight] If you know what you are doing, you can make a check non-fatal with `--ignore-preflight-errors=...`") + _, err := fmt.Fprintln( + w, + "[preflight] If you know what you are doing, you can make a check non-fatal with `--ignore-preflight-errors=...`", + ) return err } diff --git a/designs/agent-preflight.md b/designs/agent-preflight.md index 2bd02680d..55a2dcb4a 100644 --- a/designs/agent-preflight.md +++ b/designs/agent-preflight.md @@ -409,14 +409,11 @@ Rootfs provisioning checks: | Check | Purpose | |---|---| -| `machine-dir` | Validate the target machine directory state is compatible with bootstrap. | -| `oci-image-reference` | Validate the rootfs image reference parses. | | `oci-image-reachable` | Validate the configured rootfs image manifest can be resolved without pulling layers. | -| `rootfs-provisioning` | Validate rootfs provisioning prerequisites are available from installed host packages and host-side nspawn config paths are writable. | +| `nspawn-machine-provisioning` | Validate the target nspawn machine directory state, rootfs parent directory writability, and host-side nspawn config path writability. | | `kubernetes-artifacts` | Validate kubelet/kubectl/kube-proxy artifacts and checksums using the same download and verification calls used by rootfs provisioning, without installing files. | | `cri-artifacts` | Validate containerd, runc, and crictl artifacts using the same download/decompression or download calls used by rootfs provisioning, without installing files. | | `cni-artifacts` | Validate CNI plugin artifacts using the same download/decompression calls used by rootfs provisioning, without installing files. | -| `rootfs-parent-writable` | Validate the parent directory for rootfs creation can be created or written. | GPU checks: diff --git a/pkg/agent/internal/utilio/fs.go b/pkg/agent/internal/utilio/fs.go index b0cb349fc..eee35bf26 100644 --- a/pkg/agent/internal/utilio/fs.go +++ b/pkg/agent/internal/utilio/fs.go @@ -81,3 +81,20 @@ func UpdateSymlink(linkPath, targetPath string) error { return renameio.Symlink(targetPath, linkPath) } + +// ProbeWritableDir verifies that dir accepts file creation and removal without +// leaving durable state behind. +func ProbeWritableDir(dir string) error { + f, err := os.CreateTemp(dir, ".unbounded-probe-*") + if err != nil { + return err + } + + name := f.Name() + if err := f.Close(); err != nil { + os.Remove(name) //nolint:errcheck // best effort cleanup after close failure. + return err + } + + return os.Remove(name) +} diff --git a/pkg/agent/internal/utilio/fs_test.go b/pkg/agent/internal/utilio/fs_test.go index a826523df..87c6c52a2 100644 --- a/pkg/agent/internal/utilio/fs_test.go +++ b/pkg/agent/internal/utilio/fs_test.go @@ -145,3 +145,21 @@ func TestUpdateSymlink(t *testing.T) { t.Fatalf("second target = %q, want %q", target, secondTarget) } } + +func TestProbeWritableDir(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + if err := ProbeWritableDir(dir); err != nil { + t.Fatalf("probe writable dir: %v", err) + } + + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("read temp dir: %v", err) + } + + if len(entries) != 0 { + t.Fatalf("probe left entries behind: %v", entries) + } +} diff --git a/pkg/agent/phases/host/preflight_host.go b/pkg/agent/phases/host/preflight_host.go index b75bb5777..7210b69db 100644 --- a/pkg/agent/phases/host/preflight_host.go +++ b/pkg/agent/phases/host/preflight_host.go @@ -17,6 +17,7 @@ import ( "github.com/Azure/unbounded/internal/executil" "github.com/Azure/unbounded/pkg/agent/goalstates" + "github.com/Azure/unbounded/pkg/agent/internal/utilio" "github.com/Azure/unbounded/pkg/agent/preflight" ) @@ -50,7 +51,7 @@ func defaultHostCheckDeps() hostCheckDeps { statfs: syscall.Statfs, readFile: os.ReadFile, stat: os.Stat, - writeProbe: probeWritableDir, + writeProbe: utilio.ProbeWritableDir, outputCmd: executil.OutputCmd, } } @@ -93,10 +94,18 @@ func checkHostPackages(log *slog.Logger, deps hostCheckDeps) preflight.Checker { if err != nil { log.Debug("host package manager detection failed") - return preflight.ResultsError(checkHostPackagesName, "host packages", "supported host package manager is required: apt-get, tdnf, or dnf") + return preflight.ResultsError( + checkHostPackagesName, + "host packages", + "supported host package manager is required: apt-get, tdnf, or dnf", + ) } - log.Debug("detected host package manager", "packageManager", pm.name, "requiredPackages", strings.Join(pm.requiredPackages, ",")) + log.Debug( + "detected host package manager", + "packageManager", pm.name, + "requiredPackages", strings.Join(pm.requiredPackages, ","), + ) var missing []string @@ -115,7 +124,11 @@ func checkHostPackages(log *slog.Logger, deps hostCheckDeps) preflight.Checker { // TODO: when offline mode is configured, missing required host // packages should be reported as an error because bootstrap cannot // rely on package source access to remediate them. - return preflight.ResultsWarning(checkHostPackagesName, "host packages", "required host packages are missing and may be installed by bootstrap: "+strings.Join(missing, ", ")) + return preflight.ResultsWarning( + checkHostPackagesName, + "host packages", + "required host packages are missing and may be installed by bootstrap: "+strings.Join(missing, ", "), + ) } log.Debug("required host packages are installed") @@ -135,16 +148,28 @@ func checkHostOSConfiguration(log *slog.Logger, deps hostCheckDeps) preflight.Ch log.Debug("checking host OS configuration path", "path", sysctlDir) if err := deps.writeProbe(sysctlDir); err != nil { - return preflight.ResultsError(checkHostOSConfigurationName, "host OS configuration", "host OS configuration path is not writable: "+sysctlDir) + return preflight.ResultsError( + checkHostOSConfigurationName, + "host OS configuration", + "host OS configuration path is not writable: "+sysctlDir, + ) } log.Debug("checking systemd unit directory", "path", goalstates.SystemdSystemDir) if err := deps.writeProbe(goalstates.SystemdSystemDir); err != nil { - return preflight.ResultsError(checkHostOSConfigurationName, "host OS configuration", "systemd unit directory is not writable: "+goalstates.SystemdSystemDir) + return preflight.ResultsError( + checkHostOSConfigurationName, + "host OS configuration", + "systemd unit directory is not writable: "+goalstates.SystemdSystemDir, + ) } - return preflight.ResultsOK(checkHostOSConfigurationName, "host OS configuration", "host OS configuration can be applied") + return preflight.ResultsOK( + checkHostOSConfigurationName, + "host OS configuration", + "host OS configuration can be applied", + ) }} } @@ -162,7 +187,11 @@ func checkNSpawnRuntime(log *slog.Logger, deps hostCheckDeps) preflight.Checker // TODO: when offline mode is configured, missing nspawn runtime // tools should be reported as an error because bootstrap cannot rely // on package installation to remediate them. - return preflight.ResultsWarning(checkNSpawnRuntimeName, "nspawn runtime", "nspawn runtime tool is missing and may be installed by bootstrap: "+binary) + return preflight.ResultsWarning( + checkNSpawnRuntimeName, + "nspawn runtime", + "nspawn runtime tool is missing and may be installed by bootstrap: "+binary, + ) } } @@ -170,7 +199,11 @@ func checkNSpawnRuntime(log *slog.Logger, deps hostCheckDeps) preflight.Checker log.Debug("checking systemd runtime path", "path", systemdRuntimePath) if _, err := deps.stat(systemdRuntimePath); err != nil { - return preflight.ResultsWarning(checkNSpawnRuntimeName, "nspawn runtime", "systemd runtime path is not currently available: "+systemdRuntimePath) + return preflight.ResultsWarning( + checkNSpawnRuntimeName, + "nspawn runtime", + "systemd runtime path is not currently available: "+systemdRuntimePath, + ) } return preflight.ResultsOK(checkNSpawnRuntimeName, "nspawn runtime", "nspawn runtime is available") @@ -185,10 +218,19 @@ func CheckDockerActive(log *slog.Logger) preflight.Checker { func checkDockerActive(log *slog.Logger, deps hostCheckDeps) preflight.Checker { return simpleHostChecker{name: checkDockerActiveName, check: func(ctx context.Context) []preflight.Result { out, err := deps.outputCmd(ctx, log, "systemctl", "is-active", dockerServiceUnit) - log.Debug("checked Docker unit state", "unit", dockerServiceUnit, "state", strings.TrimSpace(out), "error", err != nil) + log.Debug( + "checked Docker unit state", + "unit", dockerServiceUnit, + "state", strings.TrimSpace(out), + "error", err != nil, + ) if err == nil && strings.TrimSpace(out) == "active" { - return preflight.ResultsWarning(checkDockerActiveName, "docker service", "Docker is active and bootstrap will disable it") + return preflight.ResultsWarning( + checkDockerActiveName, + "docker service", + "Docker is active and bootstrap will disable it", + ) } return preflight.ResultsOK(checkDockerActiveName, "docker service", "Docker is not active") @@ -206,7 +248,11 @@ func checkSwapActive(log *slog.Logger, deps hostCheckDeps) preflight.Checker { log.Debug("checked host swap state", "active", active, "error", err != nil) if err != nil { - return preflight.ResultsWarning(checkSwapActiveName, "host swap", "swap state could not be determined from /proc/swaps") + return preflight.ResultsWarning( + checkSwapActiveName, + "host swap", + "swap state could not be determined from /proc/swaps", + ) } if active { @@ -230,14 +276,32 @@ func checkDiskSpace(log *slog.Logger, deps hostCheckDeps) preflight.Checker { if err := deps.statfs(diskPath, &stat); err != nil { log.Debug("failed to check disk space", "path", diskPath) - return preflight.ResultsError(checkDiskSpaceName, "host disk", "available disk space could not be determined for "+diskPath) + return preflight.ResultsError( + checkDiskSpaceName, + "host disk", + "available disk space could not be determined for "+diskPath, + ) } free := stat.Bavail * uint64(stat.Bsize) - log.Debug("checked disk space", "path", diskPath, "freeGiB", gib(free), "requiredGiB", gib(minFreeDiskBytes)) + log.Debug( + "checked disk space", + "path", diskPath, + "freeGiB", gib(free), + "requiredGiB", gib(minFreeDiskBytes), + ) if free < minFreeDiskBytes { - return preflight.ResultsError(checkDiskSpaceName, "host disk", fmt.Sprintf("available disk space is below the minimum for %s: current %.1f GiB, required %.1f GiB", diskPath, gib(free), gib(minFreeDiskBytes))) + return preflight.ResultsError( + checkDiskSpaceName, + "host disk", + fmt.Sprintf( + "available disk space is below the minimum for %s: current %.1f GiB, required %.1f GiB", + diskPath, + gib(free), + gib(minFreeDiskBytes), + ), + ) } return preflight.ResultsOK(checkDiskSpaceName, "host disk", "sufficient disk space is available") @@ -266,21 +330,6 @@ func gib(bytes uint64) float64 { return float64(bytes) / (1024 * 1024 * 1024) } -func probeWritableDir(dir string) error { - f, err := os.CreateTemp(dir, ".unbounded-preflight-*") - if err != nil { - return err - } - - name := f.Name() - if err := f.Close(); err != nil { - os.Remove(name) //nolint:errcheck // best effort cleanup after close failure. - return err - } - - return os.Remove(name) -} - func swapActive(readFile func(string) ([]byte, error)) (bool, error) { data, err := readFile("/proc/swaps") if err != nil { diff --git a/pkg/agent/phases/rootfs/preflight_goal_state.go b/pkg/agent/phases/rootfs/preflight_goal_state.go index 83c0d411a..5dee3fe16 100644 --- a/pkg/agent/phases/rootfs/preflight_goal_state.go +++ b/pkg/agent/phases/rootfs/preflight_goal_state.go @@ -7,7 +7,6 @@ import ( "context" "log/slog" - "github.com/Azure/unbounded/pkg/agent/config" "github.com/Azure/unbounded/pkg/agent/goalstates" "github.com/Azure/unbounded/pkg/agent/preflight" ) @@ -18,30 +17,34 @@ const ( ) type goalStateChecker struct { - log *slog.Logger - config *config.AgentConfig - downloads *goalstates.DownloadOverrides + log *slog.Logger + err error + rootFS *goalstates.RootFS } -// CheckGoalState returns a checker that validates the agent config can be -// resolved into a machine goal state and that an OCI rootfs image is selected. -func CheckGoalState(log *slog.Logger, cfg *config.AgentConfig, downloads *goalstates.DownloadOverrides) preflight.Checker { - return goalStateChecker{log: log, config: cfg, downloads: downloads} +// CheckGoalState validates the agent config resolved into a machine goal state. +func CheckGoalState(log *slog.Logger, err error, rootFS *goalstates.RootFS) preflight.Checker { + return goalStateChecker{log: log, err: err, rootFS: rootFS} } func (c goalStateChecker) Name() string { return checkGoalStateName } func (c goalStateChecker) Check(context.Context) []preflight.Result { - gs, err := goalstates.ResolveMachine(c.log, c.config, goalstates.NSpawnMachineKube1, c.downloads) - if err != nil { + c.log.Debug("checking goal state resolution") + + if c.err != nil || c.rootFS == nil { return preflight.ResultsError(checkGoalStateName, "goal state", "goal state could not be resolved") } - if gs.RootFS.OCIImage == "" { + if c.rootFS.OCIImage == "" { // TODO: replace this with an OCI manifest reachability check that uses // the same registry parsing and plain-HTTP handling as OCI rootfs // provisioning, without pulling image layers. - return preflight.ResultsError(checkOCIImageReachableName, "rootfs image", "OCI rootfs image is required but no image was selected") + return preflight.ResultsError( + checkOCIImageReachableName, + "rootfs image", + "OCI rootfs image is required but no image was selected", + ) } return preflight.ResultsOK(checkGoalStateName, "goal state", "goal state resolved") diff --git a/pkg/agent/phases/rootfs/preflight_goal_state_test.go b/pkg/agent/phases/rootfs/preflight_goal_state_test.go index 0842e349a..3626cbf91 100644 --- a/pkg/agent/phases/rootfs/preflight_goal_state_test.go +++ b/pkg/agent/phases/rootfs/preflight_goal_state_test.go @@ -5,45 +5,37 @@ package rootfs import ( "context" + "errors" "log/slog" "testing" "github.com/stretchr/testify/assert" - "github.com/Azure/unbounded/pkg/agent/config" + "github.com/Azure/unbounded/pkg/agent/goalstates" "github.com/Azure/unbounded/pkg/agent/preflight" ) -func validGoalStateConfig() *config.AgentConfig { - return &config.AgentConfig{ - MachineName: "machine-1", - NodeName: "node-1", - Cluster: config.AgentClusterConfig{ - CaCertBase64: "Y2E=", - ClusterDNS: "10.0.0.10", - Version: "1.34.0", - }, - Kubelet: config.AgentKubeletConfig{ - ApiServer: "https://api.example.com:443", - Auth: config.KubeletAuthInfo{ - BootstrapToken: "abc123.secret456", - }, - }, - OCIImage: "registry.example.com/unbounded/rootfs:v1", +func validRootFSGoalState(t *testing.T) *goalstates.RootFS { + t.Helper() + + dir := t.TempDir() + + return &goalstates.RootFS{ + MachineDir: dir, + NSpawnConfigFile: dir + "/nspawn/kube1.nspawn", + ServiceOverrideFile: dir + "/systemd/system/systemd-nspawn@kube1.service.d/override.conf", + OCIImage: "registry.example.com/unbounded/rootfs:v1", } } func TestCheckGoalStateOK(t *testing.T) { - results := CheckGoalState(slog.New(slog.DiscardHandler), validGoalStateConfig(), nil).Check(context.Background()) + results := CheckGoalState(slog.New(slog.DiscardHandler), nil, validRootFSGoalState(t)).Check(context.Background()) assert.Equal(t, []preflight.Result{preflight.OK(checkGoalStateName, "goal state", "goal state resolved")}, results) } func TestCheckGoalStateResolveError(t *testing.T) { - cfg := validGoalStateConfig() - cfg.Cluster.CaCertBase64 = "not-base64" - - results := CheckGoalState(slog.New(slog.DiscardHandler), cfg, nil).Check(context.Background()) + results := CheckGoalState(slog.New(slog.DiscardHandler), errors.New("boom"), nil).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Equal(t, checkGoalStateName, results[0].Name) diff --git a/pkg/agent/phases/rootfs/preflight_local.go b/pkg/agent/phases/rootfs/preflight_local.go new file mode 100644 index 000000000..3543da6ba --- /dev/null +++ b/pkg/agent/phases/rootfs/preflight_local.go @@ -0,0 +1,162 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package rootfs + +import ( + "context" + "errors" + "io" + "io/fs" + "log/slog" + "os" + "path/filepath" + + "github.com/Azure/unbounded/pkg/agent/goalstates" + "github.com/Azure/unbounded/pkg/agent/internal/utilio" + "github.com/Azure/unbounded/pkg/agent/preflight" +) + +const ( + checkNSpawnMachineProvisioningName = "nspawn-machine-provisioning" +) + +type rootFSCheckDeps struct { + stat func(string) (fs.FileInfo, error) + open func(string) (*os.File, error) + writeProbe func(string) error +} + +func defaultRootFSCheckDeps() rootFSCheckDeps { + return rootFSCheckDeps{ + stat: os.Stat, + open: os.Open, + writeProbe: utilio.ProbeWritableDir, + } +} + +type nspawnMachineProvisioningChecker struct { + log *slog.Logger + gs *goalstates.RootFS + deps rootFSCheckDeps +} + +// CheckNSpawnMachineProvisioning validates local host paths needed to provision +// and configure the nspawn machine rootfs. +func CheckNSpawnMachineProvisioning(log *slog.Logger, gs *goalstates.RootFS) preflight.Checker { + return checkNSpawnMachineProvisioning(log, gs, defaultRootFSCheckDeps()) +} + +func checkNSpawnMachineProvisioning(log *slog.Logger, gs *goalstates.RootFS, deps rootFSCheckDeps) preflight.Checker { + return nspawnMachineProvisioningChecker{log: log, gs: gs, deps: deps} +} + +func (c nspawnMachineProvisioningChecker) Name() string { + return checkNSpawnMachineProvisioningName +} + +func (c nspawnMachineProvisioningChecker) Check(context.Context) []preflight.Result { + if c.gs == nil || c.gs.MachineDir == "" { + return preflight.ResultsError( + checkNSpawnMachineProvisioningName, + "nspawn machine provisioning", + "machine directory is not configured", + ) + } + + if result := c.checkMachineDir(); result != nil { + return result + } + + if result := c.checkWritableDir(filepath.Dir(c.gs.MachineDir), "rootfs parent directory"); result != nil { + return result + } + + for _, path := range []string{ + filepath.Dir(c.gs.NSpawnConfigFile), + filepath.Dir(c.gs.ServiceOverrideFile), + } { + if result := c.checkWritableDir(path, "nspawn provisioning path"); result != nil { + return result + } + } + + return preflight.ResultsOK( + checkNSpawnMachineProvisioningName, + "nspawn machine provisioning", + "nspawn machine provisioning paths are ready", + ) +} + +func (c nspawnMachineProvisioningChecker) checkMachineDir() []preflight.Result { + c.log.Debug("checking machine directory", "path", c.gs.MachineDir) + + info, err := c.deps.stat(c.gs.MachineDir) + switch { + case errors.Is(err, os.ErrNotExist): + // Missing machine directory is fine if the parent/provisioning paths are writable. + return nil + case err != nil: + return preflight.ResultsError( + checkNSpawnMachineProvisioningName, + "nspawn machine provisioning", + "machine directory cannot be inspected: "+c.gs.MachineDir, + ) + case !info.IsDir(): + return preflight.ResultsError( + checkNSpawnMachineProvisioningName, + "nspawn machine provisioning", + "machine directory path is not a directory: "+c.gs.MachineDir, + ) + } + + empty, err := isDirEmpty(c.deps.open, c.gs.MachineDir) + if err != nil { + return preflight.ResultsError( + checkNSpawnMachineProvisioningName, + "nspawn machine provisioning", + "machine directory cannot be read: "+c.gs.MachineDir, + ) + } + + if !empty { + // A populated machine directory is expected during rejoin or reuse of an + // existing kube1/kube2 rootfs. Rootfs provisioning will skip bootstrap + // rather than overwrite it. + c.log.Debug("machine directory exists and is not empty", "path", c.gs.MachineDir) + } else { + c.log.Debug("machine directory exists and is empty", "path", c.gs.MachineDir) + } + + return nil +} + +func (c nspawnMachineProvisioningChecker) checkWritableDir(path, label string) []preflight.Result { + c.log.Debug("checking "+label, "path", path) + + if err := c.deps.writeProbe(path); err != nil { + return preflight.ResultsError( + checkNSpawnMachineProvisioningName, + "nspawn machine provisioning", + label+" is not writable: "+path, + ) + } + + return nil +} + +func isDirEmpty(open func(string) (*os.File, error), dir string) (bool, error) { + f, err := open(dir) + if err != nil { + return false, err + } + + defer f.Close() //nolint:errcheck // best effort close + + _, err = f.Readdirnames(1) + if errors.Is(err, io.EOF) { + return true, nil + } + + return false, err +} diff --git a/pkg/agent/phases/rootfs/preflight_local_test.go b/pkg/agent/phases/rootfs/preflight_local_test.go new file mode 100644 index 000000000..818a07829 --- /dev/null +++ b/pkg/agent/phases/rootfs/preflight_local_test.go @@ -0,0 +1,87 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package rootfs + +import ( + "context" + "errors" + "io/fs" + "log/slog" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Azure/unbounded/pkg/agent/preflight" +) + +func TestCheckNSpawnMachineProvisioningMissingMachineDir(t *testing.T) { + gs := validRootFSGoalState(t) + gs.MachineDir = filepath.Join(t.TempDir(), "missing") + deps := defaultRootFSCheckDeps() + deps.writeProbe = func(string) error { return nil } + + results := checkNSpawnMachineProvisioning(slog.New(slog.DiscardHandler), gs, deps).Check(context.Background()) + + assert.Equal(t, preflight.SeverityOK, results[0].Severity) +} + +func TestCheckNSpawnMachineProvisioningNonEmptyMachineDir(t *testing.T) { + gs := validRootFSGoalState(t) + require.NoError(t, os.WriteFile(filepath.Join(gs.MachineDir, "file"), []byte("x"), 0o600)) + + deps := defaultRootFSCheckDeps() + deps.writeProbe = func(string) error { return nil } + + results := checkNSpawnMachineProvisioning(slog.New(slog.DiscardHandler), gs, deps).Check(context.Background()) + + assert.Equal(t, preflight.SeverityOK, results[0].Severity) +} + +func TestCheckNSpawnMachineProvisioningMachineDirFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "file") + require.NoError(t, os.WriteFile(path, []byte("x"), 0o600)) + gs := validRootFSGoalState(t) + gs.MachineDir = path + + results := CheckNSpawnMachineProvisioning(slog.New(slog.DiscardHandler), gs).Check(context.Background()) + + assert.Equal(t, preflight.SeverityError, results[0].Severity) +} + +func TestCheckNSpawnMachineProvisioningWritablePaths(t *testing.T) { + gs := validRootFSGoalState(t) + deps := defaultRootFSCheckDeps() + deps.writeProbe = func(string) error { return nil } + + results := checkNSpawnMachineProvisioning(slog.New(slog.DiscardHandler), gs, deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityOK, results[0].Severity) + + deps.writeProbe = func(string) error { return errors.New("denied") } + results = checkNSpawnMachineProvisioning(slog.New(slog.DiscardHandler), gs, deps).Check(context.Background()) + assert.Equal(t, preflight.SeverityError, results[0].Severity) +} + +func TestCheckNSpawnMachineProvisioningUnreadableMachineDir(t *testing.T) { + gs := validRootFSGoalState(t) + deps := defaultRootFSCheckDeps() + deps.stat = func(string) (fs.FileInfo, error) { return fakeDirInfo{}, nil } + deps.open = func(string) (*os.File, error) { return nil, errors.New("denied") } + + results := checkNSpawnMachineProvisioning(slog.New(slog.DiscardHandler), gs, deps).Check(context.Background()) + + assert.Equal(t, preflight.SeverityError, results[0].Severity) +} + +type fakeDirInfo struct{} + +func (fakeDirInfo) Name() string { return "dir" } +func (fakeDirInfo) Size() int64 { return 0 } +func (fakeDirInfo) Mode() fs.FileMode { return fs.ModeDir } +func (fakeDirInfo) ModTime() time.Time { return time.Time{} } +func (fakeDirInfo) IsDir() bool { return true } +func (fakeDirInfo) Sys() any { return nil } From 0fd74f306b3d43c1eca04cc481beca738554cfae Mon Sep 17 00:00:00 2001 From: Baichao He Date: Wed, 24 Jun 2026 05:24:39 +0000 Subject: [PATCH 09/16] agent: refine local preflight checks --- pkg/agent/phases/rootfs/preflight_local.go | 26 +++++++++++++++---- .../phases/rootfs/preflight_local_test.go | 10 +++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/pkg/agent/phases/rootfs/preflight_local.go b/pkg/agent/phases/rootfs/preflight_local.go index 3543da6ba..adf17b009 100644 --- a/pkg/agent/phases/rootfs/preflight_local.go +++ b/pkg/agent/phases/rootfs/preflight_local.go @@ -68,7 +68,7 @@ func (c nspawnMachineProvisioningChecker) Check(context.Context) []preflight.Res return result } - if result := c.checkWritableDir(filepath.Dir(c.gs.MachineDir), "rootfs parent directory"); result != nil { + if result := c.checkCreatableDir(filepath.Dir(c.gs.MachineDir), "rootfs parent directory"); result != nil { return result } @@ -76,7 +76,7 @@ func (c nspawnMachineProvisioningChecker) Check(context.Context) []preflight.Res filepath.Dir(c.gs.NSpawnConfigFile), filepath.Dir(c.gs.ServiceOverrideFile), } { - if result := c.checkWritableDir(path, "nspawn provisioning path"); result != nil { + if result := c.checkCreatableDir(path, "nspawn provisioning path"); result != nil { return result } } @@ -131,20 +131,36 @@ func (c nspawnMachineProvisioningChecker) checkMachineDir() []preflight.Result { return nil } -func (c nspawnMachineProvisioningChecker) checkWritableDir(path, label string) []preflight.Result { +func (c nspawnMachineProvisioningChecker) checkCreatableDir(path, label string) []preflight.Result { c.log.Debug("checking "+label, "path", path) + existing := nearestExistingParent(c.deps.stat, path) - if err := c.deps.writeProbe(path); err != nil { + if err := c.deps.writeProbe(existing); err != nil { return preflight.ResultsError( checkNSpawnMachineProvisioningName, "nspawn machine provisioning", - label+" is not writable: "+path, + label+" cannot be created under: "+existing, ) } return nil } +func nearestExistingParent(stat func(string) (fs.FileInfo, error), path string) string { + for { + if info, err := stat(path); err == nil && info.IsDir() { + return path + } + + parent := filepath.Dir(path) + if parent == path { + return path + } + + path = parent + } +} + func isDirEmpty(open func(string) (*os.File, error), dir string) (bool, error) { f, err := open(dir) if err != nil { diff --git a/pkg/agent/phases/rootfs/preflight_local_test.go b/pkg/agent/phases/rootfs/preflight_local_test.go index 818a07829..bd28963df 100644 --- a/pkg/agent/phases/rootfs/preflight_local_test.go +++ b/pkg/agent/phases/rootfs/preflight_local_test.go @@ -30,6 +30,16 @@ func TestCheckNSpawnMachineProvisioningMissingMachineDir(t *testing.T) { assert.Equal(t, preflight.SeverityOK, results[0].Severity) } +func TestCheckNSpawnMachineProvisioningMissingParentDir(t *testing.T) { + base := t.TempDir() + gs := validRootFSGoalState(t) + gs.MachineDir = filepath.Join(base, "var", "lib", "machines", "kube1") + + results := CheckNSpawnMachineProvisioning(slog.New(slog.DiscardHandler), gs).Check(context.Background()) + + assert.Equal(t, preflight.SeverityOK, results[0].Severity) +} + func TestCheckNSpawnMachineProvisioningNonEmptyMachineDir(t *testing.T) { gs := validRootFSGoalState(t) require.NoError(t, os.WriteFile(filepath.Join(gs.MachineDir, "file"), []byte("x"), 0o600)) From aeb3153102d62f36af436ef2320725132a7343fa Mon Sep 17 00:00:00 2001 From: Baichao He Date: Thu, 25 Jun 2026 20:01:30 +0000 Subject: [PATCH 10/16] agent: reduce preflight debug logging --- pkg/agent/phases/host/preflight_agent_config.go | 6 ------ pkg/agent/phases/host/preflight_cluster_credentials.go | 6 ------ pkg/agent/phases/nodestart/preflight_api_server.go | 8 -------- pkg/agent/phases/rootfs/preflight_goal_state.go | 2 -- 4 files changed, 22 deletions(-) diff --git a/pkg/agent/phases/host/preflight_agent_config.go b/pkg/agent/phases/host/preflight_agent_config.go index 948487ca2..654e4caac 100644 --- a/pkg/agent/phases/host/preflight_agent_config.go +++ b/pkg/agent/phases/host/preflight_agent_config.go @@ -30,15 +30,9 @@ func (c agentConfigChecker) Name() string { return checkAgentConfigName } // Check validates the shared agent config without mutating it. func (c agentConfigChecker) Check(context.Context) []preflight.Result { - c.log.Debug("checking agent config") - if err := c.config.Validate(); err != nil { - c.log.Debug("agent config validation failed") - return preflight.ResultsError(checkAgentConfigName, "agent config", "agent config is invalid") } - c.log.Debug("agent config validation passed") - return preflight.ResultsOK(checkAgentConfigName, "agent config", "agent config is valid") } diff --git a/pkg/agent/phases/host/preflight_cluster_credentials.go b/pkg/agent/phases/host/preflight_cluster_credentials.go index e995bb544..b8943f0b8 100644 --- a/pkg/agent/phases/host/preflight_cluster_credentials.go +++ b/pkg/agent/phases/host/preflight_cluster_credentials.go @@ -33,8 +33,6 @@ func (c clusterCredentialsChecker) Name() string { return checkClusterCredential // Check validates cluster credential inputs without printing credential values. func (c clusterCredentialsChecker) Check(context.Context) []preflight.Result { - c.log.Debug("checking cluster credentials", "attestationConfigured", c.attestationConfigured) - if c.config == nil { return preflight.ResultsError(checkClusterCredentialsName, "cluster credentials", "agent config is missing") } @@ -52,12 +50,8 @@ func (c clusterCredentialsChecker) Check(context.Context) []preflight.Result { } if len(errs) > 0 { - c.log.Debug("cluster credential validation failed", "errors", len(errs)) - return preflight.ResultsError(checkClusterCredentialsName, "cluster credentials", strings.Join(errs, "; ")) } - c.log.Debug("cluster credential validation passed") - return preflight.ResultsOK(checkClusterCredentialsName, "cluster credentials", "cluster credentials are valid") } diff --git a/pkg/agent/phases/nodestart/preflight_api_server.go b/pkg/agent/phases/nodestart/preflight_api_server.go index 0d11eb4cb..478dd7725 100644 --- a/pkg/agent/phases/nodestart/preflight_api_server.go +++ b/pkg/agent/phases/nodestart/preflight_api_server.go @@ -36,8 +36,6 @@ func CheckAPIServerReachable(log *slog.Logger, apiServer string, caCertData []by func (c apiServerReachableChecker) Name() string { return checkAPIServerReachableName } func (c apiServerReachableChecker) Check(ctx context.Context) []preflight.Result { - c.log.Debug("checking API server reachability", "target", "cluster API server", "caConfigured", len(c.caCertData) > 0) - if strings.TrimSpace(c.url) == "" { return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", "API server is required") } @@ -59,20 +57,14 @@ func (c apiServerReachableChecker) Check(ctx context.Context) []preflight.Result resp, err := client.Do(req) if err != nil { - c.log.Debug("API server reachability check failed", "target", "cluster API server") - return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", "API server is not reachable") } defer resp.Body.Close() //nolint:errcheck // best effort close if resp.StatusCode >= http.StatusInternalServerError { - c.log.Debug("API server reachability check returned server error", "target", "cluster API server", "status", resp.StatusCode) - return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", fmt.Sprintf("API server returned status %d", resp.StatusCode)) } - c.log.Debug("API server reachability check passed", "target", "cluster API server", "status", resp.StatusCode) - return preflight.ResultsOK(checkAPIServerReachableName, "cluster API server", "API server is reachable") } diff --git a/pkg/agent/phases/rootfs/preflight_goal_state.go b/pkg/agent/phases/rootfs/preflight_goal_state.go index 5dee3fe16..eacf34e80 100644 --- a/pkg/agent/phases/rootfs/preflight_goal_state.go +++ b/pkg/agent/phases/rootfs/preflight_goal_state.go @@ -30,8 +30,6 @@ func CheckGoalState(log *slog.Logger, err error, rootFS *goalstates.RootFS) pref func (c goalStateChecker) Name() string { return checkGoalStateName } func (c goalStateChecker) Check(context.Context) []preflight.Result { - c.log.Debug("checking goal state resolution") - if c.err != nil || c.rootFS == nil { return preflight.ResultsError(checkGoalStateName, "goal state", "goal state could not be resolved") } From 418eea8d36f1c1ec637049e6ff3e266f49669ff7 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Thu, 25 Jun 2026 20:11:48 +0000 Subject: [PATCH 11/16] agent: warn on restrictive nspawn machine permissions --- .../phases/nodestart/preflight_api_server.go | 3 +- pkg/agent/phases/rootfs/preflight_local.go | 43 +++++++++++++------ .../phases/rootfs/preflight_local_test.go | 36 +++++++++++++++- pkg/agent/preflight/preflight.go | 36 ++++++++++------ pkg/agent/preflight/preflight_test.go | 7 +++ 5 files changed, 98 insertions(+), 27 deletions(-) diff --git a/pkg/agent/phases/nodestart/preflight_api_server.go b/pkg/agent/phases/nodestart/preflight_api_server.go index 478dd7725..83a8e536b 100644 --- a/pkg/agent/phases/nodestart/preflight_api_server.go +++ b/pkg/agent/phases/nodestart/preflight_api_server.go @@ -7,7 +7,6 @@ import ( "context" "crypto/tls" "crypto/x509" - "fmt" "log/slog" "net/http" "net/url" @@ -62,7 +61,7 @@ func (c apiServerReachableChecker) Check(ctx context.Context) []preflight.Result defer resp.Body.Close() //nolint:errcheck // best effort close if resp.StatusCode >= http.StatusInternalServerError { - return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", fmt.Sprintf("API server returned status %d", resp.StatusCode)) + return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", "API server returned status %d", resp.StatusCode) } return preflight.ResultsOK(checkAPIServerReachableName, "cluster API server", "API server is reachable") diff --git a/pkg/agent/phases/rootfs/preflight_local.go b/pkg/agent/phases/rootfs/preflight_local.go index adf17b009..b13c1c3c2 100644 --- a/pkg/agent/phases/rootfs/preflight_local.go +++ b/pkg/agent/phases/rootfs/preflight_local.go @@ -64,12 +64,10 @@ func (c nspawnMachineProvisioningChecker) Check(context.Context) []preflight.Res ) } - if result := c.checkMachineDir(); result != nil { - return result - } + results := c.checkMachineDir() if result := c.checkCreatableDir(filepath.Dir(c.gs.MachineDir), "rootfs parent directory"); result != nil { - return result + results = append(results, result...) } for _, path := range []string{ @@ -77,10 +75,14 @@ func (c nspawnMachineProvisioningChecker) Check(context.Context) []preflight.Res filepath.Dir(c.gs.ServiceOverrideFile), } { if result := c.checkCreatableDir(path, "nspawn provisioning path"); result != nil { - return result + results = append(results, result...) } } + if len(results) > 0 { + return results + } + return preflight.ResultsOK( checkNSpawnMachineProvisioningName, "nspawn machine provisioning", @@ -90,6 +92,7 @@ func (c nspawnMachineProvisioningChecker) Check(context.Context) []preflight.Res func (c nspawnMachineProvisioningChecker) checkMachineDir() []preflight.Result { c.log.Debug("checking machine directory", "path", c.gs.MachineDir) + var results []preflight.Result info, err := c.deps.stat(c.gs.MachineDir) switch { @@ -100,23 +103,37 @@ func (c nspawnMachineProvisioningChecker) checkMachineDir() []preflight.Result { return preflight.ResultsError( checkNSpawnMachineProvisioningName, "nspawn machine provisioning", - "machine directory cannot be inspected: "+c.gs.MachineDir, + "machine directory cannot be inspected: %s", + c.gs.MachineDir, ) case !info.IsDir(): return preflight.ResultsError( checkNSpawnMachineProvisioningName, "nspawn machine provisioning", - "machine directory path is not a directory: "+c.gs.MachineDir, + "machine directory path is not a directory: %s", + c.gs.MachineDir, ) } + // The nspawn machine root needs traversal permissions so dbus inside the + // container can operate correctly when reusing an existing rootfs. + if info.Mode().Perm()&0o055 != 0o055 { + results = append(results, preflight.Warning( + checkNSpawnMachineProvisioningName, + "nspawn machine provisioning", + "machine directory permissions are too restrictive: %s", + c.gs.MachineDir, + )) + } + empty, err := isDirEmpty(c.deps.open, c.gs.MachineDir) if err != nil { - return preflight.ResultsError( + return append(results, preflight.Error( checkNSpawnMachineProvisioningName, "nspawn machine provisioning", - "machine directory cannot be read: "+c.gs.MachineDir, - ) + "machine directory cannot be read: %s", + c.gs.MachineDir, + )) } if !empty { @@ -128,7 +145,7 @@ func (c nspawnMachineProvisioningChecker) checkMachineDir() []preflight.Result { c.log.Debug("machine directory exists and is empty", "path", c.gs.MachineDir) } - return nil + return results } func (c nspawnMachineProvisioningChecker) checkCreatableDir(path, label string) []preflight.Result { @@ -139,7 +156,9 @@ func (c nspawnMachineProvisioningChecker) checkCreatableDir(path, label string) return preflight.ResultsError( checkNSpawnMachineProvisioningName, "nspawn machine provisioning", - label+" cannot be created under: "+existing, + "%s cannot be created under: %s", + label, + existing, ) } diff --git a/pkg/agent/phases/rootfs/preflight_local_test.go b/pkg/agent/phases/rootfs/preflight_local_test.go index bd28963df..858dfbac2 100644 --- a/pkg/agent/phases/rootfs/preflight_local_test.go +++ b/pkg/agent/phases/rootfs/preflight_local_test.go @@ -63,6 +63,31 @@ func TestCheckNSpawnMachineProvisioningMachineDirFile(t *testing.T) { assert.Equal(t, preflight.SeverityError, results[0].Severity) } +func TestCheckNSpawnMachineProvisioningRestrictiveMachineDir(t *testing.T) { + gs := validRootFSGoalState(t) + require.NoError(t, os.Chmod(gs.MachineDir, 0o700)) + + results := CheckNSpawnMachineProvisioning(slog.New(slog.DiscardHandler), gs).Check(context.Background()) + + assert.Equal(t, preflight.SeverityWarning, results[0].Severity) + assert.Equal(t, "machine directory permissions are too restrictive: "+gs.MachineDir, results[0].Message) +} + +func TestCheckNSpawnMachineProvisioningCollectsWarningAndError(t *testing.T) { + gs := validRootFSGoalState(t) + deps := defaultRootFSCheckDeps() + deps.stat = func(string) (fs.FileInfo, error) { return fakeRestrictiveDirInfo{}, nil } + deps.open = func(string) (*os.File, error) { return nil, errors.New("denied") } + deps.writeProbe = func(string) error { return errors.New("denied") } + + results := checkNSpawnMachineProvisioning(slog.New(slog.DiscardHandler), gs, deps).Check(context.Background()) + + require.GreaterOrEqual(t, len(results), 2) + assert.Equal(t, preflight.SeverityWarning, results[0].Severity) + assert.Equal(t, preflight.SeverityError, results[1].Severity) + assert.Equal(t, "machine directory cannot be read: "+gs.MachineDir, results[1].Message) +} + func TestCheckNSpawnMachineProvisioningWritablePaths(t *testing.T) { gs := validRootFSGoalState(t) deps := defaultRootFSCheckDeps() @@ -91,7 +116,16 @@ type fakeDirInfo struct{} func (fakeDirInfo) Name() string { return "dir" } func (fakeDirInfo) Size() int64 { return 0 } -func (fakeDirInfo) Mode() fs.FileMode { return fs.ModeDir } +func (fakeDirInfo) Mode() fs.FileMode { return fs.ModeDir | 0o755 } func (fakeDirInfo) ModTime() time.Time { return time.Time{} } func (fakeDirInfo) IsDir() bool { return true } func (fakeDirInfo) Sys() any { return nil } + +type fakeRestrictiveDirInfo struct{} + +func (fakeRestrictiveDirInfo) Name() string { return "dir" } +func (fakeRestrictiveDirInfo) Size() int64 { return 0 } +func (fakeRestrictiveDirInfo) Mode() fs.FileMode { return fs.ModeDir | 0o700 } +func (fakeRestrictiveDirInfo) ModTime() time.Time { return time.Time{} } +func (fakeRestrictiveDirInfo) IsDir() bool { return true } +func (fakeRestrictiveDirInfo) Sys() any { return nil } diff --git a/pkg/agent/preflight/preflight.go b/pkg/agent/preflight/preflight.go index f34110ec9..ce7bffb25 100644 --- a/pkg/agent/preflight/preflight.go +++ b/pkg/agent/preflight/preflight.go @@ -110,14 +110,16 @@ func OK(name, target, message string) Result { return Result{Name: name, Target: target, Severity: SeverityOK, Message: message} } -// Warning returns a warning check result. -func Warning(name, target, message string) Result { - return Result{Name: name, Target: target, Severity: SeverityWarning, Message: message} +// Warning returns a warning check result. When args are provided, message is +// formatted with fmt.Sprintf. +func Warning(name, target, message string, args ...any) Result { + return Result{Name: name, Target: target, Severity: SeverityWarning, Message: formatMessage(message, args...)} } -// Error returns a fatal check result. -func Error(name, target, message string) Result { - return Result{Name: name, Target: target, Severity: SeverityError, Message: message} +// Error returns a fatal check result. When args are provided, message is +// formatted with fmt.Sprintf. +func Error(name, target, message string, args ...any) Result { + return Result{Name: name, Target: target, Severity: SeverityError, Message: formatMessage(message, args...)} } // Results returns a result slice for concise checker returns and test fixtures. @@ -130,14 +132,24 @@ func ResultsOK(name, target, message string) []Result { return Results(OK(name, target, message)) } -// ResultsWarning returns a single warning check result as a slice. -func ResultsWarning(name, target, message string) []Result { - return Results(Warning(name, target, message)) +// ResultsWarning returns a single warning check result as a slice. When args +// are provided, message is formatted with fmt.Sprintf. +func ResultsWarning(name, target, message string, args ...any) []Result { + return Results(Warning(name, target, message, args...)) } -// ResultsError returns a single fatal check result as a slice. -func ResultsError(name, target, message string) []Result { - return Results(Error(name, target, message)) +// ResultsError returns a single fatal check result as a slice. When args are +// provided, message is formatted with fmt.Sprintf. +func ResultsError(name, target, message string, args ...any) []Result { + return Results(Error(name, target, message, args...)) +} + +func formatMessage(message string, args ...any) string { + if len(args) == 0 { + return message + } + + return fmt.Sprintf(message, args...) } // HasErrors reports whether any fatal errors remain after ignore handling. diff --git a/pkg/agent/preflight/preflight_test.go b/pkg/agent/preflight/preflight_test.go index 0ce04df4b..431de6e6f 100644 --- a/pkg/agent/preflight/preflight_test.go +++ b/pkg/agent/preflight/preflight_test.go @@ -61,6 +61,13 @@ func TestRunIgnoreAll(t *testing.T) { assert.True(t, report.Checks[0].Ignored) } +func TestFormattedWarningAndErrorMessages(t *testing.T) { + assert.Equal(t, "mode 700 is too restrictive", Warning("machine-dir", "machine directory", "mode %o is too restrictive", 0o700).Message) + assert.Equal(t, "status 500", Error("api-server", "cluster API server", "status %d", 500).Message) + assert.Equal(t, "path /var/lib/machines/kube1", ResultsWarning("machine-dir", "machine directory", "path %s", "/var/lib/machines/kube1")[0].Message) + assert.Equal(t, "path /var/lib/machines/kube1", ResultsError("machine-dir", "machine directory", "path %s", "/var/lib/machines/kube1")[0].Message) +} + func TestRunPreservesInputOrderWhileRunningConcurrently(t *testing.T) { release := make(chan struct{}) started := make(chan string, 2) From ed6611c72b8e200d355af4e0645fefd2214b46d6 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Thu, 25 Jun 2026 20:17:41 +0000 Subject: [PATCH 12/16] agent: collect host preflight findings --- .../host/preflight_cluster_credentials.go | 2 +- pkg/agent/phases/host/preflight_host.go | 59 ++++++++++++------- pkg/agent/phases/host/preflight_host_test.go | 9 +++ 3 files changed, 47 insertions(+), 23 deletions(-) diff --git a/pkg/agent/phases/host/preflight_cluster_credentials.go b/pkg/agent/phases/host/preflight_cluster_credentials.go index b8943f0b8..3ab68ec9c 100644 --- a/pkg/agent/phases/host/preflight_cluster_credentials.go +++ b/pkg/agent/phases/host/preflight_cluster_credentials.go @@ -50,7 +50,7 @@ func (c clusterCredentialsChecker) Check(context.Context) []preflight.Result { } if len(errs) > 0 { - return preflight.ResultsError(checkClusterCredentialsName, "cluster credentials", strings.Join(errs, "; ")) + return preflight.ResultsError(checkClusterCredentialsName, "cluster credentials", "%s", strings.Join(errs, "; ")) } return preflight.ResultsOK(checkClusterCredentialsName, "cluster credentials", "cluster credentials are valid") diff --git a/pkg/agent/phases/host/preflight_host.go b/pkg/agent/phases/host/preflight_host.go index 7210b69db..b6682eb8b 100644 --- a/pkg/agent/phases/host/preflight_host.go +++ b/pkg/agent/phases/host/preflight_host.go @@ -6,7 +6,6 @@ package host import ( "bufio" "context" - "fmt" "io/fs" "log/slog" "os" @@ -127,7 +126,8 @@ func checkHostPackages(log *slog.Logger, deps hostCheckDeps) preflight.Checker { return preflight.ResultsWarning( checkHostPackagesName, "host packages", - "required host packages are missing and may be installed by bootstrap: "+strings.Join(missing, ", "), + "required host packages are missing and may be installed by bootstrap: %s", + strings.Join(missing, ", "), ) } @@ -144,25 +144,33 @@ func CheckHostOSConfiguration(log *slog.Logger) preflight.Checker { func checkHostOSConfiguration(log *slog.Logger, deps hostCheckDeps) preflight.Checker { return simpleHostChecker{name: checkHostOSConfigurationName, check: func(context.Context) []preflight.Result { + var results []preflight.Result + sysctlDir := filepath.Dir(hostSysctlPath) log.Debug("checking host OS configuration path", "path", sysctlDir) if err := deps.writeProbe(sysctlDir); err != nil { - return preflight.ResultsError( + results = append(results, preflight.Error( checkHostOSConfigurationName, "host OS configuration", - "host OS configuration path is not writable: "+sysctlDir, - ) + "host OS configuration path is not writable: %s", + sysctlDir, + )) } log.Debug("checking systemd unit directory", "path", goalstates.SystemdSystemDir) if err := deps.writeProbe(goalstates.SystemdSystemDir); err != nil { - return preflight.ResultsError( + results = append(results, preflight.Error( checkHostOSConfigurationName, "host OS configuration", - "systemd unit directory is not writable: "+goalstates.SystemdSystemDir, - ) + "systemd unit directory is not writable: %s", + goalstates.SystemdSystemDir, + )) + } + + if len(results) > 0 { + return results } return preflight.ResultsOK( @@ -180,6 +188,8 @@ func CheckNSpawnRuntime(log *slog.Logger) preflight.Checker { func checkNSpawnRuntime(log *slog.Logger, deps hostCheckDeps) preflight.Checker { return simpleHostChecker{name: checkNSpawnRuntimeName, check: func(context.Context) []preflight.Result { + var results []preflight.Result + for _, binary := range []string{"systemctl", "machinectl", "systemd-nspawn"} { log.Debug("checking nspawn runtime tool", "binary", binary) @@ -187,11 +197,12 @@ func checkNSpawnRuntime(log *slog.Logger, deps hostCheckDeps) preflight.Checker // TODO: when offline mode is configured, missing nspawn runtime // tools should be reported as an error because bootstrap cannot rely // on package installation to remediate them. - return preflight.ResultsWarning( + results = append(results, preflight.Warning( checkNSpawnRuntimeName, "nspawn runtime", - "nspawn runtime tool is missing and may be installed by bootstrap: "+binary, - ) + "nspawn runtime tool is missing and may be installed by bootstrap: %s", + binary, + )) } } @@ -199,11 +210,16 @@ func checkNSpawnRuntime(log *slog.Logger, deps hostCheckDeps) preflight.Checker log.Debug("checking systemd runtime path", "path", systemdRuntimePath) if _, err := deps.stat(systemdRuntimePath); err != nil { - return preflight.ResultsWarning( + results = append(results, preflight.Warning( checkNSpawnRuntimeName, "nspawn runtime", - "systemd runtime path is not currently available: "+systemdRuntimePath, - ) + "systemd runtime path is not currently available: %s", + systemdRuntimePath, + )) + } + + if len(results) > 0 { + return results } return preflight.ResultsOK(checkNSpawnRuntimeName, "nspawn runtime", "nspawn runtime is available") @@ -279,7 +295,8 @@ func checkDiskSpace(log *slog.Logger, deps hostCheckDeps) preflight.Checker { return preflight.ResultsError( checkDiskSpaceName, "host disk", - "available disk space could not be determined for "+diskPath, + "available disk space could not be determined for %s", + diskPath, ) } @@ -295,12 +312,10 @@ func checkDiskSpace(log *slog.Logger, deps hostCheckDeps) preflight.Checker { return preflight.ResultsError( checkDiskSpaceName, "host disk", - fmt.Sprintf( - "available disk space is below the minimum for %s: current %.1f GiB, required %.1f GiB", - diskPath, - gib(free), - gib(minFreeDiskBytes), - ), + "available disk space is below the minimum for %s: current %.1f GiB, required %.1f GiB", + diskPath, + gib(free), + gib(minFreeDiskBytes), ) } @@ -319,7 +334,7 @@ func checkCgroups(log *slog.Logger, deps hostCheckDeps) preflight.Checker { log.Debug("checking cgroup filesystem", "path", cgroupPath) if _, err := deps.stat(cgroupPath); err != nil { - return preflight.ResultsError(checkCgroupsName, "host cgroups", "cgroup filesystem is required at "+cgroupPath) + return preflight.ResultsError(checkCgroupsName, "host cgroups", "cgroup filesystem is required at %s", cgroupPath) } return preflight.ResultsOK(checkCgroupsName, "host cgroups", "cgroup filesystem is available") diff --git a/pkg/agent/phases/host/preflight_host_test.go b/pkg/agent/phases/host/preflight_host_test.go index 2ef3978c4..6fa5127f1 100644 --- a/pkg/agent/phases/host/preflight_host_test.go +++ b/pkg/agent/phases/host/preflight_host_test.go @@ -54,8 +54,11 @@ func TestCheckHostOSConfiguration(t *testing.T) { deps.writeProbe = func(string) error { return errors.New("denied") } results = checkHostOSConfiguration(slog.New(slog.DiscardHandler), deps).Check(context.Background()) + assert.Len(t, results, 2) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Contains(t, results[0].Message, "/etc/sysctl.d") + assert.Equal(t, preflight.SeverityError, results[1].Severity) + assert.Contains(t, results[1].Message, "systemd") } func TestCheckNSpawnRuntime(t *testing.T) { @@ -71,9 +74,15 @@ func TestCheckNSpawnRuntime(t *testing.T) { assert.Equal(t, preflight.SeverityOK, results[0].Severity) deps.lookupPath = lookupPathWith(map[string]bool{"systemctl": true}) + deps.stat = statMissing() results = checkNSpawnRuntime(slog.New(slog.DiscardHandler), deps).Check(context.Background()) + assert.Len(t, results, 3) assert.Equal(t, preflight.SeverityWarning, results[0].Severity) assert.Contains(t, results[0].Message, "machinectl") + assert.Equal(t, preflight.SeverityWarning, results[1].Severity) + assert.Contains(t, results[1].Message, "systemd-nspawn") + assert.Equal(t, preflight.SeverityWarning, results[2].Severity) + assert.Contains(t, results[2].Message, "/run/systemd/system") } func TestCheckDockerActive(t *testing.T) { From 182429159739e8c9278d0dd0e049bf7c980728fc Mon Sep 17 00:00:00 2001 From: Baichao He Date: Thu, 25 Jun 2026 20:27:15 +0000 Subject: [PATCH 13/16] agent: fix preflight lint spacing --- pkg/agent/phases/rootfs/preflight_local.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/agent/phases/rootfs/preflight_local.go b/pkg/agent/phases/rootfs/preflight_local.go index b13c1c3c2..6de784011 100644 --- a/pkg/agent/phases/rootfs/preflight_local.go +++ b/pkg/agent/phases/rootfs/preflight_local.go @@ -91,9 +91,10 @@ func (c nspawnMachineProvisioningChecker) Check(context.Context) []preflight.Res } func (c nspawnMachineProvisioningChecker) checkMachineDir() []preflight.Result { - c.log.Debug("checking machine directory", "path", c.gs.MachineDir) var results []preflight.Result + c.log.Debug("checking machine directory", "path", c.gs.MachineDir) + info, err := c.deps.stat(c.gs.MachineDir) switch { case errors.Is(err, os.ErrNotExist): From c65729e3d42c0189d8ce4d3dd664e602e4ce1d32 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Thu, 25 Jun 2026 20:47:15 +0000 Subject: [PATCH 14/16] agent: simplify preflight checker inputs --- cmd/agent/internal/cmd/preflight.go | 46 ++++++++----------- .../host/preflight_cluster_credentials.go | 19 ++++---- .../preflight_cluster_credentials_test.go | 22 ++++++--- .../phases/nodestart/preflight_api_server.go | 25 ++++++---- .../nodestart/preflight_api_server_test.go | 20 ++++++-- .../phases/rootfs/preflight_goal_state.go | 7 ++- .../rootfs/preflight_goal_state_test.go | 5 +- pkg/agent/phases/rootfs/preflight_local.go | 19 ++++---- 8 files changed, 90 insertions(+), 73 deletions(-) diff --git a/cmd/agent/internal/cmd/preflight.go b/cmd/agent/internal/cmd/preflight.go index 27fc5e164..58ee6321a 100644 --- a/cmd/agent/internal/cmd/preflight.go +++ b/cmd/agent/internal/cmd/preflight.go @@ -5,7 +5,6 @@ package cmd import ( "context" - "encoding/base64" "encoding/json" "fmt" "io" @@ -58,6 +57,7 @@ func newCmdPreflight(cmdCtx *CommandContext) *cobra.Command { func (h *preflightHandler) execute(ctx context.Context) error { h.cmdCtx.Setup() + logger := h.cmdCtx.Logger if h.configPath != "" { oldConfigPath := os.Getenv(configFileEnv) @@ -68,43 +68,35 @@ func (h *preflightHandler) execute(ctx context.Context) error { } } - cfg, err := loadConfig(h.cmdCtx.Logger) + cfg, err := loadConfig(logger) if err != nil { return err } - caCertData, err := base64.StdEncoding.DecodeString(cfg.Cluster.CaCertBase64) - if err != nil { - caCertData = nil - } - - goalState, goalStateErr := goalstates.ResolveMachine( - h.cmdCtx.Logger, + goalState, err := goalstates.ResolveMachine( + logger, &cfg.AgentConfig, goalstates.NSpawnMachineKube1, provision.ResolveDownloadOverrides(cfg.Downloads), ) - - var rootFSGoalState *goalstates.RootFS - - if goalState != nil { - rootFSGoalState = goalState.RootFS + if err != nil { + return fmt.Errorf("resolve goal state: %w", err) } checks := []preflight.Checker{ - host.CheckIsPrivilegedUser(h.cmdCtx.Logger), - host.CheckAgentConfig(h.cmdCtx.Logger, &cfg.AgentConfig), - host.CheckClusterCredentials(h.cmdCtx.Logger, &cfg.AgentConfig, cfg.Attest != nil), - host.CheckHostPackages(h.cmdCtx.Logger), - host.CheckHostOSConfiguration(h.cmdCtx.Logger), - host.CheckNSpawnRuntime(h.cmdCtx.Logger), - host.CheckDockerActive(h.cmdCtx.Logger), - host.CheckSwapActive(h.cmdCtx.Logger), - host.CheckDiskSpace(h.cmdCtx.Logger), - host.CheckCgroups(h.cmdCtx.Logger), - nodestart.CheckAPIServerReachable(h.cmdCtx.Logger, cfg.Kubelet.ApiServer, caCertData), - rootfs.CheckGoalState(h.cmdCtx.Logger, goalStateErr, rootFSGoalState), - rootfs.CheckNSpawnMachineProvisioning(h.cmdCtx.Logger, rootFSGoalState), + host.CheckIsPrivilegedUser(logger), + host.CheckAgentConfig(logger, &cfg.AgentConfig), + host.CheckClusterCredentials(logger, cfg), + host.CheckHostPackages(logger), + host.CheckHostOSConfiguration(logger), + host.CheckNSpawnRuntime(logger), + host.CheckDockerActive(logger), + host.CheckSwapActive(logger), + host.CheckDiskSpace(logger), + host.CheckCgroups(logger), + nodestart.CheckAPIServerReachable(logger, &cfg.AgentConfig), + rootfs.CheckGoalState(logger, goalState.RootFS), + rootfs.CheckNSpawnMachineProvisioning(logger, goalState.RootFS), } opts := preflight.Options{ diff --git a/pkg/agent/phases/host/preflight_cluster_credentials.go b/pkg/agent/phases/host/preflight_cluster_credentials.go index 3ab68ec9c..babe7229d 100644 --- a/pkg/agent/phases/host/preflight_cluster_credentials.go +++ b/pkg/agent/phases/host/preflight_cluster_credentials.go @@ -9,23 +9,22 @@ import ( "log/slog" "strings" - "github.com/Azure/unbounded/pkg/agent/config" + "github.com/Azure/unbounded/internal/provision" "github.com/Azure/unbounded/pkg/agent/preflight" ) const checkClusterCredentialsName = "cluster-credentials" type clusterCredentialsChecker struct { - log *slog.Logger - config *config.AgentConfig - attestationConfigured bool + log *slog.Logger + config *provision.UnboundedAgentConfig } // CheckClusterCredentials returns a checker that validates cluster CA data and -// the bootstrap credential. When attestationConfigured is true, missing kubelet -// auth is allowed because attestation can provide the credential later. -func CheckClusterCredentials(log *slog.Logger, cfg *config.AgentConfig, attestationConfigured bool) preflight.Checker { - return clusterCredentialsChecker{log: log, config: cfg, attestationConfigured: attestationConfigured} +// the bootstrap credential. When attestation is configured, missing kubelet auth +// is allowed because attestation can provide the credential later. +func CheckClusterCredentials(log *slog.Logger, cfg *provision.UnboundedAgentConfig) preflight.Checker { + return clusterCredentialsChecker{log: log, config: cfg} } // Name returns the stable check name used in reports and ignore rules. @@ -42,8 +41,8 @@ func (c clusterCredentialsChecker) Check(context.Context) []preflight.Result { errs = append(errs, "cluster CA data is invalid") } - auth := c.config.Kubelet.Auth - if !c.attestationConfigured { + if c.config.Attest == nil { + auth := c.config.Kubelet.Auth if err := auth.Validate(); err != nil { errs = append(errs, "bootstrap credential is invalid") } diff --git a/pkg/agent/phases/host/preflight_cluster_credentials_test.go b/pkg/agent/phases/host/preflight_cluster_credentials_test.go index 0a9f214f1..14243d322 100644 --- a/pkg/agent/phases/host/preflight_cluster_credentials_test.go +++ b/pkg/agent/phases/host/preflight_cluster_credentials_test.go @@ -10,29 +10,31 @@ import ( "github.com/stretchr/testify/assert" + "github.com/Azure/unbounded/internal/provision" "github.com/Azure/unbounded/pkg/agent/preflight" ) func TestCheckClusterCredentialsValid(t *testing.T) { - results := CheckClusterCredentials(slog.New(slog.DiscardHandler), validPreflightConfig(), false).Check(context.Background()) + results := CheckClusterCredentials(slog.New(slog.DiscardHandler), validUnboundedPreflightConfig()).Check(context.Background()) assert.Equal(t, preflight.ResultsOK(checkClusterCredentialsName, "cluster credentials", "cluster credentials are valid"), results) } func TestCheckClusterCredentialsAllowsAttestation(t *testing.T) { - cfg := validPreflightConfig() + cfg := validUnboundedPreflightConfig() cfg.Kubelet.Auth.BootstrapToken = "" + cfg.Attest = &provision.AgentAttestConfig{URL: "http://metalman.example.com:8880"} - results := CheckClusterCredentials(slog.New(slog.DiscardHandler), cfg, true).Check(context.Background()) + results := CheckClusterCredentials(slog.New(slog.DiscardHandler), cfg).Check(context.Background()) assert.Equal(t, preflight.SeverityOK, results[0].Severity) } func TestCheckClusterCredentialsRequiresAuthWhenNoAttestation(t *testing.T) { - cfg := validPreflightConfig() + cfg := validUnboundedPreflightConfig() cfg.Kubelet.Auth.BootstrapToken = "" - results := CheckClusterCredentials(slog.New(slog.DiscardHandler), cfg, false).Check(context.Background()) + results := CheckClusterCredentials(slog.New(slog.DiscardHandler), cfg).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Equal(t, checkClusterCredentialsName, results[0].Name) @@ -41,11 +43,17 @@ func TestCheckClusterCredentialsRequiresAuthWhenNoAttestation(t *testing.T) { } func TestCheckClusterCredentialsInvalidCA(t *testing.T) { - cfg := validPreflightConfig() + cfg := validUnboundedPreflightConfig() cfg.Cluster.CaCertBase64 = "not-base64" - results := CheckClusterCredentials(slog.New(slog.DiscardHandler), cfg, false).Check(context.Background()) + results := CheckClusterCredentials(slog.New(slog.DiscardHandler), cfg).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Contains(t, results[0].Message, "cluster CA data is invalid") } + +func validUnboundedPreflightConfig() *provision.UnboundedAgentConfig { + return &provision.UnboundedAgentConfig{ + AgentConfig: *validPreflightConfig(), + } +} diff --git a/pkg/agent/phases/nodestart/preflight_api_server.go b/pkg/agent/phases/nodestart/preflight_api_server.go index 83a8e536b..3701d96fc 100644 --- a/pkg/agent/phases/nodestart/preflight_api_server.go +++ b/pkg/agent/phases/nodestart/preflight_api_server.go @@ -7,12 +7,14 @@ import ( "context" "crypto/tls" "crypto/x509" + "encoding/base64" "log/slog" "net/http" "net/url" "strings" "time" + "github.com/Azure/unbounded/pkg/agent/config" "github.com/Azure/unbounded/pkg/agent/preflight" ) @@ -20,26 +22,30 @@ const checkAPIServerReachableName = "api-server-reachable" type apiServerReachableChecker struct { log *slog.Logger - url string - caCertData []byte + config *config.AgentConfig httpClient *http.Client } // CheckAPIServerReachable returns a non-mutating checker that validates the // configured Kubernetes API server can be reached from the host. The checker // redacts the configured endpoint from result messages. -func CheckAPIServerReachable(log *slog.Logger, apiServer string, caCertData []byte) preflight.Checker { - return apiServerReachableChecker{log: log, url: apiServer, caCertData: caCertData} +func CheckAPIServerReachable(log *slog.Logger, cfg *config.AgentConfig) preflight.Checker { + return apiServerReachableChecker{log: log, config: cfg} } func (c apiServerReachableChecker) Name() string { return checkAPIServerReachableName } func (c apiServerReachableChecker) Check(ctx context.Context) []preflight.Result { - if strings.TrimSpace(c.url) == "" { + if c.config == nil { + return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", "agent config is missing") + } + + apiServer := c.config.Kubelet.ApiServer + if strings.TrimSpace(apiServer) == "" { return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", "API server is required") } - parsed, err := url.Parse(c.url) + parsed, err := url.Parse(apiServer) if err != nil || parsed.Scheme == "" || parsed.Host == "" { return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", "API server endpoint is invalid") } @@ -49,7 +55,7 @@ func (c apiServerReachableChecker) Check(ctx context.Context) []preflight.Result client = c.httpClientWithCA() } - req, err := http.NewRequestWithContext(ctx, http.MethodGet, strings.TrimRight(c.url, "/")+"/readyz", http.NoBody) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, strings.TrimRight(apiServer, "/")+"/readyz", http.NoBody) if err != nil { return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", "API server request could not be created") } @@ -73,13 +79,14 @@ func (c apiServerReachableChecker) httpClientWithCA() *http.Client { transport = defaultTransport.Clone() } - if len(c.caCertData) > 0 { + caCertData, err := base64.StdEncoding.DecodeString(c.config.Cluster.CaCertBase64) + if err == nil && len(caCertData) > 0 { pool, err := x509.SystemCertPool() if err != nil { pool = x509.NewCertPool() } - pool.AppendCertsFromPEM(c.caCertData) + pool.AppendCertsFromPEM(caCertData) transport.TLSClientConfig = &tls.Config{RootCAs: pool} //nolint:gosec // uses configured root CAs. } diff --git a/pkg/agent/phases/nodestart/preflight_api_server_test.go b/pkg/agent/phases/nodestart/preflight_api_server_test.go index 6bbf5b3f4..556ac9ac2 100644 --- a/pkg/agent/phases/nodestart/preflight_api_server_test.go +++ b/pkg/agent/phases/nodestart/preflight_api_server_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/assert" + "github.com/Azure/unbounded/pkg/agent/config" "github.com/Azure/unbounded/pkg/agent/preflight" ) @@ -22,13 +23,13 @@ func TestCheckAPIServerReachableOK(t *testing.T) { })) t.Cleanup(srv.Close) - results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), srv.URL, nil).Check(context.Background()) + results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), apiServerPreflightConfig(srv.URL)).Check(context.Background()) assert.Equal(t, preflight.ResultsOK(checkAPIServerReachableName, "cluster API server", "API server is reachable"), results) } func TestCheckAPIServerReachableInvalidEndpoint(t *testing.T) { - results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), "://bad", nil).Check(context.Background()) + results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), apiServerPreflightConfig("://bad")).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Equal(t, checkAPIServerReachableName, results[0].Name) @@ -39,7 +40,7 @@ func TestCheckAPIServerReachableInvalidEndpoint(t *testing.T) { func TestCheckAPIServerReachableRequestFailureIsRedacted(t *testing.T) { const endpoint = "https://127.0.0.1:1" - results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), endpoint, nil).Check(context.Background()) + results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), apiServerPreflightConfig(endpoint)).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Equal(t, "API server is not reachable", results[0].Message) @@ -52,8 +53,19 @@ func TestCheckAPIServerReachableServerError(t *testing.T) { })) t.Cleanup(srv.Close) - results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), srv.URL, nil).Check(context.Background()) + results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), apiServerPreflightConfig(srv.URL)).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Equal(t, "API server returned status 500", results[0].Message) } + +func apiServerPreflightConfig(apiServer string) *config.AgentConfig { + return &config.AgentConfig{ + Cluster: config.AgentClusterConfig{ + CaCertBase64: "Y2E=", + }, + Kubelet: config.AgentKubeletConfig{ + ApiServer: apiServer, + }, + } +} diff --git a/pkg/agent/phases/rootfs/preflight_goal_state.go b/pkg/agent/phases/rootfs/preflight_goal_state.go index eacf34e80..ee082b02f 100644 --- a/pkg/agent/phases/rootfs/preflight_goal_state.go +++ b/pkg/agent/phases/rootfs/preflight_goal_state.go @@ -18,19 +18,18 @@ const ( type goalStateChecker struct { log *slog.Logger - err error rootFS *goalstates.RootFS } // CheckGoalState validates the agent config resolved into a machine goal state. -func CheckGoalState(log *slog.Logger, err error, rootFS *goalstates.RootFS) preflight.Checker { - return goalStateChecker{log: log, err: err, rootFS: rootFS} +func CheckGoalState(log *slog.Logger, rootFS *goalstates.RootFS) preflight.Checker { + return goalStateChecker{log: log, rootFS: rootFS} } func (c goalStateChecker) Name() string { return checkGoalStateName } func (c goalStateChecker) Check(context.Context) []preflight.Result { - if c.err != nil || c.rootFS == nil { + if c.rootFS == nil { return preflight.ResultsError(checkGoalStateName, "goal state", "goal state could not be resolved") } diff --git a/pkg/agent/phases/rootfs/preflight_goal_state_test.go b/pkg/agent/phases/rootfs/preflight_goal_state_test.go index 3626cbf91..455bffa04 100644 --- a/pkg/agent/phases/rootfs/preflight_goal_state_test.go +++ b/pkg/agent/phases/rootfs/preflight_goal_state_test.go @@ -5,7 +5,6 @@ package rootfs import ( "context" - "errors" "log/slog" "testing" @@ -29,13 +28,13 @@ func validRootFSGoalState(t *testing.T) *goalstates.RootFS { } func TestCheckGoalStateOK(t *testing.T) { - results := CheckGoalState(slog.New(slog.DiscardHandler), nil, validRootFSGoalState(t)).Check(context.Background()) + results := CheckGoalState(slog.New(slog.DiscardHandler), validRootFSGoalState(t)).Check(context.Background()) assert.Equal(t, []preflight.Result{preflight.OK(checkGoalStateName, "goal state", "goal state resolved")}, results) } func TestCheckGoalStateResolveError(t *testing.T) { - results := CheckGoalState(slog.New(slog.DiscardHandler), errors.New("boom"), nil).Check(context.Background()) + results := CheckGoalState(slog.New(slog.DiscardHandler), nil).Check(context.Background()) assert.Equal(t, preflight.SeverityError, results[0].Severity) assert.Equal(t, checkGoalStateName, results[0].Name) diff --git a/pkg/agent/phases/rootfs/preflight_local.go b/pkg/agent/phases/rootfs/preflight_local.go index 6de784011..f558d6ca4 100644 --- a/pkg/agent/phases/rootfs/preflight_local.go +++ b/pkg/agent/phases/rootfs/preflight_local.go @@ -93,9 +93,10 @@ func (c nspawnMachineProvisioningChecker) Check(context.Context) []preflight.Res func (c nspawnMachineProvisioningChecker) checkMachineDir() []preflight.Result { var results []preflight.Result - c.log.Debug("checking machine directory", "path", c.gs.MachineDir) + machineDir := c.gs.MachineDir + c.log.Debug("checking machine directory", "path", machineDir) - info, err := c.deps.stat(c.gs.MachineDir) + info, err := c.deps.stat(machineDir) switch { case errors.Is(err, os.ErrNotExist): // Missing machine directory is fine if the parent/provisioning paths are writable. @@ -105,14 +106,14 @@ func (c nspawnMachineProvisioningChecker) checkMachineDir() []preflight.Result { checkNSpawnMachineProvisioningName, "nspawn machine provisioning", "machine directory cannot be inspected: %s", - c.gs.MachineDir, + machineDir, ) case !info.IsDir(): return preflight.ResultsError( checkNSpawnMachineProvisioningName, "nspawn machine provisioning", "machine directory path is not a directory: %s", - c.gs.MachineDir, + machineDir, ) } @@ -123,17 +124,17 @@ func (c nspawnMachineProvisioningChecker) checkMachineDir() []preflight.Result { checkNSpawnMachineProvisioningName, "nspawn machine provisioning", "machine directory permissions are too restrictive: %s", - c.gs.MachineDir, + machineDir, )) } - empty, err := isDirEmpty(c.deps.open, c.gs.MachineDir) + empty, err := isDirEmpty(c.deps.open, machineDir) if err != nil { return append(results, preflight.Error( checkNSpawnMachineProvisioningName, "nspawn machine provisioning", "machine directory cannot be read: %s", - c.gs.MachineDir, + machineDir, )) } @@ -141,9 +142,9 @@ func (c nspawnMachineProvisioningChecker) checkMachineDir() []preflight.Result { // A populated machine directory is expected during rejoin or reuse of an // existing kube1/kube2 rootfs. Rootfs provisioning will skip bootstrap // rather than overwrite it. - c.log.Debug("machine directory exists and is not empty", "path", c.gs.MachineDir) + c.log.Debug("machine directory exists and is not empty", "path", machineDir) } else { - c.log.Debug("machine directory exists and is empty", "path", c.gs.MachineDir) + c.log.Debug("machine directory exists and is empty", "path", machineDir) } return results From 6f110bd4c39c40f18116734689d1b06488074ec4 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Thu, 25 Jun 2026 21:21:53 +0000 Subject: [PATCH 15/16] agent: group preflight checkers by phase --- cmd/agent/internal/cmd/preflight.go | 24 +++----- cmd/agent/internal/cmd/preflight_test.go | 2 +- .../phases/host/preflight_agent_config.go | 38 ------------ .../host/preflight_agent_config_test.go | 51 ---------------- .../host/preflight_cluster_credentials.go | 56 ------------------ .../preflight_cluster_credentials_test.go | 59 ------------------- pkg/agent/phases/host/preflight_host.go | 16 +++++ .../phases/nodestart/preflight_api_server.go | 39 ++++++++++-- .../nodestart/preflight_api_server_test.go | 58 +++++++++++++++--- .../phases/rootfs/preflight_goal_state.go | 14 +++++ pkg/agent/preflight/preflight.go | 15 +++++ pkg/agent/preflight/preflight_test.go | 10 ++++ 12 files changed, 149 insertions(+), 233 deletions(-) delete mode 100644 pkg/agent/phases/host/preflight_agent_config.go delete mode 100644 pkg/agent/phases/host/preflight_agent_config_test.go delete mode 100644 pkg/agent/phases/host/preflight_cluster_credentials.go delete mode 100644 pkg/agent/phases/host/preflight_cluster_credentials_test.go diff --git a/cmd/agent/internal/cmd/preflight.go b/cmd/agent/internal/cmd/preflight.go index 58ee6321a..37b820c8a 100644 --- a/cmd/agent/internal/cmd/preflight.go +++ b/cmd/agent/internal/cmd/preflight.go @@ -73,6 +73,10 @@ func (h *preflightHandler) execute(ctx context.Context) error { return err } + if err := cfg.AgentConfig.Validate(); err != nil { + return fmt.Errorf("validate agent config: %w", err) + } + goalState, err := goalstates.ResolveMachine( logger, &cfg.AgentConfig, @@ -83,21 +87,11 @@ func (h *preflightHandler) execute(ctx context.Context) error { return fmt.Errorf("resolve goal state: %w", err) } - checks := []preflight.Checker{ - host.CheckIsPrivilegedUser(logger), - host.CheckAgentConfig(logger, &cfg.AgentConfig), - host.CheckClusterCredentials(logger, cfg), - host.CheckHostPackages(logger), - host.CheckHostOSConfiguration(logger), - host.CheckNSpawnRuntime(logger), - host.CheckDockerActive(logger), - host.CheckSwapActive(logger), - host.CheckDiskSpace(logger), - host.CheckCgroups(logger), - nodestart.CheckAPIServerReachable(logger, &cfg.AgentConfig), - rootfs.CheckGoalState(logger, goalState.RootFS), - rootfs.CheckNSpawnMachineProvisioning(logger, goalState.RootFS), - } + checks := preflight.Flatten( + host.Preflight(logger, cfg, goalState), + nodestart.Preflight(logger, cfg, goalState), + rootfs.Preflight(logger, cfg, goalState), + ) opts := preflight.Options{ IgnoreErrors: h.ignorePreflightErrors, diff --git a/cmd/agent/internal/cmd/preflight_test.go b/cmd/agent/internal/cmd/preflight_test.go index 1451d3b08..856b3884e 100644 --- a/cmd/agent/internal/cmd/preflight_test.go +++ b/cmd/agent/internal/cmd/preflight_test.go @@ -104,5 +104,5 @@ func TestPreflightTextOutputIncludesOK(t *testing.T) { } require.NoError(t, h.execute(context.Background())) - assert.Contains(t, out.String(), "[OK agent-config]") + assert.Contains(t, out.String(), "[OK goal-state]") } diff --git a/pkg/agent/phases/host/preflight_agent_config.go b/pkg/agent/phases/host/preflight_agent_config.go deleted file mode 100644 index 654e4caac..000000000 --- a/pkg/agent/phases/host/preflight_agent_config.go +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -package host - -import ( - "context" - "log/slog" - - "github.com/Azure/unbounded/pkg/agent/config" - "github.com/Azure/unbounded/pkg/agent/preflight" -) - -const checkAgentConfigName = "agent-config" - -type agentConfigChecker struct { - log *slog.Logger - config *config.AgentConfig -} - -// CheckAgentConfig verifies the shared agent config has been normalized and is -// internally consistent. Product-specific credential requirements are validated -// by separate checks. -func CheckAgentConfig(log *slog.Logger, cfg *config.AgentConfig) preflight.Checker { - return agentConfigChecker{log: log, config: cfg} -} - -// Name returns the stable check name used in reports and ignore rules. -func (c agentConfigChecker) Name() string { return checkAgentConfigName } - -// Check validates the shared agent config without mutating it. -func (c agentConfigChecker) Check(context.Context) []preflight.Result { - if err := c.config.Validate(); err != nil { - return preflight.ResultsError(checkAgentConfigName, "agent config", "agent config is invalid") - } - - return preflight.ResultsOK(checkAgentConfigName, "agent config", "agent config is valid") -} diff --git a/pkg/agent/phases/host/preflight_agent_config_test.go b/pkg/agent/phases/host/preflight_agent_config_test.go deleted file mode 100644 index 91cff5973..000000000 --- a/pkg/agent/phases/host/preflight_agent_config_test.go +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -package host - -import ( - "context" - "log/slog" - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/Azure/unbounded/pkg/agent/config" - "github.com/Azure/unbounded/pkg/agent/preflight" -) - -func validPreflightConfig() *config.AgentConfig { - return &config.AgentConfig{ - MachineName: "machine-1", - NodeName: "node-1", - Cluster: config.AgentClusterConfig{ - CaCertBase64: "Y2E=", - ClusterDNS: "10.0.0.10", - Version: "1.34.0", - }, - Kubelet: config.AgentKubeletConfig{ - ApiServer: "https://api.example.com:443", - Auth: config.KubeletAuthInfo{ - BootstrapToken: "abc123.secret456", - }, - }, - } -} - -func TestCheckAgentConfigValid(t *testing.T) { - results := CheckAgentConfig(slog.New(slog.DiscardHandler), validPreflightConfig()).Check(context.Background()) - - assert.Equal(t, preflight.ResultsOK(checkAgentConfigName, "agent config", "agent config is valid"), results) -} - -func TestCheckAgentConfigInvalid(t *testing.T) { - cfg := validPreflightConfig() - cfg.MachineName = "" - - results := CheckAgentConfig(slog.New(slog.DiscardHandler), cfg).Check(context.Background()) - - assert.Equal(t, preflight.SeverityError, results[0].Severity) - assert.Equal(t, checkAgentConfigName, results[0].Name) - assert.Equal(t, "agent config", results[0].Target) - assert.Equal(t, "agent config is invalid", results[0].Message) -} diff --git a/pkg/agent/phases/host/preflight_cluster_credentials.go b/pkg/agent/phases/host/preflight_cluster_credentials.go deleted file mode 100644 index babe7229d..000000000 --- a/pkg/agent/phases/host/preflight_cluster_credentials.go +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -package host - -import ( - "context" - "encoding/base64" - "log/slog" - "strings" - - "github.com/Azure/unbounded/internal/provision" - "github.com/Azure/unbounded/pkg/agent/preflight" -) - -const checkClusterCredentialsName = "cluster-credentials" - -type clusterCredentialsChecker struct { - log *slog.Logger - config *provision.UnboundedAgentConfig -} - -// CheckClusterCredentials returns a checker that validates cluster CA data and -// the bootstrap credential. When attestation is configured, missing kubelet auth -// is allowed because attestation can provide the credential later. -func CheckClusterCredentials(log *slog.Logger, cfg *provision.UnboundedAgentConfig) preflight.Checker { - return clusterCredentialsChecker{log: log, config: cfg} -} - -// Name returns the stable check name used in reports and ignore rules. -func (c clusterCredentialsChecker) Name() string { return checkClusterCredentialsName } - -// Check validates cluster credential inputs without printing credential values. -func (c clusterCredentialsChecker) Check(context.Context) []preflight.Result { - if c.config == nil { - return preflight.ResultsError(checkClusterCredentialsName, "cluster credentials", "agent config is missing") - } - - var errs []string - if _, err := base64.StdEncoding.DecodeString(c.config.Cluster.CaCertBase64); err != nil { - errs = append(errs, "cluster CA data is invalid") - } - - if c.config.Attest == nil { - auth := c.config.Kubelet.Auth - if err := auth.Validate(); err != nil { - errs = append(errs, "bootstrap credential is invalid") - } - } - - if len(errs) > 0 { - return preflight.ResultsError(checkClusterCredentialsName, "cluster credentials", "%s", strings.Join(errs, "; ")) - } - - return preflight.ResultsOK(checkClusterCredentialsName, "cluster credentials", "cluster credentials are valid") -} diff --git a/pkg/agent/phases/host/preflight_cluster_credentials_test.go b/pkg/agent/phases/host/preflight_cluster_credentials_test.go deleted file mode 100644 index 14243d322..000000000 --- a/pkg/agent/phases/host/preflight_cluster_credentials_test.go +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -package host - -import ( - "context" - "log/slog" - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/Azure/unbounded/internal/provision" - "github.com/Azure/unbounded/pkg/agent/preflight" -) - -func TestCheckClusterCredentialsValid(t *testing.T) { - results := CheckClusterCredentials(slog.New(slog.DiscardHandler), validUnboundedPreflightConfig()).Check(context.Background()) - - assert.Equal(t, preflight.ResultsOK(checkClusterCredentialsName, "cluster credentials", "cluster credentials are valid"), results) -} - -func TestCheckClusterCredentialsAllowsAttestation(t *testing.T) { - cfg := validUnboundedPreflightConfig() - cfg.Kubelet.Auth.BootstrapToken = "" - cfg.Attest = &provision.AgentAttestConfig{URL: "http://metalman.example.com:8880"} - - results := CheckClusterCredentials(slog.New(slog.DiscardHandler), cfg).Check(context.Background()) - - assert.Equal(t, preflight.SeverityOK, results[0].Severity) -} - -func TestCheckClusterCredentialsRequiresAuthWhenNoAttestation(t *testing.T) { - cfg := validUnboundedPreflightConfig() - cfg.Kubelet.Auth.BootstrapToken = "" - - results := CheckClusterCredentials(slog.New(slog.DiscardHandler), cfg).Check(context.Background()) - - assert.Equal(t, preflight.SeverityError, results[0].Severity) - assert.Equal(t, checkClusterCredentialsName, results[0].Name) - assert.Equal(t, "cluster credentials", results[0].Target) - assert.Equal(t, "bootstrap credential is invalid", results[0].Message) -} - -func TestCheckClusterCredentialsInvalidCA(t *testing.T) { - cfg := validUnboundedPreflightConfig() - cfg.Cluster.CaCertBase64 = "not-base64" - - results := CheckClusterCredentials(slog.New(slog.DiscardHandler), cfg).Check(context.Background()) - - assert.Equal(t, preflight.SeverityError, results[0].Severity) - assert.Contains(t, results[0].Message, "cluster CA data is invalid") -} - -func validUnboundedPreflightConfig() *provision.UnboundedAgentConfig { - return &provision.UnboundedAgentConfig{ - AgentConfig: *validPreflightConfig(), - } -} diff --git a/pkg/agent/phases/host/preflight_host.go b/pkg/agent/phases/host/preflight_host.go index b6682eb8b..905bfceef 100644 --- a/pkg/agent/phases/host/preflight_host.go +++ b/pkg/agent/phases/host/preflight_host.go @@ -15,6 +15,7 @@ import ( "syscall" "github.com/Azure/unbounded/internal/executil" + "github.com/Azure/unbounded/internal/provision" "github.com/Azure/unbounded/pkg/agent/goalstates" "github.com/Azure/unbounded/pkg/agent/internal/utilio" "github.com/Azure/unbounded/pkg/agent/preflight" @@ -64,6 +65,21 @@ func (c simpleHostChecker) Name() string { return c.name } func (c simpleHostChecker) Check(ctx context.Context) []preflight.Result { return c.check(ctx) } +// Preflight returns the standard host environment checks required before +// provisioning an nspawn machine. +func Preflight(log *slog.Logger, _ *provision.UnboundedAgentConfig, _ *goalstates.MachineGoalState) []preflight.Checker { + return []preflight.Checker{ + CheckIsPrivilegedUser(log), + CheckHostPackages(log), + CheckHostOSConfiguration(log), + CheckNSpawnRuntime(log), + CheckDockerActive(log), + CheckSwapActive(log), + CheckDiskSpace(log), + CheckCgroups(log), + } +} + // CheckIsPrivilegedUser verifies preflight is running as root. func CheckIsPrivilegedUser(log *slog.Logger) preflight.Checker { return checkIsPrivilegedUser(log, defaultHostCheckDeps()) diff --git a/pkg/agent/phases/nodestart/preflight_api_server.go b/pkg/agent/phases/nodestart/preflight_api_server.go index 3701d96fc..0943bcd1d 100644 --- a/pkg/agent/phases/nodestart/preflight_api_server.go +++ b/pkg/agent/phases/nodestart/preflight_api_server.go @@ -14,7 +14,8 @@ import ( "strings" "time" - "github.com/Azure/unbounded/pkg/agent/config" + "github.com/Azure/unbounded/internal/provision" + "github.com/Azure/unbounded/pkg/agent/goalstates" "github.com/Azure/unbounded/pkg/agent/preflight" ) @@ -22,14 +23,22 @@ const checkAPIServerReachableName = "api-server-reachable" type apiServerReachableChecker struct { log *slog.Logger - config *config.AgentConfig + config *provision.UnboundedAgentConfig httpClient *http.Client } +// Preflight returns the standard node-start checks that can run before the +// nspawn machine starts. +func Preflight(log *slog.Logger, cfg *provision.UnboundedAgentConfig, _ *goalstates.MachineGoalState) []preflight.Checker { + return []preflight.Checker{ + CheckAPIServerReachable(log, cfg), + } +} + // CheckAPIServerReachable returns a non-mutating checker that validates the -// configured Kubernetes API server can be reached from the host. The checker -// redacts the configured endpoint from result messages. -func CheckAPIServerReachable(log *slog.Logger, cfg *config.AgentConfig) preflight.Checker { +// cluster credentials and configured Kubernetes API server reachability. The +// checker redacts the configured endpoint from result messages. +func CheckAPIServerReachable(log *slog.Logger, cfg *provision.UnboundedAgentConfig) preflight.Checker { return apiServerReachableChecker{log: log, config: cfg} } @@ -40,6 +49,10 @@ func (c apiServerReachableChecker) Check(ctx context.Context) []preflight.Result return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", "agent config is missing") } + if errs := c.validateClusterCredentials(); len(errs) > 0 { + return preflight.ResultsError(checkAPIServerReachableName, "cluster credentials", "%s", strings.Join(errs, "; ")) + } + apiServer := c.config.Kubelet.ApiServer if strings.TrimSpace(apiServer) == "" { return preflight.ResultsError(checkAPIServerReachableName, "cluster API server", "API server is required") @@ -73,6 +86,22 @@ func (c apiServerReachableChecker) Check(ctx context.Context) []preflight.Result return preflight.ResultsOK(checkAPIServerReachableName, "cluster API server", "API server is reachable") } +func (c apiServerReachableChecker) validateClusterCredentials() []string { + var errs []string + if _, err := base64.StdEncoding.DecodeString(c.config.Cluster.CaCertBase64); err != nil { + errs = append(errs, "cluster CA data is invalid") + } + + if c.config.Attest == nil { + auth := c.config.Kubelet.Auth + if err := auth.Validate(); err != nil { + errs = append(errs, "bootstrap credential is invalid") + } + } + + return errs +} + func (c apiServerReachableChecker) httpClientWithCA() *http.Client { transport := &http.Transport{} if defaultTransport, ok := http.DefaultTransport.(*http.Transport); ok { diff --git a/pkg/agent/phases/nodestart/preflight_api_server_test.go b/pkg/agent/phases/nodestart/preflight_api_server_test.go index 556ac9ac2..eee7b5dd4 100644 --- a/pkg/agent/phases/nodestart/preflight_api_server_test.go +++ b/pkg/agent/phases/nodestart/preflight_api_server_test.go @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/assert" - "github.com/Azure/unbounded/pkg/agent/config" + "github.com/Azure/unbounded/internal/provision" "github.com/Azure/unbounded/pkg/agent/preflight" ) @@ -59,13 +59,55 @@ func TestCheckAPIServerReachableServerError(t *testing.T) { assert.Equal(t, "API server returned status 500", results[0].Message) } -func apiServerPreflightConfig(apiServer string) *config.AgentConfig { - return &config.AgentConfig{ - Cluster: config.AgentClusterConfig{ - CaCertBase64: "Y2E=", - }, - Kubelet: config.AgentKubeletConfig{ - ApiServer: apiServer, +func TestCheckAPIServerReachableAllowsAttestation(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(srv.Close) + + cfg := apiServerPreflightConfig(srv.URL) + cfg.Kubelet.Auth.BootstrapToken = "" + cfg.Attest = &provision.AgentAttestConfig{URL: "http://metalman.example.com:8880"} + + results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), cfg).Check(context.Background()) + + assert.Equal(t, preflight.SeverityOK, results[0].Severity) +} + +func TestCheckAPIServerReachableRequiresAuthWhenNoAttestation(t *testing.T) { + cfg := apiServerPreflightConfig("https://api.example.com:443") + cfg.Kubelet.Auth.BootstrapToken = "" + + results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), cfg).Check(context.Background()) + + assert.Equal(t, preflight.SeverityError, results[0].Severity) + assert.Equal(t, checkAPIServerReachableName, results[0].Name) + assert.Equal(t, "cluster credentials", results[0].Target) + assert.Equal(t, "bootstrap credential is invalid", results[0].Message) +} + +func TestCheckAPIServerReachableInvalidCA(t *testing.T) { + cfg := apiServerPreflightConfig("https://api.example.com:443") + cfg.Cluster.CaCertBase64 = "not-base64" + + results := CheckAPIServerReachable(slog.New(slog.DiscardHandler), cfg).Check(context.Background()) + + assert.Equal(t, preflight.SeverityError, results[0].Severity) + assert.Contains(t, results[0].Message, "cluster CA data is invalid") +} + +func apiServerPreflightConfig(apiServer string) *provision.UnboundedAgentConfig { + return &provision.UnboundedAgentConfig{ + AgentConfig: provision.AgentConfig{ + Cluster: provision.AgentClusterConfig{ + CaCertBase64: "Y2E=", + }, + Kubelet: provision.AgentKubeletConfig{ + ApiServer: apiServer, + Auth: provision.KubeletAuthInfo{ + BootstrapToken: "abc123.secret456", + }, + }, }, } } diff --git a/pkg/agent/phases/rootfs/preflight_goal_state.go b/pkg/agent/phases/rootfs/preflight_goal_state.go index ee082b02f..1aac02391 100644 --- a/pkg/agent/phases/rootfs/preflight_goal_state.go +++ b/pkg/agent/phases/rootfs/preflight_goal_state.go @@ -7,6 +7,7 @@ import ( "context" "log/slog" + "github.com/Azure/unbounded/internal/provision" "github.com/Azure/unbounded/pkg/agent/goalstates" "github.com/Azure/unbounded/pkg/agent/preflight" ) @@ -21,6 +22,19 @@ type goalStateChecker struct { rootFS *goalstates.RootFS } +// Preflight returns the standard rootfs checks for a resolved machine goal state. +func Preflight(log *slog.Logger, _ *provision.UnboundedAgentConfig, goalState *goalstates.MachineGoalState) []preflight.Checker { + var rootFS *goalstates.RootFS + if goalState != nil { + rootFS = goalState.RootFS + } + + return []preflight.Checker{ + CheckGoalState(log, rootFS), + CheckNSpawnMachineProvisioning(log, rootFS), + } +} + // CheckGoalState validates the agent config resolved into a machine goal state. func CheckGoalState(log *slog.Logger, rootFS *goalstates.RootFS) preflight.Checker { return goalStateChecker{log: log, rootFS: rootFS} diff --git a/pkg/agent/preflight/preflight.go b/pkg/agent/preflight/preflight.go index ce7bffb25..8221500e5 100644 --- a/pkg/agent/preflight/preflight.go +++ b/pkg/agent/preflight/preflight.go @@ -32,6 +32,21 @@ type Checker interface { Check(ctx context.Context) []Result } +// Flatten returns one checker slice from ordered checker groups. +func Flatten(groups ...[]Checker) []Checker { + total := 0 + for _, group := range groups { + total += len(group) + } + + checks := make([]Checker, 0, total) + for _, group := range groups { + checks = append(checks, group...) + } + + return checks +} + // Result describes one preflight check outcome. Message and Target must not // include raw configured values such as URLs, tokens, image references, or file // contents. diff --git a/pkg/agent/preflight/preflight_test.go b/pkg/agent/preflight/preflight_test.go index 431de6e6f..76dfbdd9f 100644 --- a/pkg/agent/preflight/preflight_test.go +++ b/pkg/agent/preflight/preflight_test.go @@ -32,6 +32,16 @@ func TestRunIncludesAllResults(t *testing.T) { assert.Len(t, report.Checks, 3) } +func TestFlatten(t *testing.T) { + first := fakeChecker{name: "first"} + second := fakeChecker{name: "second"} + third := fakeChecker{name: "third"} + + checks := Flatten([]Checker{first}, nil, []Checker{second, third}) + + assert.Equal(t, []Checker{first, second, third}, checks) +} + func TestRunDowngradesIgnoredErrors(t *testing.T) { report := Run(context.Background(), []Checker{ fakeChecker{results: []Result{Error("host-packages", "host packages", "missing")}}, From bf651fb61431a31767738ee4f7c1bd7b41042ae0 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Thu, 25 Jun 2026 21:34:47 +0000 Subject: [PATCH 16/16] agent: fix preflight config lint --- cmd/agent/internal/cmd/preflight.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/agent/internal/cmd/preflight.go b/cmd/agent/internal/cmd/preflight.go index 37b820c8a..6a15cccd1 100644 --- a/cmd/agent/internal/cmd/preflight.go +++ b/cmd/agent/internal/cmd/preflight.go @@ -73,7 +73,7 @@ func (h *preflightHandler) execute(ctx context.Context) error { return err } - if err := cfg.AgentConfig.Validate(); err != nil { + if err := cfg.Validate(); err != nil { return fmt.Errorf("validate agent config: %w", err) }