proxmox_disk: prevent trying to shrink disks

[aBUDmdBQ]

SUMMARY

Shrinking Disks is not supported by the PVE API and should be prevented by the module:
https://pve.proxmox.com/wiki/Resize_disks

Currently, when the configured size of the disk is smaller than the actual size, the module reports a change, but the change isn't actually happening. The module doesn't detect that this fails.

This PR checks if the user tries to shrink the disk and then fails instead of contacting the PVE API.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

• proxmox_disk

[ansibullbot]

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/proxmox_disk.py:820:161: E501: line too long (172 > 160 characters)

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/proxmox_disk.py:820:161: E501: line too long (172 > 160 characters)

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/proxmox_disk.py:820:161: E501: line too long (172 > 160 characters)

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/proxmox_disk.py:820:161: E501: line too long (172 > 160 characters)

click here for bot help

[ansibullbot]

cc @Ajpantuso @Thulium-Drake @UnderGreen @castorsky @joshainglis @karmab @krauthosting
click here for bot help

[felixfontein]

Thanks for your contribution!

I don't know whether it's a good idea to prevent this, since both the old and the new behavior result in a failure. But it could happen that at some point the API suddenly supports shrinking, and then this extra check would be problematic.

[castorsky]

I'm agree with Felix. Current behavior is mentioned in module documentation and may be changed in future. I think there's no need to fix since this behavior is not a bug.

[aBUDmdBQ]

You're saying that the module already should return a failed result when trying to shrink a disk?

When I try that, the module reports "changed", but the Proxmox Cluster Log shows an Error: TASK ERROR: shrinking disks is not supported

Versions we currently use:
Proxmox VE 8.1.4
community.general 8.3.0
python3-proxmoxer 1.2.0-2 (installed via apt. no other versions installed)

To make sure it's not the outdated version, i tried upgrading proxmoxer to 2.0.1 via pip, but this didn't change the behavior.

[felixfontein]

Hmm, maybe the API says "everything's fine" to the caller, queues the job, and only then the job fails while the caller (in this case the module) already returned "everything's ok"?

[castorsky]

I have checked with different PVE versions and v8 reports success before task being queued (and failed).

• v7.4.17

TASK [Modify disk] **********************************************************************************************************************************************
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Failed to resize disk scsi0 in VM 100 with exception: 500 Internal Server Error: shrinking disks is not supported"}

• v8.1.3

TASK [Modify disk] **********************************************************************************************************************************************
changed: [localhost]

[krauthosting]

@castorsky So this is a regression in PVE, right? Maybe it'd be a good idea to open a ticket with them about this.

FYI We are currently in the process of moving the proxmox_ modules to a separate collection. Hence we would like to resolve this PR asap so that we can move all issues over to community.proxmox

@aBUDmdBQ Would you be okay to close this PR and reopen it in community.proxmox if the upstream issue remains unresolved?

[castorsky]

@castorsky So this is a regression in PVE, right? Maybe it'd be a good idea to open a ticket with them about this.

I'm not sure where is the root of the problem but it seems that this call should be async (via POST, not PUT). I have already implemented this for create_disk and move_disk functions because they can operate for a very long time. Maybe it's time to convert all sync calls to async.

This PR does not solve the root cause - it just patches some random consequence. So I think it's better to close PR in this repo.

[aBUDmdBQ]

@krauthosting Yes, I'm fine with closing this and trying to get it fixed upstream. Otherwise I will open it again in community.proxmox