Skip to content

chore(api): use success state for preflight failures #3211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

chore(api): use success state for preflight failures #3211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a8de24
chore(api): use success state for preflight failures
emosbaugh Nov 14, 2025
095daf5
f
emosbaugh Nov 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions api/controllers/app/apppreflight.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"time"

apppreflightmanager "github.com/replicatedhq/embedded-cluster/api/internal/managers/app/preflight"
"github.com/replicatedhq/embedded-cluster/api/internal/statemachine"
"github.com/replicatedhq/embedded-cluster/api/internal/states"
"github.com/replicatedhq/embedded-cluster/api/types"
ecv1beta1 "github.com/replicatedhq/embedded-cluster/kinds/apis/v1beta1"
Expand Down Expand Up @@ -116,22 +117,30 @@ func (c *AppController) RunAppPreflights(ctx context.Context, opts RunAppPreflig
return fmt.Errorf("run app preflights: %w", err)
}

// Transition state machine based on whether there are failures
// This is used internally for workflow state tracking
var smState statemachine.State
if output.HasFail() {
if err := c.stateMachine.Transition(lock, states.StateAppPreflightsFailed, output); err != nil {
return fmt.Errorf("transition states: %w", err)
}
smState = states.StateAppPreflightsFailed
} else {
smState = states.StateAppPreflightsSucceeded
}

if err := c.setAppPreflightStatus(types.StateFailed, "App preflights failed"); err != nil {
return fmt.Errorf("set status to failed: %w", err)
}
if err := c.stateMachine.Transition(lock, smState, output); err != nil {
return fmt.Errorf("transition states: %w", err)
}

// Always set status to StateSucceeded to indicate execution completed successfully
// The output will contain failures/warnings for the caller to check
var statusDescription string
if output.HasFail() {
statusDescription = "App preflights completed with failures"
} else {
if err := c.stateMachine.Transition(lock, states.StateAppPreflightsSucceeded, output); err != nil {
return fmt.Errorf("transition states: %w", err)
}
statusDescription = "App preflights succeeded"
}

if err := c.setAppPreflightStatus(types.StateSucceeded, "App preflights succeeded"); err != nil {
return fmt.Errorf("set status to succeeded: %w", err)
}
if err := c.setAppPreflightStatus(types.StateSucceeded, statusDescription); err != nil {
return fmt.Errorf("set status to succeeded: %w", err)
}

return nil
Expand Down
33 changes: 21 additions & 12 deletions api/controllers/linux/install/hostpreflight.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/replicatedhq/embedded-cluster/api/internal/managers/linux/preflight"
"github.com/replicatedhq/embedded-cluster/api/internal/statemachine"
"github.com/replicatedhq/embedded-cluster/api/internal/states"
"github.com/replicatedhq/embedded-cluster/api/internal/utils"
"github.com/replicatedhq/embedded-cluster/api/types"
Expand Down Expand Up @@ -105,22 +106,30 @@ func (c *InstallController) RunHostPreflights(ctx context.Context, opts RunHostP
return fmt.Errorf("run host preflights: %w", err)
}

// Transition state machine based on whether there are failures
// This is used internally for workflow state tracking
var smState statemachine.State
if output.HasFail() {
if err := c.stateMachine.Transition(lock, states.StateHostPreflightsFailed, output); err != nil {
return fmt.Errorf("transition states: %w", err)
}
smState = states.StateHostPreflightsFailed
} else {
smState = states.StateHostPreflightsSucceeded
}

if err := c.setHostPreflightStatus(types.StateFailed, "Host preflights failed"); err != nil {
return fmt.Errorf("set status to failed: %w", err)
}
if err := c.stateMachine.Transition(lock, smState, output); err != nil {
return fmt.Errorf("transition states: %w", err)
}

// Always set status to StateSucceeded to indicate execution completed successfully
// The output will contain failures/warnings for the caller to check
var statusDescription string
if output.HasFail() {
statusDescription = "Host preflights completed with failures"
} else {
if err := c.stateMachine.Transition(lock, states.StateHostPreflightsSucceeded, output); err != nil {
return fmt.Errorf("transition states: %w", err)
}
statusDescription = "Host preflights succeeded"
}

if err := c.setHostPreflightStatus(types.StateSucceeded, "Host preflights succeeded"); err != nil {
return fmt.Errorf("set status to succeeded: %w", err)
}
if err := c.setHostPreflightStatus(types.StateSucceeded, statusDescription); err != nil {
return fmt.Errorf("set status to succeeded: %w", err)
}

return nil
Expand Down
38 changes: 20 additions & 18 deletions cmd/installer/cli/headless/install/orchestrator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,7 @@ func (o *orchestrator) runHostPreflights(ctx context.Context, ignoreFailures boo
return fmt.Errorf("run linux install host preflights: %w", err)
}

// For preflights, we poll until the operation completes (either succeeded or failed),
// then check if there are failures and decide whether to continue based on ignoreFailures
// Poll until the preflight execution completes
state, message, err := pollUntilComplete(ctx, func() (apitypes.State, string, error) {
resp, err = o.apiClient.GetLinuxInstallHostPreflightsStatus(ctx)
if err != nil {
Expand All @@ -267,15 +266,17 @@ func (o *orchestrator) runHostPreflights(ctx context.Context, ignoreFailures boo
return fmt.Errorf("poll until complete: %w", err)
}

if state == apitypes.StateFailed {
hasFailures := resp.Output != nil && resp.Output.HasFail()
// With the new behavior, preflights always return StateSucceeded when execution completes.
// We need to check the output for actual failures.
if state != apitypes.StateSucceeded {
loading.ErrorClosef("Host preflights execution failed")
return fmt.Errorf("host preflights execution failed: %s", message)
}

// If there are no failures, it means the execution failed
if !hasFailures {
loading.ErrorClosef("Host preflights execution failed")
return fmt.Errorf("host preflights execution failed: %s", message)
}
// Check if there are actual preflight failures
hasFailures := resp.Output != nil && resp.Output.HasFail()

if hasFailures {
loading.ErrorClosef("Host preflights completed with failures")

o.logger.Warn("\n⚠ Warning: Host preflight checks completed with failures\n")
Expand Down Expand Up @@ -400,8 +401,7 @@ func (o *orchestrator) runAppPreflights(ctx context.Context, ignoreFailures bool
return fmt.Errorf("run linux install app preflights: %w", err)
}

// For preflights, we poll until the operation completes (either succeeded or failed),
// then check if there are failures and decide whether to continue based on ignoreFailures
// Poll until the preflight execution completes
state, message, err := pollUntilComplete(ctx, func() (apitypes.State, string, error) {
resp, err = o.apiClient.GetLinuxInstallAppPreflightsStatus(ctx)
if err != nil {
Expand All @@ -414,15 +414,17 @@ func (o *orchestrator) runAppPreflights(ctx context.Context, ignoreFailures bool
return fmt.Errorf("poll until complete: %w", err)
}

if state == apitypes.StateFailed {
hasFailures := resp.Output != nil && resp.Output.HasFail()
// With the new behavior, preflights always return StateSucceeded when execution completes.
// We need to check the output for actual failures.
if state != apitypes.StateSucceeded {
loading.ErrorClosef("App preflights execution failed")
return fmt.Errorf("app preflights execution failed: %s", message)
}

// If there are no failures, it means the execution failed
if !hasFailures {
loading.ErrorClosef("App preflights execution failed")
return fmt.Errorf("app preflights execution failed: %s", message)
}
// Check if there are actual preflight failures
hasFailures := resp.Output != nil && resp.Output.HasFail()

if hasFailures {
loading.ErrorClosef("App preflights completed with failures")

o.logger.Warn("\n⚠ Warning: Application preflight checks completed with failures\n")
Expand Down
8 changes: 4 additions & 4 deletions cmd/installer/cli/headless/install/orchestrator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ func Test_orchestrator_runHostPreflights(t *testing.T) {
mockGetStatusFunc: func(ctx context.Context) (apitypes.InstallHostPreflightsStatusResponse, error) {
return apitypes.InstallHostPreflightsStatusResponse{
Status: apitypes.Status{
State: apitypes.StateFailed,
State: apitypes.StateSucceeded,
Description: "Completed with failures",
},
Output: &apitypes.PreflightsOutput{
Expand Down Expand Up @@ -328,7 +328,7 @@ func Test_orchestrator_runHostPreflights(t *testing.T) {
mockGetStatusFunc: func(ctx context.Context) (apitypes.InstallHostPreflightsStatusResponse, error) {
return apitypes.InstallHostPreflightsStatusResponse{
Status: apitypes.Status{
State: apitypes.StateFailed,
State: apitypes.StateSucceeded,
Description: "Completed with failures",
},
Output: &apitypes.PreflightsOutput{
Expand Down Expand Up @@ -645,7 +645,7 @@ func Test_orchestrator_runAppPreflights(t *testing.T) {
mockGetStatusFunc: func(ctx context.Context) (apitypes.InstallAppPreflightsStatusResponse, error) {
return apitypes.InstallAppPreflightsStatusResponse{
Status: apitypes.Status{
State: apitypes.StateFailed,
State: apitypes.StateSucceeded,
Description: "Completed with failures",
},
Output: &apitypes.PreflightsOutput{
Expand Down Expand Up @@ -677,7 +677,7 @@ func Test_orchestrator_runAppPreflights(t *testing.T) {
mockGetStatusFunc: func(ctx context.Context) (apitypes.InstallAppPreflightsStatusResponse, error) {
return apitypes.InstallAppPreflightsStatusResponse{
Status: apitypes.Status{
State: apitypes.StateFailed,
State: apitypes.StateSucceeded,
Description: "Completed with failures",
},
Output: &apitypes.PreflightsOutput{
Expand Down
30 changes: 14 additions & 16 deletions tests/dryrun/v3_install_preflights_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,13 @@ func TestV3Install_HostPreflights_WithFailuresBypass(t *testing.T) {
}()

runV3Install(t, v3InstallArgs{
managerPort: 30080,
password: "password123",
isAirgap: false,
configValuesFile: configFile,
installationConfig: apitypes.LinuxInstallationConfig{},
ignoreHostPreflights: true, // Bypass host preflight failures
ignoreAppPreflights: false,
shouldHostPreflightsFail: true,
managerPort: 30080,
password: "password123",
isAirgap: false,
configValuesFile: configFile,
installationConfig: apitypes.LinuxInstallationConfig{},
ignoreHostPreflights: true, // Bypass host preflight failures
ignoreAppPreflights: false,
})

require.NoError(t, dryrun.Dump(), "fail to dump dryrun output")
Expand Down Expand Up @@ -332,14 +331,13 @@ func TestV3Install_AppPreflights_WithFailuresBypass(t *testing.T) {
}()

runV3Install(t, v3InstallArgs{
managerPort: 30080,
password: "password123",
isAirgap: false,
configValuesFile: configFile,
installationConfig: apitypes.LinuxInstallationConfig{},
ignoreHostPreflights: false,
ignoreAppPreflights: true, // Bypass app preflight failures
shouldAppPreflightsFail: true,
managerPort: 30080,
password: "password123",
isAirgap: false,
configValuesFile: configFile,
installationConfig: apitypes.LinuxInstallationConfig{},
ignoreHostPreflights: false,
ignoreAppPreflights: true, // Bypass app preflight failures
})

preflightRunner.AssertExpectations(t)
Expand Down
10 changes: 8 additions & 2 deletions web/src/components/wizard/installation/phases/AppPreflightCheck.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ const AppPreflightCheck: React.FC<AppPreflightCheckProps> = ({ onRun, onComplete
const hasFailures = (output?: PreflightsOutput) => (output?.fail?.length ?? 0) > 0;
const hasWarnings = (output?: PreflightsOutput) => (output?.warn?.length ?? 0) > 0;
const hasStrictFailures = (response?: AppPreflightsResponse) => response?.hasStrictAppPreflightFailures ?? false;
const isSuccessful = (response?: AppPreflightsResponse) => response?.status?.state === "Succeeded";
const isSuccessful = (response?: AppPreflightsResponse) =>
response?.status?.state === "Succeeded" && !hasFailures(response?.output);

const getErrorMessage = () => {
if (runAppPreflights.error) {
Expand Down Expand Up @@ -63,13 +64,18 @@ const AppPreflightCheck: React.FC<AppPreflightCheckProps> = ({ onRun, onComplete

// Handle preflight status changes
useEffect(() => {
if (preflightResponse?.status?.state === "Succeeded" || preflightResponse?.status?.state === "Failed") {
if (preflightResponse?.status?.state === "Succeeded") {
// Execution completed successfully - check if there are preflight failures
setIsPreflightsPolling(false);
onComplete(
!hasFailures(preflightResponse.output),
preflightResponse.allowIgnoreAppPreflights ?? false,
hasStrictFailures(preflightResponse)
);
} else if (preflightResponse?.status?.state === "Failed") {
// Execution failed - treat as failure regardless of output
setIsPreflightsPolling(false);
onComplete(false, false, false);
}
}, [preflightResponse]);

Expand Down
10 changes: 8 additions & 2 deletions web/src/components/wizard/installation/phases/LinuxPreflightCheck.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ const LinuxPreflightCheck: React.FC<LinuxPreflightCheckProps> = ({ onRun, onComp

const hasFailures = (output?: PreflightsOutput) => (output?.fail?.length ?? 0) > 0;
const hasWarnings = (output?: PreflightsOutput) => (output?.warn?.length ?? 0) > 0;
const isSuccessful = (response?: HostPreflightResponse) => response?.status?.state === "Succeeded";
const isSuccessful = (response?: HostPreflightResponse) =>
response?.status?.state === "Succeeded" && !hasFailures(response?.output);

const getErrorMessage = () => {
if (runHostPreflights.error) {
Expand Down Expand Up @@ -69,9 +70,14 @@ const LinuxPreflightCheck: React.FC<LinuxPreflightCheckProps> = ({ onRun, onComp

// Handle preflight status changes
useEffect(() => {
if (preflightResponse?.status?.state === "Succeeded" || preflightResponse?.status?.state === "Failed") {
if (preflightResponse?.status?.state === "Succeeded") {
// Execution completed successfully - check if there are preflight failures
setIsPreflightsPolling(false);
onComplete(!hasFailures(preflightResponse.output), preflightResponse.allowIgnoreHostPreflights ?? false);
} else if (preflightResponse?.status?.state === "Failed") {
// Execution failed - treat as failure regardless of output
setIsPreflightsPolling(false);
onComplete(false, false);
}
}, [preflightResponse]);

Expand Down
22 changes: 11 additions & 11 deletions web/src/components/wizard/installation/tests/AppPreflightCheck.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const TEST_TOKEN = "test-auth-token";

const createServer = (target: 'linux' | 'kubernetes') => setupServer(
mockHandlers.preflights.app.getStatus({
status: { state: "Failed" },
status: { state: "Succeeded" },
output: {
pass: [{ title: "CPU Check", message: "CPU requirements met" }],
warn: [{ title: "Memory Warning", message: "Memory is below recommended" }],
Expand Down Expand Up @@ -131,10 +131,10 @@ describe.each([
});

it("receives allowIgnoreAppPreflights field in preflight response", async () => {
// Mock preflight status endpoint with allowIgnoreAppPreflights: true
// Mock preflight status endpoint with allowIgnoreAppPreflights: true - execution succeeds but checks fail
server.use(
mockHandlers.preflights.app.getStatus({
status: { state: "Failed" },
status: { state: "Succeeded" },
output: {
pass: [{ title: "CPU Check", message: "CPU requirements met" }],
warn: [],
Expand All @@ -161,10 +161,10 @@ describe.each([
});

it("passes allowIgnoreAppPreflights false to onComplete callback", async () => {
// Mock preflight status endpoint with allowIgnoreAppPreflights: false
// Mock preflight status endpoint with allowIgnoreAppPreflights: false - execution succeeds but checks fail
server.use(
mockHandlers.preflights.app.getStatus({
status: { state: "Failed" },
status: { state: "Succeeded" },
output: {
pass: [{ title: "CPU Check", message: "CPU requirements met" }],
warn: [],
Expand Down Expand Up @@ -192,10 +192,10 @@ describe.each([

// New tests for strict preflight functionality
it("displays strict failures with visual distinction", async () => {
// Mock preflight status endpoint with strict and non-strict failures
// Mock preflight status endpoint with strict and non-strict failures - execution succeeds but checks fail
server.use(
mockHandlers.preflights.app.getStatus({
status: { state: "Failed" },
status: { state: "Succeeded" },
output: {
pass: [],
warn: [],
Expand Down Expand Up @@ -232,10 +232,10 @@ describe.each([
});

it("shows appropriate guidance for strict failures in What's Next section", async () => {
// Mock preflight status endpoint with strict failures
// Mock preflight status endpoint with strict failures - execution succeeds but checks fail
server.use(
mockHandlers.preflights.app.getStatus({
status: { state: "Failed" },
status: { state: "Succeeded" },
output: {
pass: [],
warn: [],
Expand All @@ -262,10 +262,10 @@ describe.each([
});

it("passes hasStrictAppPreflightFailures from API response to onComplete", async () => {
// Mock preflight status endpoint with hasStrictAppPreflightFailures field
// Mock preflight status endpoint with hasStrictAppPreflightFailures field - execution succeeds but checks fail
server.use(
mockHandlers.preflights.app.getStatus({
status: { state: "Failed" },
status: { state: "Succeeded" },
output: {
pass: [],
warn: [],
Expand Down
Loading
Loading