Skip to content

Conversation

Derioss
Copy link

@Derioss Derioss commented Aug 4, 2025

Thanks for your module, we added SDN features for our needs

Copy link
Owner

@lae lae left a comment

Choose a reason for hiding this comment

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

The proxmox_query is really only meant to be a diagnostic module (i.e. a module for collecting facts from the cluster to support other tasks), and not one for configuring PVE. And given there appear to already be set /cluster/sdn/?/? calls in proxmox_sdn.py, I'm not sure why the set /cluster/sdn is being separated out of it in 6a4f28c.

There also doesn't appear to be a way to use the proxmox_sdn module via the role in the current changeset. Please add the relevant task for managing it via role variables and documentation in the README. Since this is a big feature it should probably have its own section. See a42e6b5 for a decent (but simpler than this) example of adding a new management module.

@Derioss
Copy link
Author

Derioss commented Aug 6, 2025

I admit I've enhanced the query module because I use it for other purposes (we have specific customized tasks files), but you're right that it's not logical to integrate the SDN application within the module itself. I should have a time slot available for testing in early September.

@Derioss
Copy link
Author

Derioss commented Aug 6, 2025

And ensure that the PR matches the required logic. Thanks for your's work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants