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

Feature: Add Support for LAGG Interfaces #83

Merged
merged 3 commits into from
Oct 6, 2024

Conversation

jiuka
Copy link
Contributor

@jiuka jiuka commented Aug 14, 2024

Link to API

https://docs.opnsense.org/development/api/core/interfaces.html#id3

Description

It would be nice to setup/manage the LAGG Interfaces with Ansible

@jiuka
Copy link
Contributor Author

jiuka commented Aug 14, 2024

Its the first time I try to implement a Module with the collection_opnsense Framework. It works for me currently but I am not so sure if the approach with match_fields is the correct one.

@ansibleguy
Copy link
Owner

Greetings!

@jiuka Thank you for this very clean PR!

  1. You are right - the match_fields do not make sense in that case.

The match_fields are used to match the configured entry with any one that exists on the firewall. So in practice this value should never change as you could think of it as the ID of the entry.

Fields like members, primary_member or proto are not static. They could change. We would only fall-back to using those if we have no other option. But in this case we CAN use description or device as unique ID field.
Most modules of this collection only use the description field if it is available.

As an example:

You can see - if only one ID-field is used, we do not set the match_fields but the FIELD_ID (as there is no user-choice).

In this case you would need to set the description to be a required module argument.

  1. Some issue with the module arguments:
  • Please add the flowid alias to the use_flowid argument
  • Please add the fast_timeout alias to the lacp_fast_timeout argument
  • Please add the parent alias to the members argument - as that is the name in the WebUI
  • Please add the hash and hash_layers aliases to the lagghash argument
  • As I see it - the default value of lagghash should be unset/empty as that is the default in the WebUI
  • The option default of the argument lacp_strict and use_flowid should not be an option as the option in the WebUI just sets the value to None/empty-string

@ansibleguy ansibleguy self-assigned this Aug 15, 2024
@ansibleguy ansibleguy added the enhancement New feature or request label Aug 15, 2024
@ansibleguy ansibleguy marked this pull request as ready for review October 6, 2024 12:38
@ansibleguy ansibleguy merged commit 84ca0ad into ansibleguy:latest Oct 6, 2024
1 of 2 checks passed
@jiuka jiuka deleted the lagg branch October 11, 2024 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants