Repository: ansible/ansible
Pull request: win_disk_facts - Set output array order to be by disk number property
##### SUMMARY
Not really a bug but not a feature either.

Fixes #63998

##### ISSUE TYPE
- Bugfix Pull Request

##### COMPONENT NAME
win_disk_facts.ps1

##### ADDITIONAL INFORMATION


Problem statement:
Order of elements in ansible_facts['disks'] array does not match actual disk order
##### SUMMARY
On a system with multiple disks, ansible_facts['disks'][0] is not guaranteed to be the first disk.

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
win_disk_facts

##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes -->
```paste below
Ansible 2.8.5 on Tower 3.5.2
```

##### CONFIGURATION
<!--- Paste verbatim output from "ansible-config dump --only-changed" between quotes -->
```paste below

```

##### OS / ENVIRONMENT
Windows Server 2019 Datacenter running in AWS 

##### STEPS TO REPRODUCE
Use the following playbook to illustrate the issue

<!--- Paste example playbooks or commands between quotes below -->
```yaml
---
- name: Configure local storage
  hosts: win1.example.com

  tasks:
    - name: Disk facts are populated
      win_disk_facts:

    - name: Print second disk size
      debug:
        var: ansible_facts['disks'][1]['size']

    - name: Extract second disk as standalone fact
      set_fact:
        second_disk: "{{ ansible_facts['disks'] | json_query('[?number==`1`]') }}"

    - name: Print actual second disk size
      debug:
        var: second_disk[0]['size']

```

<!--- HINT: You can paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
ansible_facts['disks'][0] should contain information for the first disk (number: 0)
ansible_facts['disks'][1] should contain information for the second disk (number: 1)


##### ACTUAL RESULTS
<!--- Describe what actually happened. If possible run with extra verbosity (-vvvv) -->

The first element in the ansible_facts['disks'] array is the second disk instead of the first.

<!--- Paste verbatim command output between quotes -->
```paste below
SSH password: 
PLAY [Configure local storage] *************************************************

TASK [Gathering Facts] *********************************************************
ok: [win1.example.com]

TASK [Disk facts are populated] ************************************************
ok: [win1.example.com]

TASK [Print second disk size] **************************************************
ok: [win1.example.com] => {
    "ansible_facts['disks'][1]['size']": "32212254720"
}

TASK [Extract second disk as standalone fact] **********************************
ok: [win1.example.com]

TASK [Print actual second disk size] *******************************************
ok: [win1.example.com] => {
    "second_disk[0]['size']": "5368709120"
}

PLAY RECAP *********************************************************************
win1.example.com           : ok=5    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   
```
Users should not need to add json_query filters to obtain the correct disk. This may be a bug, or an enhancement, or a documentation issue.

Hints from issue discussion:
Files identified in the description:
* [`lib/ansible/modules/windows/win_disk_facts.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/windows/win_disk_facts.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 @ShachafGoldstein @if-meaton @jborean93 @jhawkesworth @marqelme @nitzmahone
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->

Commit message: win_disk_facts - Set output array order to be by disk number property - https://github.com/ansible/ansible/issues/63998

Patch to review:
diff --git a/changelogs/fragments/win_disk_facts-Set-output-array-order-by-disk-number.yml b/changelogs/fragments/win_disk_facts-Set-output-array-order-by-disk-number.yml
new file mode 100644
index 00000000000000..fecf703047d5d5
--- /dev/null
+++ b/changelogs/fragments/win_disk_facts-Set-output-array-order-by-disk-number.yml
@@ -0,0 +1,2 @@
+minor_changes:
+  - "win_disk_facts - Set output array order to be by disk number property - https://github.com/ansible/ansible/issues/63998"
\ No newline at end of file
diff --git a/lib/ansible/modules/windows/win_disk_facts.ps1 b/lib/ansible/modules/windows/win_disk_facts.ps1
index efa5be5687eab5..1d132c171930c6 100644
--- a/lib/ansible/modules/windows/win_disk_facts.ps1
+++ b/lib/ansible/modules/windows/win_disk_facts.ps1
@@ -244,5 +244,8 @@ foreach ($disk in $disks) {
     $result.ansible_facts.ansible_disks += $disk_info
 }
 
+# Sort by disk number property
+$result.ansible_facts.ansible_disks = @() + ($result.ansible_facts.ansible_disks | Sort-Object -Property Number)
+
 # Return result
 Exit-Json -obj $result


Code review comments:
diff_hunk: @@ -244,5 +244,8 @@ foreach ($disk in $disks) {
     $result.ansible_facts.ansible_disks += $disk_info
 }
 
+# Sort by disk number property
+$result.ansible_facts.ansible_disks = @() + ($result.ansible_facts.ansible_disks | Sort-Object -Property Number)
path: lib/ansible/modules/windows/win_disk_facts.ps1
text: @user1:
`ansible_disks` is a hashtable so `-Property Name` does not work normally. What you need to do is sort by a property expression like `| Sort-Object -Property @{e={$_.number}}`.

@author:
But ansible_disks is an array. From tests on my machine it worked for multiple disks as well. You mean on lower versions maybe? 

@user1:
It's an array of hashtables and we are trying to sort it by the "Number" property. Because each element in the array is a hashtable it doesn't actually have a property called `Number` but rather a key called `number`. Typically accessing this key is the same as getting an object's property, i.e. `$hashtable.number`. This is because PowerShell hides these differences in the background but this can affect things like `Sort-Object`. As an example here is where it can go wrong

```
$data = @(
    @{name='a'}
    @{name='c'}
    @{name='e'}
    @{name='f'}
    @{name='d'}
    @{name='b'}
)
$data | Sort-Object -Property name
```

Running that on PS v5.1 I get

```
PS C:\Users\vagrant-domain> $data | Sort-Object -Property name

Name                           Value
----                           -----
name                           f
name                           d
name                           b
name                           a
name                           c
name                           e
```

Instead we should just be providing a way for `Sort-Object` to get the value to sort by, e.g. `-Property {$_.name}` which then becomes

```
PS C:\Users\vagrant-domain> $data | Sort-Object -Property {$_.name}

Name                           Value
----                           -----
name                           a
name                           b
name                           c
name                           d
name                           e
name                           f
```

It seems like this may or may not be the case for PowerShell Core but we have to worry about Windows PowerShell.