Skip to content

Conversation

@patrykw-splunk
Copy link
Collaborator

@patrykw-splunk patrykw-splunk commented Nov 5, 2025

Description

This PR aims to fix FIPS Error Handling:
Modified isAppAlreadyInstalled() and installApp() functions to properly handle FIPS provider informational messages in stderr by treating them as non-error conditions rather than installation failures.
Also added fanout CR status synchronization (installPhaseManager()) to ensure that when app installation fails after max retries on any replica, the main PhaseInfo is properly updated to reflect the overall deployment failure state.

Key Changes

FIPS error handling

  • Modified isAppAlreadyInstalled() function to detect and ignore FIPS provider informational messages in stderr instead of treating them as errors
  • Updated installApp() function to handle FIPS messages during app installation, preventing false installation failures
  • Added FIPS message detection logic that checks for "FIPS provider enabled" string and logs as informational rather than error

Fanout CR Status Management

  • Enhanced failure handling for fanout CRs (Standalone with multiple replicas) to properly update main PhaseInfo when max retries are reached
  • Added synchronization logic to ensure overall deployment status reflects individual replica failures
  • Improved status reporting by setting DeployStatus to Error when legitimate failures occur after retry exhaustion

Testing and Verification

Fixed TestPremiumAppScopedPlaybook:

  • Test3: Updated to test real error conditions (exit code 2) instead of just stderr content
  • Test4: Fixed to properly simulate grep exit code 1 (app not enabled scenario)
  • Test7: Added new test case specifically for apps already installed with FIPS messages
  • Fixed mock setup to ensure cleanup operations succeed when app is already installed

Added comprehensive TestIsAppAlreadyInstalled function:

  • Tests all scenarios: app enabled, app not found, FIPS messages with different conditions
  • Covers both success and failure cases with proper FIPS message handling
  • Uses proper mock client setup with CR and target pod name

Related Issues

N/A

PR Checklist

  • Code changes adhere to the project's coding standards.
  • Relevant unit and integration tests are included.
  • Documentation has been updated accordingly.
  • All tests pass locally.
  • The PR description follows the project's guidelines.

@coveralls
Copy link
Collaborator

coveralls commented Nov 14, 2025

Pull Request Test Coverage Report for Build 19497371957

Details

  • 29 of 37 (78.38%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.003%) to 86.533%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/splunk/enterprise/afwscheduler.go 29 37 78.38%
Files with Coverage Reduction New Missed Lines %
pkg/splunk/enterprise/afwscheduler.go 1 92.78%
Totals Coverage Status
Change from base Build 19427516711: -0.003%
Covered Lines: 10731
Relevant Lines: 12401

💛 - Coveralls

stdOut, stdErr, err := localCtx.podExecClient.RunPodExecCommand(rctx, streamOptions, []string{"/bin/sh"})

// Handle FIPS messages in stderr - these are informational, not errors
if stdErr != "" && strings.Contains(stdErr, "FIPS provider enabled") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer to ignore stdErr at all - error code should indicate an error. Did we run into problems that are indicated only in stdErr and not in error code?

@patrykw-splunk patrykw-splunk force-pushed the feature/fix-error-handling-in-afwscheduler branch from 977edc7 to 6546a3f Compare November 19, 2025 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants