Skip to content

Conversation

@emosbaugh
Copy link
Member

@emosbaugh emosbaugh commented Nov 10, 2025

Summary

Fixes a critical bug where preflight execution failures incorrectly reported as successful in headless installs, adds comprehensive test coverage, and refactors preflight result handling for consistency across the codebase.

Bug Fix

Problem: The headless orchestrator (cmd/installer/cli/headless/install/orchestrator.go) only checked resp.Output for failures. When preflights failed to execute (nil output), it incorrectly reported "Host preflights passed" and continued installation.

Impact: Headless installs would proceed on incompatible systems when preflights failed to run due to infrastructure issues, permissions, or runtime errors.

Fix: Now checks resp.Status.State first. When StateFailed:

  • If output has failures → report check failures (can be bypassed)
  • If output is nil or empty → report execution failure (cannot be bypassed)
  • Otherwise → report success

Applied to both host and app preflights in the orchestrator and API controllers.

Changes

Preflight State Handling Refactor

  • Updated orchestrator to check Status.State before examining output
  • Refactored API controller methods (getStateFromAppPreflightsOutput, getStateFromPreflightsOutput) to use the same pattern
  • Now properly distinguishes execution failures from check failures

Dependency Injection

  • Added PreflightRunnerInterface throughout the codebase (API, controllers, handlers, managers)
  • Enables mocking for tests and future dry-run capabilities

Test Coverage

  • Integration tests: End-to-end headless install scenarios with execution failures and check failures
  • Unit tests: Orchestrator and controller state determination logic
  • Covers all edge cases: nil output, empty output, status errors, unexpected states

Testing

Integration Tests (tests/dryrun/v3_install_test.go)

  • Execution failures properly block installation (cannot be bypassed)
  • Check failures block installation (can be bypassed with flags)
  • Both host and app preflights tested

Unit Tests

  • Orchestrator: cmd/installer/cli/headless/install/orchestrator_test.go
  • Controllers: api/controllers/app/apppreflight_test.go, api/controllers/linux/install/hostpreflight_test.go
  • Covers success, execution failures, check failures, and edge cases

Related

Part of SC-130867 (headless install orchestrator implementation).

@github-actions
Copy link

github-actions bot commented Nov 10, 2025

This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID.

Online Installer:

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci/appver-dev-ff359aa" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Airgap Installer (may take a few minutes before the airgap bundle is built):

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci-airgap/appver-dev-ff359aa?airgap=true" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Happy debugging!

@emosbaugh emosbaugh force-pushed the emosbaugh/sc-130867/headless-install-headless-orchestrator-implementation-3 branch 2 times, most recently from 5eaf752 to 7cdd5f4 Compare November 11, 2025 18:08
@emosbaugh emosbaugh marked this pull request as ready for review November 11, 2025 21:10
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Installation Leaves Orphaned Temporary Files

The install method creates a temporary config values file via createConfigValuesFile but never cleans it up, causing a resource leak. Each installation leaves an orphaned temp file in the system's temp directory. The upgrade manager correctly uses defer os.Remove(configValuesFile) after creating the file, but the install manager is missing this cleanup.

api/internal/managers/app/install/install.go#L76-L81

configValuesFile, err := m.createConfigValuesFile(configValues)
if err != nil {
return fmt.Errorf("creating config values file: %w", err)
}
installOpts.ConfigValuesFile = configValuesFile

Fix in Cursor Fix in Web


@emosbaugh emosbaugh enabled auto-merge (squash) November 11, 2025 21:38
@emosbaugh emosbaugh merged commit e909ebc into main Nov 12, 2025
141 of 146 checks passed
@emosbaugh emosbaugh deleted the emosbaugh/sc-130867/headless-install-headless-orchestrator-implementation-3 branch November 12, 2025 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants