From e907322e573663b9e6992888f06c362d8a683f4e Mon Sep 17 00:00:00 2001 From: Brian Barthel Date: Fri, 26 Jan 2024 16:04:40 -0500 Subject: [PATCH 1/4] Add initial support for multiple domains in Build Artifacts --- .../Artifact/FileContainerProvider.cs | 51 ++++- .../Artifact/PipelineArtifactServer.cs | 4 +- src/Agent.Sdk/Knob/AgentKnobs.cs | 6 + src/Agent.Worker/Build/FileContainerServer.cs | 46 ++++- .../Blob/BlobStoreUtils.cs | 2 +- .../Blob/BlobstoreClientSettings.cs | 121 +++++++++++ .../DedupManifestArtifactClientFactory.cs | 190 ++++-------------- .../JobServer.cs | 16 +- .../MockDedupManifestArtifactClientFactory.cs | 15 +- 9 files changed, 270 insertions(+), 181 deletions(-) create mode 100644 src/Microsoft.VisualStudio.Services.Agent/Blob/BlobstoreClientSettings.cs diff --git a/src/Agent.Plugins/Artifact/FileContainerProvider.cs b/src/Agent.Plugins/Artifact/FileContainerProvider.cs index 310815aa59..450bf5916b 100644 --- a/src/Agent.Plugins/Artifact/FileContainerProvider.cs +++ b/src/Agent.Plugins/Artifact/FileContainerProvider.cs @@ -11,6 +11,7 @@ using Microsoft.VisualStudio.Services.Agent.Util; using Microsoft.VisualStudio.Services.BlobStore.Common; using Microsoft.VisualStudio.Services.BlobStore.WebApi; +using Microsoft.VisualStudio.Services.BlobStore.WebApi.Contracts; using Microsoft.VisualStudio.Services.Content.Common; using Microsoft.VisualStudio.Services.Content.Common.Tracing; using Microsoft.VisualStudio.Services.FileContainer; @@ -149,19 +150,34 @@ private async Task DownloadFileContainerAsync(IEnumerable ite // Only initialize these clients if we know we need to download from Blobstore // If a client cannot connect to Blobstore, we shouldn't stop them from downloading from FCS var downloadFromBlob = !AgentKnobs.DisableBuildArtifactsToBlob.GetValue(context).AsBoolean(); - DedupStoreClient dedupClient = null; + Dictionary dedupClientTable = new Dictionary(); BlobStoreClientTelemetryTfs clientTelemetry = null; if (downloadFromBlob && fileItems.Any(x => x.BlobMetadata != null)) { + // this is not the most efficient but good enough for now: + var domains = fileItems.Select(x => GetDomainIdAndDedupIdFromArtifactHash(x.BlobMetadata.ArtifactHash).domainId).Distinct(); + DedupStoreClient dedupClient = null; try { - (dedupClient, clientTelemetry) = await DedupManifestArtifactClientFactory.Instance.CreateDedupClientAsync( - false, - (str) => this.tracer.Info(str), - this.connection, - DedupManifestArtifactClientFactory.Instance.GetDedupStoreClientMaxParallelism(context), + BlobstoreClientSettings clientSettings = await BlobstoreClientSettings.GetClientSettingsAsync( + connection, Microsoft.VisualStudio.Services.BlobStore.WebApi.Contracts.Client.BuildArtifact, + tracer, cancellationToken); + + foreach(var domainId in domains) + { + (dedupClient, clientTelemetry) = DedupManifestArtifactClientFactory.Instance.CreateDedupClient( + this.connection, + domainId, + DedupManifestArtifactClientFactory.Instance.GetDedupStoreClientMaxParallelism(context), + clientSettings.GetRedirectTimeout(), + false, + (str) => this.tracer.Info(str), + cancellationToken); + + dedupClientTable.Add(domainId, dedupClient); + } } catch (SocketException e) { @@ -169,7 +185,7 @@ private async Task DownloadFileContainerAsync(IEnumerable ite } catch { - var blobStoreHost = dedupClient.Client.BaseAddress.Host; + var blobStoreHost = dedupClient?.Client.BaseAddress.Host; var allowListLink = BlobStoreWarningInfoProvider.GetAllowListLinkForCurrentPlatform(); var warningMessage = StringUtil.Loc("BlobStoreDownloadWarning", blobStoreHost, allowListLink); @@ -191,7 +207,8 @@ await AsyncHttpRetryHelper.InvokeVoidAsync( tracer.Info($"Downloading: {targetPath}"); if (item.BlobMetadata != null && downloadFromBlob) { - await this.DownloadFileFromBlobAsync(context, containerIdAndRoot, targetPath, projectId, item, dedupClient, clientTelemetry, cancellationToken); + var client = dedupClientTable[GetDomainIdAndDedupIdFromArtifactHash(item.BlobMetadata.ArtifactHash).domainId]; + await this.DownloadFileFromBlobAsync(context, containerIdAndRoot, targetPath, projectId, item, client, clientTelemetry, cancellationToken); } else { @@ -336,6 +353,22 @@ private async Task DownloadFileAsync( return responseStream; } + private static (IDomainId domainId, DedupIdentifier dedupId) GetDomainIdAndDedupIdFromArtifactHash(string artifactHash) + { + string[] parts = artifactHash.Split(','); + if(parts.Length == 1) + { + // legacy format is always in the default domain: + return (WellKnownDomainIds.DefaultDomainId, DedupIdentifier.Deserialize(parts[0])); + } + else if(parts.Length==2) + { + // Multidomain format is in the form of , + return (DomainIdFactory.Create(parts[0]), DedupIdentifier.Deserialize(parts[1])); + } + throw new ArgumentException($"Invalid artifact hash: {artifactHash}", nameof(artifactHash)); + } + private async Task DownloadFileFromBlobAsync( AgentTaskPluginExecutionContext context, (long, string) containerIdAndRoot, @@ -346,7 +379,7 @@ private async Task DownloadFileFromBlobAsync( BlobStoreClientTelemetryTfs clientTelemetry, CancellationToken cancellationToken) { - var dedupIdentifier = DedupIdentifier.Deserialize(item.BlobMetadata.ArtifactHash); + (var domainId, var dedupIdentifier) = GetDomainIdAndDedupIdFromArtifactHash(item.BlobMetadata.ArtifactHash); var downloadRecord = clientTelemetry.CreateRecord((level, uri, type) => new BuildArtifactActionRecord(level, uri, type, nameof(DownloadFileContainerAsync), context)); diff --git a/src/Agent.Plugins/Artifact/PipelineArtifactServer.cs b/src/Agent.Plugins/Artifact/PipelineArtifactServer.cs index 90798ca33a..1767b3eb33 100644 --- a/src/Agent.Plugins/Artifact/PipelineArtifactServer.cs +++ b/src/Agent.Plugins/Artifact/PipelineArtifactServer.cs @@ -44,14 +44,14 @@ internal async Task UploadAsync( // Get the client settings, if any. var tracer = DedupManifestArtifactClientFactory.CreateArtifactsTracer(verbose: false, (str) => context.Output(str)); VssConnection connection = context.VssConnection; - var clientSettings = await DedupManifestArtifactClientFactory.GetClientSettingsAsync( + var clientSettings = await BlobstoreClientSettings.GetClientSettingsAsync( connection, Microsoft.VisualStudio.Services.BlobStore.WebApi.Contracts.Client.PipelineArtifact, tracer, cancellationToken); // Get the default domain to use: - IDomainId domainId = DedupManifestArtifactClientFactory.GetDefaultDomainId(clientSettings, tracer); + IDomainId domainId = clientSettings.GetDefaultDomainId(); var (dedupManifestClient, clientTelemetry) = DedupManifestArtifactClientFactory.Instance .CreateDedupManifestClient( diff --git a/src/Agent.Sdk/Knob/AgentKnobs.cs b/src/Agent.Sdk/Knob/AgentKnobs.cs index cc95440fb3..5f706a8b2a 100644 --- a/src/Agent.Sdk/Knob/AgentKnobs.cs +++ b/src/Agent.Sdk/Knob/AgentKnobs.cs @@ -374,6 +374,12 @@ public class AgentKnobs new RuntimeKnobSource("DISABLE_BUILD_ARTIFACTS_TO_BLOB"), new EnvironmentKnobSource("DISABLE_BUILD_ARTIFACTS_TO_BLOB"), new BuiltInDefaultKnobSource("false")); + public static readonly Knob DisableBuildArtifactsToBlobAlternateDomain = new Knob( + nameof(DisableBuildArtifactsToBlobAlternateDomain), + "The agent may upload build artifacts to alternate domains in Blobstore. If enabled, all artifacts will go to the legacy domain.", + new RuntimeKnobSource("DISABLE_BUILD_ARTIFACTS_TO_BLOB_ALTERNATE_DOMAIN"), + new EnvironmentKnobSource("DISABLE_BUILD_ARTIFACTS_TO_BLOB_ALTERNATE_DOMAIN"), + new BuiltInDefaultKnobSource("false")); public static readonly Knob EnableIncompatibleBuildArtifactsPathResolution = new Knob( nameof(EnableIncompatibleBuildArtifactsPathResolution), diff --git a/src/Agent.Worker/Build/FileContainerServer.cs b/src/Agent.Worker/Build/FileContainerServer.cs index 353862d13e..9016aa12ae 100644 --- a/src/Agent.Worker/Build/FileContainerServer.cs +++ b/src/Agent.Worker/Build/FileContainerServer.cs @@ -3,8 +3,13 @@ using Agent.Sdk.Knob; using Agent.Sdk.Util; +using BuildXL.Cache.ContentStore.Hashing; +using Microsoft.TeamFoundation.DistributedTask.WebApi; using Microsoft.VisualStudio.Services.Agent.Blob; using Microsoft.VisualStudio.Services.Agent.Util; +using Microsoft.VisualStudio.Services.BlobStore.WebApi; +using Microsoft.VisualStudio.Services.BlobStore.Common; +using Microsoft.VisualStudio.Services.BlobStore.WebApi.Contracts; using Microsoft.VisualStudio.Services.FileContainer.Client; using System; using System.Collections.Concurrent; @@ -18,8 +23,6 @@ using System.Net.Http; using System.Net; using System.Net.Sockets; -using Microsoft.TeamFoundation.DistributedTask.WebApi; -using Microsoft.VisualStudio.Services.BlobStore.WebApi; namespace Microsoft.VisualStudio.Services.Agent.Worker.Build @@ -326,6 +329,16 @@ private async Task UploadAsync(IAsyncCommandContext context, int u return new UploadResult(failedFiles, uploadedSize); } + public static string CreateDomainHash(IDomainId domainId, DedupIdentifier dedupId) + { + if (domainId != WellKnownDomainIds.DefaultDomainId) + { + // Only use the new format domainId,dedupId if we aren't going to the default domain as this is a breaking change: + return $"{domainId.Serialize()},{dedupId.ValueString}"; + } + // We are still uploading to the default domain so use the don't use the new format: + return dedupId.ValueString; + } private async Task BlobUploadAsync(IAsyncCommandContext context, IReadOnlyList files, int concurrentUploads, CancellationToken token) { @@ -342,20 +355,31 @@ private async Task BlobUploadAsync(IAsyncCommandContext context, I BlobStoreClientTelemetryTfs clientTelemetry = null; try { + bool forceDefaultDomain = AgentKnobs.DisableBuildArtifactsToBlobAlternateDomain.GetValue(context).AsBoolean(); + var verbose = String.Equals(context.GetVariableValueOrDefault("system.debug"), "true", StringComparison.InvariantCultureIgnoreCase); - int maxParallelism = context.GetHostContext().GetService().GetSettings().MaxDedupParallelism; - (dedupClient, clientTelemetry) = await DedupManifestArtifactClientFactory.Instance - .CreateDedupClientAsync( - verbose, - (str) => context.Output(str), - this._connection, - maxParallelism, - BlobStore.WebApi.Contracts.Client.BuildArtifact, + Action tracer = (str) => context.Output(str); + + var clientSettings = await BlobstoreClientSettings.GetClientSettingsAsync( + _connection, + Microsoft.VisualStudio.Services.BlobStore.WebApi.Contracts.Client.BuildArtifact, + DedupManifestArtifactClientFactory.CreateArtifactsTracer(verbose, tracer), token); + IDomainId domainId = forceDefaultDomain ? WellKnownDomainIds.DefaultDomainId : clientSettings.GetDefaultDomainId(); + + (dedupClient, clientTelemetry) = DedupManifestArtifactClientFactory.Instance + .CreateDedupClient( + _connection, + domainId, + context.GetHostContext().GetService().GetSettings().MaxDedupParallelism, + clientSettings.GetRedirectTimeout(), + verbose, + tracer, + token); // Upload to blobstore var results = await BlobStoreUtils.UploadBatchToBlobstore(verbose, files, (level, uri, type) => - new BuildArtifactActionRecord(level, uri, type, nameof(BlobUploadAsync), context), (str) => context.Output(str), dedupClient, clientTelemetry, token, enableReporting: true); + new BuildArtifactActionRecord(level, uri, type, nameof(BlobUploadAsync), context), tracer, dedupClient, clientTelemetry, token, enableReporting: true); // Associate with TFS context.Output(StringUtil.Loc("AssociateFiles")); diff --git a/src/Microsoft.VisualStudio.Services.Agent/Blob/BlobStoreUtils.cs b/src/Microsoft.VisualStudio.Services.Agent/Blob/BlobStoreUtils.cs index 56928caca8..f04e6cb9a9 100644 --- a/src/Microsoft.VisualStudio.Services.Agent/Blob/BlobStoreUtils.cs +++ b/src/Microsoft.VisualStudio.Services.Agent/Blob/BlobStoreUtils.cs @@ -120,7 +120,7 @@ private static async Task> GenerateHashes(IReadOnlyList + /// Get the client settings for the given client. + /// + /// This should only be called once per client type. This is intended to fail fast so it has no retries. + public static async Task GetClientSettingsAsync( + VssConnection connection, + BlobStore.WebApi.Contracts.Client? client, + IAppTraceSource tracer, + CancellationToken cancellationToken) + { + if(client.HasValue) + { + try + { + ArtifactHttpClientFactory factory = new( + connection.Credentials, + connection.Settings.SendTimeout, + tracer, + cancellationToken); + + var blobUri = connection.GetClient().BaseAddress; + var clientSettingsHttpClient = factory.CreateVssHttpClient(blobUri); + return new BlobstoreClientSettings(await clientSettingsHttpClient.GetSettingsAsync(client.Value, userState: null, cancellationToken), tracer); + } + catch (Exception exception) + { + // Use info cause we don't want to fail builds with warnings as errors... + tracer.Info($"Error while retrieving client Settings for {client}. Exception: {exception}. Falling back to defaults."); + } + } + return new BlobstoreClientSettings(null, tracer); + } + + public IDomainId GetDefaultDomainId() + { + IDomainId domainId = WellKnownDomainIds.DefaultDomainId; + if (clientSettings != null && clientSettings.Properties.ContainsKey(ClientSettingsConstants.DefaultDomainId)) + { + try + { + domainId = DomainIdFactory.Create(clientSettings.Properties[ClientSettingsConstants.DefaultDomainId]); + tracer.Verbose($"Using domain id '{domainId}' from client settings."); + } + catch (Exception exception) + { + tracer.Info($"Error converting the domain id '{clientSettings.Properties[ClientSettingsConstants.DefaultDomainId]}': {exception.Message}. Falling back to default."); + } + } + else + { + tracer.Verbose($"No client settings found, using the default domain id '{domainId}'."); + } + return domainId; + } + + public HashType GetClientHashType(AgentTaskPluginExecutionContext context) + { + HashType hashType = ChunkerHelper.DefaultChunkHashType; + + // Note: 9/6/2023 Remove the below check in couple of months. + if (AgentKnobs.AgentEnablePipelineArtifactLargeChunkSize.GetValue(context).AsBoolean()) + { + if (clientSettings != null && clientSettings.Properties.ContainsKey(ClientSettingsConstants.ChunkSize)) + { + try + { + HashTypeExtensions.Deserialize(clientSettings.Properties[ClientSettingsConstants.ChunkSize], out hashType); + } + catch (Exception exception) + { + tracer.Info($"Error converting the chunk size '{clientSettings.Properties[ClientSettingsConstants.ChunkSize]}': {exception.Message}. Falling back to default."); + } + } + } + + return ChunkerHelper.IsHashTypeChunk(hashType) ? hashType : ChunkerHelper.DefaultChunkHashType; + } + + public int? GetRedirectTimeout() + { + if (int.TryParse(clientSettings?.Properties.GetValueOrDefault(ClientSettingsConstants.RedirectTimeout), out int redirectTimeoutSeconds)) + { + return redirectTimeoutSeconds; + } + else + { + return null; + } + } + } +} \ No newline at end of file diff --git a/src/Microsoft.VisualStudio.Services.Agent/Blob/DedupManifestArtifactClientFactory.cs b/src/Microsoft.VisualStudio.Services.Agent/Blob/DedupManifestArtifactClientFactory.cs index ba7c09215f..eb7001c4cd 100644 --- a/src/Microsoft.VisualStudio.Services.Agent/Blob/DedupManifestArtifactClientFactory.cs +++ b/src/Microsoft.VisualStudio.Services.Agent/Blob/DedupManifestArtifactClientFactory.cs @@ -38,7 +38,7 @@ public interface IDedupManifestArtifactClientFactory VssConnection connection, int maxParallelism, IDomainId domainId, - ClientSettingsInfo clientSettings, + BlobstoreClientSettings clientSettings, AgentTaskPluginExecutionContext context, CancellationToken cancellationToken); @@ -58,20 +58,22 @@ public interface IDedupManifestArtifactClientFactory /// /// Creates a DedupStoreClient client. /// - /// If true emit verbose telemetry. - /// Action used for logging. /// VssConnection + /// Storage domain to use, if null pulls the default domain for the given client type. /// Maximum number of parallel threads that should be used for download. If 0 then /// use the system default. - /// The client type for client settings + /// Number of seconds to wait for an http redirect. + /// If true emit verbose telemetry. + /// Action used for logging. /// Cancellation token used for both creating clients and verifying client conneciton. - /// Tuple of the client and the telemtery client - Task<(DedupStoreClient client, BlobStoreClientTelemetryTfs telemetry)> CreateDedupClientAsync( - bool verbose, - Action traceOutput, + /// Tuple of the domain, client and the telemetry client + (DedupStoreClient client, BlobStoreClientTelemetryTfs telemetry) CreateDedupClient( VssConnection connection, + IDomainId domainId, int maxParallelism, - BlobStore.WebApi.Contracts.Client? clientType, + int? redirectTimeoutSeconds, + bool verbose, + Action traceOutput, CancellationToken cancellationToken); /// @@ -84,9 +86,6 @@ public interface IDedupManifestArtifactClientFactory public class DedupManifestArtifactClientFactory : IDedupManifestArtifactClientFactory { - // NOTE: this should be set to ClientSettingsConstants.DefaultDomainId when the latest update from Azure Devops is added. - private static string DefaultDomainIdKey = "DefaultDomainId"; - // Old default for hosted agents was 16*2 cores = 32. // In my tests of a node_modules folder, this 32x parallelism was consistently around 47 seconds. // At 192x it was around 16 seconds and 256x was no faster. @@ -113,7 +112,7 @@ private DedupManifestArtifactClientFactory() AgentTaskPluginExecutionContext context, CancellationToken cancellationToken) { - var clientSettings = await GetClientSettingsAsync( + var clientSettings = await BlobstoreClientSettings.GetClientSettingsAsync( connection, client, CreateArtifactsTracer(verbose, traceOutput), @@ -136,7 +135,7 @@ private DedupManifestArtifactClientFactory() VssConnection connection, int maxParallelism, IDomainId domainId, - ClientSettingsInfo clientSettings, + BlobstoreClientSettings clientSettings, AgentTaskPluginExecutionContext context, CancellationToken cancellationToken) { @@ -150,6 +149,25 @@ private DedupManifestArtifactClientFactory() traceOutput($"Max dedup parallelism: {maxParallelism}"); traceOutput($"DomainId: {domainId}"); + IDedupStoreHttpClient dedupStoreHttpClient = GetDedupStoreHttpClient(connection, domainId, maxRetries, tracer, cancellationToken); + + var telemetry = new BlobStoreClientTelemetry(tracer, dedupStoreHttpClient.BaseAddress); + this.HashType = clientSettings.GetClientHashType(context); + + if (this.HashType == BuildXL.Cache.ContentStore.Hashing.HashType.Dedup1024K) + { + dedupStoreHttpClient.RecommendedChunkCountPerCall = 10; // This is to workaround IIS limit - https://learn.microsoft.com/en-us/iis/configuration/system.webserver/security/requestfiltering/requestlimits/ + } + traceOutput($"Hashtype: {this.HashType.Value}"); + + dedupStoreHttpClient.SetRedirectTimeout(clientSettings.GetRedirectTimeout()); + + var dedupClient = new DedupStoreClientWithDataport(dedupStoreHttpClient, new DedupStoreClientContext(maxParallelism), this.HashType.Value); + return (new DedupManifestArtifactClient(telemetry, dedupClient, tracer), telemetry); + } + + private static IDedupStoreHttpClient GetDedupStoreHttpClient(VssConnection connection, IDomainId domainId, int maxRetries, IAppTraceSource tracer, CancellationToken cancellationToken) + { ArtifactHttpClientFactory factory = new ArtifactHttpClientFactory( connection.Credentials, connection.Settings.SendTimeout, @@ -178,28 +196,15 @@ private DedupManifestArtifactClientFactory() return dedupHttpclient; }); - - var telemetry = new BlobStoreClientTelemetry(tracer, dedupStoreHttpClient.BaseAddress); - this.HashType = GetClientHashType(clientSettings, context, tracer); - - if (this.HashType == BuildXL.Cache.ContentStore.Hashing.HashType.Dedup1024K) - { - dedupStoreHttpClient.RecommendedChunkCountPerCall = 10; // This is to workaround IIS limit - https://learn.microsoft.com/en-us/iis/configuration/system.webserver/security/requestfiltering/requestlimits/ - } - traceOutput($"Hashtype: {this.HashType.Value}"); - - dedupStoreHttpClient.SetRedirectTimeout(GetRedirectTimeoutFromClientSettings(clientSettings)); - - var dedupClient = new DedupStoreClientWithDataport(dedupStoreHttpClient, new DedupStoreClientContext(maxParallelism), this.HashType.Value); - return (new DedupManifestArtifactClient(telemetry, dedupClient, tracer), telemetry); + return dedupStoreHttpClient; } - - public async Task<(DedupStoreClient client, BlobStoreClientTelemetryTfs telemetry)> CreateDedupClientAsync( - bool verbose, - Action traceOutput, + public (DedupStoreClient client, BlobStoreClientTelemetryTfs telemetry) CreateDedupClient( VssConnection connection, + IDomainId domainId, int maxParallelism, - BlobStore.WebApi.Contracts.Client? clientType, + int? redirectTimeoutSeconds, + bool verbose, + Action traceOutput, CancellationToken cancellationToken) { const int maxRetries = 5; @@ -208,37 +213,13 @@ private DedupManifestArtifactClientFactory() { maxParallelism = DefaultDedupStoreClientMaxParallelism; } - traceOutput($"Max dedup parallelism: {maxParallelism}"); - - ClientSettingsInfo clientSettings = clientType is null - ? null - : await GetClientSettingsAsync( - connection, - clientType.Value, - tracer, - cancellationToken); - - var dedupStoreHttpClient = await AsyncHttpRetryHelper.InvokeAsync( - () => - { - ArtifactHttpClientFactory factory = new ArtifactHttpClientFactory( - connection.Credentials, - connection.Settings.SendTimeout, // copy timeout settings from connection provided by agent - tracer, - cancellationToken); - - // this is actually a hidden network call to the location service: - return Task.FromResult(factory.CreateVssHttpClient(connection.GetClient().BaseAddress)); - }, - maxRetries: maxRetries, - tracer: tracer, - canRetryDelegate: e => true, - context: nameof(CreateDedupManifestClientAsync), - cancellationToken: cancellationToken, - continueOnCapturedContext: false); - - dedupStoreHttpClient.SetRedirectTimeout(GetRedirectTimeoutFromClientSettings(clientSettings)); + traceOutput("Creating dedup client:"); + traceOutput($" - Max dedup parallelism: {maxParallelism}"); + traceOutput($" - Using blobstore domain: {domainId}"); + traceOutput($" - Using redirect timeout: {redirectTimeoutSeconds}"); + var dedupStoreHttpClient = GetDedupStoreHttpClient(connection, domainId, maxRetries, tracer, cancellationToken); + dedupStoreHttpClient.SetRedirectTimeout(redirectTimeoutSeconds); var telemetry = new BlobStoreClientTelemetryTfs(tracer, dedupStoreHttpClient.BaseAddress, connection); var client = new DedupStoreClient(dedupStoreHttpClient, maxParallelism); return (client, telemetry); @@ -301,88 +282,5 @@ public static IAppTraceSource CreateArtifactsTracer(bool verbose, Action : System.Diagnostics.SourceLevels.Information, includeSeverityLevel: verbose); } - - /// - /// Get the client settings for the given client. - /// - /// This should only be called once per client type. This is intended to fail fast so it has no retries. - public static async Task GetClientSettingsAsync( - VssConnection connection, - BlobStore.WebApi.Contracts.Client client, - IAppTraceSource tracer, - CancellationToken cancellationToken) - { - try - { - ArtifactHttpClientFactory factory = new( - connection.Credentials, - connection.Settings.SendTimeout, - tracer, - cancellationToken); - - var blobUri = connection.GetClient().BaseAddress; - var clientSettingsHttpClient = factory.CreateVssHttpClient(blobUri); - return await clientSettingsHttpClient.GetSettingsAsync(client, userState: null, cancellationToken); - } - catch (Exception exception) - { - // Use info cause we don't want to fail builds with warnings as errors... - tracer.Info($"Error while retrieving client Settings for {client}. Exception: {exception}. Falling back to defaults."); - } - return null; - } - - public static IDomainId GetDefaultDomainId(ClientSettingsInfo clientSettings, IAppTraceSource tracer) - { - IDomainId domainId = WellKnownDomainIds.DefaultDomainId; - if (clientSettings != null && clientSettings.Properties.ContainsKey(DefaultDomainIdKey)) - { - try - { - domainId = DomainIdFactory.Create(clientSettings.Properties[DefaultDomainIdKey]); - } - catch (Exception exception) - { - tracer.Info($"Error converting the domain id '{clientSettings.Properties[DefaultDomainIdKey]}': {exception.Message}. Falling back to default."); - } - } - - return domainId; - } - - private static HashType GetClientHashType(ClientSettingsInfo clientSettings, AgentTaskPluginExecutionContext context, IAppTraceSource tracer) - { - HashType hashType = ChunkerHelper.DefaultChunkHashType; - - // Note: 9/6/2023 Remove the below check in couple of months. - if (AgentKnobs.AgentEnablePipelineArtifactLargeChunkSize.GetValue(context).AsBoolean()) - { - if (clientSettings != null && clientSettings.Properties.ContainsKey(ClientSettingsConstants.ChunkSize)) - { - try - { - HashTypeExtensions.Deserialize(clientSettings.Properties[ClientSettingsConstants.ChunkSize], out hashType); - } - catch (Exception exception) - { - tracer.Info($"Error converting the chunk size '{clientSettings.Properties[ClientSettingsConstants.ChunkSize]}': {exception.Message}. Falling back to default."); - } - } - } - - return ChunkerHelper.IsHashTypeChunk(hashType) ? hashType : ChunkerHelper.DefaultChunkHashType; - } - - private int? GetRedirectTimeoutFromClientSettings(ClientSettingsInfo clientSettings) - { - if (int.TryParse(clientSettings?.Properties.GetValueOrDefault(ClientSettingsConstants.RedirectTimeout), out int redirectTimeoutSeconds)) - { - return redirectTimeoutSeconds; - } - else - { - return null; - } - } } } \ No newline at end of file diff --git a/src/Microsoft.VisualStudio.Services.Agent/JobServer.cs b/src/Microsoft.VisualStudio.Services.Agent/JobServer.cs index 6c359a71ac..b59f6c5de3 100644 --- a/src/Microsoft.VisualStudio.Services.Agent/JobServer.cs +++ b/src/Microsoft.VisualStudio.Services.Agent/JobServer.cs @@ -18,6 +18,7 @@ using Microsoft.VisualStudio.Services.Content.Common; using Microsoft.VisualStudio.Services.Content.Common.Tracing; using Agent.Sdk.Util; +using Microsoft.VisualStudio.Services.BlobStore.Common; namespace Microsoft.VisualStudio.Services.Agent { @@ -172,13 +173,18 @@ public async Task UploadLogToBlobStore(Stream blob, st public async Task<(DedupIdentifier dedupId, ulong length)> UploadAttachmentToBlobStore(bool verbose, string itemPath, Guid planId, Guid jobId, CancellationToken cancellationToken) { int maxParallelism = HostContext.GetService().GetSettings().MaxDedupParallelism; - var (dedupClient, clientTelemetry) = await DedupManifestArtifactClientFactory.Instance - .CreateDedupClientAsync( + var clientSettings = await BlobstoreClientSettings.GetClientSettingsAsync( + _connection, + client: null, + DedupManifestArtifactClientFactory.CreateArtifactsTracer(verbose, (str) => Trace.Info(str)), cancellationToken); + var (dedupClient, clientTelemetry) = DedupManifestArtifactClientFactory.Instance + .CreateDedupClient( + _connection, + WellKnownDomainIds.DefaultDomainId, + maxParallelism, + clientSettings.GetRedirectTimeout(), verbose, (str) => Trace.Info(str), - this._connection, - maxParallelism, - clientType: null, cancellationToken); var results = await BlobStoreUtils.UploadToBlobStore(verbose, itemPath, (level, uri, type) => diff --git a/src/Test/L0/Plugin/TestFileShareProvider/MockDedupManifestArtifactClientFactory.cs b/src/Test/L0/Plugin/TestFileShareProvider/MockDedupManifestArtifactClientFactory.cs index 0380b35fb5..84f4631621 100644 --- a/src/Test/L0/Plugin/TestFileShareProvider/MockDedupManifestArtifactClientFactory.cs +++ b/src/Test/L0/Plugin/TestFileShareProvider/MockDedupManifestArtifactClientFactory.cs @@ -43,7 +43,7 @@ public class MockDedupManifestArtifactClientFactory : IDedupManifestArtifactClie VssConnection connection, int maxParallelism, IDomainId domainId, - ClientSettingsInfo clientSettings, + BlobstoreClientSettings clientSettings, AgentTaskPluginExecutionContext context, CancellationToken cancellationToken) { @@ -54,20 +54,21 @@ public class MockDedupManifestArtifactClientFactory : IDedupManifestArtifactClie telemetrySender)); } - public Task<(DedupStoreClient client, BlobStoreClientTelemetryTfs telemetry)> CreateDedupClientAsync( - bool verbose, - Action traceOutput, + public (DedupStoreClient client, BlobStoreClientTelemetryTfs telemetry) CreateDedupClient( VssConnection connection, + IDomainId domainId, int maxParallelism, - BlobStore.WebApi.Contracts.Client? clientType, + int? redirectTimeoutSeconds, + bool verbose, + Action traceOutput, CancellationToken cancellationToken) { telemetrySender = new TestTelemetrySender(); - return Task.FromResult((client: (DedupStoreClient)null, telemetry: new BlobStoreClientTelemetryTfs( + return (client: (DedupStoreClient)null, telemetry: new BlobStoreClientTelemetryTfs( NoopAppTraceSource.Instance, baseAddress, connection, - telemetrySender))); + telemetrySender)); } From 5f550fd7e8072c64cf7eac17df44373c3f07b894 Mon Sep 17 00:00:00 2001 From: Brian Barthel Date: Fri, 26 Jan 2024 17:13:36 -0500 Subject: [PATCH 2/4] Add knob to override the storage domain used. --- src/Agent.Plugins/Artifact/PipelineArtifactServer.cs | 6 ++++-- src/Agent.Sdk/Knob/AgentKnobs.cs | 12 ++++++------ src/Agent.Worker/Build/FileContainerServer.cs | 12 +++++++----- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/Agent.Plugins/Artifact/PipelineArtifactServer.cs b/src/Agent.Plugins/Artifact/PipelineArtifactServer.cs index 1767b3eb33..302b0f4b42 100644 --- a/src/Agent.Plugins/Artifact/PipelineArtifactServer.cs +++ b/src/Agent.Plugins/Artifact/PipelineArtifactServer.cs @@ -18,6 +18,7 @@ using Microsoft.VisualStudio.Services.WebApi; using Microsoft.VisualStudio.Services.Agent.Util; using Microsoft.VisualStudio.Services.BlobStore.Common; +using Agent.Sdk.Knob; namespace Agent.Plugins { @@ -50,8 +51,9 @@ internal async Task UploadAsync( tracer, cancellationToken); - // Get the default domain to use: - IDomainId domainId = clientSettings.GetDefaultDomainId(); + // Check if the pipeline has an override domain set, if not, use the default domain from the client settings. + string overrideDomain = AgentKnobs.SendArtifactsToBlobstoreDomain.GetValue(context).AsString(); + IDomainId domainId = String.IsNullOrWhiteSpace(overrideDomain) ? clientSettings.GetDefaultDomainId() : DomainIdFactory.Create(overrideDomain); var (dedupManifestClient, clientTelemetry) = DedupManifestArtifactClientFactory.Instance .CreateDedupManifestClient( diff --git a/src/Agent.Sdk/Knob/AgentKnobs.cs b/src/Agent.Sdk/Knob/AgentKnobs.cs index 5f706a8b2a..fbfe333f2b 100644 --- a/src/Agent.Sdk/Knob/AgentKnobs.cs +++ b/src/Agent.Sdk/Knob/AgentKnobs.cs @@ -374,12 +374,12 @@ public class AgentKnobs new RuntimeKnobSource("DISABLE_BUILD_ARTIFACTS_TO_BLOB"), new EnvironmentKnobSource("DISABLE_BUILD_ARTIFACTS_TO_BLOB"), new BuiltInDefaultKnobSource("false")); - public static readonly Knob DisableBuildArtifactsToBlobAlternateDomain = new Knob( - nameof(DisableBuildArtifactsToBlobAlternateDomain), - "The agent may upload build artifacts to alternate domains in Blobstore. If enabled, all artifacts will go to the legacy domain.", - new RuntimeKnobSource("DISABLE_BUILD_ARTIFACTS_TO_BLOB_ALTERNATE_DOMAIN"), - new EnvironmentKnobSource("DISABLE_BUILD_ARTIFACTS_TO_BLOB_ALTERNATE_DOMAIN"), - new BuiltInDefaultKnobSource("false")); + public static readonly Knob SendArtifactsToBlobstoreDomain = new Knob( + nameof(SendArtifactsToBlobstoreDomain), + "When set, defines the domain to use to send (Build|Pipeline) artifacts to.", + new RuntimeKnobSource("SEND_ARTIFACTS_TO_BLOBSTORE_DOMAIN"), + new EnvironmentKnobSource("SEND_ARTIFACTS_TO_BLOBSTORE_DOMAIN"), + new BuiltInDefaultKnobSource(string.Empty)); public static readonly Knob EnableIncompatibleBuildArtifactsPathResolution = new Knob( nameof(EnableIncompatibleBuildArtifactsPathResolution), diff --git a/src/Agent.Worker/Build/FileContainerServer.cs b/src/Agent.Worker/Build/FileContainerServer.cs index 9016aa12ae..e0daf86893 100644 --- a/src/Agent.Worker/Build/FileContainerServer.cs +++ b/src/Agent.Worker/Build/FileContainerServer.cs @@ -355,7 +355,6 @@ private async Task BlobUploadAsync(IAsyncCommandContext context, I BlobStoreClientTelemetryTfs clientTelemetry = null; try { - bool forceDefaultDomain = AgentKnobs.DisableBuildArtifactsToBlobAlternateDomain.GetValue(context).AsBoolean(); var verbose = String.Equals(context.GetVariableValueOrDefault("system.debug"), "true", StringComparison.InvariantCultureIgnoreCase); Action tracer = (str) => context.Output(str); @@ -366,7 +365,9 @@ private async Task BlobUploadAsync(IAsyncCommandContext context, I DedupManifestArtifactClientFactory.CreateArtifactsTracer(verbose, tracer), token); - IDomainId domainId = forceDefaultDomain ? WellKnownDomainIds.DefaultDomainId : clientSettings.GetDefaultDomainId(); + // Check if the pipeline has an override domain set, if not, use the default domain from the client settings. + string overrideDomain = AgentKnobs.SendArtifactsToBlobstoreDomain.GetValue(context).AsString(); + IDomainId domainId = String.IsNullOrWhiteSpace(overrideDomain) ? clientSettings.GetDefaultDomainId() : DomainIdFactory.Create(overrideDomain); (dedupClient, clientTelemetry) = DedupManifestArtifactClientFactory.Instance .CreateDedupClient( @@ -377,6 +378,7 @@ private async Task BlobUploadAsync(IAsyncCommandContext context, I verbose, tracer, token); + // Upload to blobstore var results = await BlobStoreUtils.UploadBatchToBlobstore(verbose, files, (level, uri, type) => new BuildArtifactActionRecord(level, uri, type, nameof(BlobUploadAsync), context), tracer, dedupClient, clientTelemetry, token, enableReporting: true); @@ -397,7 +399,7 @@ private async Task BlobUploadAsync(IAsyncCommandContext context, I var parallelAssociateTasks = new List>(); for (int uploader = 0; uploader < concurrentUploads; uploader++) { - parallelAssociateTasks.Add(AssociateAsync(context, queue, token)); + parallelAssociateTasks.Add(AssociateAsync(context, domainId, queue, token)); } // Wait for parallel associate tasks to finish. @@ -443,7 +445,7 @@ private async Task BlobUploadAsync(IAsyncCommandContext context, I return uploadResult; } - private async Task AssociateAsync(IAsyncCommandContext context, ConcurrentQueue associateQueue, CancellationToken token) + private async Task AssociateAsync(IAsyncCommandContext context, IDomainId domainId, ConcurrentQueue associateQueue, CancellationToken token) { var uploadResult = new UploadResult(); @@ -467,7 +469,7 @@ private async Task AssociateAsync(IAsyncCommandContext context, Co { var length = (long)file.Node.TransitiveContentBytes; response = await retryHelper.Retry(async () => await _fileContainerHttpClient.CreateItemForArtifactUpload(_containerId, itemPath, _projectId, - file.DedupId.ValueString, length, token), + CreateDomainHash(domainId, file.DedupId), length, token), (retryCounter) => (int)Math.Pow(retryCounter, 2) * 5, (exception) => true); uploadResult.TotalFileSizeUploaded += length; From 183538be46135a4f58a9a0bcdad0471c7610b345 Mon Sep 17 00:00:00 2001 From: Brian Barthel Date: Sun, 28 Jan 2024 14:06:37 -0500 Subject: [PATCH 3/4] Split domain knob into separate build|pipeline artifact knobs. --- .../Artifact/PipelineArtifactServer.cs | 2 +- src/Agent.Sdk/Knob/AgentKnobs.cs | 16 +++++++++++----- src/Agent.Worker/Build/FileContainerServer.cs | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/Agent.Plugins/Artifact/PipelineArtifactServer.cs b/src/Agent.Plugins/Artifact/PipelineArtifactServer.cs index 302b0f4b42..580bafbe4a 100644 --- a/src/Agent.Plugins/Artifact/PipelineArtifactServer.cs +++ b/src/Agent.Plugins/Artifact/PipelineArtifactServer.cs @@ -52,7 +52,7 @@ internal async Task UploadAsync( cancellationToken); // Check if the pipeline has an override domain set, if not, use the default domain from the client settings. - string overrideDomain = AgentKnobs.SendArtifactsToBlobstoreDomain.GetValue(context).AsString(); + string overrideDomain = AgentKnobs.SendPipelineArtifactsToBlobstoreDomain.GetValue(context).AsString(); IDomainId domainId = String.IsNullOrWhiteSpace(overrideDomain) ? clientSettings.GetDefaultDomainId() : DomainIdFactory.Create(overrideDomain); var (dedupManifestClient, clientTelemetry) = DedupManifestArtifactClientFactory.Instance diff --git a/src/Agent.Sdk/Knob/AgentKnobs.cs b/src/Agent.Sdk/Knob/AgentKnobs.cs index fbfe333f2b..27ee0b44ae 100644 --- a/src/Agent.Sdk/Knob/AgentKnobs.cs +++ b/src/Agent.Sdk/Knob/AgentKnobs.cs @@ -374,11 +374,17 @@ public class AgentKnobs new RuntimeKnobSource("DISABLE_BUILD_ARTIFACTS_TO_BLOB"), new EnvironmentKnobSource("DISABLE_BUILD_ARTIFACTS_TO_BLOB"), new BuiltInDefaultKnobSource("false")); - public static readonly Knob SendArtifactsToBlobstoreDomain = new Knob( - nameof(SendArtifactsToBlobstoreDomain), - "When set, defines the domain to use to send (Build|Pipeline) artifacts to.", - new RuntimeKnobSource("SEND_ARTIFACTS_TO_BLOBSTORE_DOMAIN"), - new EnvironmentKnobSource("SEND_ARTIFACTS_TO_BLOBSTORE_DOMAIN"), + public static readonly Knob SendBuildArtifactsToBlobstoreDomain = new Knob( + nameof(SendBuildArtifactsToBlobstoreDomain), + "When set, defines the domain to use to send Build artifacts to.", + new RuntimeKnobSource("SEND_BUILD_ARTIFACTS_TO_BLOBSTORE_DOMAIN"), + new EnvironmentKnobSource("SEND_BUILD_ARTIFACT_ARTIFACTS_TO_BLOBSTORE_DOMAIN"), + new BuiltInDefaultKnobSource(string.Empty)); + public static readonly Knob SendPipelineArtifactsToBlobstoreDomain = new Knob( + nameof(SendPipelineArtifactsToBlobstoreDomain), + "When set, defines the domain to use to send Pipeline artifacts to.", + new RuntimeKnobSource("SEND_PIPELINE_ARTIFACTS_TO_BLOBSTORE_DOMAIN"), + new EnvironmentKnobSource("SEND_PIPELINE_ARTIFACT_ARTIFACTS_TO_BLOBSTORE_DOMAIN"), new BuiltInDefaultKnobSource(string.Empty)); public static readonly Knob EnableIncompatibleBuildArtifactsPathResolution = new Knob( diff --git a/src/Agent.Worker/Build/FileContainerServer.cs b/src/Agent.Worker/Build/FileContainerServer.cs index e0daf86893..d8695df9d7 100644 --- a/src/Agent.Worker/Build/FileContainerServer.cs +++ b/src/Agent.Worker/Build/FileContainerServer.cs @@ -366,7 +366,7 @@ private async Task BlobUploadAsync(IAsyncCommandContext context, I token); // Check if the pipeline has an override domain set, if not, use the default domain from the client settings. - string overrideDomain = AgentKnobs.SendArtifactsToBlobstoreDomain.GetValue(context).AsString(); + string overrideDomain = AgentKnobs.SendBuildArtifactsToBlobstoreDomain.GetValue(context).AsString(); IDomainId domainId = String.IsNullOrWhiteSpace(overrideDomain) ? clientSettings.GetDefaultDomainId() : DomainIdFactory.Create(overrideDomain); (dedupClient, clientTelemetry) = DedupManifestArtifactClientFactory.Instance From b5929cd958e8dd14b862470cf7109edb550597bd Mon Sep 17 00:00:00 2001 From: Brian Barthel Date: Mon, 29 Jan 2024 09:49:14 -0500 Subject: [PATCH 4/4] Add fallback for socket exception as well --- src/Agent.Plugins/Artifact/FileContainerProvider.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Agent.Plugins/Artifact/FileContainerProvider.cs b/src/Agent.Plugins/Artifact/FileContainerProvider.cs index 450bf5916b..9f8be74b9a 100644 --- a/src/Agent.Plugins/Artifact/FileContainerProvider.cs +++ b/src/Agent.Plugins/Artifact/FileContainerProvider.cs @@ -182,6 +182,8 @@ private async Task DownloadFileContainerAsync(IEnumerable ite catch (SocketException e) { ExceptionsUtil.HandleSocketException(e, connection.Uri.ToString(), context.Warning); + // Fall back to streaming through TFS if we cannot reach blobstore for any reason + downloadFromBlob = false; } catch {