Repository: ansible/ansible
Pull request: win_format - Add allocation_unit_size
##### SUMMARY
Fixes #56961 

##### ISSUE TYPE
- Bugfix Pull Request

##### COMPONENT NAME
win_format

##### ADDITIONAL INFORMATION
Added `allocation_unit_size` support for `win_format`. Was added initially then removed after the re-write. Putting it back in with this.

Added a note in the docs to specify usage of physical sector size as well.

Problem statement:
win_format: allocation_unit_size has no effect
<!--- Verify first that your issue is not already reported on GitHub -->
<!--- Also test if the latest release and devel branch are affected too -->
<!--- Complete *all* sections as described, this form is processed automatically -->

##### SUMMARY
<!--- Explain the problem briefly below -->
When allocation_unit_size  parameter is set, the partition is formatted with the default one instead

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below, use your best guess if unsure -->
win_format

##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes -->
```paste below
ansible 2.8.0
```

##### OS / ENVIRONMENT
<!--- Provide all relevant information below, e.g. target OS versions, network device firmware, etc. -->
Windows Server 2019

##### STEPS TO REPRODUCE
<!--- Describe exactly how to reproduce the problem, using a minimal test-case -->
Run playbook with win_format module, where custom allocation unit size is set:

<!--- Paste example playbooks or commands between quotes below -->
```yaml
- name: Format partition
  win_format:
    drive_letter: D
    file_system: NTFS
    allocation_unit_size: 512
    full: False
```

<!--- HINT: You can paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- Describe what you expected to happen when running the steps above -->
Partition allocation size unit should be set to 512

##### ACTUAL RESULTS
<!--- Describe what actually happened. If possible run with extra verbosity (-vvvv) -->
Partition was formatted with default (4096) allocation size unit
<!--- Paste verbatim command output between quotes -->
```paste below

TASK [Format partition] ***********************************************************************************************************
changed: [win2019] => {
    "changed": true, 
    "invocation": {
        "module_args": {
            "allocation_unit_size": 512, 
            "compress": null, 
            "drive_letter": "D", 
            "file_system": "NTFS", 
            "force": false, 
            "full": false, 
            "integrity_streams": null, 
            "label": null, 
            "large_frs": null, 
            "new_label": null, 
            "path": null
        }
    }
}

TASK [debug] ***********************************************************************************************************
ok: [win2019] => {
    "ansible_facts.disks[1].partitions[0].volumes[0]": {
        "allocation_unit_size": 4096, 
        "drive_type": "Fixed", 
        "health_status": "Healthy", 
        "label": "", 
        "object_id": "{1}\\\\win2019\\root/Microsoft/Windows/Storage/Providers_v2\\WSP_Volume.ObjectId=\"{c2b6d3ca-7f00-11e9-b427-806e6f6e6963}:VO:\\\\?\\Volume{509328f7-0000-0000-0000-100000000000}\\\"", 
        "path": "\\\\?\\Volume{509328f7-0000-0000-0000-100000000000}\\", 
        "size": 85897244672, 
        "size_remaining": 85794185216, 
        "type": "NTFS"
    }
}

```

Hints from issue discussion:
Files identified in the description:
* [lib/ansible/modules/windows/win_format.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/windows/win_format.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @chopraaa @dagwieers @if-meaton @jborean93 @jhawkesworth @nitzmahone
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
This is just sad on my part. Fix for this soon.

Please test https://github.com/ansible/ansible/pull/56966 if possible and let me know if the issue persists.
With changes from #56966 it works as expected. 
cc @ShachafGoldstein
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->

Commit message: Update tests for 2012/R2

Patch to review:
diff --git a/changelogs/fragments/56966-win_format-allocation-unit-size.yml b/changelogs/fragments/56966-win_format-allocation-unit-size.yml
new file mode 100644
index 00000000000000..5ad05610e7eb29
--- /dev/null
+++ b/changelogs/fragments/56966-win_format-allocation-unit-size.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - win_format - fixed issue where module would not change allocation unit size (https://github.com/ansible/ansible/issues/56961)
diff --git a/lib/ansible/modules/windows/win_format.ps1 b/lib/ansible/modules/windows/win_format.ps1
index 6302ad8754cc8c..468dd780391250 100644
--- a/lib/ansible/modules/windows/win_format.ps1
+++ b/lib/ansible/modules/windows/win_format.ps1
@@ -108,7 +108,8 @@ function Format-AnsibleVolume {
         $Full,
         $UseLargeFRS,
         $Compress,
-        $SetIntegrityStreams
+        $SetIntegrityStreams,
+        $AllocationUnitSize
     )
     $parameters = @{
         Path = $Path
@@ -129,6 +130,9 @@ function Format-AnsibleVolume {
     if ($null -ne $FileSystem) {
         $parameters.Add("FileSystem", $FileSystem)
     }
+    if ($null -ne $AllocationUnitSize) {
+        $parameters.Add("AllocationUnitSize", $AllocationUnitSize)
+    }
 
     Format-Volume @parameters -Confirm:$false | Out-Null
 
@@ -137,9 +141,14 @@ function Format-AnsibleVolume {
 $ansible_volume = Get-AnsibleVolume -DriveLetter $drive_letter -Path $path -Label $label
 $ansible_file_system = $ansible_volume.FileSystem
 $ansible_volume_size = $ansible_volume.Size
+$ansible_volume_alu = (Get-CimInstance -ClassName Win32_Volume | Where-Object { $_.DeviceId -eq $ansible_volume.Path } | Select-Object BlockSize).BlockSize
 
 $ansible_partition = Get-Partition -Volume $ansible_volume
 
+if (-not $force_format -and $null -ne $allocation_unit_size -and $ansible_volume_alu -ne 0 -and $null -ne $ansible_volume_alu -and $allocation_unit_size -ne $ansible_volume_alu) {
+        $module.FailJson("Force format must be specified since target allocation unit size: $($allocation_unit_size) is different from the current allocation unit size of the volume: $($ansible_volume_alu)")
+}
+
 foreach ($access_path in $ansible_partition.AccessPaths) {
     if ($access_path -ne $Path) {
         $files_in_volume = (Get-ChildItem -LiteralPath $access_path -ErrorAction SilentlyContinue | Measure-Object).Count
@@ -161,7 +170,7 @@ foreach ($access_path in $ansible_partition.AccessPaths) {
 
 if ($force_format) {
     if (-not $module.CheckMode) {
-        Format-AnsibleVolume -Path $ansible_volume.Path -Full $full_format -Label $new_label -FileSystem $file_system -SetIntegrityStreams $integrity_streams -UseLargeFRS $large_frs -Compress $compress_volume
+        Format-AnsibleVolume -Path $ansible_volume.Path -Full $full_format -Label $new_label -FileSystem $file_system -SetIntegrityStreams $integrity_streams -UseLargeFRS $large_frs -Compress $compress_volume -AllocationUnitSize $allocation_unit_size
     }
     $module.Result.changed = $true
 }
@@ -174,7 +183,7 @@ else {
         if ($ansible_volume_size -eq 0 -or
             $ansible_volume.FileSystemLabel -ne $new_label) {
             if (-not $module.CheckMode) {
-                Format-AnsibleVolume -Path $ansible_volume.Path -Full $full_format -Label $new_label -FileSystem $file_system -SetIntegrityStreams $integrity_streams -UseLargeFRS $large_frs -Compress $compress_volume
+                Format-AnsibleVolume -Path $ansible_volume.Path -Full $full_format -Label $new_label -FileSystem $file_system -SetIntegrityStreams $integrity_streams -UseLargeFRS $large_frs -Compress $compress_volume -AllocationUnitSize $allocation_unit_size
             }
             $module.Result.changed = $true
         }
diff --git a/lib/ansible/modules/windows/win_format.py b/lib/ansible/modules/windows/win_format.py
index 8b85cce81318b4..b1bd033937651d 100644
--- a/lib/ansible/modules/windows/win_format.py
+++ b/lib/ansible/modules/windows/win_format.py
@@ -43,6 +43,7 @@
       - Specifies the cluster size to use when formatting the volume.
       - If no cluster size is specified when you format a partition, defaults are selected based on
         the size of the partition.
+      - This value must be a multiple of the physical sector size of the disk.
     type: int
   large_frs:
     description:
diff --git a/test/integration/targets/win_format/tasks/tests.yml b/test/integration/targets/win_format/tasks/tests.yml
index 1db4576cfef880..fb67479d251ace 100644
--- a/test/integration/targets/win_format/tasks/tests.yml
+++ b/test/integration/targets/win_format/tasks/tests.yml
@@ -28,10 +28,12 @@
     drive_letter: T
     new_label: Formatted
     full: True
+    allocation_unit_size: 8192
   register: format_result_check
   check_mode: True
+  ignore_errors: True
 
-- win_shell: $AnsiPart = Get-Partition -DriveLetter T; $AnsiVol = Get-Volume -DriveLetter T; "$($AnsiPart.Size),$($AnsiVol.Size),$($AnsiVol.FileSystemLabel)"
+- win_shell: $AnsiPart = Get-Partition -DriveLetter T; $AnsiVol = Get-Volume -DriveLetter T; "$($AnsiPart.Size),$($AnsiVol.Size),$($AnsiVol.FileSystemLabel),$((Get-CimInstance -ClassName Win32_Volume | Where-Object { $_.DriveLetter -eq 'T:' } | Select-Object BlockSize).BlockSize)"
   register: formatted_value_result_check
 
 - name: Fully format volume and assign label
@@ -39,17 +41,18 @@
     drive_letter: T
     new_label: Formatted
     full: True
+    allocation_unit_size: 8192
   register: format_result
 
-- win_shell: $AnsiPart = Get-Partition -DriveLetter T; $AnsiVol = Get-Volume -DriveLetter T; "$($AnsiPart.Size),$($AnsiVol.Size),$($AnsiVol.FileSystemLabel)"
+- win_shell: $AnsiPart = Get-Partition -DriveLetter T; $AnsiVol = Get-Volume -DriveLetter T; "$($AnsiPart.Size),$($AnsiVol.Size),$($AnsiVol.FileSystemLabel),$((Get-CimInstance -ClassName Win32_Volume | Where-Object { $_.DriveLetter -eq 'T:' } | Select-Object BlockSize).BlockSize)"
   register: formatted_value_result
 
 - assert:
     that:
       - format_result_check is changed
       - format_result is changed
-      - formatted_value_result_check.stdout | trim == "2096037888,0,"
-      - formatted_value_result.stdout | trim == "2096037888,2096033792,Formatted"
+      - formatted_value_result_check.stdout | trim == "2096037888,0,,"
+      - formatted_value_result.stdout | trim == "2096037888,2096029696,Formatted,8192"
 
 - name: Format NTFS volume with integrity streams enabled
   win_format:
@@ -136,3 +139,24 @@
       - format_volume_without_force is failed
       - 'format_volume_without_force.msg == "Force format must be specified to format non-pristine volumes"'
       - format_volume_with_force is changed
+
+- name: Reformat using different alu without force format
+  win_format:
+    path: "{{ shell_partition_result.stdout | trim }}"
+    allocation_unit_size: 8192
+    file_system: ntfs
+  register: reformat_using_alu_without_force
+  ignore_errors: True
+
+- name: Reformat using different alu using force format
+  win_format:
+    path: "{{ shell_partition_result.stdout | trim }}"
+    allocation_unit_size: 8192
+    file_system: ntfs
+    force: True
+  register: reformat_using_alu_with_force
+
+- assert:
+    that:
+      - reformat_using_alu_without_force is failed
+      - reformat_using_alu_with_force is changed


Code review comments:
diff_hunk: @@ -28,28 +28,31 @@
     drive_letter: T
     new_label: Formatted
     full: True
+    allocation_unit_size: 8192
   register: format_result_check
   check_mode: True
+  ignore_errors: True
path: test/integration/targets/win_format/tasks/tests.yml
text: @user1:
Why needed?

@author:
Must've slipped in while testing. 😄

diff_hunk: @@ -28,28 +28,31 @@
     drive_letter: T
     new_label: Formatted
     full: True
+    allocation_unit_size: 8192
   register: format_result_check
   check_mode: True
+  ignore_errors: True
 
-- win_shell: $AnsiPart = Get-Partition -DriveLetter T; $AnsiVol = Get-Volume -DriveLetter T; "$($AnsiPart.Size),$($AnsiVol.Size),$($AnsiVol.FileSystemLabel)"
+- win_shell: $AnsiPart = Get-Partition -DriveLetter T; $AnsiVol = Get-Volume -DriveLetter T; "$($AnsiPart.Size),$($AnsiVol.Size),$($AnsiVol.FileSystemLabel),$((Get-CimInstance -ClassName Win32_Volume | Where-Object { $_.DriveLetter -eq 'T:' } | Select-Object BlockSize).BlockSize)"
   register: formatted_value_result_check
 
 - name: Fully format volume and assign label
   win_format:
     drive_letter: T
     new_label: Formatted
     full: True
+    allocation_unit_size: 8192
   register: format_result
 
-- win_shell: $AnsiPart = Get-Partition -DriveLetter T; $AnsiVol = Get-Volume -DriveLetter T; "$($AnsiPart.Size),$($AnsiVol.Size),$($AnsiVol.FileSystemLabel)"
+- win_shell: $AnsiPart = Get-Partition -DriveLetter T; $AnsiVol = Get-Volume -DriveLetter T; "$($AnsiPart.Size),$($AnsiVol.Size),$($AnsiVol.FileSystemLabel),$((Get-CimInstance -ClassName Win32_Volume | Where-Object { $_.DriveLetter -eq 'T:' } | Select-Object BlockSize).BlockSize)"
path: test/integration/targets/win_format/tasks/tests.yml
text: @user1:
`(Get-CimInstance -ClassName Win32_Volume -Filter "DriveLetter = 'T:'" -Property BlockSize).BlockSize`

diff_hunk: @@ -28,28 +28,31 @@
     drive_letter: T
     new_label: Formatted
     full: True
+    allocation_unit_size: 8192
   register: format_result_check
   check_mode: True
+  ignore_errors: True
 
-- win_shell: $AnsiPart = Get-Partition -DriveLetter T; $AnsiVol = Get-Volume -DriveLetter T; "$($AnsiPart.Size),$($AnsiVol.Size),$($AnsiVol.FileSystemLabel)"
+- win_shell: $AnsiPart = Get-Partition -DriveLetter T; $AnsiVol = Get-Volume -DriveLetter T; "$($AnsiPart.Size),$($AnsiVol.Size),$($AnsiVol.FileSystemLabel),$((Get-CimInstance -ClassName Win32_Volume | Where-Object { $_.DriveLetter -eq 'T:' } | Select-Object BlockSize).BlockSize)"
path: test/integration/targets/win_format/tasks/tests.yml
text: @user1:
`(Get-CimInstance -ClassName Win32_Volume -Filter "DriveLetter = 'T:'" -Property BlockSize).BlockSize`

diff_hunk: @@ -137,9 +141,14 @@ function Format-AnsibleVolume {
 $ansible_volume = Get-AnsibleVolume -DriveLetter $drive_letter -Path $path -Label $label
 $ansible_file_system = $ansible_volume.FileSystem
 $ansible_volume_size = $ansible_volume.Size
+$ansible_volume_alu = (Get-CimInstance -ClassName Win32_Volume | Where-Object { $_.DeviceId -eq $ansible_volume.Path } | Select-Object BlockSize).BlockSize
path: lib/ansible/modules/windows/win_format.ps1
text: @user1:
More efficient like this `(Get-CimInstance -ClassName Win32_Volume -Filter "DeviceId = '$($ansible_volume.path.replace('\','\\'))'" -Property BlockSize).BlockSize`