proxmox: Add clone parameter #3930

m-rtijn · 2021-12-20T22:19:51Z

SUMMARY

This PR adds a 'clone' parameter to the proxmox module to clone a container. The implementation is based on the clone option in the proxmox_kvm module.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

proxmox

WIP

A small todo list of things which I already know I should do:

~~Check if the older OpenVZ type also supports the clone API operation just like the LXC one. If not, make sure that the clone operation is only used with LXC-enabled proxmox nodes.~~ Not solved, just ignored: it now fails when using this on a node that does not use LXC. A note is added in the documentation.
Actually test out the code
Check development conventions
Check documentation conventions
Add changelog fragment
Change the required_if[('state', 'present' ... accordingly
Check if cloned container is a template or not (if not, a 'storage' parameter can't be passed/should be ignored)
Add extra examples

ansibullbot · 2021-12-20T22:23:52Z

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

m-rtijn · 2021-12-20T23:08:30Z

Hm. It fails with an error that the clone parameter is not supported. It is however supplied to the AnsibleModule(argument_spec), so I'm not sure what wrong here.

changelogs/fragments/3930-proxmox-add-clone.yaml

plugins/modules/cloud/misc/proxmox.py

Co-authored-by: Felix Fontein <[email protected]>

m-rtijn · 2021-12-21T10:18:56Z

Hm. It fails with an error that the clone parameter is not supported. It is however supplied to the AnsibleModule(argument_spec), so I'm not sure what wrong here.

Ah never mind this, I wasn't running my own code :')

…ion pre-checks fail

m-rtijn · 2021-12-21T15:08:24Z

While debugging I discovered that I get "400 Bad Request: Parameter verification failed" (PVE v7.1-8) errors even when not running my own changes when I try to create a new container. So that's a bit of a bummer (and oversight that I didn't try that first 😅)

m-rtijn · 2021-12-22T16:48:51Z

While debugging I discovered that I get "400 Bad Request: Parameter verification failed" (PVE v7.1-8) errors even when not running my own changes when I try to create a new container. So that's a bit of a bummer (and oversight that I didn't try that first sweat_smile)

Ah, I found the culprit. It's a bit disappointing that proxmoxer only gives a 400 error even though the PVE API neatly tells the user what the error is.

…a default choice

plugins/modules/cloud/misc/proxmox.py

Co-authored-by: Felix Fontein <[email protected]>

felixfontein

Looks reasonable to me. Let's see what the modue maintainers say about this.

felixfontein · 2022-01-03T18:23:01Z

If nobody complains, I'll merge this in a few days.

patchback · 2022-01-06T06:01:29Z

Backport to stable-4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4/b0c27f7a688344359875be604927f216df847ca7/pr-3930

Backported as #3992

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

* proxmox: Add clone parameter * Add changelog fragment * Add version_added Co-authored-by: Felix Fontein <[email protected]> * Add PR URL to changelog fragment Co-authored-by: Felix Fontein <[email protected]> * Clarify what content_check does * Split up try/except block to give a separate error message when creation pre-checks fail * Create seperate case for cloning * Prevent 'clone' argument from being removed * Fix double argument, add todo's * Check if to be cloned container actually exists * Adjust module options dependencies * Require 'storage' parameter when cloned container is not a template and ignore otherwise * Don't only create linked clones of template containers * Fix pylint errors * Add extra example * Minor language fix * Add clone_type parameter to specify cloning behaviour * I can't find if openvz nodes have this clone API, so just don't support it * Remove unrelated changes * Don't pass unused kwargs * Revert more unrelated changes * Remove required_together clone and clone_type because clone_type has a default choice * Fix clone_type reference Co-authored-by: Felix Fontein <[email protected]> * Fix missing period Co-authored-by: Felix Fontein <[email protected]> * Fix redundant period Co-authored-by: Felix Fontein <[email protected]> * Fix redundant period Co-authored-by: Felix Fontein <[email protected]> Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit b0c27f7)

felixfontein · 2022-01-06T06:01:37Z

@m-rtijn thanks for your contribution!

* proxmox: Add clone parameter * Add changelog fragment * Add version_added Co-authored-by: Felix Fontein <[email protected]> * Add PR URL to changelog fragment Co-authored-by: Felix Fontein <[email protected]> * Clarify what content_check does * Split up try/except block to give a separate error message when creation pre-checks fail * Create seperate case for cloning * Prevent 'clone' argument from being removed * Fix double argument, add todo's * Check if to be cloned container actually exists * Adjust module options dependencies * Require 'storage' parameter when cloned container is not a template and ignore otherwise * Don't only create linked clones of template containers * Fix pylint errors * Add extra example * Minor language fix * Add clone_type parameter to specify cloning behaviour * I can't find if openvz nodes have this clone API, so just don't support it * Remove unrelated changes * Don't pass unused kwargs * Revert more unrelated changes * Remove required_together clone and clone_type because clone_type has a default choice * Fix clone_type reference Co-authored-by: Felix Fontein <[email protected]> * Fix missing period Co-authored-by: Felix Fontein <[email protected]> * Fix redundant period Co-authored-by: Felix Fontein <[email protected]> * Fix redundant period Co-authored-by: Felix Fontein <[email protected]> Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit b0c27f7) Co-authored-by: Martijn <[email protected]>

m-rtijn added 2 commits December 20, 2021 23:07

proxmox: Add clone parameter

5e12c6c

Add changelog fragment

2875fa3

ansibullbot added WIP Work in progress cloud feature This issue/PR relates to a feature request module module new_contributor Help guide this first time contributor plugins plugin (any type) labels Dec 20, 2021

felixfontein added backport-4 check-before-release PR will be looked at again shortly before release and merged if possible. labels Dec 21, 2021

felixfontein reviewed Dec 21, 2021

View reviewed changes

changelogs/fragments/3930-proxmox-add-clone.yaml Outdated Show resolved Hide resolved

plugins/modules/cloud/misc/proxmox.py Show resolved Hide resolved

m-rtijn and others added 2 commits December 21, 2021 11:01

Add version_added

0f85b00

Co-authored-by: Felix Fontein <[email protected]>

Add PR URL to changelog fragment

99a56c5

Co-authored-by: Felix Fontein <[email protected]>

m-rtijn added 3 commits December 21, 2021 11:50

Clarify what content_check does

2647f75

Split up try/except block to give a separate error message when creat…

a0f39f0

…ion pre-checks fail

Create seperate case for cloning

992590b

This comment has been minimized.

Sign in to view

ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Dec 21, 2021

Prevent 'clone' argument from being removed

46a1b7b

ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Dec 21, 2021

This comment has been minimized.

Sign in to view

ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Dec 21, 2021

Fix double argument, add todo's

5e7e256

ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Dec 22, 2021

This comment has been minimized.

Sign in to view

m-rtijn changed the title ~~[WIP] proxmox: Add clone parameter~~ proxmox: Add clone parameter Dec 22, 2021

ansibullbot removed the WIP Work in progress label Dec 22, 2021

m-rtijn added 4 commits December 23, 2021 13:45

Remove unrelated changes

33c7517

Don't pass unused kwargs

47911e2

Revert more unrelated changes

b6f855b

Remove required_together clone and clone_type because clone_type has …

e44ad88

…a default choice

m-rtijn requested a review from felixfontein December 23, 2021 19:03

felixfontein reviewed Dec 24, 2021

View reviewed changes

m-rtijn and others added 4 commits December 24, 2021 18:02

Fix clone_type reference

9de1ab9

Co-authored-by: Felix Fontein <[email protected]>

Fix missing period

68c83ec

Co-authored-by: Felix Fontein <[email protected]>

Fix redundant period

5729576

Co-authored-by: Felix Fontein <[email protected]>

Fix redundant period

9693741

Co-authored-by: Felix Fontein <[email protected]>

m-rtijn requested a review from felixfontein December 24, 2021 17:05

felixfontein approved these changes Dec 26, 2021

View reviewed changes

m-rtijn mentioned this pull request Dec 29, 2021

Proxmox 7 LXC Creation 400 Bad Request: Parameter verification failed #3956

Closed

1 task

ansibullbot added stale_ci CI is older than 7 days, rerun before merging has_issue labels Jan 3, 2022

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jan 6, 2022

felixfontein merged commit b0c27f7 into ansible-collections:main Jan 6, 2022

patchback bot mentioned this pull request Jan 6, 2022

[PR #3930/b0c27f7a backport][stable-4] proxmox: Add clone parameter #3992

Merged

8 tasks

m-rtijn mentioned this pull request Jan 13, 2022

Refactor all Proxmox modules to use shared module_utils. #4029

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

proxmox: Add clone parameter #3930

proxmox: Add clone parameter #3930

m-rtijn commented Dec 20, 2021 •

edited

Loading

ansibullbot commented Dec 20, 2021

m-rtijn commented Dec 20, 2021

m-rtijn commented Dec 21, 2021

This comment has been minimized.

This comment has been minimized.

m-rtijn commented Dec 21, 2021 •

edited

Loading

m-rtijn commented Dec 22, 2021

This comment has been minimized.

felixfontein left a comment

felixfontein commented Jan 3, 2022

patchback bot commented Jan 6, 2022 •

edited

Loading

felixfontein commented Jan 6, 2022

proxmox: Add clone parameter #3930

proxmox: Add clone parameter #3930

Conversation

m-rtijn commented Dec 20, 2021 • edited Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

WIP

ansibullbot commented Dec 20, 2021

m-rtijn commented Dec 20, 2021

m-rtijn commented Dec 21, 2021

This comment has been minimized.

This comment has been minimized.

m-rtijn commented Dec 21, 2021 • edited Loading

m-rtijn commented Dec 22, 2021

This comment has been minimized.

felixfontein left a comment

Choose a reason for hiding this comment

felixfontein commented Jan 3, 2022

patchback bot commented Jan 6, 2022 • edited Loading

Backport to stable-4: 💚 backport PR created

felixfontein commented Jan 6, 2022

m-rtijn commented Dec 20, 2021 •

edited

Loading

m-rtijn commented Dec 21, 2021 •

edited

Loading

patchback bot commented Jan 6, 2022 •

edited

Loading