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.

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

OpenNebula/one_vm implement the one.vm.updateconf API call #5812

Merged

Conversation

sk4zuzu
Copy link
Contributor

@sk4zuzu sk4zuzu commented Jan 10, 2023

SUMMARY

OpenNebula provides the one.vm.updateconf API call, that can be used to modify the context of a VM. For example SSH_PUBLIC_KEY, PASSWORD or even START_SCRIPT variables can be set. This PR implements exactly that, it allows for updating running VMs and setting the same parameters for freshly created ones. We believe this can prove useful for some users.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

one_vm module

ADDITIONAL INFORMATION

NA

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added cloud feature This issue/PR relates to a feature request module module module_utils module_utils new_contributor Help guide this first time contributor new_plugin New plugin plugins plugin (any type) tests tests unit tests/unit labels Jan 10, 2023
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 10, 2023
@github-actions
Copy link

github-actions bot commented Jan 10, 2023

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

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

Hi @sk4zuzu
Thank you for the contribution! Please add a changelog fragment.

I saw you wrote tests, but only for the module_utils stuff. It would be great if you could think of tests to the actual update_conf() code. It might be something for another PR though, I will defer that decision to your judgement.

Cheers

plugins/module_utils/opennebula.py Outdated Show resolved Hide resolved
plugins/modules/one_vm.py Outdated Show resolved Hide resolved
tests/unit/plugins/module_utils/test_opennebula.py Outdated Show resolved Hide resolved
Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>
@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI and removed ci_verified Push fixes to PR branch to re-run CI labels Jan 11, 2023
@ansibullbot ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Jan 11, 2023
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Jan 11, 2023
@sk4zuzu
Copy link
Contributor Author

sk4zuzu commented Jan 11, 2023

Hello @russoz

Thank you for the review.

Please add a changelog fragment.

Done. :)

I saw you wrote tests, but only for the module_utils stuff. It would be great if you could think of tests to the actual update_conf() code. It might be something for another PR though, I will defer that decision to your judgement.

Actually I was thinking about it and concluded this should be some other PR, that's because there were no tests for one_vm module of any kind. I'd rather create something more comprehensive for the whole module if you won't mind. 🤔

The CI checks fail for python2.6, is it something that needs to addressed? I believe python 2.6 is not listed here. 🤔

Cheers

@felixfontein
Copy link
Collaborator

The CI checks fail for python2.6, is it something that needs to addressed? I believe python 2.6 is not listed here. thinking

Yes, this needs to be adressed (otherwise CI wouldn't complain). ansible-core 2.11 (which this collection still supports) supports Python 2.6.

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. I've added some comments.

plugins/modules/one_vm.py Show resolved Hide resolved
plugins/modules/one_vm.py Outdated Show resolved Hide resolved
plugins/modules/one_vm.py Show resolved Hide resolved
plugins/modules/one_vm.py Outdated Show resolved Hide resolved
plugins/modules/one_vm.py Show resolved Hide resolved
plugins/modules/one_vm.py Outdated Show resolved Hide resolved
plugins/modules/one_vm.py Outdated Show resolved Hide resolved
plugins/modules/one_vm.py Show resolved Hide resolved
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-6 labels Jan 12, 2023
@ansibullbot ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 13, 2023
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.

I'm not familiar with this codebase, but from a generic Ansible POV this looks ok. If nobody objects in the next two weeks, I'll merge this.

@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Jan 23, 2023
@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jan 28, 2023
@felixfontein felixfontein merged commit 8818a6f into ansible-collections:main Jan 28, 2023
@patchback
Copy link

patchback bot commented Jan 28, 2023

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/8818a6f2421d50980229db464a710a84e4748eff/pr-5812

Backported as #5905

🤖 @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 Jan 28, 2023
* opennebula: Add template manipulation helpers

* one_vm: Use 'updateconf' API call to modify running VMs

* one_vm: Emulate 'updateconf' API call for newly created VMs

* opennebula/one_vm: Satisfy linter checks

* opennebula/one_vm: Apply suggestions from code review

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

* opennebula/one_vm: Drop 'extend' function, use 'dict_merge' instead

* Add changelog fragment

* one_vm: Refactor 'parse_updateconf' function

* opennebula/one_vm: Apply suggestions from code review

Co-authored-by: Felix Fontein <felix@fontein.de>

* one_vm: Allow for using updateconf in all scenarios

---------

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>
Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 8818a6f)
@felixfontein
Copy link
Collaborator

@sk4zuzu thanks for your contribution!
@russoz thanks for reviewing!

felixfontein pushed a commit that referenced this pull request Jan 28, 2023
…e one.vm.updateconf API call (#5905)

OpenNebula/one_vm implement the one.vm.updateconf API call (#5812)

* opennebula: Add template manipulation helpers

* one_vm: Use 'updateconf' API call to modify running VMs

* one_vm: Emulate 'updateconf' API call for newly created VMs

* opennebula/one_vm: Satisfy linter checks

* opennebula/one_vm: Apply suggestions from code review

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

* opennebula/one_vm: Drop 'extend' function, use 'dict_merge' instead

* Add changelog fragment

* one_vm: Refactor 'parse_updateconf' function

* opennebula/one_vm: Apply suggestions from code review

Co-authored-by: Felix Fontein <felix@fontein.de>

* one_vm: Allow for using updateconf in all scenarios

---------

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>
Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 8818a6f)

Co-authored-by: Michal Opala <mopala@opennebula.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cloud feature This issue/PR relates to a feature request module_utils module_utils module module new_contributor Help guide this first time contributor new_plugin New plugin plugins plugin (any type) stale_ci CI is older than 7 days, rerun before merging tests tests unit tests/unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants