Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: Duplicate vmid in proxmox_disk module #5492 #5493

Merged
merged 1 commit into from
Nov 13, 2022
Merged

Fix: Duplicate vmid in proxmox_disk module #5492 #5493

merged 1 commit into from
Nov 13, 2022

Conversation

Doc-Tiebeau
Copy link
Contributor

@Doc-Tiebeau Doc-Tiebeau commented Nov 7, 2022 •
edited
Loading

#5492

SUMMARY

Remove duplicated vmid reference in URI builder.

Fixes #5492

ISSUE TYPE

BUG: Resize disk blocking feature

@ansibullbot
Copy link
Collaborator

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

@ansibullbot ansibullbot added bug This issue/PR relates to a bug cloud module module new_contributor Help guide this first time contributor plugins plugin (any type) small_patch Hopefully easy to review labels Nov 7, 2022
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-5 labels Nov 7, 2022
felixfontein
felixfontein reviewed Nov 7, 2022
View reviewed changes
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! Could you please add a changelog fragment? Thanks.

@castorsky
Copy link
Contributor

I have tested right now both versions (original and patched) and both work without complaints.
Environment:

  • Controller: openSUSE Tumbleweed (python 3.10)
  • Target: Proxmox 7.2-11
  • Ansible:
    • core 2.14.0
    • community-general 6.0.0
  • Proxmoxer: 1.3.1
  • Requests: 2.28.1

@castorsky
Copy link
Contributor

Have to mention this: such a construction is being used multiple times in proxmox modules. So I think that it will be sane to check this modules and their functions that use vmid=vmid along with qemu(vmid) and patch them too.
If such a patch will pass all tests then I think it is possible to remove vmid=vmid (that seems to be redundant) from API calls.

@felixfontein
Copy link
Collaborator

@castorsky should this be merged then? There will be a 6.0.1 release tomorrow, so this PR could make it in there.

@castorsky
Copy link
Contributor

should this be merged then?

Sure. I think there will be no degrade after merge as it works for me and Doc-Tiebeau.
In couple of weeks I will test if such change can be done in all functions of proxmox_disk and proxmox_nic modules and create separate PR.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Nov 13, 2022
@felixfontein felixfontein merged commit 27a4ffc into ansible-collections:main Nov 13, 2022
@patchback
Copy link

patchback bot commented Nov 13, 2022 •
edited
Loading

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/27a4ffc2931e5ccdd3a2d4d09260351d1050c4f9/pr-5493

Backported as #5536

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Nov 13, 2022
@Doc-Tiebeau @patchback
Fix: Duplicate vmid in proxmox_disk module #5492 (#5493)
c33beed 
#5492
(cherry picked from commit 27a4ffc)
@patchback patchback bot mentioned this pull request Nov 13, 2022
Merged
@patchback
Copy link

patchback bot commented Nov 13, 2022 •
edited
Loading

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/27a4ffc2931e5ccdd3a2d4d09260351d1050c4f9/pr-5493

Backported as #5537

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Nov 13, 2022
@Doc-Tiebeau @patchback
Fix: Duplicate vmid in proxmox_disk module #5492 (#5493)
4c0e419 
#5492
(cherry picked from commit 27a4ffc)
@patchback patchback bot mentioned this pull request Nov 13, 2022
Merged
@felixfontein
Copy link
Collaborator

@Doc-Tiebeau thanks for your contribution!
@castorsky thanks for reviewing and testing!

@felixfontein
Copy link
Collaborator

Oh, I merged this a bit too quickly, there's no changelog fragment... I'll add one in a follow-up commit...

felixfontein added a commit that referenced this pull request Nov 13, 2022
@patchback @Doc-Tiebeau @felixfontein
[PR #5493/27a4ffc2 backport][stable-6] Fix: Duplicate vmid in proxmox…
09ea441 
…_disk module #5492 (#5537)

* Fix:  Duplicate vmid in proxmox_disk module #5492 (#5493)

#5492
(cherry picked from commit 27a4ffc)

* Add changelog fragment.

(cherry picked from commit 6723853)

Co-authored-by: Doc_Tiebeau <elie.saintfelix@gmail.com>
Co-authored-by: Felix Fontein <felix@fontein.de>
felixfontein added a commit that referenced this pull request Nov 13, 2022
@patchback @Doc-Tiebeau @felixfontein
[PR #5493/27a4ffc2 backport][stable-5] Fix: Duplicate vmid in proxmox…
bc83586 
…_disk module #5492 (#5536)

* Fix:  Duplicate vmid in proxmox_disk module #5492 (#5493)

#5492
(cherry picked from commit 27a4ffc)

* Add changelog fragment.

(cherry picked from commit 6723853)

Co-authored-by: Doc_Tiebeau <elie.saintfelix@gmail.com>
Co-authored-by: Felix Fontein <felix@fontein.de>
@castorsky castorsky mentioned this pull request Dec 9, 2022
Merged
@patchback patchback bot mentioned this pull request Dec 19, 2022
Merged
@patchback patchback bot mentioned this pull request Dec 19, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixfontein felixfontein felixfontein left review comments

Assignees
No one assigned
Labels
bug This issue/PR relates to a bug cloud module module new_contributor Help guide this first time contributor plugins plugin (any type) small_patch Hopefully easy to review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Duplicate vmid in proxmox_disk module
4 participants
@Doc-Tiebeau @ansibullbot @castorsky @felixfontein