one_image/one_image_info: refactor #8889

abakanovskii · 2024-09-20T17:52:32Z

SUMMARY

Refactor one_image and one_image_info modules to support more parameters and make it more simillar to one_vnet and one_template to reuse already existing OpenNebula class from module_utils
Fixes #3578

ISSUE TYPE

Refactoring Pull Request

COMPONENT NAME

one_image
one_image_info

ADDITIONAL INFORMATION

As i already mention the goal is to make these 2 modules use already existing OpenNebula class from module_utils to reuse already existing methods, i also added fully extended parameters that these 2 modules in return (using xsd scheme from https://github.com/privazio/pyone/blob/master/pyone/xsd/image.xsd)
There were also some problems with images that have id=0 because of incorrect if statements so i fixed them in process
I also added persistent to images (see related issue) and tested all of these on my OpenNebula server and everything is working as intended

# Now this is much more verbose
- name: Gather facts about all images
  community.general.one_image_info:
  register: result

# As well as this
- name: Fetch the IMAGE by id
  community.general.one_image:
    id: 45
    state: present
  register: result

# Now this would work
- name: Make the IMAGE persistent
  community.general.one_image:
    id: 37
    persistent: true

felixfontein

Thanks for your contribution! A first comment below:

plugins/modules/one_image.py

ansibullbot · 2024-09-20T18:13:46Z

cc @ilicmilan @meerkampdvv @nilsding @rsmontero @xorel
click here for bot help

russoz

hi @abakanovskii , thanks for your contribution!

I am not familiar with Open Nebula's API, and I have not spent much time looking into this. That being said, I got a couple of adjustments listed below, the rest looks good but again, I am no expert.

changelogs/fragments/8889-refactor-one-image-modules.yml

russoz · 2024-09-22T22:56:37Z

plugins/modules/one_image_info.py

+    def get_image_info(self, image):
+        info = {
+            'id': image.ID,
+            'name': image.NAME,
+            'state': IMAGE_STATES[image.STATE],
+            'running_vms': image.RUNNING_VMS,
+            'used': bool(image.RUNNING_VMS),
+            'user_name': image.UNAME,
+            'user_id': image.UID,
+            'group_name': image.GNAME,
+            'group_id': image.GID,
+            'permissions': {
+                'owner_u': image.PERMISSIONS.OWNER_U,
+                'owner_m': image.PERMISSIONS.OWNER_M,
+                'owner_a': image.PERMISSIONS.OWNER_A,
+                'group_u': image.PERMISSIONS.GROUP_U,
+                'group_m': image.PERMISSIONS.GROUP_M,
+                'group_a': image.PERMISSIONS.GROUP_A,
+                'other_u': image.PERMISSIONS.OTHER_U,
+                'other_m': image.PERMISSIONS.OTHER_M,
+                'other_a': image.PERMISSIONS.OTHER_A
+            },
+            'type': image.TYPE,
+            'disk_type': image.DISK_TYPE,
+            'persistent': image.PERSISTENT,
+            'regtime': image.REGTIME,
+            'source': image.SOURCE,
+            'path': image.PATH,
+            'fstype': getattr(image, 'FSTYPE', 'Null'),
+            'size': image.SIZE,
+            'cloning_ops': image.CLONING_OPS,
+            'cloning_id': image.CLONING_ID,
+            'target_snapshot': image.TARGET_SNAPSHOT,
+            'datastore_id': image.DATASTORE_ID,
+            'datastore': image.DATASTORE,
+            'vms': self.get_image_list_id(image, 'VMS'),
+            'clones': self.get_image_list_id(image, 'CLONES'),
+            'app_clones': self.get_image_list_id(image, 'APP_CLONES'),
+            'snapshots': self.get_image_snapshots_list(image),
+            'template': image.TEMPLATE,
+        }
+
+        return info


I have not compared line by line but this looks very much the same as the matching method in one_image.py - if so, then they should be moved to the module_utils/opennebula.py file and reused in both. Or, if there is a tiny difference between them, maybe a the bulk of the image info could go to a common function in the utils, while these functions here and in the other module just make these smaller adjustments.

that seems like a good idea, I can do something similar with other one_X modules later, Thanks!

russoz

LGTM

plugins/modules/one_image.py

plugins/modules/one_image_info.py

felixfontein · 2024-10-02T06:55:21Z

Looks good from my side. If nobody objects, I'll merge on the weekend.

patchback · 2024-10-05T13:02:05Z

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/fea0ffa5aa8d90a01616e596e03c6e78fb3f887c/pr-8889

Backported as #8984

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

* Refactor one_image * Refactor one_image_info * Add examples one_image * Add CHANGELOG fragment * Add integration tests for one_image * Add integration tests for one_image_info * Update one_image DOC * Update one_image_info DOC * Update one_image DOC * Update one_image_info DOC * Fix f-strings for one_image * Update CHANGELOG fragment * PR fixes * PR fixes (cherry picked from commit fea0ffa)

felixfontein · 2024-10-05T13:02:22Z

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

…ctor (#8984) one_image/one_image_info: refactor (#8889) * Refactor one_image * Refactor one_image_info * Add examples one_image * Add CHANGELOG fragment * Add integration tests for one_image * Add integration tests for one_image_info * Update one_image DOC * Update one_image_info DOC * Update one_image DOC * Update one_image_info DOC * Fix f-strings for one_image * Update CHANGELOG fragment * PR fixes * PR fixes (cherry picked from commit fea0ffa) Co-authored-by: alexander <[email protected]>

abakanovskii added 4 commits September 20, 2024 20:38

Refactor one_image

a2d522b

Refactor one_image_info

2baab7b

Add examples one_image

da68574

Add CHANGELOG fragment

5e87b63

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-9 Automatically create a backport for the stable-9 branch labels Sep 20, 2024

felixfontein reviewed Sep 20, 2024

View reviewed changes

plugins/modules/one_image.py Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

ansibullbot added ci_verified Push fixes to PR branch to re-run CI module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR plugins plugin (any type) labels Sep 20, 2024

abakanovskii added 2 commits September 20, 2024 22:20

Add integration tests for one_image

fe5c499

Add integration tests for one_image_info

7a4e885

ansibullbot added integration tests/integration tests tests 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 Sep 20, 2024

abakanovskii added 2 commits September 20, 2024 22:34

Update one_image DOC

9d3a8e2

Update one_image_info DOC

2fe2f88

This comment was marked as outdated.

Sign in to view

abakanovskii added 3 commits September 20, 2024 22:55

Update one_image DOC

2fc0fcc

Update one_image_info DOC

2d3abbf

Fix f-strings for one_image

5f46d7d

abakanovskii requested a review from felixfontein September 20, 2024 20:09

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 Sep 20, 2024

abakanovskii changed the title ~~One image update~~ one_image/one_image_info: refactor Sep 20, 2024

Update CHANGELOG fragment

8a6ed75

russoz reviewed Sep 22, 2024

View reviewed changes

PR fixes

702d150

ansibullbot added the module_utils module_utils label Sep 23, 2024

russoz approved these changes Sep 27, 2024

View reviewed changes

felixfontein reviewed Sep 30, 2024

View reviewed changes

plugins/modules/one_image.py Show resolved Hide resolved

plugins/modules/one_image.py Outdated Show resolved Hide resolved

plugins/modules/one_image.py Show resolved Hide resolved

plugins/modules/one_image_info.py Show resolved Hide resolved

PR fixes

135ad7f

abakanovskii requested a review from felixfontein September 30, 2024 14:06

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Oct 5, 2024

felixfontein merged commit fea0ffa into ansible-collections:main Oct 5, 2024
141 checks passed

patchback bot mentioned this pull request Oct 5, 2024

[PR #8889/fea0ffa5 backport][stable-9] one_image/one_image_info: refactor #8984

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

one_image/one_image_info: refactor #8889

one_image/one_image_info: refactor #8889

abakanovskii commented Sep 20, 2024 •

edited

Loading

felixfontein left a comment

This comment was marked as outdated.

ansibullbot commented Sep 20, 2024

This comment was marked as outdated.

russoz left a comment

russoz Sep 22, 2024

abakanovskii Sep 23, 2024 •

edited

Loading

russoz left a comment

felixfontein commented Oct 2, 2024

patchback bot commented Oct 5, 2024 •

edited

Loading

felixfontein commented Oct 5, 2024

one_image/one_image_info: refactor #8889

one_image/one_image_info: refactor #8889

Conversation

abakanovskii commented Sep 20, 2024 • edited Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

felixfontein left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

ansibullbot commented Sep 20, 2024

This comment was marked as outdated.

russoz left a comment

Choose a reason for hiding this comment

russoz Sep 22, 2024

Choose a reason for hiding this comment

abakanovskii Sep 23, 2024 • edited Loading

Choose a reason for hiding this comment

russoz left a comment

Choose a reason for hiding this comment

felixfontein commented Oct 2, 2024

patchback bot commented Oct 5, 2024 • edited Loading

Backport to stable-9: 💚 backport PR created

felixfontein commented Oct 5, 2024

abakanovskii commented Sep 20, 2024 •

edited

Loading

abakanovskii Sep 23, 2024 •

edited

Loading

patchback bot commented Oct 5, 2024 •

edited

Loading