Skip to content

Fix for OSS pipelines transient failures #5275

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

Jump to bottom
Merged
jestradaMS merged 21 commits into from
Dec 31, 2025
Merged

Fix for OSS pipelines transient failures #5275

jestradaMS merged 21 commits into from
Dec 31, 2025

Conversation

@jestradaMS
Copy link
Contributor

@jestradaMS jestradaMS commented Dec 11, 2025 •
edited by azure-boards bot
Loading

Description

This pull request refactors the CI pipeline to separate CosmosDB and SQL Server integration tests for each FHIR version (Stu3, R4, R4B, R5), introduces dedicated job templates for CosmosDB and SQL Server tests, and improves pipeline robustness by adding retry logic to key tasks. It also enhances the health check process with configurable timeout and success criteria.

Pipeline structure and job separation:

  • Split test stages in build/ci-pipeline.yml to run CosmosDB and SQL Server integration tests separately for each FHIR version, using new templates (run-cosmos-tests.yml and run-sql-tests.yml). This enables more granular control and reporting for each data store and version. [1] [2] [3] [4] [5] [6]
  • Created a new build/jobs/run-cosmos-tests.yml template for CosmosDB integration and E2E tests, and renamed run-tests.yml to run-sql-tests.yml for SQL Server integration tests. The CosmosDB and SQL Server jobs are now fully separated. [1] [2] [3] [4]

Pipeline robustness and reliability:

  • Added retryCountOnTaskFailure: 1 to critical DotNetCoreCLI, AzurePowerShell, and test tasks across build, test, and deployment jobs to improve resilience against transient failures. [1] [2] [3] [4] [5] [6] [7] [8] [9]

Health check improvements:

  • Enhanced the provision-healthcheck.yml script to support configurable timeout and required consecutive successes before passing, providing more robust validation of service health before proceeding.

These changes collectively improve test isolation, reporting, and reliability of the CI pipeline for all supported FHIR versions and data stores.

Related issues

Addresses AB#166378.

Testing

Describe how this change was tested.

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 65 characters
  • Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
  • Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
  • Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
  • Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
  • When changing or adding behavior, if your code modifies the system design or changes design assumptions, please create and include an ADR.
  • CI is green before merge Build Status
  • Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

@jestradaMS jestradaMS requested a review from a team as a code owner December 11, 2025 23:04
@jestradaMS
Copy link
Contributor Author

jestradaMS commented Dec 12, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 12, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@jestradaMS
Copy link
Contributor Author

jestradaMS commented Dec 12, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 12, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@jestradaMS
Copy link
Contributor Author

jestradaMS commented Dec 12, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 12, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@jestradaMS
Copy link
Contributor Author

jestradaMS commented Dec 12, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 12, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@jestradaMS
Copy link
Contributor Author

jestradaMS commented Dec 12, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 12, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@jestradaMS
Copy link
Contributor Author

jestradaMS commented Dec 18, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 18, 2025

Azure Pipelines successfully started running 1 pipeline(s).

github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 18, 2025
View reviewed changes
test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/RetryableCredentialProvider.cs
Comment on lines +57 to +61
catch
{
// If we can't parse the token, assume a short expiration
_tokenExpiration = DateTime.UtcNow.AddMinutes(30);
}

Check notice

Code scanning / CodeQL

Generic catch clause Note test

Generic catch clause.

Copilot Autofix

AI 18 days ago

To fix the problem, replace the generic catch clause in the GetBearerTokenAsync method's token parsing logic with catch clauses for specific exceptions that can actually be thrown by JwtSecurityTokenHandler.ReadJwtToken. According to Microsoft documentation and typical usage, possible exceptions include ArgumentException (invalid token argument), SecurityTokenException (token format issues), and possibly FormatException (malformed token string). Therefore, change the generic catch on line 57 to:

catch (ArgumentException)
catch (SecurityTokenException)
catch (FormatException)

The catch body remains unchanged, as the fallback behavior (setting expiration to 30 minutes from now) is still desired in these cases. Ensure that Microsoft.IdentityModel.Tokens is properly imported for SecurityTokenException (but do not add imports outside demonstrated code regions). No other functionality should be changed.

Suggested changeset 1
test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/RetryableCredentialProvider.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/RetryableCredentialProvider.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/RetryableCredentialProvider.cs
--- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/RetryableCredentialProvider.cs
+++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/RetryableCredentialProvider.cs
@@ -54,11 +54,21 @@
                     var jwtToken = handler.ReadJwtToken(token);
                     _tokenExpiration = jwtToken.ValidTo;
                 }
-                catch
+                catch (ArgumentException)
                 {
                     // If we can't parse the token, assume a short expiration
                     _tokenExpiration = DateTime.UtcNow.AddMinutes(30);
                 }
+                catch (System.IdentityModel.Tokens.Jwt.SecurityTokenException)
+                {
+                    // If we can't parse the token, assume a short expiration
+                    _tokenExpiration = DateTime.UtcNow.AddMinutes(30);
+                }
+                catch (FormatException)
+                {
+                    // If we can't parse the token, assume a short expiration
+                    _tokenExpiration = DateTime.UtcNow.AddMinutes(30);
+                }
             }
 
             return token;
EOF
@@ -54,11 +54,21 @@
var jwtToken = handler.ReadJwtToken(token);
_tokenExpiration = jwtToken.ValidTo;
}
catch
catch (ArgumentException)
{
// If we can't parse the token, assume a short expiration
_tokenExpiration = DateTime.UtcNow.AddMinutes(30);
}
catch (System.IdentityModel.Tokens.Jwt.SecurityTokenException)
{
// If we can't parse the token, assume a short expiration
_tokenExpiration = DateTime.UtcNow.AddMinutes(30);
}
catch (FormatException)
{
// If we can't parse the token, assume a short expiration
_tokenExpiration = DateTime.UtcNow.AddMinutes(30);
}
}

return token;
Copilot is powered by AI and may make mistakes. Always verify output.
@jestradaMS
test: replace Fact with RetryFact for high contention scenario in Sea…
3ef5d06 
…rchParameterOptimisticConcurrencyIntegrationTests
@jestradaMS
Revert "feat(tests): implement multi-layer retry strategy for transie…
aa75324 
…nt failures in E2E and integration tests"

This reverts commit 8879a39.
@jestradaMS jestradaMS changed the title [DO NOT REVIEW YET] Fix for e2e transient auth failures Fix for pipeline transient failures Dec 19, 2025
@jestradaMS jestradaMS changed the title Fix for pipeline transient failures Fix for OSS pipelines transient failures Dec 19, 2025
@jestradaMS jestradaMS added this to the CY25Q3/2Wk13 milestone Dec 19, 2025
@jestradaMS jestradaMS added Enhancement-Test Enhancement on tests. Build labels Dec 19, 2025
@jestradaMS
Copy link
Contributor Author

jestradaMS commented Dec 19, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 19, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@jestradaMS
Copy link
Contributor Author

jestradaMS commented Dec 19, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 19, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@jestradaMS
Copy link
Contributor Author

jestradaMS commented Dec 19, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 19, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@jestradaMS
Copy link
Contributor Author

jestradaMS commented Dec 22, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 22, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@jestradaMS
Copy link
Contributor Author

jestradaMS commented Dec 22, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Dec 22, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@jestradaMS jestradaMS merged commit de52856 into main Dec 31, 2025
61 checks passed
@jestradaMS jestradaMS deleted the users/jestrada/fixe2etestauthfailures branch December 31, 2025 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fhibf fhibf fhibf approved these changes

Assignees

No one assigned

Labels

Build Enhancement-Test Enhancement on tests.

Projects

None yet

Milestone

CY25Q3/2Wk13

Development

Successfully merging this pull request may close these issues.

3 participants

@jestradaMS @fhibf