Skip to content

Commit f16c288

Browse files
tippmar-nrtippmar-nr
authored
ci: retry test jobs once on transient failure; raise SE.Redis timeouts (#3684)
1 parent 0d111f6 commit f16c288

9 files changed

Lines changed: 130 additions & 17 deletions

File tree

‎.github/workflows/all_solutions.yml‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,30 @@ jobs:
657657
$json | Add-Member -Name "parallelizeTestCollections" -Value $parallelize -MemberType NoteProperty
658658
$json | ConvertTo-Json | Out-File "${{ env.integration_tests_path }}/xunit.runner.json"
659659
660-
${{ env.integration_tests_path }}/NewRelic.Agent.IntegrationTests.exe -namespace NewRelic.Agent.IntegrationTests.${{ matrix.namespace }} -trx "C:\IntegrationTestWorkingDirectory\TestResults\${{ matrix.namespace }}_testResults.trx"
660+
# Retry once on failure to absorb transient infrastructure blips (e.g. port
661+
# contention, SSL cert / tooling startup races, or a flaky test-app launch) that
662+
# can surface as a spurious failure while the code under test is healthy. A
663+
# retried-but-passing job still emits a ::warning:: so genuine regressions /
664+
# flake churn stay visible in the UI.
665+
$maxAttempts = 2
666+
$attempt = 1
667+
while ($true) {
668+
& "${{ env.integration_tests_path }}/NewRelic.Agent.IntegrationTests.exe" -namespace NewRelic.Agent.IntegrationTests.${{ matrix.namespace }} -trx "C:\IntegrationTestWorkingDirectory\TestResults\${{ matrix.namespace }}_testResults.trx"
669+
$exitCode = $LASTEXITCODE
670+
if ($exitCode -eq 0) {
671+
if ($attempt -gt 1) {
672+
Write-Host "::warning::Integration tests for ${{ matrix.namespace }} passed on attempt $attempt after a transient failure. Investigate if this recurs."
673+
}
674+
break
675+
}
676+
if ($attempt -ge $maxAttempts) {
677+
Write-Host "Integration tests for ${{ matrix.namespace }} failed after $attempt attempt(s) (exit code $exitCode)."
678+
exit $exitCode
679+
}
680+
Write-Host "Integration tests for ${{ matrix.namespace }} failed on attempt $attempt (exit code $exitCode); retrying once to absorb a possible transient blip..."
681+
$attempt++
682+
Start-Sleep -Seconds 10
683+
}
661684
662685
if ($Env:enhanced_logging -eq $True) {
663686
Write-Host "Get HostableWebCore errors (if any)"
@@ -850,7 +873,29 @@ jobs:
850873
$json | Add-Member -Name "parallelizeTestCollections" -Value $parallelize -MemberType NoteProperty
851874
$json | ConvertTo-Json | Out-File "${{ env.unbounded_tests_path }}/xunit.runner.json"
852875
853-
${{ env.unbounded_tests_path }}/NewRelic.Agent.UnboundedIntegrationTests.exe -namespace NewRelic.Agent.UnboundedIntegrationTests.${{ matrix.namespace }} -trx "C:\IntegrationTestWorkingDirectory\TestResults\${{ matrix.namespace }}_testResults.trx"
876+
# Retry once on failure to absorb transient WAN/infra blips between the GitHub-hosted
877+
# runner and the Azure-hosted unbounded services (e.g. brief packet loss surfacing as a
878+
# client command timeout while the service itself is healthy). A retried-but-passing job
879+
# still emits a ::warning:: so genuine regressions / flake churn stay visible in the UI.
880+
$maxAttempts = 2
881+
$attempt = 1
882+
while ($true) {
883+
& "${{ env.unbounded_tests_path }}/NewRelic.Agent.UnboundedIntegrationTests.exe" -namespace NewRelic.Agent.UnboundedIntegrationTests.${{ matrix.namespace }} -trx "C:\IntegrationTestWorkingDirectory\TestResults\${{ matrix.namespace }}_testResults.trx"
884+
$exitCode = $LASTEXITCODE
885+
if ($exitCode -eq 0) {
886+
if ($attempt -gt 1) {
887+
Write-Host "::warning::Unbounded tests for ${{ matrix.namespace }} passed on attempt $attempt after a transient failure. Investigate if this recurs."
888+
}
889+
break
890+
}
891+
if ($attempt -ge $maxAttempts) {
892+
Write-Host "Unbounded tests for ${{ matrix.namespace }} failed after $attempt attempt(s) (exit code $exitCode)."
893+
exit $exitCode
894+
}
895+
Write-Host "Unbounded tests for ${{ matrix.namespace }} failed on attempt $attempt (exit code $exitCode); retrying once to absorb a possible transient blip..."
896+
$attempt++
897+
Start-Sleep -Seconds 10
898+
}
854899
855900
if ($Env:enhanced_logging -eq $True) {
856901
Write-Host "Get HostableWebCore errors (if any)"

‎.github/workflows/check_modified_files.yml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,11 @@ jobs:
3535
uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1
3636
id: filter
3737
with:
38-
base: ${{ github.ref }}
38+
# 'base' is ignored (and warns) on pull_request events, where the action
39+
# auto-detects the PR base branch; only pass it for push/schedule/etc.
40+
# Note: the '!=' form avoids the GitHub Actions ternary pitfall where an
41+
# empty-string middle operand is falsy and falls through to github.ref.
42+
base: ${{ github.event_name != 'pull_request' && github.ref || '' }}
3943
# Look for source files that were modified, excluding workflow files and unbounded services
4044
predicate-quantifier: 'every'
4145
filters: |

‎.github/workflows/linux_container_tests.yml‎

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,38 @@ jobs:
128128
- name: Build & Run Linux Container Integration Tests
129129
env:
130130
BUILD_ARCH: ${{ matrix.arch }}
131-
run: >-
132-
dotnet test ./tests/Agent/IntegrationTests/ContainerIntegrationTests/ContainerIntegrationTests.csproj
133-
--framework net10.0
134-
--filter "Architecture=${{ matrix.arch }}&Distro=${{ matrix.distro }}"
135-
--logger "console;verbosity=detailed"
136-
--logger "trx;verbosity=detailed"
137-
--results-directory ${{ env.test_results_path }}
131+
run: |
132+
# Retry once on failure to absorb transient infrastructure blips (e.g. container
133+
# image pulls, Docker networking, or service-container startup races) that can
134+
# surface as a spurious failure while the code under test is healthy. A
135+
# retried-but-passing job still emits a ::warning:: so genuine regressions /
136+
# flake churn stay visible in the UI.
137+
max_attempts=2
138+
attempt=1
139+
while true; do
140+
set +e
141+
dotnet test ./tests/Agent/IntegrationTests/ContainerIntegrationTests/ContainerIntegrationTests.csproj \
142+
--framework net10.0 \
143+
--filter "Architecture=${{ matrix.arch }}&Distro=${{ matrix.distro }}" \
144+
--logger "console;verbosity=detailed" \
145+
--logger "trx;verbosity=detailed" \
146+
--results-directory ${{ env.test_results_path }}
147+
exit_code=$?
148+
set -e
149+
if [ $exit_code -eq 0 ]; then
150+
if [ $attempt -gt 1 ]; then
151+
echo "::warning::Container tests for ${{ matrix.arch }}/${{ matrix.distro }} passed on attempt $attempt after a transient failure. Investigate if this recurs."
152+
fi
153+
break
154+
fi
155+
if [ $attempt -ge $max_attempts ]; then
156+
echo "Container tests for ${{ matrix.arch }}/${{ matrix.distro }} failed after $attempt attempt(s) (exit code $exit_code)."
157+
exit $exit_code
158+
fi
159+
echo "Container tests for ${{ matrix.arch }}/${{ matrix.distro }} failed on attempt $attempt (exit code $exit_code); retrying once to absorb a possible transient blip..."
160+
attempt=$((attempt + 1))
161+
sleep 10
162+
done
138163
139164
- name: Extract payload bytes log from TRX
140165
if: inputs.aggregate_payload_data == true

‎tests/Agent/IntegrationTests/ContainerApplications/AwsSdkTestApp/Dockerfile‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,15 @@ EXPOSE 80
1010
FROM --platform=${BUILD_ARCH} mcr.microsoft.com/dotnet/sdk:${DOTNET_VERSION}-${DISTRO_TAG} AS build
1111
ARG TARGET_ARCH
1212
WORKDIR /src
13+
COPY ["AwsSdkTestApp/AwsSdkTestApp.csproj", "AwsSdkTestApp/"]
14+
RUN dotnet restore "AwsSdkTestApp/AwsSdkTestApp.csproj" -a ${TARGET_ARCH}
1315

14-
COPY ./AwsSdkTestApp ./ContainerApplications/AwsSdkTestApp
15-
RUN dotnet restore "ContainerApplications/AwsSdkTestApp/AwsSdkTestApp.csproj" -a ${TARGET_ARCH}
16-
17-
WORKDIR "/src/ContainerApplications/AwsSdkTestApp"
18-
RUN dotnet build "AwsSdkTestApp.csproj" -c Release -o /app/build --os linux -a ${TARGET_ARCH}
16+
COPY . .
17+
WORKDIR "/src/AwsSdkTestApp"
18+
RUN dotnet build "AwsSdkTestApp.csproj" -c Release -o /app/build --os linux -a ${TARGET_ARCH} --no-restore
1919

2020
FROM build AS publish
21-
RUN dotnet publish "AwsSdkTestApp.csproj" -c Release -o /app/publish /p:UseAppHost=false --os linux -a ${TARGET_ARCH}
21+
RUN dotnet publish "AwsSdkTestApp.csproj" -c Release -o /app/publish /p:UseAppHost=false --os linux -a ${TARGET_ARCH} --no-restore
2222

2323
FROM base AS final
2424

‎tests/Agent/IntegrationTests/ContainerApplications/Directory.Build.props‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<Project>
2+
<!-- Host builds: chain to the IntegrationTests/root Directory.Build.props so
3+
these projects keep TreatWarningsAsErrors, the shared NoWarn set, etc.
4+
Inside the container the Docker build context is only this folder, so
5+
GetPathOfFileAbove finds nothing above /src and the import is skipped.
6+
The path is resolved into a property first because nesting the
7+
GetPathOfFileAbove call (with its quoted args) directly inside an Import
8+
Condition trips the MSBuild condition parser (MSB4092). -->
9+
<PropertyGroup>
10+
<_ParentDirectoryBuildProps>$([MSBuild]::GetPathOfFileAbove('Directory.Build.props', '$(MSBuildThisFileDirectory)..'))</_ParentDirectoryBuildProps>
11+
</PropertyGroup>
12+
<Import Project="$(_ParentDirectoryBuildProps)" Condition="'$(_ParentDirectoryBuildProps)' != ''" />
13+
<PropertyGroup>
14+
<!-- Container apps compile inside an isolated Docker context that never sees the
15+
IntegrationTests Directory.Build.props, so restate the naming-convention
16+
suppression here. VSTHRD002 is handled per call site with pragmas instead. -->
17+
<NoWarn>$(NoWarn);VSTHRD200</NoWarn>
18+
</PropertyGroup>
19+
</Project>

‎tests/Agent/IntegrationTests/ContainerApplications/KafkaTestApp/Producer.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,14 @@ public void CreateTopic(IConfiguration configuration)
125125
throw new InvalidOperationException("Kafka metadata returned zero brokers.");
126126
}
127127

128+
// Deliberate synchronous wait: CreateTopic is synchronous startup setup and
129+
// Confluent.Kafka's AdminClient only exposes CreateTopicsAsync.
130+
#pragma warning disable VSTHRD002
128131
adminClient.CreateTopicsAsync(new TopicSpecification[] {
129132
new TopicSpecification { Name = _topic, ReplicationFactor = 1, NumPartitions = 1 },
130133
new TopicSpecification { Name = _burstTopic, ReplicationFactor = 1, NumPartitions = 1 }
131134
}).GetAwaiter().GetResult();
135+
#pragma warning restore VSTHRD002
132136

133137
_logger.LogInformation("Created topics '{Topic}' and '{BurstTopic}' on attempt {Attempt}.",
134138
_topic, _burstTopic, attempt);

‎tests/Agent/IntegrationTests/ContainerApplications/W3CTestApp/Controllers/W3CController.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,13 @@ private void ProcessModels(List<W3CTestModel> models)
6363
foreach (var model in models)
6464
{
6565
var request = BuildHttpRequest(model);
66+
// Deliberate synchronous wait: the calls must run synchronously and in order so the
67+
// agent injects the W3C trace context headers the harness validates (see comment above).
68+
#pragma warning disable VSTHRD002
6669
_ = client
6770
.SendAsync(request, HttpCompletionOption.ResponseHeadersRead)
6871
.Result;
72+
#pragma warning restore VSTHRD002
6973
}
7074
}
7175

‎tests/Agent/IntegrationTests/Directory.Build.props‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
<NoWarn>
66
VSTHRD200, <!-- Use `Async` suffix for async methods -->
77
VSTHRD105, <!-- Avoid method overloads that assume `TaskScheduler.Current` -->
8-
VSTHRD002 <!-- Avoid problematic synchronous waits -->
8+
VSTHRD002, <!-- Avoid problematic synchronous waits -->
9+
NETSDK1198 <!-- SDK-style test apps have no classic LocalDeploy MSDeploy profile; PublishProfile is applied solution-wide -->
910
</NoWarn>
1011
<NuGetAudit>false</NuGetAudit>
1112
</PropertyGroup>

‎tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/StackExchangeRedisExerciser.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,17 @@ private ConfigurationOptions GetRedisConnectionOptions()
133133
var connectionString = StackExchangeRedisConfiguration.StackExchangeRedisConnectionString;
134134
_redisConfigOptions = ConfigurationOptions.Parse(connectionString);
135135
_redisConfigOptions.Password = StackExchangeRedisConfiguration.StackExchangeRedisPassword;
136+
137+
// These tests run from GitHub-hosted runners across the public internet to Azure-hosted
138+
// Redis. Transient WAN packet-loss stalls can exceed the 5s SE.Redis defaults and surface
139+
// as "Timeout performing <cmd>" flakes even when the server is healthy. Raise the timeouts
140+
// to ride out brief blips. Command-level retries are deliberately NOT used -- they would
141+
// add extra instrumented calls and perturb the metric call counts these tests assert on.
142+
// Note: only properties present since SE.Redis 1.x are set here (oldest pinned is 1.x);
143+
// AsyncTimeout (2.0+) is intentionally omitted, and in 1.x SyncTimeout governs async waits too.
144+
_redisConfigOptions.SyncTimeout = 30000;
145+
_redisConfigOptions.ConnectTimeout = 30000;
146+
_redisConfigOptions.ConnectRetry = 5;
136147
}
137148
return _redisConfigOptions;
138149
}

0 commit comments

Comments
 (0)