Skip to content
This repository was archived by the owner on Jul 30, 2024. It is now read-only.

Commit f965fa5

Browse files
agragr
authored
Fix for missing packages in public container. (#334)
* Moved unrelated calls out of try block that contains Gallery DB udpate. * Enhanced HandleAsync call information * Tracking nupkgs missing in public container for packages that are marked as available. * Dropped call IDs * null check in ValidationMessageHandler.HandleAsync * Tracking Validation tracking ID * Test that verifies that package is not deleted when success notification sending fails * Log message * Updated to the latest NuGet.Services.* * Using release version of the NuGet.Services.*
1 parent 63ef8f4 commit f965fa5

8 files changed

Lines changed: 128 additions & 31 deletions

File tree

‎src/NuGet.Services.Validation.Orchestrator/NuGet.Services.Validation.Orchestrator.csproj‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,25 +133,25 @@
133133
<Version>1.1.2</Version>
134134
</PackageReference>
135135
<PackageReference Include="NuGet.Services.Configuration">
136-
<Version>2.13.0</Version>
136+
<Version>2.14.0</Version>
137137
</PackageReference>
138138
<PackageReference Include="NuGet.Services.Contracts">
139-
<Version>2.13.0</Version>
139+
<Version>2.14.0</Version>
140140
</PackageReference>
141141
<PackageReference Include="NuGet.Services.KeyVault">
142-
<Version>2.13.0</Version>
142+
<Version>2.14.0</Version>
143143
</PackageReference>
144144
<PackageReference Include="NuGet.Services.Logging">
145-
<Version>2.13.0</Version>
145+
<Version>2.14.0</Version>
146146
</PackageReference>
147147
<PackageReference Include="NuGet.Services.ServiceBus">
148-
<Version>2.13.0</Version>
148+
<Version>2.14.0</Version>
149149
</PackageReference>
150150
<PackageReference Include="NuGet.Services.Validation">
151-
<Version>2.13.0</Version>
151+
<Version>2.14.0</Version>
152152
</PackageReference>
153153
<PackageReference Include="NuGet.Services.Validation.Issues">
154-
<Version>2.13.0</Version>
154+
<Version>2.14.0</Version>
155155
</PackageReference>
156156
<PackageReference Include="NuGet.Versioning">
157157
<Version>4.3.0</Version>

‎src/NuGet.Services.Validation.Orchestrator/Telemetry/ITelemetryService.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,5 +78,11 @@ public interface ITelemetryService
7878
/// <param name="validatorType">The validator type (name) the produced the issue.</param>
7979
/// <param name="clientCode">The client code.</param>
8080
void TrackClientValidationIssue(string validatorType, string clientCode);
81+
82+
/// <summary>
83+
/// A metric for the case when orchestrator sees a package marked as available, but the blob is missing
84+
/// in the public container.
85+
/// </summary>
86+
void TrackMissingNupkgForAvailablePackage(string packageId, string normalizedVersion, string validationTrackingId);
8187
}
8288
}

‎src/NuGet.Services.Validation.Orchestrator/Telemetry/TelemetryService.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,17 @@ public class TelemetryService : ITelemetryService
2121
private const string ValidatorDurationSeconds = Prefix + "ValidatorDurationSeconds";
2222
private const string ValidatorStarted = Prefix + "ValidatorStarted";
2323
private const string ClientValidationIssue = Prefix + "ClientValidationIssue";
24+
private const string MissingNupkgForAvailablePackage = Prefix + "MissingNupkgForAvailablePackage";
2425

2526
private const string FromStatus = "FromStatus";
2627
private const string ToStatus = "ToStatus";
2728
private const string IsSuccess = "IsSuccess";
2829
private const string ValidatorType = "ValidatorType";
2930
private const string IssueCode = "IssueCode";
3031
private const string ClientCode = "ClientCode";
32+
private const string PackageId = "PackageId";
33+
private const string NormalizedVersion = "NormalizedVersion";
34+
private const string ValidationTrackingId = "ValidationTrackingId";
3135

3236
private readonly TelemetryClient _telemetryClient;
3337

@@ -135,5 +139,16 @@ public void TrackClientValidationIssue(string validatorType, string clientCode)
135139
{ ClientCode, clientCode },
136140
});
137141
}
142+
143+
public void TrackMissingNupkgForAvailablePackage(string packageId, string normalizedVersion, string validationTrackingId)
144+
=> _telemetryClient.TrackMetric(
145+
MissingNupkgForAvailablePackage,
146+
1,
147+
new Dictionary<string, string>
148+
{
149+
{ PackageId, packageId },
150+
{ NormalizedVersion, normalizedVersion },
151+
{ ValidationTrackingId, validationTrackingId },
152+
});
138153
}
139154
}

‎src/NuGet.Services.Validation.Orchestrator/ValidationMessageHandler.cs‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,27 @@ public ValidationMessageHandler(
3333

3434
public async Task<bool> HandleAsync(PackageValidationMessageData message)
3535
{
36-
var package = _galleryPackageService.FindPackageByIdAndVersionStrict(message.PackageId, message.PackageVersion);
37-
38-
if (package == null)
36+
if (message == null)
3937
{
40-
// no package in DB yet. Might have received message a bit early, need to retry later
41-
_logger.LogInformation("Did not find information in DB for package {PackageId} {PackageVersion}",
42-
message.PackageId,
43-
message.PackageVersion);
44-
return false;
38+
throw new ArgumentNullException(nameof(message));
4539
}
4640

47-
using (_logger.BeginScope("Handling message for {PackageId} {PackageVersion} validation set {ValidationSetId}", message.PackageId, message.PackageVersion, message.ValidationTrackingId))
41+
using (_logger.BeginScope("Handling message for {PackageId} {PackageVersion} validation set {ValidationSetId}",
42+
message.PackageId,
43+
message.PackageVersion,
44+
message.ValidationTrackingId))
4845
{
46+
var package = _galleryPackageService.FindPackageByIdAndVersionStrict(message.PackageId, message.PackageVersion);
47+
48+
if (package == null)
49+
{
50+
// no package in DB yet. Might have received message a bit early, need to retry later
51+
_logger.LogInformation("Did not find information in DB for package {PackageId} {PackageVersion}",
52+
message.PackageId,
53+
message.PackageVersion);
54+
return false;
55+
}
56+
4957
var validationSet = await _validationSetProvider.TryGetOrCreateValidationSetAsync(message.ValidationTrackingId, package);
5058

5159
if (validationSet == null)

‎src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,20 @@ ValidationConfigurationItem GetValidationConfigurationItem(string validationName
122122
package.PackageRegistration.Id,
123123
package.NormalizedVersion,
124124
validationSet.ValidationTrackingId);
125+
126+
if (!await _packageFileService.DoesPackageFileExistAsync(package))
127+
{
128+
var validationPackageAvailable = await _packageFileService.DoesValidationPackageFileExistAsync(package);
129+
130+
_logger.LogWarning("Package {PackageId} {PackageVersion} is marked as available, but does not exist " +
131+
"in public container. Does package exist in validation container: {ExistsInValidation}",
132+
package.PackageRegistration.Id,
133+
package.NormalizedVersion,
134+
validationPackageAvailable);
135+
136+
// report missing package, don't try to fix up anything. This shouldn't happen and needs an investigation.
137+
TrackMissingNupkgForAvailablePackage(validationSet);
138+
}
125139
}
126140
_logger.LogInformation("Done processing {PackageId} {PackageVersion} {ValidationSetId}",
127141
package.PackageRegistration.Id,
@@ -139,6 +153,14 @@ ValidationConfigurationItem GetValidationConfigurationItem(string validationName
139153
}
140154
}
141155

156+
private void TrackMissingNupkgForAvailablePackage(PackageValidationSet validationSet)
157+
{
158+
_telemetryService.TrackMissingNupkgForAvailablePackage(
159+
validationSet.PackageId,
160+
validationSet.PackageNormalizedVersion,
161+
validationSet.ValidationTrackingId.ToString());
162+
}
163+
142164
private void TrackTotalValidationDuration(PackageValidationSet validationSet, bool isSuccess)
143165
{
144166
_telemetryService.TrackTotalValidationDuration(
@@ -155,17 +177,15 @@ private async Task MoveFileToPublicStorageAndMarkPackageAsAvailable(PackageValid
155177
var packageStream = await _packageFileService.DownloadValidationPackageFileAsync(package);
156178
await _packageFileService.SavePackageFileAsync(package, packageStream);
157179

180+
_logger.LogInformation("Marking package {PackageId} {PackageVersion}, validation set {ValidationSetId} as {PackageStatus} in DB",
181+
package.PackageRegistration.Id,
182+
package.NormalizedVersion,
183+
validationSet.ValidationTrackingId,
184+
PackageStatus.Available);
185+
158186
try
159187
{
160-
_logger.LogInformation("Marking package {PackageId} {PackageVersion}, validation set {ValidationSetId} as {PackageStatus} in DB",
161-
package.PackageRegistration.Id,
162-
package.NormalizedVersion,
163-
validationSet.ValidationTrackingId,
164-
PackageStatus.Available);
165-
166188
await UpdatePackageStatusAsync(package, PackageStatus.Available);
167-
168-
_messageService.SendPackagePublishedMessage(package);
169189
}
170190
catch (Exception e)
171191
{
@@ -182,6 +202,8 @@ private async Task MoveFileToPublicStorageAndMarkPackageAsAvailable(PackageValid
182202
throw;
183203
}
184204

205+
_messageService.SendPackagePublishedMessage(package);
206+
185207
_logger.LogInformation("Deleting from the source for package {PackageId} {PackageVersion}, validation set {ValidationSetId}",
186208
package.PackageRegistration.Id,
187209
package.NormalizedVersion,

‎src/Validation.PackageSigning.ExtractAndValidateSignature/Validation.PackageSigning.ExtractAndValidateSignature.csproj‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,22 +109,22 @@
109109
<Version>4.7.0-preview1-4886</Version>
110110
</PackageReference>
111111
<PackageReference Include="NuGet.Services.Configuration">
112-
<Version>2.13.0</Version>
112+
<Version>2.14.0</Version>
113113
</PackageReference>
114114
<PackageReference Include="NuGet.Services.Contracts">
115-
<Version>2.13.0</Version>
115+
<Version>2.14.0</Version>
116116
</PackageReference>
117117
<PackageReference Include="NuGet.Services.KeyVault">
118-
<Version>2.13.0</Version>
118+
<Version>2.14.0</Version>
119119
</PackageReference>
120120
<PackageReference Include="NuGet.Services.Logging">
121-
<Version>2.13.0</Version>
121+
<Version>2.14.0</Version>
122122
</PackageReference>
123123
<PackageReference Include="NuGet.Services.ServiceBus">
124-
<Version>2.13.0</Version>
124+
<Version>2.14.0</Version>
125125
</PackageReference>
126126
<PackageReference Include="NuGet.Services.Validation">
127-
<Version>2.13.0</Version>
127+
<Version>2.14.0</Version>
128128
</PackageReference>
129129
<PackageReference Include="NuGetGallery.Core">
130130
<Version>4.4.4-dev-19677</Version>

‎tests/NuGet.Services.Validation.Orchestrator.Tests/NuGet.Services.Validation.Orchestrator.Tests.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@
6969
<Version>4.7.145</Version>
7070
</PackageReference>
7171
<PackageReference Include="NuGet.Services.Validation.Issues">
72-
<Version>2.13.0</Version>
72+
<Version>2.14.0</Version>
7373
</PackageReference>
7474
<PackageReference Include="System.Net.Http">
7575
<Version>4.3.3</Version>

‎tests/NuGet.Services.Validation.Orchestrator.Tests/ValidationOutcomeProcessorFacts.cs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,52 @@ public async Task PrefersDbOverConfigurationForDeterminingSuccess(
370370
}
371371
}
372372

373+
[Theory]
374+
[InlineData(true)]
375+
[InlineData(false)]
376+
public async Task TracksMissingNupkgForAvailablePackage(bool validationFileExists)
377+
{
378+
Package.PackageStatusKey = PackageStatus.Available;
379+
PackageFileServiceMock
380+
.Setup(pfs => pfs.DoesPackageFileExistAsync(Package))
381+
.ReturnsAsync(false);
382+
PackageFileServiceMock
383+
.Setup(pfs => pfs.DoesValidationPackageFileExistAsync(Package))
384+
.ReturnsAsync(validationFileExists);
385+
386+
var processor = CreateProcessor();
387+
await processor.ProcessValidationOutcomeAsync(ValidationSet, Package);
388+
389+
TelemetryServiceMock
390+
.Verify(ts => ts.TrackMissingNupkgForAvailablePackage(
391+
ValidationSet.PackageId,
392+
ValidationSet.PackageNormalizedVersion,
393+
ValidationSet.ValidationTrackingId.ToString()),
394+
Times.Once());
395+
}
396+
397+
[Fact]
398+
public async Task PublishedNotificationSendFailureDoesNotCausePackageToBeDeleted()
399+
{
400+
var exception = new Exception("Something baaad happened");
401+
402+
MessageServiceMock
403+
.Setup(ms => ms.SendPackagePublishedMessage(It.IsAny<Package>()))
404+
.Throws(exception);
405+
406+
Package.PackageStatusKey = PackageStatus.Validating;
407+
408+
var processor = CreateProcessor();
409+
var thrownException = await Record.ExceptionAsync(async () => await processor.ProcessValidationOutcomeAsync(ValidationSet, Package));
410+
411+
Assert.NotNull(thrownException);
412+
PackageServiceMock
413+
.Verify(ps => ps.UpdatePackageStatusAsync(Package, PackageStatus.Available, true),
414+
Times.Once());
415+
PackageFileServiceMock
416+
.Verify(pfs => pfs.DeletePackageFileAsync(It.IsAny<string>(), It.IsAny<string>()), Times.Never());
417+
}
418+
373419
public ValidationOutcomeProcessorFacts()
374420
{
375421
PackageServiceMock = new Mock<ICorePackageService>();

0 commit comments

Comments
 (0)