Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions src/Altinn.App.Api/Extensions/ServiceCollectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Altinn.App.Api.Helpers.Patch;
using Altinn.App.Api.Infrastructure.Filters;
using Altinn.App.Api.Infrastructure.Health;
using Altinn.App.Api.Infrastructure.Lifetime;
using Altinn.App.Api.Infrastructure.Middleware;
using Altinn.App.Api.Infrastructure.Telemetry;
using Altinn.App.Core.Constants;
Expand Down Expand Up @@ -124,6 +125,8 @@ IWebHostEnvironment env
options.AllowSynchronousIO = true;
});

ConfigureGracefulShutdown(services);

Comment on lines +128 to +129
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Do not override ConsoleLifetime in Development; gate registration to tt02/prod

Registering a custom IHostLifetime unconditionally replaces ConsoleLifetime, which can break Ctrl+C handling locally. Limit registration to staging++ (tt02) and prod, and pass env to the helper.

Apply:

-        ConfigureGracefulShutdown(services);
+        ConfigureGracefulShutdown(services, env);

And update the helper signature and gating (see diff on the method below).

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/Altinn.App.Api/Extensions/ServiceCollectionExtensions.cs around lines
128-129, ConfigureGracefulShutdown(services) is registering a custom
IHostLifetime unconditionally which overrides ConsoleLifetime in Development;
change the call to accept the current environment and only register the custom
lifetime when the environment is tt02 or Production. Update the
ConfigureGracefulShutdown helper signature to accept IHostEnvironment (or
environment name) and move the conditional there so that in Development the
default ConsoleLifetime remains untouched while in tt02/prod the custom
IHostLifetime is added to the services.

// HttpClients for platform functionality. Registered as HttpClient so default HttpClientFactory is used
services.AddHttpClient<AuthorizationApiClient>();

Expand Down Expand Up @@ -546,4 +549,34 @@ IHostEnvironment env

return $"InstrumentationKey={key}";
}

private static void ConfigureGracefulShutdown(IServiceCollection services)
{
// Need to coordinate graceful shutdown (let's assume k8s as the scheduler/runtime):
// - deployment is configured with a terminationGracePeriod of 30s (default timeout before SIGKILL)
// - k8s flow of information is eventually consistent.
// it takes time for knowledge of SIGTERM on the worker node to propagate to e.g. networking layers
// (k8s Service -> Endspoints rotation. It takes time to be taken out of Endpoint rotation)
// - we want to gracefully drain ASP.NET core for requests, leaving some time for active requests to complete
// This leaves us with the following sequence of events
// - container receives SIGTERM
// - `AppHostLifetime` intercepts SIGTERM and delays for `shutdownDelay`
// - `AppHostLifetime` calls `IHostApplicationLifetime.StopApplication`, to start ASP.NET Core shutdown process
// - ASP.NET Core will spend a maximum of `shutdownTimeout` trying to drain active requests
// (cancelable requests can combine cancellation tokens with `IHostApplicationLifetime.ApplicationStopping`)
// - If ASP.NET Core completes shutdown within `shutdownTimeout`, everything is fine
// - If ASP.NET Core is stuck or in some way can't terminate, kubelet will eventually SIGKILL
var shutdownDelay = TimeSpan.FromSeconds(5);
var shutdownTimeout = TimeSpan.FromSeconds(20);

services.AddSingleton<IHostLifetime>(serviceProvider =>
{
var logger = serviceProvider.GetRequiredService<ILogger<AppHostLifetime>>();
var env = serviceProvider.GetRequiredService<IHostEnvironment>();
var lifetime = serviceProvider.GetRequiredService<IHostApplicationLifetime>();
return new AppHostLifetime(logger, env, lifetime, shutdownDelay);
});

services.Configure<HostOptions>(options => options.ShutdownTimeout = shutdownTimeout);
}
Comment on lines +552 to +581
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Gate by environment; make idempotent values configurable; fix typo

  • Gate lifetime registration to staging++ (tt02) and Production only.
  • Consider configuring delay/timeout from config.
  • Minor: “Endspoints” -> “Endpoints”.
-    private static void ConfigureGracefulShutdown(IServiceCollection services)
+    private static void ConfigureGracefulShutdown(IServiceCollection services, IHostEnvironment env)
     {
-        // Need to coordinate graceful shutdown (let's assume k8s as the scheduler/runtime):
+        // Need to coordinate graceful shutdown (let's assume k8s as the scheduler/runtime):
         // - deployment is configured with a terminationGracePeriod of 30s (default timeout before SIGKILL)
         // - k8s flow of information is eventually consistent.
-        //   it takes time for knowledge of SIGTERM on the worker node to propagate to e.g. networking layers
-        //   (k8s Service -> Endspoints rotation. It takes time to be taken out of Endpoint rotation)
+        //   it takes time for knowledge of SIGTERM on the worker node to propagate to e.g. networking layers
+        //   (k8s Service -> Endpoints rotation. It takes time to be taken out of Endpoint rotation)
         // - we want to gracefully drain ASP.NET core for requests, leaving some time for active requests to complete
         // This leaves us with the following sequence of events
         // - container receives SIGTERM
         // - `AppHostLifetime` intercepts SIGTERM and delays for `shutdownDelay`
         // - `AppHostLifetime` calls `IHostApplicationLifetime.StopApplication`, to start ASP.NET Core shutdown process
         // - ASP.NET Core will spend a maximum of `shutdownTimeout` trying to drain active requests
         //   (cancelable requests can combine cancellation tokens with `IHostApplicationLifetime.ApplicationStopping`)
         // - If ASP.NET Core completes shutdown within `shutdownTimeout`, everything is fine
         // - If ASP.NET Core is stuck or in some way can't terminate, kubelet will eventually SIGKILL
-        var shutdownDelay = TimeSpan.FromSeconds(5);
-        var shutdownTimeout = TimeSpan.FromSeconds(20);
+        var shutdownDelay = TimeSpan.FromSeconds(
+            services.BuildServiceProvider().GetRequiredService<IConfiguration>()
+                .GetValue("GracefulShutdown:DelaySeconds", 5)
+        );
+        var shutdownTimeout = TimeSpan.FromSeconds(
+            services.BuildServiceProvider().GetRequiredService<IConfiguration>()
+                .GetValue("GracefulShutdown:TimeoutSeconds", 20)
+        );
 
-        services.AddSingleton<IHostLifetime>(serviceProvider =>
-        {
-            var logger = serviceProvider.GetRequiredService<ILogger<AppHostLifetime>>();
-            var env = serviceProvider.GetRequiredService<IHostEnvironment>();
-            var lifetime = serviceProvider.GetRequiredService<IHostApplicationLifetime>();
-            return new AppHostLifetime(logger, env, lifetime, shutdownDelay);
-        });
+        // Only wire up in staging++ (tt02) and Production to avoid replacing ConsoleLifetime in Development/test-like envs
+        if (IsStagingPlus(env))
+        {
+            services.AddSingleton<IHostLifetime>(serviceProvider =>
+            {
+                var logger = serviceProvider.GetRequiredService<ILogger<AppHostLifetime>>();
+                var lifetime = serviceProvider.GetRequiredService<IHostApplicationLifetime>();
+                return new AppHostLifetime(logger, env, lifetime, shutdownDelay);
+            });
+        }
 
         services.Configure<HostOptions>(options => options.ShutdownTimeout = shutdownTimeout);
     }
+
+    private static bool IsStagingPlus(IHostEnvironment env) =>
+        env.IsProduction() || string.Equals(env.EnvironmentName, "tt02", StringComparison.OrdinalIgnoreCase);

Committable suggestion skipped: line range outside the PR's diff.

}
49 changes: 49 additions & 0 deletions src/Altinn.App.Api/Infrastructure/Lifetime/AppHostLifetime.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
using System.Runtime.InteropServices;

namespace Altinn.App.Api.Infrastructure.Lifetime;

// Based on guidance in:
// https://github.com/dotnet/dotnet-docker/blob/2a6f35b9361d1aacb664b0ce09e529698b622d2b/samples/kubernetes/graceful-shutdown/graceful-shutdown.md
internal sealed class AppHostLifetime(
ILogger<AppHostLifetime> _logger,
IHostEnvironment _environment,
IHostApplicationLifetime _applicationLifetime,
TimeSpan _delay
) : IHostLifetime, IDisposable
{
private IEnumerable<IDisposable>? _disposables;

public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask;

public Task WaitForStartAsync(CancellationToken cancellationToken)
{
if (!_environment.IsDevelopment())
{
_disposables =
[
PosixSignalRegistration.Create(PosixSignal.SIGINT, HandleSignal),
PosixSignalRegistration.Create(PosixSignal.SIGQUIT, HandleSignal),
PosixSignalRegistration.Create(PosixSignal.SIGTERM, HandleSignal),
];
}
return Task.CompletedTask;
}

private void HandleSignal(PosixSignalContext ctx)
{
_logger.LogInformation("Received shutdown signal: {Signal}, delaying shutdown", ctx.Signal);
ctx.Cancel = true; // Signal intercepted here, we are now responsible for calling `StopApplication`
Task.Delay(_delay)
.ContinueWith(t =>
{
_logger.LogInformation("Starting host shutdown...");
_applicationLifetime.StopApplication();
});
}

public void Dispose()
{
foreach (var disposable in _disposables ?? [])
disposable.Dispose();
}
}
Loading