Skip to content

Commit 75be809

Browse files
brdeyobrdeyo
committed
Fix bug where metadata supplied on command line is NOT passed to client/server executors that are created outside of the bounds of the ProfileExecutor. This was causing required metadata to resolve content path templates to be unavailable.
1 parent 330cd68 commit 75be809

File tree

11 files changed

+163
-38
lines changed

11 files changed

+163
-38
lines changed

‎.pipelines/azure-pipelines-linux.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ resources:
1616
options: --entrypoint=""
1717

1818
variables:
19-
VcVersion : 1.11.2
19+
VcVersion : 1.11.3
2020
ROOT: $(Build.SourcesDirectory)
2121
CDP_DEFINITION_BUILD_COUNT: $[counter('', 0)] # needed for onebranch.pipeline.version task https://aka.ms/obpipelines/versioning
2222
ENABLE_PRS_DELAYSIGN: 1

‎.pipelines/azure-pipelines.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pool:
1818
vmImage: windows-latest
1919

2020
variables:
21-
VcVersion : 1.11.2
21+
VcVersion : 1.11.3
2222
ROOT: $(Build.SourcesDirectory)
2323
CDP_DEFINITION_BUILD_COUNT: $[counter('', 0)] # needed for onebranch.pipeline.version task https://aka.ms/obpipelines/versioning
2424
ENABLE_PRS_DELAYSIGN: 1

‎src/VirtualClient/VirtualClient.Contracts.UnitTests/FileUploadDescriptorFactoryTests.cs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,62 @@ public void FileUploadDescriptorFactoryCreatesTheExpectedDescriptorWithCustomCon
373373
Assert.AreEqual(expectedFilePath, descriptor.FilePath);
374374
}
375375

376+
[Test]
377+
[TestCase("contentpathtemplate")]
378+
[TestCase("CONTENTPATHTEMPLATE")]
379+
[TestCase("ContentPathTemplate")]
380+
[TestCase("contentPathTemplate")]
381+
public void FileUploadDescriptorFactoryIsNotCaseSensitiveOnContentPathTemplatesDefinedInCommandLineMetadata(string contentPathTemplateParameterName)
382+
{
383+
this.SetupDefaults();
384+
385+
string expectedContentPathTemplate = "customcontainer/{ExperimentDefinitionId}/{ExperimentName}/{ExperimentId},{Revision}/{AgentId}/{Scenario}/{Role}";
386+
string expectedExperimentName = "Test_Experiment";
387+
string expectedExperimentId = "cfad01a9-8F5c-4210-841e-63210ed6a85d";
388+
string expectedExperimentDefinitionId = "645f6639-98c6-41ee-a853-b0a3ab8ce0a3";
389+
string expectedAgentId = "642,042728166da7,37,192.168.2.155";
390+
string expectedRevision = "Revision01";
391+
string expectedToolName = "Toolkit";
392+
string expectedScenario = "Cycle-VegaServer";
393+
string expectedRole = "Client";
394+
string expectedContentType = HttpContentType.PlainText;
395+
string expectedContentEncoding = Encoding.UTF8.WebName;
396+
string expectedFilePath = this.mockFile.Object.FullName;
397+
string expectedFileName = $"{this.mockFile.Object.CreationTimeUtc.ToString("yyyy-MM-ddTHH-mm-ss-fffffZ")}-{this.mockFile.Object.Name}";
398+
399+
// The blob path itself is lower-cased. However, the file name casing is NOT modified.
400+
string expectedContainer = "customcontainer";
401+
string expectedBlobPath = $"/645f6639-98c6-41ee-a853-b0a3ab8ce0a3/test_experiment/cfad01a9-8f5c-4210-841e-63210ed6a85d,revision01/642,042728166da7,37,192.168.2.155/cycle-vegaserver/client/{expectedFileName}";
402+
403+
FileContext context = new FileContext(
404+
this.mockFile.Object,
405+
expectedContentType,
406+
expectedContentEncoding,
407+
expectedExperimentId,
408+
expectedAgentId,
409+
expectedToolName,
410+
expectedScenario,
411+
null,
412+
expectedRole);
413+
414+
IDictionary<string, IConvertible> componentMetadata = new Dictionary<string, IConvertible>(StringComparer.OrdinalIgnoreCase)
415+
{
416+
{ "Revision", expectedRevision },
417+
{ "ExperimentDefinitionId", expectedExperimentDefinitionId },
418+
{ "ExperimentName", expectedExperimentName },
419+
{ contentPathTemplateParameterName, expectedContentPathTemplate }
420+
};
421+
422+
FileUploadDescriptor descriptor = FileUploadDescriptorFactory.CreateDescriptor(context, metadata: componentMetadata, timestamped: true);
423+
424+
Assert.AreEqual(expectedContainer, descriptor.ContainerName);
425+
Assert.AreEqual(expectedFileName, descriptor.BlobName);
426+
Assert.AreEqual(expectedBlobPath, descriptor.BlobPath);
427+
Assert.AreEqual(expectedContentEncoding, descriptor.ContentEncoding);
428+
Assert.AreEqual(expectedContentType, descriptor.ContentType);
429+
Assert.AreEqual(expectedFilePath, descriptor.FilePath);
430+
}
431+
376432
[Test]
377433
[TestCase
378434
(

‎src/VirtualClient/VirtualClient.Contracts.UnitTests/VirtualClientComponentTests.cs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,13 @@ public void VirtualClientComponentConstructorsValidateRequiredParameters()
3535
[Test]
3636
public void VirtualClientComponentConstructorsSetPropertiesToExpectedValues()
3737
{
38+
// The existence of the layout causes the 'Role' parameter to be added automatically.
39+
// We want to do a pure parameter comparison.
40+
this.mockFixture.Dependencies.RemoveAll<EnvironmentLayout>();
3841
VirtualClientComponent component = new TestVirtualClientComponent(this.mockFixture.Dependencies, this.mockFixture.Parameters);
3942

4043
Assert.IsTrue(object.ReferenceEquals(this.mockFixture.Dependencies, component.Dependencies));
44+
4145
CollectionAssert.AreEquivalent(
4246
this.mockFixture.Parameters.Select(p => $"{p.Key}={p.Value}"),
4347
component.Parameters.Select(p => $"{p.Key}={p.Value}"));
@@ -64,6 +68,43 @@ public void VirtualClientComponentTagsSupportsCommonListMergingScenario()
6468
Assert.IsTrue(mergedTags.First() == "AnyOtherTag");
6569
}
6670

71+
[Test]
72+
[TestCase("Client", "Client")]
73+
[TestCase("Client,User", "Client,User")]
74+
[TestCase("Client;User", "Client,User")]
75+
public void VirtualClientComponentRolesMatchThoseDefinedInTheParameters(string roles, string expectedRoles)
76+
{
77+
// Both 'Role' and 'Roles' are supported parameters.
78+
this.mockFixture.Parameters["Role"] = roles;
79+
TestVirtualClientComponent component = new TestVirtualClientComponent(this.mockFixture.Dependencies, this.mockFixture.Parameters);
80+
Assert.AreEqual(expectedRoles, string.Join(",", component.Roles));
81+
82+
this.mockFixture.Parameters["Roles"] = roles;
83+
component = new TestVirtualClientComponent(this.mockFixture.Dependencies, this.mockFixture.Parameters);
84+
Assert.AreEqual(expectedRoles, string.Join(",", component.Roles));
85+
}
86+
87+
[Test]
88+
[TestCase("Client", "Client")]
89+
[TestCase("Client,User", "Client,User")]
90+
[TestCase("Client;User", "Client,User")]
91+
public void VirtualClientComponentRolesMatchThoseDefinedInTheEnvironmentLayoutWhenTheyAreNotDefinedInTheParameters(string roles, string expectedRoles)
92+
{
93+
this.mockFixture.Layout = new EnvironmentLayout(new List<ClientInstance>
94+
{
95+
new ClientInstance(Environment.MachineName, "1.2.3.4", roles),
96+
new ClientInstance("AnyOtherClientInstance", "1.2.3.5", "Server")
97+
});
98+
99+
// Ensure there are no parameters that define the role.
100+
this.mockFixture.Parameters.Remove("Role");
101+
this.mockFixture.Parameters.Remove("Roles");
102+
103+
TestVirtualClientComponent component = new TestVirtualClientComponent(this.mockFixture.Dependencies, this.mockFixture.Parameters);
104+
105+
Assert.AreEqual(expectedRoles, string.Join(",", component.Roles));
106+
}
107+
67108
[Test]
68109
public void VirtualClientComponentCorrectlyDeterminesWhenItIsInAGivenRole_Scenario1()
69110
{

‎src/VirtualClient/VirtualClient.Contracts/ClientInstance.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ namespace VirtualClient.Contracts
1515
/// <summary>
1616
/// Represents a Virtual Client instance (e.g. name, IP address, etc.)
1717
/// </summary>
18-
[DebuggerDisplay("{Name}({Role}):{PrivateIPAddress}")]
18+
[DebuggerDisplay("{Name}({Role}):{IPAddress}")]
1919
[JsonObject(MemberSerialization = MemberSerialization.OptIn)]
2020
public class ClientInstance : IEquatable<ClientInstance>
2121
{

‎src/VirtualClient/VirtualClient.Contracts/VirtualClientComponent.cs

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public abstract class VirtualClientComponent : IDisposable
3737
/// </summary>
3838
public static readonly Assembly ExecutingAssembly = Assembly.GetExecutingAssembly();
3939

40+
private const string Role = "Role";
4041
private ISystemInfo systemInfo;
4142

4243
/// <summary>
@@ -60,10 +61,23 @@ protected VirtualClientComponent(IServiceCollection dependencies, IDictionary<st
6061
this.Parameters = new Dictionary<string, IConvertible>(StringComparer.OrdinalIgnoreCase);
6162
}
6263

63-
this.Metadata = new Dictionary<string, IConvertible>(StringComparer.OrdinalIgnoreCase);
64-
this.MetadataContract = new MetadataContract();
64+
this.systemInfo = dependencies.GetService<ISystemInfo>();
65+
this.AgentId = this.systemInfo.AgentId;
66+
this.CpuArchitecture = this.systemInfo.CpuArchitecture;
6567
this.Dependencies = dependencies;
68+
this.ExperimentId = this.systemInfo.ExperimentId;
6669
this.Logger = NullLogger.Instance;
70+
this.Metadata = new Dictionary<string, IConvertible>(StringComparer.OrdinalIgnoreCase);
71+
this.MetadataContract = new MetadataContract();
72+
this.PlatformSpecifics = this.systemInfo.PlatformSpecifics;
73+
this.Platform = this.systemInfo.Platform;
74+
this.SupportingExecutables = new List<string>();
75+
this.CleanupTasks = new List<Action>();
76+
77+
if (VirtualClientRuntime.Metadata?.Any() == true)
78+
{
79+
this.Metadata.AddRange(VirtualClientRuntime.Metadata, true);
80+
}
6781

6882
if (dependencies.TryGetService<ILogger>(out ILogger logger))
6983
{
@@ -73,16 +87,19 @@ protected VirtualClientComponent(IServiceCollection dependencies, IDictionary<st
7387
if (dependencies.TryGetService<EnvironmentLayout>(out EnvironmentLayout layout))
7488
{
7589
this.Layout = layout;
76-
}
7790

78-
this.systemInfo = this.Dependencies.GetService<ISystemInfo>();
79-
this.AgentId = this.systemInfo.AgentId;
80-
this.ExperimentId = this.systemInfo.ExperimentId;
81-
this.PlatformSpecifics = this.systemInfo.PlatformSpecifics;
82-
this.Platform = this.systemInfo.Platform;
83-
this.CpuArchitecture = this.systemInfo.CpuArchitecture;
84-
this.SupportingExecutables = new List<string>();
85-
this.CleanupTasks = new List<Action>();
91+
if (this.Roles?.Any() != true)
92+
{
93+
// Backwards Compatibility:
94+
// Add in the roles from the layout if they are defined within it.
95+
ClientInstance clientInstance = this.GetLayoutClientInstance(throwIfNotExists: false);
96+
97+
if (clientInstance != null && !string.IsNullOrWhiteSpace(clientInstance.Role))
98+
{
99+
this.Parameters[VirtualClientComponent.Role] = clientInstance.Role;
100+
}
101+
}
102+
}
86103
}
87104

88105
/// <summary>
@@ -375,7 +392,9 @@ public IEnumerable<string> Roles
375392
get
376393
{
377394
IEnumerable<string> rolesList = null;
378-
if (this.Parameters.TryGetValue("Role", out IConvertible roles))
395+
IConvertible roles;
396+
397+
if (this.Parameters.TryGetValue("Role", out roles) || this.Parameters.TryGetValue(nameof(this.Roles), out roles))
379398
{
380399
rolesList = roles?.ToString().Split(VirtualClientComponent.CommonDelimiters, StringSplitOptions.None);
381400
}

‎src/VirtualClient/VirtualClient.Contracts/VirtualClientRuntime.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
namespace VirtualClient
55
{
66
using System;
7+
using System.Collections.Concurrent;
78
using System.Collections.Generic;
89
using System.Linq;
910
using System.Threading;
@@ -69,6 +70,16 @@ public static class VirtualClientRuntime
6970
/// </summary>
7071
public static bool IsRebootRequested { get; set; }
7172

73+
/// <summary>
74+
/// Metadata provided to the application on the command line.
75+
/// </summary>
76+
public static IDictionary<string, IConvertible> Metadata { get; } = new ConcurrentDictionary<string, IConvertible>(StringComparer.OrdinalIgnoreCase);
77+
78+
/// <summary>
79+
/// Parameters provided to the application on the command line.
80+
/// </summary>
81+
public static IDictionary<string, IConvertible> Parameters { get; } = new ConcurrentDictionary<string, IConvertible>(StringComparer.OrdinalIgnoreCase);
82+
7283
/// <summary>
7384
/// Cleans up any tracked resources.
7485
/// </summary>

‎src/VirtualClient/VirtualClient.Core.UnitTests/ProfileExecutorTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -747,8 +747,8 @@ public async Task ProfileExecutorExitsImmediatelyOnAnyErrorWheneverTheFailFastOp
747747

748748
private class TestProfileExecutor : ProfileExecutor
749749
{
750-
public TestProfileExecutor(ExecutionProfile profile, IServiceCollection dependencies, IEnumerable<string> scenarios = null, IDictionary<string, IConvertible> metadata = null, ILogger logger = null)
751-
: base(profile, dependencies, scenarios, metadata, logger)
750+
public TestProfileExecutor(ExecutionProfile profile, IServiceCollection dependencies, IEnumerable<string> scenarios = null, ILogger logger = null)
751+
: base(profile, dependencies, scenarios, logger)
752752
{
753753
}
754754

‎src/VirtualClient/VirtualClient.Core/ProfileExecutor.cs

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ public class ProfileExecutor : IDisposable
3232
/// <param name="profile">The profile to execute.</param>
3333
/// <param name="dependencies">Shared platform dependencies to pass along to individual components in the profile.</param>
3434
/// <param name="scenarios">A specific set of profile scenarios to execute or to exclude.</param>
35-
/// <param name="metadata">Metadata supplied</param>
3635
/// <param name="logger">A logger to use for capturing telemetry.</param>
37-
public ProfileExecutor(ExecutionProfile profile, IServiceCollection dependencies, IEnumerable<string> scenarios = null, IDictionary<string, IConvertible> metadata = null, ILogger logger = null)
36+
public ProfileExecutor(ExecutionProfile profile, IServiceCollection dependencies, IEnumerable<string> scenarios = null, ILogger logger = null)
3837
{
3938
profile.ThrowIfNull(nameof(profile));
4039
dependencies.ThrowIfNull(nameof(dependencies));
@@ -50,11 +49,6 @@ public ProfileExecutor(ExecutionProfile profile, IServiceCollection dependencies
5049
this.Logger = logger ?? NullLogger.Instance;
5150

5251
this.metadataContact = new MetadataContract();
53-
this.Metadata = new Dictionary<string, IConvertible>(StringComparer.OrdinalIgnoreCase);
54-
if (metadata?.Any() == true)
55-
{
56-
this.Metadata.AddRange(metadata, true);
57-
}
5852
}
5953

6054
/// <summary>
@@ -145,11 +139,6 @@ public static int CurrentIteration
145139
/// </summary>
146140
public ILogger Logger { get; }
147141

148-
/// <summary>
149-
/// Metadata supplied to the application on the command line.
150-
/// </summary>
151-
public IDictionary<string, IConvertible> Metadata { get; }
152-
153142
/// <summary>
154143
/// The profile to execute.
155144
/// </summary>
@@ -204,6 +193,11 @@ public async Task ExecuteAsync(ProfileTiming timing, CancellationToken cancellat
204193
CancellationToken profileCancellationToken = tokenSource.Token;
205194
if (!profileCancellationToken.IsCancellationRequested)
206195
{
196+
if (this.Profile.Metadata?.Any() == true)
197+
{
198+
VirtualClientRuntime.Metadata.AddRange(this.Profile.Metadata, true);
199+
}
200+
207201
// The parent context is created when the profile operations start up. We can use the
208202
// activity ID of the parent to enable correlation of events all the way down the
209203
// callstack.
@@ -716,12 +710,6 @@ private List<VirtualClientComponent> CreateComponents(
716710
VirtualClientComponent runtimeComponent = ComponentFactory.CreateComponent(component, this.Dependencies, this.RandomizationSeed);
717711
runtimeComponent.FailFast = this.FailFast;
718712

719-
// Metadata: Profile-level (global)
720-
if (this.Metadata?.Any() == true)
721-
{
722-
runtimeComponent.Metadata.AddRange(this.Metadata, true);
723-
}
724-
725713
// Metadata: Profile Component-level (overrides global)
726714
if (component.Metadata?.Any() == true)
727715
{

‎src/VirtualClient/VirtualClient.Main/CommandBase.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,16 @@ protected virtual IServiceCollection InitializeDependencies(string[] args)
168168

169169
this.InitializeGlobalTelemetryProperties(args);
170170

171+
if (this.Metadata?.Any() == true)
172+
{
173+
VirtualClientRuntime.Metadata.AddRange(this.Metadata, true);
174+
}
175+
176+
if (this.Parameters?.Any() == true)
177+
{
178+
VirtualClientRuntime.Parameters.AddRange(this.Parameters, true);
179+
}
180+
171181
IConfiguration configuration = Program.LoadAppSettings();
172182

173183
IConvertible telemetrySource = null;

0 commit comments

Comments
 (0)