Skip to content

Commit

Permalink
Make PCS process all subscriptions, stop Maestro's triggerer (#4249)
Browse files Browse the repository at this point in the history
  • Loading branch information
@premun
premun authored Dec 16, 2024
1 parent edf2d20 commit 1f6977f
Show file tree
Hide file tree
Showing 21 changed files with 74 additions and 176 deletions.
109 changes: 49 additions & 60 deletions src/Maestro/DependencyUpdater/DependencyUpdater.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,25 +43,21 @@ public sealed class DependencyUpdater : IServiceImplementation, IDependencyUpdat
private readonly ILogger<DependencyUpdater> _logger;
private readonly BuildAssetRegistryContext _context;
private readonly IActorProxyFactory<ISubscriptionActor> _subscriptionActorFactory;
// TODO (https://github.com/dotnet/arcade-services/issues/3880) - Remove subscriptionIdGenerator
private readonly SubscriptionIdGenerator _subscriptionIdGenerator;

public DependencyUpdater(
IReliableStateManager stateManager,
ILogger<DependencyUpdater> logger,
BuildAssetRegistryContext context,
IBasicBarClient barClient,
IActorProxyFactory<ISubscriptionActor> subscriptionActorFactory,
OperationManager operations,
SubscriptionIdGenerator subscriptionIdGenerator)
OperationManager operations)
{
_operations = operations;
_stateManager = stateManager;
_logger = logger;
_context = context;
_barClient = barClient;
_subscriptionActorFactory = subscriptionActorFactory;
_subscriptionIdGenerator = subscriptionIdGenerator;
}

public async Task StartUpdateDependenciesAsync(int buildId, int channelId)
Expand Down Expand Up @@ -186,7 +182,7 @@ public async Task<TimeSpan> RunAsync(CancellationToken cancellationToken)
/// </summary>
/// <param name="cancellationToken"></param>
/// <returns></returns>
[CronSchedule("0 0 5 1/1 * ? *", TimeZones.PST)]
// [CronSchedule("0 0 5 1/1 * ? *", TimeZones.PST)]
public async Task CheckDailySubscriptionsAsync(CancellationToken cancellationToken)
{
await CheckSubscriptionsAsync(UpdateFrequency.EveryDay, cancellationToken);
Expand All @@ -197,7 +193,7 @@ public async Task CheckDailySubscriptionsAsync(CancellationToken cancellationTok
/// </summary>
/// <param name="cancellationToken"></param>
/// <returns></returns>
[CronSchedule("0 0 5,19 * * ?", TimeZones.PST)]
// [CronSchedule("0 0 5,19 * * ?", TimeZones.PST)]
public async Task CheckTwiceDailySubscriptionsAsync(CancellationToken cancellationToken)
{
await CheckSubscriptionsAsync(UpdateFrequency.TwiceDaily, cancellationToken);
Expand All @@ -208,58 +204,60 @@ public async Task CheckTwiceDailySubscriptionsAsync(CancellationToken cancellati
/// </summary>
/// <param name="cancellationToken"></param>
/// <returns></returns>
[CronSchedule("0 0 5 ? * MON", TimeZones.PST)]
// [CronSchedule("0 0 5 ? * MON", TimeZones.PST)]
public async Task CheckWeeklySubscriptionsAsync(CancellationToken cancellationToken)
{
await CheckSubscriptionsAsync(UpdateFrequency.EveryWeek, cancellationToken);
}

private async Task CheckSubscriptionsAsync(UpdateFrequency targetUpdateFrequency, CancellationToken cancellationToken)
private static Task CheckSubscriptionsAsync(UpdateFrequency targetUpdateFrequency, CancellationToken cancellationToken)
{
using (_operations.BeginOperation($"Updating {targetUpdateFrequency} subscriptions"))
{
var enabledSubscriptionsWithTargetFrequency = (await _context.Subscriptions
.Where(s => s.Enabled)
.ToListAsync(cancellationToken))
.Where(s => s.PolicyObject?.UpdateFrequency == targetUpdateFrequency);

int subscriptionsUpdated = 0;
foreach (var subscription in enabledSubscriptionsWithTargetFrequency)
{
Subscription subscriptionWithBuilds = await _context.Subscriptions
.Where(s => s.Id == subscription.Id)
.Include(s => s.Channel)
.ThenInclude(c => c.BuildChannels)
.ThenInclude(bc => bc.Build)
.FirstOrDefaultAsync(cancellationToken);

if (subscriptionWithBuilds == null)
{
_logger.LogWarning("Subscription {subscriptionId} was not found in the BAR. Not applying updates", subscription.Id.ToString());
continue;
}

Build latestBuildInTargetChannel = subscriptionWithBuilds.Channel.BuildChannels.Select(bc => bc.Build)
.Where(b => (subscription.SourceRepository == b.GitHubRepository || subscription.SourceRepository == b.AzureDevOpsRepository))
.OrderByDescending(b => b.DateProduced)
.FirstOrDefault();

bool isThereAnUnappliedBuildInTargetChannel = latestBuildInTargetChannel != null &&
(subscription.LastAppliedBuild == null || subscription.LastAppliedBuildId != latestBuildInTargetChannel.Id);

if (isThereAnUnappliedBuildInTargetChannel)
{
_logger.LogInformation("Will update {subscriptionId} to build {latestBuildInTargetChannelId}", subscription.Id, latestBuildInTargetChannel.Id);
await UpdateSubscriptionAsync(subscription.Id, latestBuildInTargetChannel.Id);
subscriptionsUpdated++;
}
}

_logger.LogInformation("Updated '{SubscriptionsUpdated}' '{targetUpdateFrequency}' subscriptions", subscriptionsUpdated, targetUpdateFrequency.ToString());
}
// Disabling scheduled subscription updates in Maestro (https://github.com/dotnet/arcade-services/issues/3808)
return Task.CompletedTask;
//using (_operations.BeginOperation($"Updating {targetUpdateFrequency} subscriptions"))
//{
// var enabledSubscriptionsWithTargetFrequency = (await _context.Subscriptions
// .Where(s => s.Enabled)
// .ToListAsync(cancellationToken))
// .Where(s => s.PolicyObject?.UpdateFrequency == targetUpdateFrequency);

// int subscriptionsUpdated = 0;
// foreach (var subscription in enabledSubscriptionsWithTargetFrequency)
// {
// Subscription subscriptionWithBuilds = await _context.Subscriptions
// .Where(s => s.Id == subscription.Id)
// .Include(s => s.Channel)
// .ThenInclude(c => c.BuildChannels)
// .ThenInclude(bc => bc.Build)
// .FirstOrDefaultAsync(cancellationToken);

// if (subscriptionWithBuilds == null)
// {
// _logger.LogWarning("Subscription {subscriptionId} was not found in the BAR. Not applying updates", subscription.Id.ToString());
// continue;
// }

// Build latestBuildInTargetChannel = subscriptionWithBuilds.Channel.BuildChannels.Select(bc => bc.Build)
// .Where(b => (subscription.SourceRepository == b.GitHubRepository || subscription.SourceRepository == b.AzureDevOpsRepository))
// .OrderByDescending(b => b.DateProduced)
// .FirstOrDefault();

// bool isThereAnUnappliedBuildInTargetChannel = latestBuildInTargetChannel != null &&
// (subscription.LastAppliedBuild == null || subscription.LastAppliedBuildId != latestBuildInTargetChannel.Id);

// if (isThereAnUnappliedBuildInTargetChannel)
// {
// _logger.LogInformation("Will update {subscriptionId} to build {latestBuildInTargetChannelId}", subscription.Id, latestBuildInTargetChannel.Id);
// await UpdateSubscriptionAsync(subscription.Id, latestBuildInTargetChannel.Id);
// subscriptionsUpdated++;
// }
// }

// _logger.LogInformation("Updated '{SubscriptionsUpdated}' '{targetUpdateFrequency}' subscriptions", subscriptionsUpdated, targetUpdateFrequency.ToString());
//}
}

[CronSchedule("0 0 0 1/1 * ? *", TimeZones.PST)]
// [CronSchedule("0 0 0 1/1 * ? *", TimeZones.PST)]
public async Task UpdateLongestBuildPathAsync(CancellationToken cancellationToken)
{
using (_operations.BeginOperation($"Updating Longest Build Path table"))
Expand Down Expand Up @@ -312,9 +310,6 @@ public async Task UpdateLongestBuildPathAsync(CancellationToken cancellationToke
/// <summary>
/// Update dependencies for a new build in a channel
/// </summary>
/// <param name="buildId"></param>
/// <param name="channelId"></param>
/// <returns></returns>
public async Task UpdateDependenciesAsync(int buildId, int channelId)
{
Build build = await _context.Builds.FindAsync(buildId);
Expand All @@ -335,12 +330,6 @@ where sub.Enabled

private async Task UpdateSubscriptionAsync(Guid subscriptionId, int buildId)
{
if (!_subscriptionIdGenerator.ShouldTriggerSubscription(subscriptionId))
{
_logger.LogInformation("Skipping subscription '{subscriptionId}', Maestro won't trigger PCS subscriptions", subscriptionId);
return;
}

using (_operations.BeginOperation(
"Updating subscription '{subscriptionId}' with build '{buildId}'",
subscriptionId,
Expand Down
2 changes: 0 additions & 2 deletions src/Maestro/DependencyUpdater/Program.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,5 @@ public static void Configure(IServiceCollection services)
services.AddScoped<IRemoteFactory, RemoteFactory>();
services.AddTransient<IBasicBarClient, SqlBarClient>();
services.AddKustoClientProvider("Kusto");
// TODO (https://github.com/dotnet/arcade-services/issues/3880) - Remove subscriptionIdGenerator
services.AddSingleton<SubscriptionIdGenerator>(sp => new(RunningService.Maestro));
}
}
4 changes: 2 additions & 2 deletions src/Maestro/FeedCleanerService/FeedCleanerService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ public FeedCleanerService(

public Task<TimeSpan> RunAsync(CancellationToken cancellationToken)
{
return Task.FromResult(TimeSpan.FromMinutes(5));
return Task.FromResult(TimeSpan.MaxValue);
}

/// <summary>
/// Updates assets that are now available from one of the non-stable feeds,
/// delete those package versions from the stable feeds and delete any feeds
/// where all packages versions have been deleted every day at 2 AM.
/// </summary>
[CronSchedule("0 0 2 1/1 * ? *", TimeZones.PST)]
// [CronSchedule("0 0 2 1/1 * ? *", TimeZones.PST)]
public async Task CleanManagedFeedsAsync()
{
if (!Options.Enabled)
Expand Down
43 changes: 0 additions & 43 deletions src/Maestro/Maestro.Data/SubscriptionIdGenerator.cs
View file

This file was deleted.

13 changes: 2 additions & 11 deletions src/Maestro/Maestro.Web/Api/v2018_07_16/Controllers/SubscriptionsController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,13 @@ public class SubscriptionsController : Controller
{
private readonly BuildAssetRegistryContext _context;
private readonly IBackgroundQueue _queue;
protected readonly SubscriptionIdGenerator _subscriptionIdGenerator;

public SubscriptionsController(
BuildAssetRegistryContext context,
IBackgroundQueue queue,
SubscriptionIdGenerator subscriptionIdGenerator)
IBackgroundQueue queue)
{
_context = context;
_queue = queue;
_subscriptionIdGenerator = subscriptionIdGenerator;
}

/// <summary>
Expand Down Expand Up @@ -122,11 +119,6 @@ public virtual async Task<IActionResult> TriggerSubscription(Guid id, [FromQuery

protected async Task<IActionResult> TriggerSubscriptionCore(Guid id, int buildId)
{
// TODO (https://github.com/dotnet/arcade-services/issues/3880) - Remove subscriptionIdGenerator
if (!_subscriptionIdGenerator.ShouldTriggerSubscription(id))
{
return BadRequest(new ApiError("Maestro shouldn't trigger PCS subscriptions"));
}
Data.Models.Subscription subscription = await _context.Subscriptions.Include(sub => sub.LastAppliedBuild)
.Include(sub => sub.Channel)
.FirstOrDefaultAsync(sub => sub.Id == id);
Expand Down Expand Up @@ -475,8 +467,7 @@ public virtual async Task<IActionResult> Create([FromBody, Required] Subscriptio

Data.Models.Subscription subscriptionModel = subscription.ToDb();
subscriptionModel.Channel = channel;
// TODO (https://github.com/dotnet/arcade-services/issues/3880) - Remove subscriptionIdGenerator
subscriptionModel.Id = _subscriptionIdGenerator.GenerateSubscriptionId();
subscriptionModel.Id = Guid.NewGuid();

Data.Models.Subscription equivalentSubscription = await FindEquivalentSubscription(subscriptionModel);
if (equivalentSubscription != null)
Expand Down
8 changes: 3 additions & 5 deletions src/Maestro/Maestro.Web/Api/v2019_01_16/Controllers/SubscriptionsController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ public class SubscriptionsController : v2018_07_16.Controllers.SubscriptionsCont

public SubscriptionsController(
BuildAssetRegistryContext context,
IBackgroundQueue queue,
SubscriptionIdGenerator subscriptionIdGenerator)
: base(context, queue, subscriptionIdGenerator)
IBackgroundQueue queue)
: base(context, queue)
{
_context = context;
}
Expand Down Expand Up @@ -275,8 +274,7 @@ public override async Task<IActionResult> Create([FromBody, Required] Maestro.Ap

Data.Models.Subscription subscriptionModel = subscription.ToDb();
subscriptionModel.Channel = channel;
// TODO (https://github.com/dotnet/arcade-services/issues/3880) - Remove subscriptionIdGenerator
subscriptionModel.Id = _subscriptionIdGenerator.GenerateSubscriptionId();
subscriptionModel.Id = Guid.NewGuid();

// Check that we're not about add an existing subscription that is identical
Data.Models.Subscription equivalentSubscription = await FindEquivalentSubscription(subscriptionModel);
Expand Down
8 changes: 3 additions & 5 deletions src/Maestro/Maestro.Web/Api/v2020_02_20/Controllers/SubscriptionsController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ public class SubscriptionsController : v2019_01_16.Controllers.SubscriptionsCont
public SubscriptionsController(
BuildAssetRegistryContext context,
IBackgroundQueue queue,
IGitHubClientFactory gitHubClientFactory,
SubscriptionIdGenerator subscriptionIdGenerator)
: base(context, queue, subscriptionIdGenerator)
IGitHubClientFactory gitHubClientFactory)
: base(context, queue)
{
_context = context;
_gitHubClientFactory = gitHubClientFactory;
Expand Down Expand Up @@ -445,8 +444,7 @@ public async Task<IActionResult> Create([FromBody, Required] SubscriptionData su

Data.Models.Subscription subscriptionModel = subscription.ToDb();
subscriptionModel.Channel = channel;
// TODO (https://github.com/dotnet/arcade-services/issues/3880) - Remove subscriptionIdGenerator
subscriptionModel.Id = _subscriptionIdGenerator.GenerateSubscriptionId();
subscriptionModel.Id = Guid.NewGuid();

// Check that we're not about add an existing subscription that is identical
Data.Models.Subscription equivalentSubscription = await FindEquivalentSubscription(subscriptionModel);
Expand Down
3 changes: 0 additions & 3 deletions src/Maestro/Maestro.Web/Startup.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,6 @@ public override void ConfigureServices(IServiceCollection services)
// in such a way that will work with sizing.
services.AddSingleton<DarcRemoteMemoryCache>();

// TODO (https://github.com/dotnet/arcade-services/issues/3880) - Remove subscriptionIdGenerator
services.AddSingleton<SubscriptionIdGenerator>(sp => new(RunningService.Maestro));

services.AddTransient<IProcessManager>(sp =>
new ProcessManager(
sp.GetRequiredService<ILogger<ProcessManager>>(),
Expand Down
15 changes: 2 additions & 13 deletions ...ice/ProductConstructionService.Api/Api/v2018_07_16/Controllers/SubscriptionsController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,15 @@ public class SubscriptionsController : ControllerBase
private readonly BuildAssetRegistryContext _context;
private readonly IWorkItemProducerFactory _workItemProducerFactory;
private readonly ILogger<SubscriptionsController> _logger;
// TODO (https://github.com/dotnet/arcade-services/issues/3880) - Remove subscriptionIdGenerator
protected readonly SubscriptionIdGenerator _subscriptionIdGenerator;

public SubscriptionsController(
BuildAssetRegistryContext context,
IWorkItemProducerFactory workItemProducerFactory,
ILogger<SubscriptionsController> logger,
SubscriptionIdGenerator subscriptionIdGenerator)
ILogger<SubscriptionsController> logger)
{
_context = context;
_workItemProducerFactory = workItemProducerFactory;
_logger = logger;
_subscriptionIdGenerator = subscriptionIdGenerator;
}

/// <summary>
Expand Down Expand Up @@ -116,13 +112,6 @@ public virtual async Task<IActionResult> TriggerSubscription(Guid id, [FromQuery

protected async Task<IActionResult> TriggerSubscriptionCore(Guid id, int buildId)
{
// TODO (https://github.com/dotnet/arcade-services/issues/3880) - Remove subscriptionIdGenerator
if (!_subscriptionIdGenerator.ShouldTriggerSubscription(id))
{
return BadRequest(
new ApiError($"PCS can only trigger subscriptions which ids start with ${SubscriptionIdGenerator.PcsSubscriptionIdPrefix}")
);
}
Maestro.Data.Models.Subscription? subscription = await _context.Subscriptions.Include(sub => sub.LastAppliedBuild)
.Include(sub => sub.Channel)
.FirstOrDefaultAsync(sub => sub.Id == id);
Expand Down Expand Up @@ -438,7 +427,7 @@ public virtual async Task<IActionResult> Create([FromBody, Required] Subscriptio

Maestro.Data.Models.Subscription subscriptionModel = subscription.ToDb();
subscriptionModel.Channel = channel;
subscriptionModel.Id = _subscriptionIdGenerator.GenerateSubscriptionId();
subscriptionModel.Id = Guid.NewGuid();

Maestro.Data.Models.Subscription? equivalentSubscription = await FindEquivalentSubscription(subscriptionModel);
if (equivalentSubscription != null)
Expand Down
Loading

0 comments on commit 1f6977f

Please sign in to comment.