Added SetDiskSanPolicy dependency to enable writable JBOD disks on Windows (#679)

Co-authored-by: Ankit Sharma <ankitshar@microsoft.com>
Co-authored-by: Bryan DeYoung <35380894+brdeyo@users.noreply.github.com>

Author: 3 people

src/VirtualClient/VirtualClient.Core.UnitTests/WindowsDiskManagerTests.cs

@@ -1670,6 +1670,106 @@ public async Task WindowsDiskManagerGetsTheExpectedDisks_Scenario1()
             actualDisks.ToList().ForEach(disk => Assert.IsTrue(disk.Volumes.Count() == 2));
         }
 
+        [Test]
+        public async Task WindowsDiskManagerCallsTheExpectedDiskPartCommandsToSetSanPolicy()
+        {
+            this.testProcess.OnHasExited = () => true;
+            this.testProcess.OnStart = () => true;
+
+            List<string> expectedCommands = new List<string>
+            {
+                "san",
+                "san policy=onlineall",
+                "exit"
+            };
+
+            List<string> actualCommands = new List<string>();
+
+            this.standardInput.BytesWritten += (sender, data) =>
+            {
+                string input = data.ToString().Trim();
+                actualCommands.Add(input);
+
+                if (input == "san")
+                {
+                    // Simulate a policy that is NOT already OnlineAll.
+                    this.testProcess.StandardOutput.Append("SAN Policy  : Offline Shared");
+                }
+                else if (input.Contains("san policy=onlineall"))
+                {
+                    this.testProcess.StandardOutput.Append("DiskPart successfully changed the SAN policy for the current operating system.");
+                }
+                else if (input == "exit")
+                {
+                    // Expected
+                }
+                else
+                {
+                    Assert.Fail($"Unexpected command called: {input}");
+                }
+            };
+
+            await this.diskManager.SetSanPolicyAsync(CancellationToken.None).ConfigureAwait(false);
+
+            Assert.IsNotEmpty(actualCommands);
+            Assert.AreEqual(expectedCommands.Count, actualCommands.Count);
+            CollectionAssert.AreEquivalent(expectedCommands, actualCommands);
+        }
+
+        [Test]
+        public async Task WindowsDiskManagerSkipsSettingSanPolicyWhenItIsAlreadyOnlineAll()
+        {
+            this.testProcess.OnHasExited = () => true;
+            this.testProcess.OnStart = () => true;
+
+            // Only "san" and "exit" — no "san policy=onlineall".
+            List<string> expectedCommands = new List<string>
+            {
+                "san",
+                "exit"
+            };
+
+            List<string> actualCommands = new List<string>();
+
+            this.standardInput.BytesWritten += (sender, data) =>
+            {
+                string input = data.ToString().Trim();
+                actualCommands.Add(input);
+
+                if (input == "san")
+                {
+                    // Simulate a policy that is already OnlineAll.
+                    this.testProcess.StandardOutput.Append("SAN Policy  : Online All");
+                }
+                else if (input == "exit")
+                {
+                    // Expected
+                }
+                else
+                {
+                    Assert.Fail($"Unexpected command called: {input}");
+                }
+            };
+
+            await this.diskManager.SetSanPolicyAsync(CancellationToken.None).ConfigureAwait(false);
+
+            Assert.IsNotEmpty(actualCommands);
+            Assert.AreEqual(expectedCommands.Count, actualCommands.Count);
+            CollectionAssert.AreEquivalent(expectedCommands, actualCommands);
+        }
+
+        [Test]
+        public void WindowsDiskManagerThrowsWhenSettingSanPolicyTimesOut()
+        {
+            this.testProcess.OnHasExited = () => true;
+            this.testProcess.OnStart = () => true;
+
+            // Do not write any response to standard output — the WaitForResponseAsync will time out.
+            this.standardInput.BytesWritten += (sender, data) => { };
+
+            Assert.ThrowsAsync<ProcessException>(() => this.diskManager.SetSanPolicyAsync(CancellationToken.None));
+        }
+
         private class TestWindowsDiskManager : WindowsDiskManager
         {
             public TestWindowsDiskManager(ProcessManager processManager)

src/VirtualClient/VirtualClient.Core/DiskManager.cs

@@ -45,5 +45,8 @@ protected DiskManager(ILogger logger = null)
 
         /// <inheritdoc/>
         public abstract Task<IEnumerable<Disk>> GetDisksAsync(CancellationToken cancellationToken);
+
+        /// <inheritdoc/>
+        public abstract Task SetSanPolicyAsync(CancellationToken cancellationToken);
     }
 }

src/VirtualClient/VirtualClient.Core/IDiskManager.cs

@@ -38,5 +38,14 @@ public interface IDiskManager
         /// </summary>
         /// <param name="cancellationToken">A token that can be used to cancel the operation.</param>
         Task<IEnumerable<Disk>> GetDisksAsync(CancellationToken cancellationToken);
+
+        /// <summary>
+        /// Sets the SAN (Storage Area Network) policy so that newly discovered disks are brought
+        /// online and writable rather than left offline or read-only. On Windows this runs the
+        /// DiskPart command <c>san policy=onlineall</c>, which prevents JBOD disks from being
+        /// marked read-only by the OS. This operation is a no-op on Linux.
+        /// </summary>
+        /// <param name="cancellationToken">A token that can be used to cancel the operation.</param>
+        Task SetSanPolicyAsync(CancellationToken cancellationToken);
     }
 }

src/VirtualClient/VirtualClient.Core/UnixDiskManager.cs

@@ -81,6 +81,15 @@ public override Task CreateMountPointAsync(DiskVolume volume, string mountPoint,
             });
         }
 
+        /// <summary>
+        /// SAN policy is a Windows-only concept. This operation is a no-op on Linux/Unix.
+        /// </summary>
+        /// <param name="cancellationToken">A token that can be used to cancel the operation.</param>
+        public override Task SetSanPolicyAsync(CancellationToken cancellationToken)
+        {
+            return Task.CompletedTask;
+        }
+
         /// <summary>
         /// Partitions and formats the disk for file system operations.
         /// </summary>

src/VirtualClient/VirtualClient.Core/WindowsDiskManager.cs

@@ -151,6 +151,90 @@ await process.WriteInput(command)
             });
         }
 
+        /// <summary>
+        /// Sets the SAN policy to <c>onlineall</c> so that newly discovered JBOD disks are brought
+        /// online and writable rather than remaining offline or read-only.
+        /// </summary>
+        /// <param name="cancellationToken">A token that can be used to cancel the operation.</param>
+        public override Task SetSanPolicyAsync(CancellationToken cancellationToken)
+        {
+            EventContext context = EventContext.Persisted();
+
+            return this.Logger.LogMessageAsync($"{nameof(WindowsDiskManager)}.SetSanPolicy", context, async () =>
+            {
+                string command = string.Empty;
+                int retries = -1;
+
+                try
+                {
+                    await this.RetryPolicy.ExecuteAsync(async () =>
+                    {
+                        retries++;
+                        if (!cancellationToken.IsCancellationRequested)
+                        {
+                            using (IProcessProxy process = this.ProcessManager.CreateProcess("DiskPart", string.Empty))
+                            {
+                                try
+                                {
+                                    process.Interactive();
+                                    if (!process.Start())
+                                    {
+                                        throw new ProcessException("Failed to enter DiskPart session.", ErrorReason.DiskFormatFailed);
+                                    }
+
+                                    // Query the current SAN policy first.
+                                    command = "san";
+                                    await process.WriteInput(command)
+                                        .WaitForResponseAsync(@"SAN Policy\s*:", cancellationToken, timeout: TimeSpan.FromSeconds(30))
+                                        .ConfigureAwait(false);
+
+                                    string sanOutput = process.StandardOutput.ToString();
+                                    this.Logger.LogTraceMessage($"Current SAN policy output: {sanOutput}", context);
+
+                                    // Only set the policy if it is not already OnlineAll.
+                                    // DiskPart reports the policy in the format: "SAN Policy  : Online All"
+                                    if (Regex.IsMatch(sanOutput, @"SAN Policy\s*:\s*Online All", RegexOptions.IgnoreCase))
+                                    {
+                                        this.Logger.LogTraceMessage("SAN policy is already set to OnlineAll. No change required.", context);
+                                    }
+                                    else
+                                    {
+                                        // Set SAN policy to OnlineAll so that newly discovered disks are
+                                        // brought online and writable instead of remaining offline/read-only.
+                                        command = "san policy=onlineall";
+                                        await process.WriteInput(command)
+                                            .WaitForResponseAsync(@"DiskPart successfully changed the SAN policy for the current operating system\.", cancellationToken, timeout: TimeSpan.FromSeconds(30))
+                                            .ConfigureAwait(false);
+
+                                        this.Logger.LogTraceMessage("SAN policy set to OnlineAll.", context);
+                                    }
+                                }
+                                catch (TimeoutException exc)
+                                {
+                                    throw new ProcessException(
+                                        $"Failed to set SAN policy. DiskPart command timed out (command={command}, retries={retries}). {Environment.NewLine}{process.StandardOutput}",
+                                        exc,
+                                        ErrorReason.DiskFormatFailed);
+                                }
+                                finally
+                                {
+                                    process.WriteInput("exit");
+                                    await Task.Delay(this.WaitTime).ConfigureAwait(false);
+                                    context.AddProcessDetails(process.ToProcessDetails("diskpart"), "diskpartProcess");
+                                }
+                            }
+                        }
+                    }).ConfigureAwait(false);
+                }
+                catch (Win32Exception exc) when (exc.Message.Contains("requires elevation"))
+                {
+                    throw new ProcessException(
+                        $"Requires elevated permissions. The current operation set requires the application to be run with administrator privileges.",
+                        ErrorReason.Unauthorized);
+                }
+            });
+        }
+
         /// <summary>
         /// Partitions and formats the disk for file system operations.
         /// </summary>

src/VirtualClient/VirtualClient.Dependencies.UnitTests/SetDiskSanPolicyTests.cs

@@ -0,0 +1,71 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+namespace VirtualClient.Dependencies
+{
+    using System;
+    using System.Collections.Generic;
+    using System.Threading;
+    using System.Threading.Tasks;
+    using Moq;
+    using NUnit.Framework;
+    using VirtualClient.Contracts;
+
+    [TestFixture]
+    [Category("Unit")]
+    public class SetDiskSanPolicyTests
+    {
+        private MockFixture mockFixture;
+
+        [Test]
+        public async Task SetDiskSanPolicyCallsDiskManagerSetSanPolicyOnWindows()
+        {
+            this.mockFixture = new MockFixture();
+            this.mockFixture.Setup(PlatformID.Win32NT);
+
+            using (SetDiskSanPolicy component = new SetDiskSanPolicy(this.mockFixture.Dependencies, this.mockFixture.Parameters))
+            {
+                await component.ExecuteAsync(CancellationToken.None);
+
+                this.mockFixture.DiskManager.Verify(
+                    mgr => mgr.SetSanPolicyAsync(It.IsAny<CancellationToken>()),
+                    Times.Once);
+            }
+        }
+
+        [Test]
+        public async Task SetDiskSanPolicyDoesNotCallDiskManagerSetSanPolicyOnLinux()
+        {
+            this.mockFixture = new MockFixture();
+            this.mockFixture.Setup(PlatformID.Unix);
+
+            using (SetDiskSanPolicy component = new SetDiskSanPolicy(this.mockFixture.Dependencies, this.mockFixture.Parameters))
+            {
+                await component.ExecuteAsync(CancellationToken.None);
+
+                this.mockFixture.DiskManager.Verify(
+                    mgr => mgr.SetSanPolicyAsync(It.IsAny<CancellationToken>()),
+                    Times.Never);
+            }
+        }
+
+        [Test]
+        public void SetDiskSanPolicyPropagatesExceptionsThrownByDiskManagerOnWindows()
+        {
+            this.mockFixture = new MockFixture();
+            this.mockFixture.Setup(PlatformID.Win32NT);
+
+            this.mockFixture.DiskManager
+                .Setup(mgr => mgr.SetSanPolicyAsync(It.IsAny<CancellationToken>()))
+                .ThrowsAsync(new ProcessException("DiskPart SAN policy command failed.", ErrorReason.DiskFormatFailed));
+
+            using (SetDiskSanPolicy component = new SetDiskSanPolicy(this.mockFixture.Dependencies, this.mockFixture.Parameters))
+            {
+                ProcessException exc = Assert.ThrowsAsync<ProcessException>(
+                    () => component.ExecuteAsync(CancellationToken.None));
+
+                Assert.AreEqual(ErrorReason.DiskFormatFailed, exc.Reason);
+            }
+        }
+    }
+}

src/VirtualClient/VirtualClient.Dependencies/SetDiskSanPolicy.cs

@@ -0,0 +1,58 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+namespace VirtualClient.Dependencies
+{
+    using System;
+    using System.Collections.Generic;
+    using System.Threading;
+    using System.Threading.Tasks;
+    using Microsoft.Extensions.DependencyInjection;
+    using VirtualClient.Common.Extensions;
+    using VirtualClient.Common.Telemetry;
+    using VirtualClient.Contracts;
+
+    /// <summary>
+    /// A dependency that sets the Windows SAN (Storage Area Network) policy to <c>OnlineAll</c>
+    /// so that newly discovered JBOD disks are brought online and writable rather than being
+    /// left offline or marked as read-only by the operating system.
+    /// </summary>
+    /// <remarks>
+    /// On Windows, disks discovered through SAN controllers (including JBOD configurations)
+    /// are sometimes left offline or marked read-only depending on the SAN policy in effect.
+    /// Running the DiskPart commands <c>san</c> followed by <c>san policy=onlineall</c> configures
+    /// Windows to automatically bring all newly discovered disks online and writable.
+    /// This dependency is a no-op on Linux.
+    /// </remarks>
+    public class SetDiskSanPolicy : VirtualClientComponent
+    {
+        private ISystemManagement systemManagement;
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="SetDiskSanPolicy"/> class.
+        /// </summary>
+        public SetDiskSanPolicy(IServiceCollection dependencies, IDictionary<string, IConvertible> parameters)
+            : base(dependencies, parameters)
+        {
+            this.systemManagement = this.Dependencies.GetService<ISystemManagement>();
+        }
+
+        /// <summary>
+        /// Executes the DiskPart SAN policy change. Only runs on Windows; skipped on Linux.
+        /// </summary>
+        protected override async Task ExecuteAsync(EventContext telemetryContext, CancellationToken cancellationToken)
+        {
+            if (this.Platform == PlatformID.Win32NT)
+            {
+                await this.systemManagement.DiskManager.SetSanPolicyAsync(cancellationToken)
+                    .ConfigureAwait(false);
+            }
+            else
+            {
+                this.Logger.LogTraceMessage(
+                    $"{nameof(SetDiskSanPolicy)}: SAN policy is a Windows-only concept. Skipping on platform '{this.Platform}'.",
+                    telemetryContext);
+            }
+        }
+    }
+}

src/VirtualClient/VirtualClient.TestFramework/InMemoryDiskManager.cs

@@ -129,6 +129,15 @@ public Task<IEnumerable<Disk>> GetDisksAsync(CancellationToken cancellationToken
             return Task.FromResult((IEnumerable<Disk>)this);
         }
 
+        /// <summary>
+        /// No-op in the test/in-memory disk manager. SAN policy changes are Windows-only.
+        /// </summary>
+        /// <param name="cancellationToken">A token that can be used to cancel the operation.</param>
+        public Task SetSanPolicyAsync(CancellationToken cancellationToken)
+        {
+            return Task.CompletedTask;
+        }
+
         private void AddVolumeToDisk(Disk disk, FileSystemType fileSystemType)
         {
             DiskVolume newVolume = null;