diff --git a/src/VirtualClient/VirtualClient.Actions.FunctionalTests/TestDependencies.cs b/src/VirtualClient/VirtualClient.Actions.FunctionalTests/TestDependencies.cs index 7ffa41c715..1b0a6ef3e1 100644 --- a/src/VirtualClient/VirtualClient.Actions.FunctionalTests/TestDependencies.cs +++ b/src/VirtualClient/VirtualClient.Actions.FunctionalTests/TestDependencies.cs @@ -4,6 +4,7 @@ namespace VirtualClient.Actions { using System; + using System.Collections.Generic; using System.IO; using System.Reflection; using Microsoft.Extensions.DependencyInjection; @@ -39,13 +40,44 @@ static TestDependencies() /// /// Creates a for the workload profile provided (e.g. PERF-IO-FIO-STRESS.json). /// - public static ProfileExecutor CreateProfileExecutor(string profile, IServiceCollection dependencies, bool dependenciesOnly = false) + /// The name of the workload profile file (e.g. PERF-IO-DISKSPD.json). + /// Service dependencies required by the profile executor. + /// True to execute only profile dependencies, not actions. + /// + /// Optional parameter overrides applied to every non-disk-fill action in the profile after inlining + /// (e.g. new Dictionary<string, IConvertible> {{ "DiskFilter", "DiskIndex:6,7" }}). + /// Useful for testing scenarios that would normally be driven by CLI --parameters. + /// + public static ProfileExecutor CreateProfileExecutor( + string profile, + IServiceCollection dependencies, + bool dependenciesOnly = false, + IDictionary parameterOverrides = null) { ExecutionProfile workloadProfile = ExecutionProfile.ReadProfileAsync(Path.Combine(TestDependencies.ProfileDirectory, profile)) .GetAwaiter().GetResult(); workloadProfile.Inline(); + if (parameterOverrides?.Count > 0) + { + foreach (ExecutionProfileElement action in workloadProfile.Actions) + { + // Do not apply overrides to disk-fill actions — DiskFill is incompatible with + // DiskIndex: targeting and would fail validation if both are set. + bool isDiskFill = action.Parameters.TryGetValue("DiskFill", out IConvertible df) + && bool.TryParse(df?.ToString(), out bool dfValue) && dfValue; + + if (!isDiskFill) + { + foreach (KeyValuePair kvp in parameterOverrides) + { + action.Parameters[kvp.Key] = kvp.Value; + } + } + } + } + ComponentSettings settings = new ComponentSettings { ExitWait = TimeSpan.Zero diff --git a/src/VirtualClient/VirtualClient.Actions.UnitTests/DiskSpd/DiskSpdExecutorTests.cs b/src/VirtualClient/VirtualClient.Actions.UnitTests/DiskSpd/DiskSpdExecutorTests.cs index dffe0d439a..4477d03220 100644 --- a/src/VirtualClient/VirtualClient.Actions.UnitTests/DiskSpd/DiskSpdExecutorTests.cs +++ b/src/VirtualClient/VirtualClient.Actions.UnitTests/DiskSpd/DiskSpdExecutorTests.cs @@ -366,6 +366,484 @@ public async Task DiskSpdExecutorDeletesTestFilesByDefault() } } + [Test] + public void DiskSpdExecutorWithDiskIndexFilterUsesPhysicalDeviceNumberSyntax_SingleProcessModel() + { + // Bare disks use VC's internal \\.\.PHYSICALDISK{N} identifier. + // The executor uses DiskSpd's native #N syntax (e.g. #1, #2) derived from disk.Index. + IEnumerable bareDisks = new List + { + this.CreateDisk(1, PlatformID.Win32NT, os: false, @"\\.\PHYSICALDISK1"), + this.CreateDisk(2, PlatformID.Win32NT, os: false, @"\\.\PHYSICALDISK2") + }; + + this.profileParameters[nameof(DiskSpdExecutor.DiskFilter)] = "DiskIndex:1,2"; + this.profileParameters[nameof(DiskSpdExecutor.ProcessModel)] = WorkloadProcessModel.SingleProcess; + + List capturedArguments = new List(); + this.ProcessManager.OnCreateProcess = (exe, arguments, workingDir) => + { + capturedArguments.Add(arguments); + return new InMemoryProcess + { + StartInfo = new ProcessStartInfo { FileName = exe, Arguments = arguments } + }; + }; + + using (TestDiskSpdExecutor executor = new TestDiskSpdExecutor(this.Dependencies, this.profileParameters)) + { + executor.CreateWorkloadProcesses("diskspd.exe", "-b4K -r4K -t1 -o1 -w100", bareDisks, WorkloadProcessModel.SingleProcess); + } + + Assert.AreEqual(1, capturedArguments.Count); + // DiskSpd #N syntax -- derived from disk.Index, not disk.DevicePath. + // DiskSpd uses IOCTL_DISK_GET_DRIVE_GEOMETRY_EX internally; no -c needed. + Assert.IsTrue(capturedArguments[0].Contains(" #1")); + Assert.IsTrue(capturedArguments[0].Contains(" #2")); + // No file path style references should appear. + Assert.IsFalse(capturedArguments[0].Contains("diskspd-test.dat")); + } + + [Test] + public void DiskSpdExecutorWithDiskIndexFilterUsesPhysicalDeviceNumberSyntax_SingleProcessPerDiskModel() + { + IEnumerable bareDisks = new List + { + this.CreateDisk(1, PlatformID.Win32NT, os: false, @"\\.\PHYSICALDISK1"), + this.CreateDisk(2, PlatformID.Win32NT, os: false, @"\\.\PHYSICALDISK2") + }; + + this.profileParameters[nameof(DiskSpdExecutor.DiskFilter)] = "DiskIndex:1,2"; + + List capturedArguments = new List(); + int processCount = 0; + this.ProcessManager.OnCreateProcess = (exe, arguments, workingDir) => + { + capturedArguments.Add(arguments); + processCount++; + return new InMemoryProcess + { + StartInfo = new ProcessStartInfo { FileName = exe, Arguments = arguments } + }; + }; + + using (TestDiskSpdExecutor executor = new TestDiskSpdExecutor(this.Dependencies, this.profileParameters)) + { + executor.CreateWorkloadProcesses("diskspd.exe", "-b4K -r4K -t1 -o1 -w100", bareDisks, WorkloadProcessModel.SingleProcessPerDisk); + } + + Assert.AreEqual(2, processCount); + // Each process targets exactly one drive via DiskSpd's #N syntax (derived from disk.Index). + Assert.IsTrue(capturedArguments[0].Contains(" #1")); + Assert.IsFalse(capturedArguments[0].Contains(" #2")); + Assert.IsTrue(capturedArguments[1].Contains(" #2")); + Assert.IsFalse(capturedArguments[1].Contains(" #1")); + } + + [Test] + public void DiskSpdExecutorWithDiskIndexFilterDoesNotAppendFilenamesToCommandLine() + { + Disk bareDisk = this.CreateDisk(1, PlatformID.Win32NT, os: false, @"\\.\PHYSICALDISK1"); + + this.profileParameters[nameof(DiskSpdExecutor.DiskFilter)] = "DiskIndex:1"; + + string capturedArguments = null; + this.ProcessManager.OnCreateProcess = (exe, arguments, workingDir) => + { + capturedArguments = arguments; + return new InMemoryProcess + { + StartInfo = new ProcessStartInfo { FileName = exe, Arguments = arguments } + }; + }; + + using (TestDiskSpdExecutor executor = new TestDiskSpdExecutor(this.Dependencies, this.profileParameters)) + { + executor.CreateWorkloadProcesses( + "diskspd.exe", + "-b4K -r4K -t1 -o1 -w100", + new[] { bareDisk }, + WorkloadProcessModel.SingleProcess); + } + + Assert.IsNotNull(capturedArguments); + // DiskSpd's #N syntax is used -- derived from disk.Index=1. + Assert.IsTrue(capturedArguments.Contains(" #1")); + // No test-file extension should be present. + Assert.IsFalse(capturedArguments.Contains(".dat")); + } + + [Test] + public void DiskSpdExecutorWithDiskIndexFilterStoresDeviceNumberPathsInTestFiles() + { + // TestFiles is iterated by DeleteTestFilesAsync. For raw disk targets the paths must be + // the #N device number strings -- not file paths. File.Exists("#1") returns false, + // so DeleteTestFilesAsync becomes a correct no-op. + IEnumerable bareDisks = new List + { + this.CreateDisk(1, PlatformID.Win32NT, os: false, @"\\.\PHYSICALDISK1"), + this.CreateDisk(2, PlatformID.Win32NT, os: false, @"\\.\PHYSICALDISK2") + }; + + this.profileParameters[nameof(DiskSpdExecutor.DiskFilter)] = "DiskIndex:1,2"; + + this.ProcessManager.OnCreateProcess = (exe, arguments, workingDir) => + new InMemoryProcess { StartInfo = new ProcessStartInfo { FileName = exe, Arguments = arguments } }; + + IEnumerable processes; + using (TestDiskSpdExecutor executor = new TestDiskSpdExecutor(this.Dependencies, this.profileParameters)) + { + processes = executor.CreateWorkloadProcesses( + "diskspd.exe", "-b4K -r4K -t1 -o1 -w100", bareDisks, WorkloadProcessModel.SingleProcessPerDisk).ToList(); + } + + Assert.AreEqual(2, processes.Count()); + CollectionAssert.AreEqual(new[] { "#1" }, processes.ElementAt(0).TestFiles); + CollectionAssert.AreEqual(new[] { "#2" }, processes.ElementAt(1).TestFiles); + } + + // ----------------------------------------------------------------------- + // DiskIndex filter in ExecuteAsync tests + // ----------------------------------------------------------------------- + + [Test] + public void DiskSpdExecutorWithDiskIndexFilter_BypassesDiskManagerEnumeration() + { + // When DiskFilter=DiskIndex:6-8, TryGetDiskIndexes resolves disks without any DiskManager call. + bool diskManagerCalled = false; + this.DiskManager.OnGetDisks().Returns((CancellationToken token) => + { + diskManagerCalled = true; + return Task.FromResult(this.disks); + }); + + bool result = DiskFilters.TryGetDiskIndexes("DiskIndex:6-8", out IEnumerable indexes); + + Assert.IsTrue(result); + Assert.IsNotNull(indexes); + Assert.AreEqual(3, indexes.Count()); + Assert.IsFalse(diskManagerCalled, "DiskManager must NOT be called by TryGetDiskIndexes."); + } + + [Test] + public void DiskSpdExecutorWithDiskIndexFilter_CreatesOneProcessPerDisk() + { + // DiskFilter=DiskIndex:6-8 → disks 6, 7, 8 → 3 processes (SingleProcessPerDisk model). + this.profileParameters[nameof(DiskSpdExecutor.DiskFilter)] = "DiskIndex:6-8"; + + List capturedArguments = new List(); + this.ProcessManager.OnCreateProcess = (exe, arguments, workingDir) => + { + capturedArguments.Add(arguments); + return new InMemoryProcess + { + StartInfo = new ProcessStartInfo { FileName = exe, Arguments = arguments } + }; + }; + + DiskFilters.TryGetDiskIndexes("DiskIndex:6-8", out IEnumerable indexes); + IEnumerable disksFromRange = indexes.Select(i => new Disk(i, $@"\\.\PHYSICALDISK{i}")).ToList(); + + using (TestDiskSpdExecutor executor = new TestDiskSpdExecutor(this.Dependencies, this.profileParameters)) + { + executor.CreateWorkloadProcesses( + "diskspd.exe", + "-b128K -d60 -o32 -t1 -r -w0 -Sh -L -Rxml", + disksFromRange, + WorkloadProcessModel.SingleProcessPerDisk); + } + + Assert.AreEqual(3, capturedArguments.Count, "Expected one process per disk in the range."); + Assert.IsTrue(capturedArguments[0].TrimEnd().EndsWith("#6")); + Assert.IsTrue(capturedArguments[1].TrimEnd().EndsWith("#7")); + Assert.IsTrue(capturedArguments[2].TrimEnd().EndsWith("#8")); + } + + [Test] + public void DiskSpdExecutorWithDiskIndexFilter_UsesPhysicalDiskIndexSyntaxInCommandLine() + { + // Confirm #N notation (not a file path) is written to each spawned process. + this.profileParameters[nameof(DiskSpdExecutor.DiskFilter)] = "DiskIndex:10-11"; + + List capturedArguments = new List(); + this.ProcessManager.OnCreateProcess = (exe, arguments, workingDir) => + { + capturedArguments.Add(arguments); + return new InMemoryProcess + { + StartInfo = new ProcessStartInfo { FileName = exe, Arguments = arguments } + }; + }; + + DiskFilters.TryGetDiskIndexes("DiskIndex:10-11", out IEnumerable indexes); + IEnumerable disksFromRange = indexes.Select(i => new Disk(i, $@"\\.\PHYSICALDISK{i}")).ToList(); + + using (TestDiskSpdExecutor executor = new TestDiskSpdExecutor(this.Dependencies, this.profileParameters)) + { + executor.CreateWorkloadProcesses( + "diskspd.exe", + "-b128K -d60 -o32 -t1 -r -w0 -Sh -L -Rxml", + disksFromRange, + WorkloadProcessModel.SingleProcessPerDisk); + } + + Assert.AreEqual(2, capturedArguments.Count); + foreach (string args in capturedArguments) + { + Assert.IsTrue(args.Contains(" #10") || args.Contains(" #11"), + $"Expected '#N' syntax but got: {args}"); + Assert.IsFalse(args.Contains(".dat"), + $"Unexpected .dat file path: {args}"); + } + } + + [Test] + public void DiskSpdExecutorWithoutDiskIndexFilter_GetDisksToTestUsesFilteredDiskSet() + { + // When RawDiskIndexRange is NOT set, GetDisksToTest must use DiskFilters and return + // disks from the DiskManager-sourced collection (not a hardcoded range). + // This is the normal (non-raw-range) code path. + IEnumerable nonOsDisks = this.disks.Where(disk => !disk.IsOperatingSystem()); + + using (TestDiskSpdExecutor executor = new TestDiskSpdExecutor(this.Dependencies, this.profileParameters)) + { + IEnumerable disksToTest = executor.GetDisksToTest(this.disks); + + // The default filter excludes the OS disk, so only non-OS disks should be returned. + CollectionAssert.AreEquivalent( + nonOsDisks.Select(d => d.DevicePath), + disksToTest.Select(d => d.DevicePath)); + } + } + + [Test] + public void DiskSpdExecutorThrowsWhenBothDiskIndexFilterAndDiskFillAreEnabled() + { + this.profileParameters[nameof(DiskSpdExecutor.DiskFilter)] = "DiskIndex:6-8"; + this.profileParameters[nameof(DiskSpdExecutor.DiskFill)] = true; + this.profileParameters[nameof(DiskSpdExecutor.DiskFillSize)] = "500G"; + + using (TestDiskSpdExecutor executor = new TestDiskSpdExecutor(this.Dependencies, this.profileParameters)) + { + WorkloadException exc = Assert.Throws(() => executor.Validate()); + Assert.AreEqual(ErrorReason.InvalidProfileDefinition, exc.Reason); + } + } + + // ----------------------------------------------------------------------- + // Log file naming tests (ExecuteWorkloadAsync) + // ----------------------------------------------------------------------- + + [Test] + public async Task DiskSpdExecutorWithDiskIndexFilter_UsesScenarioAndDiskIndexAsLogFileName() + { + // When the test file is a #N physical disk target the log filename must encode both + // the scenario and the disk ordinal: "{Scenario}_disk{N}.log" + this.profileParameters[nameof(DiskSpdExecutor.Scenario)] = "RandomRead_128k_BlockSize"; + this.profileParameters[nameof(DiskSpdExecutor.LogToFile)] = true; + + string capturedLogFilePath = null; + this.FileSystem + .Setup(fs => fs.File.WriteAllTextAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((path, content, ct) => + { + if (path.EndsWith(".log", StringComparison.OrdinalIgnoreCase)) + { + capturedLogFilePath = path; + } + }) + .Returns(Task.CompletedTask); + + InMemoryProcess process = new InMemoryProcess + { + OnStart = () => true, + OnHasExited = () => true + }; + process.StandardOutput.Append(this.output); + + DiskWorkloadProcess workload = new DiskWorkloadProcess(process, "SingleProcessPerDisk,OsDisk:false,1", "#6"); + + using (TestDiskSpdExecutor executor = new TestDiskSpdExecutor(this.Dependencies, this.profileParameters)) + { + await executor.ExecuteWorkloadsAsync(new[] { workload }, CancellationToken.None).ConfigureAwait(false); + } + + Assert.IsNotNull(capturedLogFilePath, "Expected a log file to be written."); + Assert.IsTrue( + Path.GetFileName(capturedLogFilePath).Contains("_disk6"), + $"Expected log filename to contain '_disk6' but was: {capturedLogFilePath}"); + } + + [Test] + public async Task DiskSpdExecutorWithoutDiskIndexFilter_DoesNotAddDiskIndexToLogFileName() + { + // When the test file is a regular file path the log filename must NOT contain a '_disk' + // suffix; it should fall back to the default (scenario name only). + this.profileParameters[nameof(DiskSpdExecutor.Scenario)] = "RandomRead_128k_BlockSize"; + this.profileParameters[nameof(DiskSpdExecutor.LogToFile)] = true; + + string capturedLogFilePath = null; + this.FileSystem + .Setup(fs => fs.File.WriteAllTextAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((path, content, ct) => + { + if (path.EndsWith(".log", StringComparison.OrdinalIgnoreCase)) + { + capturedLogFilePath = path; + } + }) + .Returns(Task.CompletedTask); + + InMemoryProcess process = new InMemoryProcess + { + OnStart = () => true, + OnHasExited = () => true + }; + process.StandardOutput.Append(this.output); + + DiskWorkloadProcess workload = new DiskWorkloadProcess(process, "SingleProcessPerDisk,OsDisk:false,1", "D:\\any\\file.dat"); + + using (TestDiskSpdExecutor executor = new TestDiskSpdExecutor(this.Dependencies, this.profileParameters)) + { + await executor.ExecuteWorkloadsAsync(new[] { workload }, CancellationToken.None).ConfigureAwait(false); + } + + Assert.IsNotNull(capturedLogFilePath, "Expected a log file to be written."); + Assert.IsFalse( + Path.GetFileName(capturedLogFilePath).Contains("_disk"), + $"Expected no '_disk' suffix in log filename when test file is not a '#N' target, but was: {capturedLogFilePath}"); + } + + [Test] + public async Task DiskSpdExecutorWithDiskIndexFilter_LogFilenameContainsCorrectDiskOrdinalForEachProcess() + { + // Multiple workloads (#42 and #180) must each produce a log file whose + // name encodes their own disk index, not a shared or wrong value. + this.profileParameters[nameof(DiskSpdExecutor.Scenario)] = "SequentialRead_1024k_BlockSize"; + this.profileParameters[nameof(DiskSpdExecutor.LogToFile)] = true; + + List capturedPaths = new List(); + this.FileSystem + .Setup(fs => fs.File.WriteAllTextAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((path, content, ct) => + { + if (path.EndsWith(".log", StringComparison.OrdinalIgnoreCase)) + { + capturedPaths.Add(path); + } + }) + .Returns(Task.CompletedTask); + + InMemoryProcess p42 = new InMemoryProcess { OnStart = () => true, OnHasExited = () => true }; + InMemoryProcess p180 = new InMemoryProcess { OnStart = () => true, OnHasExited = () => true }; + p42.StandardOutput.Append(this.output); + p180.StandardOutput.Append(this.output); + + List workloads = new List + { + new DiskWorkloadProcess(p42, "instance", "#42"), + new DiskWorkloadProcess(p180, "instance", "#180") + }; + + using (TestDiskSpdExecutor executor = new TestDiskSpdExecutor(this.Dependencies, this.profileParameters)) + { + await executor.ExecuteWorkloadsAsync(workloads, CancellationToken.None).ConfigureAwait(false); + } + + Assert.AreEqual(2, capturedPaths.Count, "Expected one log file per workload."); + Assert.IsTrue( + capturedPaths.Any(p => Path.GetFileName(p).Contains("_disk42")), + $"Expected a log file with '_disk42'. Paths: {string.Join(", ", capturedPaths)}"); + Assert.IsTrue( + capturedPaths.Any(p => Path.GetFileName(p).Contains("_disk180")), + $"Expected a log file with '_disk180'. Paths: {string.Join(", ", capturedPaths)}"); + } + + [Test] + public async Task DiskSpdExecutorRawDiskTarget_ParsesGetPhysicalDiskOutput() + { + // Simulate Get-PhysicalDisk output: one integer DeviceId per line. + string psOutput = "6\r\n7\r\n8\r\n180"; + + this.ProcessManager.OnCreateProcess = (exe, arguments, workingDir) => + { + if (exe.Equals("powershell.exe", StringComparison.OrdinalIgnoreCase)) + { + InMemoryProcess p = new InMemoryProcess { OnStart = () => true, OnHasExited = () => true }; + p.StandardOutput.Append(psOutput); + return p; + } + + return new InMemoryProcess { OnStart = () => true, OnHasExited = () => true }; + }; + + using (TestDiskSpdExecutor executor = new TestDiskSpdExecutor(this.Dependencies, this.profileParameters)) + { + IEnumerable disks = await executor.DiscoverRawDisksAsync(CancellationToken.None); + + Assert.AreEqual(4, disks.Count()); + Assert.AreEqual(6, disks.ElementAt(0).Index); + Assert.AreEqual(7, disks.ElementAt(1).Index); + Assert.AreEqual(8, disks.ElementAt(2).Index); + Assert.AreEqual(180, disks.ElementAt(3).Index); + } + } + + [Test] + public async Task DiskSpdExecutorRawDiskTarget_InvokesGetPhysicalDiskViaPowerShell() + { + // The discovery method must invoke powershell.exe with Get-PhysicalDisk. + string capturedExe = null; + string capturedArgs = null; + + this.ProcessManager.OnCreateProcess = (exe, arguments, workingDir) => + { + capturedExe = exe; + capturedArgs = arguments; + InMemoryProcess p = new InMemoryProcess { OnStart = () => true, OnHasExited = () => true }; + p.StandardOutput.Append("6\r\n7"); + return p; + }; + + using (TestDiskSpdExecutor executor = new TestDiskSpdExecutor(this.Dependencies, this.profileParameters)) + { + await executor.DiscoverRawDisksAsync(CancellationToken.None); + } + + Assert.AreEqual("powershell.exe", capturedExe, "Expected powershell.exe to be invoked."); + Assert.IsTrue( + capturedArgs.Contains("Get-PhysicalDisk"), + $"Expected 'Get-PhysicalDisk' in the PowerShell command but got: {capturedArgs}"); + Assert.IsTrue( + capturedArgs.Contains("MediaType") && capturedArgs.Contains("HDD"), + $"Expected HDD MediaType filter in the PowerShell command but got: {capturedArgs}"); + } + + [Test] + public void DiskSpdExecutorWithExplicitDiskIndexRange_DoesNotInvokePowerShellDiscovery() + { + // When DiskFilter=DiskIndex: (not "hdd"), TryGetDiskIndexes returns + // explicit indexes without any OS call. PowerShell must NOT be invoked. + bool powershellInvoked = false; + this.ProcessManager.OnCreateProcess = (exe, arguments, workingDir) => + { + if (exe.Equals("powershell.exe", StringComparison.OrdinalIgnoreCase)) + { + powershellInvoked = true; + } + + return new InMemoryProcess { OnStart = () => true, OnHasExited = () => true }; + }; + + bool result = DiskFilters.TryGetDiskIndexes("DiskIndex:6-8", out IEnumerable indexes); + + Assert.IsTrue(result); + Assert.IsNotNull(indexes); + Assert.IsFalse(powershellInvoked, "PowerShell must not be invoked for an explicit DiskIndex range."); + Assert.AreEqual(3, indexes.Count()); + Assert.IsTrue(indexes.All(i => i >= 6 && i <= 8)); + } + private IEnumerable SetupWorkloadScenario( bool testRemoteDisks = false, bool testOSDisk = false, string processModel = WorkloadProcessModel.SingleProcess) { @@ -416,6 +894,11 @@ public TestDiskSpdExecutor(IServiceCollection dependencies, IDictionary> DiscoverRawDisksAsync(CancellationToken cancellationToken) + { + return base.DiscoverRawDisksAsync(cancellationToken); + } + public new void Validate() { base.Validate(); diff --git a/src/VirtualClient/VirtualClient.Actions/DiskSpd/DiskSpdExecutor.cs b/src/VirtualClient/VirtualClient.Actions/DiskSpd/DiskSpdExecutor.cs index 0ee10a2e34..59c84e6c00 100644 --- a/src/VirtualClient/VirtualClient.Actions/DiskSpd/DiskSpdExecutor.cs +++ b/src/VirtualClient/VirtualClient.Actions/DiskSpd/DiskSpdExecutor.cs @@ -311,8 +311,22 @@ protected override async Task CleanupAsync(EventContext telemetryContext, Cancel /// The disks under test. protected DiskWorkloadProcess CreateWorkloadProcess(string executable, string commandArguments, string testedInstance, params Disk[] disksToTest) { - string[] testFiles = disksToTest.Select(disk => this.GetTestFiles(disk.GetPreferredAccessPath(this.Platform))).ToArray(); - string diskSpdArguments = $"{commandArguments} {string.Join(" ", testFiles)}"; + string diskSpdArguments; + string[] testFiles; + + if (DiskFilters.TryGetDiskIndexes(this.DiskFilter, out _)) + { + // DiskSpd has a native syntax for targeting a physical drive by its index: #. + // This is the correct format for raw physical disk access; DiskSpd uses + // IOCTL_DISK_GET_DRIVE_GEOMETRY_EX internally to determine the drive capacity. + testFiles = disksToTest.Select(disk => $"#{disk.Index}").ToArray(); + diskSpdArguments = $"{commandArguments} {string.Join(" ", testFiles)}"; + } + else + { + testFiles = disksToTest.Select(disk => this.GetTestFiles(disk.GetPreferredAccessPath(this.Platform))).ToArray(); + diskSpdArguments = $"{commandArguments} {string.Join(" ", testFiles)}"; + } IProcessProxy process = this.SystemManagement.ProcessManager.CreateProcess(executable, diskSpdArguments); @@ -413,16 +427,47 @@ protected override async Task ExecuteAsync(EventContext telemetryContext, Cancel // Apply parameters to the DiskSpd command line options. await this.EvaluateParametersAsync(telemetryContext); - IEnumerable disks = await this.SystemManagement.DiskManager.GetDisksAsync(cancellationToken); + IEnumerable disksToTest; - if (disks?.Any() != true) + if (DiskFilters.TryGetDiskIndexes(this.DiskFilter, out IEnumerable diskIndexes) && diskIndexes != null) { - throw new WorkloadException( - "Unexpected scenario. The disks defined for the system could not be properly enumerated.", - ErrorReason.WorkloadUnexpectedAnomaly); + // Explicit index range supplied (e.g. DiskFilter=DiskIndex:6-180 or DiskIndex:6,7,8). + // Build disk list directly without any OS enumeration. + disksToTest = diskIndexes.Select(i => new Disk(i, $@"\\.\PHYSICALDISK{i}")).ToList(); + + this.Logger.LogMessage($"{nameof(DiskSpdExecutor)}.SelectDisks", telemetryContext.Clone() + .AddContext("disks", disksToTest) + .AddContext("diskFilter", this.DiskFilter)); + } + else if (DiskFilters.TryGetDiskIndexes(this.DiskFilter, out _)) + { + // DiskIndex:hdd sentinel — discover HDD indices at runtime via Get-PhysicalDisk. + // This sees offline JBOD drives that DiskPart/DiskManager cannot enumerate, + // and filters to MediaType=HDD to exclude OS SSDs/NVMe devices. + disksToTest = await this.DiscoverRawDisksAsync(cancellationToken); + + this.Logger.LogMessage($"{nameof(DiskSpdExecutor)}.SelectDisks", telemetryContext.Clone() + .AddContext("disks", disksToTest) + .AddContext("discoveryMethod", "GetPhysicalDisk")); } + else + { + IEnumerable disks = await this.SystemManagement.DiskManager.GetDisksAsync(cancellationToken); + + if (disks?.Any() != true) + { + throw new WorkloadException( + "Unexpected scenario. The disks defined for the system could not be properly enumerated.", + ErrorReason.WorkloadUnexpectedAnomaly); + } + + disksToTest = this.GetDisksToTest(disks); - IEnumerable disksToTest = this.GetDisksToTest(disks); + this.Logger.LogMessage($"{nameof(DiskSpdExecutor)}.SelectDisks", telemetryContext.Clone() + .AddContext("disks", disksToTest)); + + telemetryContext.AddContext(nameof(disks), disks); + } if (disksToTest?.Any() != true) { @@ -433,13 +478,9 @@ protected override async Task ExecuteAsync(EventContext telemetryContext, Cancel ErrorReason.DependencyNotFound); } - this.Logger.LogMessage($"{nameof(DiskSpdExecutor)}.SelectDisks", telemetryContext.Clone() - .AddContext("disks", disksToTest)); - disksToTest.ToList().ForEach(disk => this.Logger.LogTraceMessage($"Disk Target: '{disk}'")); telemetryContext.AddContext("executable", this.ExecutablePath); - telemetryContext.AddContext(nameof(disks), disks); telemetryContext.AddContext(nameof(disksToTest), disksToTest); this.workloadProcesses.AddRange(this.CreateWorkloadProcesses(this.ExecutablePath, this.CommandLine, disksToTest, this.ProcessModel)); @@ -488,6 +529,47 @@ protected Task ExecuteWorkloadsAsync(IEnumerable workloads, } } + /// + /// Discovers physical disk indices at runtime by running Get-PhysicalDisk via PowerShell. + /// Get-PhysicalDisk enumerates offline drives (e.g. JBOD) that DiskPart/DiskManager does not. + /// Used when DiskFilter=DiskIndex:hdd is specified. + /// + protected virtual async Task> DiscoverRawDisksAsync(CancellationToken cancellationToken) + { + // Filter to HDD media type only + // This excludes OS SSDs (disk0-5) and other non-HDD devices that Get-PhysicalDisk + // would otherwise return, ensuring only JBOD HDDs are targeted. + // Output: one integer DeviceId per line (sorted numerically), e.g. "6\r\n7\r\n8\r\n...\r\n180" + const string psArguments = "-NonInteractive -NoProfile -Command " + + "\"Get-PhysicalDisk | Where-Object { $_.MediaType -eq 'HDD' } | Select-Object -ExpandProperty DeviceId | Sort-Object { [int]$_ }\""; + + List disks = new List(); + + using (IProcessProxy process = this.SystemManagement.ProcessManager.CreateProcess("powershell.exe", psArguments)) + { + await process.StartAndWaitAsync(cancellationToken); + + if (process.ExitCode != 0) + { + throw new WorkloadException( + $"Failed to discover raw disks using 'Get-PhysicalDisk'. " + + $"Exit code: {process.ExitCode}. {process.StandardError}", + ErrorReason.WorkloadUnexpectedAnomaly); + } + + foreach (string line in process.StandardOutput.ToString() + .Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries)) + { + if (int.TryParse(line.Trim(), out int index)) + { + disks.Add(new Disk(index, $@"\\.\PHYSICALDISK{index}")); + } + } + } + + return disks; + } + /// /// Returns the disks to test from the set of all disks. /// @@ -667,6 +749,15 @@ protected override void Validate() $"to be defined (e.g. 496G).", ErrorReason.InvalidProfileDefinition); } + + if (DiskFilters.TryGetDiskIndexes(this.DiskFilter, out _) && this.DiskFill) + { + throw new WorkloadException( + $"Invalid profile definition. The '{nameof(DiskSpdExecutor.DiskFill)}' option cannot be used together with " + + $"a 'DiskIndex:' disk filter. Disk fill operations create test files on a mounted volume and are " + + $"not applicable to raw physical device access.", + ErrorReason.InvalidProfileDefinition); + } } private void CaptureMetrics(DiskWorkloadProcess workload, EventContext telemetryContext) @@ -717,7 +808,14 @@ await this.Logger.LogMessageAsync($"{nameof(DiskSpdExecutor)}.ExecuteProcess", t if (!cancellationToken.IsCancellationRequested) { - await this.LogProcessDetailsAsync(workload.Process, telemetryContext, "DiskSpd", logToFile: true); + string logFileName = null; + if (workload.TestFiles?.Any() == true && workload.TestFiles.First().StartsWith("#", StringComparison.Ordinal)) + { + string diskIndex = workload.TestFiles.First().TrimStart('#'); + logFileName = $"{this.Scenario}_disk{diskIndex}"; + } + + await this.LogProcessDetailsAsync(workload.Process, telemetryContext, "DiskSpd", logToFile: true, logFileName: logFileName); if (this.DiskFill) { diff --git a/src/VirtualClient/VirtualClient.Contracts.UnitTests/DiskExtensionsTests.cs b/src/VirtualClient/VirtualClient.Contracts.UnitTests/DiskExtensionsTests.cs index 8c32998dd6..8b5245ec20 100644 --- a/src/VirtualClient/VirtualClient.Contracts.UnitTests/DiskExtensionsTests.cs +++ b/src/VirtualClient/VirtualClient.Contracts.UnitTests/DiskExtensionsTests.cs @@ -189,5 +189,52 @@ public void GetDefaultMountPointNameExtensionUsesASpecificPrefixWhenProvided() Assert.AreEqual(expectedMountPointName, actualMountPointName); } } + + [Test] + public void GetPreferredAccessPathThrowsForBareDiskWithNoVolumes_Windows() + { + // GetPreferredAccessPath is for file-based workloads only. A disk with no volumes + // cannot be used for file I/O, so the original throw behavior is correct. + // Raw disk access bypasses this method entirely via RawDiskTarget in DiskSpdExecutor. + Disk bareDisk = this.fixture.CreateDisk(1, PlatformID.Win32NT, os: false, @"\\.\PHYSICALDISK1"); + + Assert.Throws(() => bareDisk.GetPreferredAccessPath(PlatformID.Win32NT)); + } + + [Test] + public void GetPreferredAccessPathThrowsForBareDiskWithNoVolumes_Unix() + { + // Same as Windows: a bare Linux disk with no volumes cannot be used for file I/O. + Disk bareDisk = this.fixture.CreateDisk(1, PlatformID.Unix, os: false, @"/dev/sdb"); + + Assert.Throws(() => bareDisk.GetPreferredAccessPath(PlatformID.Unix)); + } + + [Test] + public void GetPreferredAccessPathReturnsVolumeMountPointForFormattedNonOsDisk_Windows() + { + // A formatted non-OS Windows disk with a volume should return the volume access path. + this.disks = this.fixture.CreateDisks(PlatformID.Win32NT, true); + Disk dataDisk = this.disks.First(d => !d.IsOperatingSystem()); + + string path = dataDisk.GetPreferredAccessPath(PlatformID.Win32NT); + string expectedPath = dataDisk.Volumes.First().AccessPaths.First(); + + Assert.AreEqual(expectedPath, path); + } + + [Test] + public void GetPreferredAccessPathReturnsVolumeMountPointForFormattedNonOsDisk_Unix() + { + this.disks = this.fixture.CreateDisks(PlatformID.Unix, true); + Disk dataDisk = this.disks.First(d => !d.IsOperatingSystem()); + + string path = dataDisk.GetPreferredAccessPath(PlatformID.Unix); + string expectedPath = dataDisk.Volumes + .OrderByDescending(v => v.SizeInBytes(PlatformID.Unix)) + .First().AccessPaths.First(); + + Assert.AreEqual(expectedPath, path); + } } } diff --git a/src/VirtualClient/VirtualClient.Contracts.UnitTests/DiskFiltersTests.cs b/src/VirtualClient/VirtualClient.Contracts.UnitTests/DiskFiltersTests.cs index a6a9179616..aae4bd2533 100644 --- a/src/VirtualClient/VirtualClient.Contracts.UnitTests/DiskFiltersTests.cs +++ b/src/VirtualClient/VirtualClient.Contracts.UnitTests/DiskFiltersTests.cs @@ -524,5 +524,182 @@ public void DiskFiltersHandlesAnomaliesEncounters_1() Assert.AreEqual("/dev/sdi", filteredDisks.ElementAt(30).DevicePath); Assert.AreEqual("/dev/sdj", filteredDisks.ElementAt(31).DevicePath); } + + [Test] + public void DiskFiltersIncludeOfflineFilterKeepsOfflineDisksOnWindows() + { + // Arrange: create 4 disks; mark one as offline. + this.disks = this.mockFixture.CreateDisks(PlatformID.Win32NT, true); + this.disks.ElementAt(0).Properties["Status"] = "Online"; + this.disks.ElementAt(1).Properties["Status"] = "Online"; + this.disks.ElementAt(2).Properties["Status"] = "Offline (Policy)"; + this.disks.ElementAt(3).Properties["Status"] = "Online"; + + // "none" alone would remove the offline disk; "none&IncludeOffline" should retain it. + string filterString = "none&IncludeOffline"; + IEnumerable result = DiskFilters.FilterDisks(this.disks, filterString, PlatformID.Win32NT); + + Assert.AreEqual(4, result.Count()); + Assert.IsTrue(result.Any(d => d.Properties["Status"].ToString().Contains("Offline"))); + } + + [Test] + public void DiskFiltersIncludeOfflineFilterIsCaseInsensitive() + { + this.disks = this.mockFixture.CreateDisks(PlatformID.Win32NT, true); + this.disks.ElementAt(1).Properties["Status"] = "Offline"; + + // All casing variants should be accepted. + foreach (string variant in new[] { "none&IncludeOffline", "none&includeoffline", "none&INCLUDEOFFLINE" }) + { + IEnumerable result = DiskFilters.FilterDisks(this.disks, variant, PlatformID.Win32NT); + Assert.AreEqual(4, result.Count(), $"Expected offline disk retained for filter '{variant}'"); + } + } + + [Test] + public void DiskFiltersWithoutIncludeOfflineDoesNotRetainOfflineDisksOnWindows() + { + this.disks = this.mockFixture.CreateDisks(PlatformID.Win32NT, true); + this.disks.ElementAt(2).Properties["Status"] = "Offline (Policy)"; + + // Default behaviour: the offline disk is excluded. + IEnumerable result = DiskFilters.FilterDisks(this.disks, "none", PlatformID.Win32NT); + + Assert.AreEqual(3, result.Count()); + Assert.IsFalse(result.Any(d => d.Properties.ContainsKey("Status") && + d.Properties["Status"].ToString().Contains("Offline"))); + } + + [Test] + public void DiskFiltersIncludeOfflineCanBeCombinedWithBiggestSizeFilter() + { + this.disks = this.mockFixture.CreateDisks(PlatformID.Win32NT, true); + // Make the offline disk the biggest. + this.disks.ElementAt(0).Properties["Size"] = "100 GB"; + this.disks.ElementAt(1).Properties["Size"] = "100 GB"; + this.disks.ElementAt(2).Properties["Size"] = "2000 GB"; // offline + biggest + this.disks.ElementAt(2).Properties["Status"] = "Offline"; + this.disks.ElementAt(3).Properties["Size"] = "100 GB"; + + string filterString = "BiggestSize&IncludeOffline"; + IEnumerable result = DiskFilters.FilterDisks(this.disks, filterString, PlatformID.Win32NT); + + Assert.AreEqual(1, result.Count()); + Assert.IsTrue(object.ReferenceEquals(this.disks.ElementAt(2), result.First())); + } + + // ----------------------------------------------------------------------- + // TryGetDiskIndexes tests + // ----------------------------------------------------------------------- + + [Test] + public void DiskFiltersTryGetDiskIndexes_ReturnsFalseForNonDiskIndexFilter() + { + Assert.IsFalse(DiskFilters.TryGetDiskIndexes("BiggestSize", out _)); + Assert.IsFalse(DiskFilters.TryGetDiskIndexes("none", out _)); + Assert.IsFalse(DiskFilters.TryGetDiskIndexes("osdisk:false", out _)); + Assert.IsFalse(DiskFilters.TryGetDiskIndexes(null, out _)); + Assert.IsFalse(DiskFilters.TryGetDiskIndexes(string.Empty, out _)); + Assert.IsFalse(DiskFilters.TryGetDiskIndexes(" ", out _)); + } + + [Test] + public void DiskFiltersTryGetDiskIndexes_ParsesDashRange_CorrectCount() + { + bool result = DiskFilters.TryGetDiskIndexes("DiskIndex:6-10", out IEnumerable indexes); + + Assert.IsTrue(result); + Assert.IsNotNull(indexes); + // Indices 6, 7, 8, 9, 10 → 5 entries + Assert.AreEqual(5, indexes.Count()); + } + + [Test] + public void DiskFiltersTryGetDiskIndexes_ParsesDashRange_IndicesAreCorrect() + { + DiskFilters.TryGetDiskIndexes("DiskIndex:6-10", out IEnumerable indexes); + + CollectionAssert.AreEqual(new[] { 6, 7, 8, 9, 10 }, indexes.ToList()); + } + + [Test] + public void DiskFiltersTryGetDiskIndexes_ParsesDashRange_SingleDisk() + { + DiskFilters.TryGetDiskIndexes("DiskIndex:42-42", out IEnumerable indexes); + + Assert.AreEqual(1, indexes.Count()); + Assert.AreEqual(42, indexes.First()); + } + + [Test] + public void DiskFiltersTryGetDiskIndexes_ParsesDashRange_IgnoresWhitespace() + { + DiskFilters.TryGetDiskIndexes("DiskIndex: 6 - 8 ", out IEnumerable indexes); + + Assert.AreEqual(3, indexes.Count()); + CollectionAssert.AreEqual(new[] { 6, 7, 8 }, indexes.ToList()); + } + + [Test] + public void DiskFiltersTryGetDiskIndexes_ParsesDashRange_LargeJBODRange() + { + bool result = DiskFilters.TryGetDiskIndexes("DiskIndex:6-180", out IEnumerable indexes); + + Assert.IsTrue(result); + Assert.AreEqual(175, indexes.Count()); + Assert.AreEqual(6, indexes.First()); + Assert.AreEqual(180, indexes.Last()); + } + + [Test] + public void DiskFiltersTryGetDiskIndexes_ParsesCommaSeparatedList_CorrectCount() + { + DiskFilters.TryGetDiskIndexes("DiskIndex:6,10,15", out IEnumerable indexes); + + Assert.AreEqual(3, indexes.Count()); + } + + [Test] + public void DiskFiltersTryGetDiskIndexes_ParsesCommaSeparatedList_IndicesAreCorrect() + { + DiskFilters.TryGetDiskIndexes("DiskIndex:6,10,15", out IEnumerable indexes); + + CollectionAssert.AreEqual(new[] { 6, 10, 15 }, indexes.ToList()); + } + + [Test] + public void DiskFiltersTryGetDiskIndexes_ParsesCommaSeparatedList_IgnoresWhitespace() + { + DiskFilters.TryGetDiskIndexes("DiskIndex: 6 , 7 , 8 ", out IEnumerable indexes); + + CollectionAssert.AreEqual(new[] { 6, 7, 8 }, indexes.ToList()); + } + + [Test] + public void DiskFiltersTryGetDiskIndexes_IsCaseInsensitive() + { + Assert.IsTrue(DiskFilters.TryGetDiskIndexes("diskindex:6-8", out _)); + Assert.IsTrue(DiskFilters.TryGetDiskIndexes("DISKINDEX:6-8", out _)); + Assert.IsTrue(DiskFilters.TryGetDiskIndexes("DiskIndex:6-8", out _)); + } + + [Test] + public void DiskFiltersTryGetDiskIndexes_HddSentinel_ReturnsTrueWithNullIndexes() + { + bool result = DiskFilters.TryGetDiskIndexes("DiskIndex:hdd", out IEnumerable indexes); + + Assert.IsTrue(result, "Expected true for the 'hdd' auto-discover sentinel."); + Assert.IsNull(indexes, "Expected null indexes for the 'hdd' sentinel — caller should use OS discovery."); + } + + [Test] + public void DiskFiltersTryGetDiskIndexes_HddSentinel_IsCaseInsensitive() + { + Assert.IsTrue(DiskFilters.TryGetDiskIndexes("DiskIndex:HDD", out IEnumerable i1)); + Assert.IsNull(i1); + Assert.IsTrue(DiskFilters.TryGetDiskIndexes("DiskIndex:hdd", out IEnumerable i2)); + Assert.IsNull(i2); + } } } diff --git a/src/VirtualClient/VirtualClient.Contracts/DiskFilters.cs b/src/VirtualClient/VirtualClient.Contracts/DiskFilters.cs index 9fa9682e54..a86ff663bf 100644 --- a/src/VirtualClient/VirtualClient.Contracts/DiskFilters.cs +++ b/src/VirtualClient/VirtualClient.Contracts/DiskFilters.cs @@ -18,6 +18,82 @@ public static class DiskFilters /// public const string DefaultDiskFilter = "BiggestSize"; + /// + /// Attempts to parse a "DiskIndex:" filter string and extract explicit physical disk indexes. + /// + /// + /// Supported forms: + /// + /// DiskIndex:6-180 — inclusive range, populates with 6..180. + /// DiskIndex:6,10,15 — comma-separated list, populates with {6, 10, 15}. + /// + /// DiskIndex:hdd — auto-discover sentinel; returns true with set to + /// null, signalling the caller to perform OS-based HDD discovery (e.g. Get-PhysicalDisk on Windows). + /// + /// + /// + /// The disk filter string (e.g. the value of the DiskFilter parameter). + /// + /// The parsed disk indexes when an explicit range or list is provided; null when the value is "hdd" + /// (indicating OS-based auto-discovery should be used instead). + /// + /// + /// true if begins with "DiskIndex:" (whether explicit or the "hdd" sentinel); + /// false if the filter is not a DiskIndex filter at all. + /// + public static bool TryGetDiskIndexes(string diskFilter, out IEnumerable indexes) + { + indexes = null; + + if (string.IsNullOrWhiteSpace(diskFilter)) + { + return false; + } + + const string prefix = "DiskIndex:"; + + if (!diskFilter.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + string value = diskFilter.Substring(prefix.Length).Trim(); + + // "DiskIndex:hdd" is a sentinel meaning "discover HDD disks at runtime via Get-PhysicalDisk". + if (value.Equals("hdd", StringComparison.OrdinalIgnoreCase)) + { + // indexes remains null — caller should perform OS-based auto-discovery. + return true; + } + + List parsed = new List(); + + if (value.Contains('-')) + { + string[] parts = value.Split('-', 2); + if (int.TryParse(parts[0].Trim(), out int start) && int.TryParse(parts[1].Trim(), out int end)) + { + for (int i = start; i <= end; i++) + { + parsed.Add(i); + } + } + } + else + { + foreach (string token in value.Split(',', StringSplitOptions.RemoveEmptyEntries)) + { + if (int.TryParse(token.Trim(), out int idx)) + { + parsed.Add(idx); + } + } + } + + indexes = parsed; + return true; + } + /// /// Filters the disks based on filter strings /// @@ -31,8 +107,15 @@ public static IEnumerable FilterDisks(IEnumerable disks, string filt // filterName1:value1&filterName2:value2&filterDoesNotRequireValue&filter4:value4 List filters = filterString.Split("&", StringSplitOptions.RemoveEmptyEntries).ToList(); + // Allow callers to opt into keeping offline disks (e.g. bare/unformatted disks for raw disk I/O). + bool includeOffline = filters.Any(f => f.Trim().Equals(Filters.IncludeOffline, StringComparison.OrdinalIgnoreCase)); + disks = DiskFilters.FilterStoragePathByPrefix(disks, platform); - disks = DiskFilters.FilterOfflineDisksOnWindows(disks, platform); + if (!includeOffline) + { + disks = DiskFilters.FilterOfflineDisksOnWindows(disks, platform); + } + disks = DiskFilters.FilterReadOnlyDisksOnWindows(disks, platform); foreach (string filter in filters) @@ -86,6 +169,10 @@ public static IEnumerable FilterDisks(IEnumerable disks, string filt disks = DiskFilters.DiskPathFilter(disks, filterValue); break; + case Filters.IncludeOffline: + // Already handled before the filter loop; treated as a no-op here. + break; + default: throw new EnvironmentSetupException($"Disk filter '{filter}' is not supported.", ErrorReason.DiskInformationNotAvailable); } @@ -248,6 +335,12 @@ private static class Filters /// Disk path filter. /// public const string DiskPath = "diskpath"; + + /// + /// Include offline disks filter. By default offline disks are excluded on Windows. + /// Use this filter to include them (e.g. bare/unformatted disks for raw disk I/O). + /// + public const string IncludeOffline = "includeoffline"; } } }