Get interfaces for Proxmox LXC containers #8713

scottlangendyk · 2024-08-04T14:16:54Z

SUMMARY

Add fact for proxmox lxc interfaces

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

proxmox

ADDITIONAL INFORMATION

Useful for setting the ansible_host variable for lxc containers interfaces using dhcp.

Fact example:

"proxmox_lxc_interfaces": [
    {
        "hwaddr": "00:00:00:00:00:00",
        "inet": "127.0.0.1/8",
        "inet6": "::1/128",
        "name": "lo"
    },
    {
        "hwaddr": "00:00:5e:00:53:a1",
        "inet": "192.0.2.12/24",
        "inet6": "2001:db8:3c4d:0015:0000:0000:1a2f:1a2b/48",
        "name": "eth0"
    }
]

ansibullbot · 2024-08-04T14:27:52Z

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

felixfontein

Thanks for your contribution! I've added some first comments.

changelogs/fragments/8713-proxmox_lxc_interfaces.yml

felixfontein · 2024-08-05T19:02:56Z

plugins/inventory/proxmox.py

+        try:
+            ret = self._get_json("%s/api2/json/nodes/%s/lxc/%s/interfaces" % (self.proxmox_url, node, vmid))
+        except Exception:
+            return


Simply ignoring all errors seems wrong to me. For which reasons can this call fail?

I did this to avoid introducing a breaking change, since this particular API is somewhat new (introduced in proxmox 8.1).

Maybe a version check makes more sense?

A version check would probably be best.

Otherwise you need to distinguish between "API doesn't support this" (can be ignored) and "other kind of error" (needs to be reported).

@felixfontein I've made some small changes.

Requests for a container that are not running will return null, so there is an explicit check for the status fact.

From my tests and a bit of reading, previous versions of the API will return 501 for requests to unknown endpoint. I verified this behavior with a server running version 7.

Adding an explicit version check can be done, but is more complicated. How many versions back does this plugin need to support?

Adding an explicit version check can be done, but is more complicated.

Checking for 501 sounds good to me.

How many versions back does this plugin need to support?

I cannot find anything in the documenation, so basically "all versions the plugin worked with so far".

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

felixfontein · 2024-08-10T20:12:45Z

Please note that the unit tests are currently failing.

scottlangendyk · 2024-08-11T16:24:36Z

@felixfontein Unit test failure has been addressed

felixfontein · 2024-08-11T18:09:04Z

If nobody objects, I'll merge this tomorrow for the next release.

patchback · 2024-08-12T05:35:46Z

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/0f59bb7a99258be7f885f8f77e5fcf1de8a0b0e5/pr-8713

Backported as #8750

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

* Get interfaces for Proxmox LXC containers * Add changelog * Don't use bare except * Update changelog from suggestion Co-authored-by: Felix Fontein <[email protected]> * Only lookup interfaces for running containers * Ignore not implemented status * Check that key exists in properties dict * define ignore errors in mock * Use not in --------- Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit 0f59bb7)

felixfontein · 2024-08-12T05:35:54Z

@scottlangendyk thanks for your contribution!

… containers (#8750) Get interfaces for Proxmox LXC containers (#8713) * Get interfaces for Proxmox LXC containers * Add changelog * Don't use bare except * Update changelog from suggestion Co-authored-by: Felix Fontein <[email protected]> * Only lookup interfaces for running containers * Ignore not implemented status * Check that key exists in properties dict * define ignore errors in mock * Use not in --------- Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit 0f59bb7) Co-authored-by: Scott Langendyk <[email protected]>

* Get interfaces for Proxmox LXC containers * Add changelog * Don't use bare except * Update changelog from suggestion Co-authored-by: Felix Fontein <[email protected]> * Only lookup interfaces for running containers * Ignore not implemented status * Check that key exists in properties dict * define ignore errors in mock * Use not in --------- Co-authored-by: Felix Fontein <[email protected]>

scottlangendyk added 2 commits August 4, 2024 10:03

Get interfaces for Proxmox LXC containers

8588c21

Add changelog

805e79f

This comment was marked as outdated.

Sign in to view

Don't use bare except

a493ebc

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 Aug 4, 2024

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 Aug 5, 2024

felixfontein reviewed Aug 5, 2024

View reviewed changes

scottlangendyk and others added 3 commits August 5, 2024 21:31

Update changelog from suggestion

cde2d23

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

Only lookup interfaces for running containers

606c45c

Ignore not implemented status

1a7b79f

ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Aug 10, 2024

Check that key exists in properties dict

fd325cd

This comment was marked as outdated.

Sign in to view

define ignore errors in mock

63d934a

ansibullbot added tests tests unit tests/unit labels Aug 11, 2024

Use not in

d904cc2

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Aug 11, 2024

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

felixfontein merged commit 0f59bb7 into ansible-collections:main Aug 12, 2024
147 checks passed

patchback bot mentioned this pull request Aug 12, 2024

[PR #8713/0f59bb7a backport][stable-9] Get interfaces for Proxmox LXC containers #8750

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get interfaces for Proxmox LXC containers #8713

Get interfaces for Proxmox LXC containers #8713

scottlangendyk commented Aug 4, 2024 •

edited

Loading

This comment was marked as outdated.

ansibullbot commented Aug 4, 2024

felixfontein left a comment

felixfontein Aug 5, 2024

scottlangendyk Aug 6, 2024

felixfontein Aug 7, 2024

scottlangendyk Aug 10, 2024

felixfontein Aug 10, 2024

felixfontein commented Aug 10, 2024

This comment was marked as outdated.

scottlangendyk commented Aug 11, 2024

felixfontein commented Aug 11, 2024

patchback bot commented Aug 12, 2024 •

edited

Loading

felixfontein commented Aug 12, 2024

Get interfaces for Proxmox LXC containers #8713

Get interfaces for Proxmox LXC containers #8713

Conversation

scottlangendyk commented Aug 4, 2024 • edited Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

This comment was marked as outdated.

ansibullbot commented Aug 4, 2024

felixfontein left a comment

Choose a reason for hiding this comment

felixfontein Aug 5, 2024

Choose a reason for hiding this comment

scottlangendyk Aug 6, 2024

Choose a reason for hiding this comment

felixfontein Aug 7, 2024

Choose a reason for hiding this comment

scottlangendyk Aug 10, 2024

Choose a reason for hiding this comment

felixfontein Aug 10, 2024

Choose a reason for hiding this comment

felixfontein commented Aug 10, 2024

This comment was marked as outdated.

scottlangendyk commented Aug 11, 2024

felixfontein commented Aug 11, 2024

patchback bot commented Aug 12, 2024 • edited Loading

Backport to stable-9: 💚 backport PR created

felixfontein commented Aug 12, 2024

scottlangendyk commented Aug 4, 2024 •

edited

Loading

patchback bot commented Aug 12, 2024 •

edited

Loading