-
Notifications
You must be signed in to change notification settings - Fork 117
Implement Script Isolation #1803
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| using System; | ||
| using System.IO; | ||
| using System.Threading.Tasks; | ||
|
|
||
| namespace Calamari.Common.Features.Processes.ScriptIsolation; | ||
|
|
||
| public static class FileLock | ||
| { | ||
| public static ILockHandle Acquire(LockOptions lockOptions) | ||
| { | ||
| var fileShareMode = GetFileShareMode(lockOptions.Type); | ||
| try | ||
| { | ||
| var fileStream = File.Open(lockOptions.LockFile, FileMode.OpenOrCreate, FileAccess.ReadWrite, fileShareMode); | ||
| return new LockHandle(fileStream); | ||
| } | ||
| catch (IOException e) | ||
| { | ||
| throw new LockRejectedException(e); | ||
| } | ||
| } | ||
|
|
||
| static FileShare GetFileShareMode(LockType isolationLevel) | ||
| { | ||
| return isolationLevel switch | ||
| { | ||
| LockType.Exclusive => FileShare.None, | ||
| LockType.Shared => FileShare.ReadWrite, | ||
|
Comment on lines
+27
to
+28
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opted to use a custom enum to map to the appropriate |
||
| _ => throw new ArgumentOutOfRangeException(nameof(isolationLevel), isolationLevel, null) | ||
| }; | ||
| } | ||
|
|
||
| sealed class LockHandle(FileStream fileStream) : ILockHandle | ||
| { | ||
| public void Dispose() | ||
| { | ||
| fileStream.Dispose(); | ||
| } | ||
|
|
||
| public async ValueTask DisposeAsync() | ||
| { | ||
| await fileStream.DisposeAsync(); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| using System; | ||
|
|
||
| namespace Calamari.Common.Features.Processes.ScriptIsolation; | ||
|
|
||
| public interface ILockHandle : IAsyncDisposable, IDisposable; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| using System; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using Calamari.Common.Plumbing.Logging; | ||
| using Polly; | ||
|
|
||
| namespace Calamari.Common.Features.Processes.ScriptIsolation; | ||
|
|
||
| public static class Isolation | ||
| { | ||
| // Compare these values with the standard script isolation mutex strategy | ||
| static readonly TimeSpan RetryInitialDelay = TimeSpan.FromMilliseconds(10); | ||
| static readonly TimeSpan RetryMaxDelay = TimeSpan.FromMilliseconds(500); | ||
|
Comment on lines
+12
to
+13
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need to use a polling strategy in Tentacle's |
||
|
|
||
| public static ILockHandle Enforce() | ||
| { | ||
| var lockOptions = LockOptions.FromEnvironmentOrNull(); | ||
| if (lockOptions is null) | ||
| { | ||
| Log.Verbose("No lock required"); | ||
| return new NoLock(); | ||
| } | ||
|
|
||
| var pipeline = BuildLockAcquisitionPipeline(lockOptions); | ||
| LogIsolation(lockOptions); | ||
| try | ||
| { | ||
| return pipeline.Execute(FileLock.Acquire, lockOptions); | ||
| } | ||
| catch (Exception exception) | ||
| { | ||
| LockRejectedException.Throw(exception); | ||
| throw; // Satisfy the compiler | ||
| } | ||
| } | ||
|
|
||
| public static async Task<ILockHandle> EnforceAsync(CancellationToken cancellationToken) | ||
| { | ||
| var lockOptions = LockOptions.FromEnvironmentOrNull(); | ||
| if (lockOptions is null) | ||
| { | ||
| return new NoLock(); | ||
| } | ||
|
|
||
| var pipeline = BuildLockAcquisitionPipeline(lockOptions); | ||
| LogIsolation(lockOptions); | ||
| try | ||
| { | ||
| return await pipeline.ExecuteAsync(static (o, _) => ValueTask.FromResult(FileLock.Acquire(o)), lockOptions, cancellationToken); | ||
| } | ||
| catch (Exception exception) | ||
| { | ||
| LockRejectedException.Throw(exception); | ||
| throw; // Satisfy the compiler | ||
| } | ||
| } | ||
|
|
||
| static void LogIsolation(LockOptions lockOptions) | ||
| { | ||
| Log.Verbose($"Acquiring script isolation mutex {lockOptions.Name} with {lockOptions.Type} lock"); | ||
| } | ||
|
|
||
| static ResiliencePipeline<ILockHandle> BuildLockAcquisitionPipeline(LockOptions lockOptions) | ||
| { | ||
| var builder = new ResiliencePipelineBuilder<ILockHandle>(); | ||
| // Timeout must be between 10ms and 1 day. (Polly) | ||
| // If it's 10ms or less, we'll skip timeout and limit retries | ||
| // If it's more than 1 day, we'll assume indefinite retries with no timeout | ||
| var retryAttempts = lockOptions.Timeout <= TimeSpan.FromMilliseconds(10) | ||
| ? 1 | ||
| : int.MaxValue; | ||
| if (lockOptions.Timeout < TimeSpan.FromDays(1) && lockOptions.Timeout > TimeSpan.FromMilliseconds(10)) | ||
| { | ||
| builder.AddTimeout(lockOptions.Timeout); | ||
| } | ||
|
|
||
| builder.AddRetry( | ||
| new() | ||
| { | ||
| BackoffType = DelayBackoffType.Exponential, | ||
| Delay = RetryInitialDelay, | ||
| MaxDelay = RetryMaxDelay, | ||
| MaxRetryAttempts = retryAttempts, | ||
| ShouldHandle = new PredicateBuilder<ILockHandle>().Handle<LockRejectedException>(), | ||
| UseJitter = true | ||
| } | ||
| ); | ||
| return builder.Build(); | ||
| } | ||
|
|
||
| class NoLock : ILockHandle | ||
| { | ||
| public ValueTask DisposeAsync() => ValueTask.CompletedTask; | ||
|
|
||
| public void Dispose() | ||
| { | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| using System; | ||
| using Calamari.Common.Plumbing.Logging; | ||
|
|
||
| namespace Calamari.Common.Features.Processes.ScriptIsolation; | ||
|
|
||
| public sealed record LockOptions( | ||
| LockType Type, | ||
| string Name, | ||
| string LockFile, | ||
| TimeSpan Timeout | ||
| ) | ||
| { | ||
| public static LockOptions? FromEnvironmentOrNull() | ||
| { | ||
| Log.Verbose("Attempting to load LockOptions from environment variables"); | ||
|
|
||
| var envLevel = GetAndLogEnvironmentVariable(EnvironmentVariables.CalamariScriptIsolationLevel); | ||
| var envMutexName = GetAndLogEnvironmentVariable(EnvironmentVariables.CalamariScriptIsolationMutexName); | ||
| var envMutexTimeout = GetAndLogEnvironmentVariable(EnvironmentVariables.CalamariScriptIsolationMutexTimeout); | ||
| var tentacleHome = GetAndLogEnvironmentVariable(EnvironmentVariables.TentacleHome); | ||
|
|
||
| if (string.IsNullOrWhiteSpace(envLevel) || string.IsNullOrWhiteSpace(envMutexName) || string.IsNullOrWhiteSpace(envMutexTimeout) || string.IsNullOrWhiteSpace(tentacleHome)) | ||
| { | ||
| // This is for initial debugging - This will indicate that Calamari shouldn't perform locking | ||
| Log.Verbose("One or more required environment variables are missing or empty. Unable to create LockOptions."); | ||
| return null; | ||
| } | ||
|
|
||
| var lockType = MapScriptIsolationLevelToLockTypeOrNull(envLevel); | ||
| if (lockType == null) | ||
| { | ||
| Log.Verbose($"Failed to map script isolation level '{envLevel}' to a valid LockType. Expected 'FullIsolation' or 'NoIsolation' (case-insensitive)."); | ||
| return null; | ||
| } | ||
|
|
||
| Log.Verbose($"Mapped isolation level '{envLevel}' to LockType.{lockType.Value}"); | ||
|
|
||
| if (!TimeSpan.TryParse(envMutexTimeout, out var timeout)) | ||
| { | ||
| Log.Verbose($"Failed to parse mutex timeout value '{envMutexTimeout}' as TimeSpan. Defaulting to TimeSpan.MaxValue."); | ||
| // What should we do if the timeout is invalid? Default to max value? | ||
| timeout = TimeSpan.MaxValue; | ||
| } | ||
| else | ||
| { | ||
| Log.Verbose($"Parsed mutex timeout: {timeout}"); | ||
| } | ||
|
|
||
| var lockFilePath = GetLockFilePath(tentacleHome, envMutexName); | ||
| Log.Verbose($"Calculated lock file path: {lockFilePath}"); | ||
|
|
||
| Log.Verbose($"Successfully created LockOptions with Type={lockType.Value}, Name={envMutexName}, LockFile={lockFilePath}, Timeout={timeout}"); | ||
| return new LockOptions(lockType.Value, envMutexName, lockFilePath, timeout); | ||
| } | ||
|
|
||
| static string? GetAndLogEnvironmentVariable(string environmentVariableName) | ||
| { | ||
| var result = Environment.GetEnvironmentVariable(environmentVariableName); | ||
| Log.Verbose($"Environment variable '{environmentVariableName}': {(string.IsNullOrWhiteSpace(result) ? "<null or whitespace>" : result)}"); | ||
| return result; | ||
| } | ||
|
|
||
| static string GetLockFilePath(string tentacleHome, string mutexName) => | ||
| System.IO.Path.Combine(tentacleHome, $"ScriptIsolation.{mutexName}.lock"); // Should we sanitize the mutex name or just allow it to be invalid? | ||
|
|
||
| static LockType? MapScriptIsolationLevelToLockTypeOrNull(string isolationLevel) => | ||
| isolationLevel.ToLowerInvariant() switch | ||
| { | ||
| "fullisolation" => LockType.Exclusive, | ||
| "noisolation" => LockType.Shared, | ||
| _ => null | ||
| }; | ||
|
|
||
| static class EnvironmentVariables | ||
| { | ||
| public const string CalamariScriptIsolationLevel = "CalamariScriptIsolationLevel"; | ||
| public const string CalamariScriptIsolationMutexName = "CalamariScriptIsolationMutexName"; | ||
| public const string CalamariScriptIsolationMutexTimeout = "CalamariScriptIsolationMutexTimeout"; | ||
| public const string TentacleHome = "TentacleHome"; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| using System; | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using Polly.Timeout; | ||
|
|
||
| namespace Calamari.Common.Features.Processes.ScriptIsolation; | ||
|
|
||
| public sealed class LockRejectedException(string message, Exception? innerException) | ||
| : Exception(message, innerException) | ||
| { | ||
| public LockRejectedException(Exception innerException) : this("Lock acquisition failed", innerException) | ||
| { | ||
| } | ||
|
|
||
| [DoesNotReturn] | ||
| public static void Throw(Exception innerException) | ||
| { | ||
| if (innerException is LockRejectedException lockRejectedException) | ||
| { | ||
| throw lockRejectedException; | ||
| } | ||
|
|
||
| if (innerException is TimeoutRejectedException timeoutRejectedException) | ||
| { | ||
| var message = $"Lock acquisition failed after {timeoutRejectedException.Timeout}"; | ||
| throw new LockRejectedException(message, timeoutRejectedException); | ||
| } | ||
|
|
||
| throw new LockRejectedException("Lock acquisition failed", innerException); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| namespace Calamari.Common.Features.Processes.ScriptIsolation; | ||
|
|
||
| public enum LockType | ||
| { | ||
| Shared, | ||
| Exclusive | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
IOExceptionis possibly a little bit broad here. We could potentially perform additional checks, but that could get fragile if we rely on the error message string.