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

nmcli: Add macvlan connection type support #6312

Merged
merged 7 commits into from
Apr 16, 2023
Merged

nmcli: Add macvlan connection type support #6312

merged 7 commits into from
Apr 16, 2023

Conversation

psvmcc
Copy link
Contributor

@psvmcc psvmcc commented Apr 10, 2023

SUMMARY

Add macvlan connection type support to nmcli module

Fixes #5316

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

nmcli

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request module module net_tools new_contributor Help guide this first time contributor plugins plugin (any type) tests tests unit tests/unit labels Apr 10, 2023
@psvmcc psvmcc changed the title add nmcli macvlan type nmcli: Add macvlan connection type support Apr 10, 2023
@ansibullbot ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Apr 10, 2023
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Apr 10, 2023
@github-actions
Copy link

github-actions bot commented Apr 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 @psvmcc Thanks for your contribution!

A couple of comments though.

plugins/modules/nmcli.py Outdated Show resolved Hide resolved
plugins/modules/nmcli.py Show resolved Hide resolved
plugins/modules/nmcli.py Outdated Show resolved Hide resolved
@ansibullbot
Copy link
Collaborator

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/nmcli.py:2407:25: E122: continuation line missing indentation or outdented

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/nmcli.py:2407:25: E122: continuation line missing indentation or outdented

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/nmcli.py:2407:25: E122: continuation line missing indentation or outdented

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/nmcli.py:2407:25: E122: continuation line missing indentation or outdented

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/nmcli.py:2407:25: E122: continuation line missing indentation or outdented

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

plugins/modules/nmcli.py:0:0: doc-choices-do-not-match-spec: Argument 'mode' in argument_spec found in macvlan defines choices as ([]) but documentation defines choices as ([0, 1, 2, 3, 4, 5])

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

plugins/modules/nmcli.py:0:0: doc-choices-do-not-match-spec: Argument 'mode' in argument_spec found in macvlan defines choices as ([]) but documentation defines choices as ([0, 1, 2, 3, 4, 5])

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

plugins/modules/nmcli.py:0:0: doc-choices-do-not-match-spec: Argument 'mode' in argument_spec found in macvlan defines choices as ([]) but documentation defines choices as ([0, 1, 2, 3, 4, 5])

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

plugins/modules/nmcli.py:0:0: doc-choices-do-not-match-spec: Argument 'mode' in argument_spec found in macvlan defines choices as ([]) but documentation defines choices as ([0, 1, 2, 3, 4, 5])

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

plugins/modules/nmcli.py:0:0: doc-choices-do-not-match-spec: Argument 'mode' in argument_spec found in macvlan defines choices as ([]) but documentation defines choices as ([0, 1, 2, 3, 4, 5])

click here for bot help

@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 Apr 11, 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 Apr 11, 2023
@russoz
Copy link
Collaborator

russoz commented Apr 11, 2023

maybe we could use required_if=('state', 'macvlan', ['macvlan']) in the module definition?

@russoz russoz closed this Apr 11, 2023
@russoz russoz reopened this Apr 11, 2023
@russoz
Copy link
Collaborator

russoz commented Apr 11, 2023

oopsie, closed by mistake

@psvmcc
Copy link
Contributor Author

psvmcc commented Apr 11, 2023

maybe we could use required_if=('state', 'macvlan', ['macvlan']) in the module definition?

@russoz if I correctly understand your idea, you suggest do next thing:

        mutually_exclusive=[['never_default4', 'gw4'],
                            ['routes4_extended', 'routes4'],
                            ['routes6_extended', 'routes6']],
-       required_if=[("type", "wifi", [("ssid")])],
+       required_if=[("type", "wifi", [("ssid")]),
+                    ("type", "macvlan", ["macvlan"])],
        supports_check_mode=True,

But in this case for absent state, will be required to set macvlan dict also, I think this is not good idea.

Or I'm wrong?

@russoz
Copy link
Collaborator

russoz commented Apr 11, 2023

maybe we could use required_if=('state', 'macvlan', ['macvlan']) in the module definition?

@russoz if I correctly understand your idea, you suggest do next thing:

        mutually_exclusive=[['never_default4', 'gw4'],
                            ['routes4_extended', 'routes4'],
                            ['routes6_extended', 'routes6']],
-       required_if=[("type", "wifi", [("ssid")])],
+       required_if=[("type", "wifi", [("ssid")]),
+                    ("type", "macvlan", ["macvlan"])],
        supports_check_mode=True,

But in this case for absent state, will be required to set macvlan dict also, I think this is not good idea.

Or I'm wrong?

No, you're right on spot. But then, it would be nice to have a neat message coming back to the user, in case they fail to provide the parameter - instead of an exception popping up being silently ignored.

@psvmcc
Copy link
Contributor Author

psvmcc commented Apr 12, 2023

maybe we could use required_if=('state', 'macvlan', ['macvlan']) in the module definition?

@russoz if I correctly understand your idea, you suggest do next thing:

        mutually_exclusive=[['never_default4', 'gw4'],
                            ['routes4_extended', 'routes4'],
                            ['routes6_extended', 'routes6']],
-       required_if=[("type", "wifi", [("ssid")])],
+       required_if=[("type", "wifi", [("ssid")]),
+                    ("type", "macvlan", ["macvlan"])],
        supports_check_mode=True,

But in this case for absent state, will be required to set macvlan dict also, I think this is not good idea.
Or I'm wrong?

No, you're right on spot. But then, it would be nice to have a neat message coming back to the user, in case they fail to provide the parameter - instead of an exception popping up being silently ignored.

Can you please check now, I just added raise error, if required params not set.

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.

LGTM

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-6 labels Apr 13, 2023
@felixfontein felixfontein merged commit 29a7d24 into ansible-collections:main Apr 16, 2023
144 checks passed
@patchback
Copy link

patchback bot commented Apr 16, 2023

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/29a7d24d758869dab67cdff5443fae2fb8883ac3/pr-6312

Backported as #6341

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

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Apr 16, 2023
patchback bot pushed a commit that referenced this pull request Apr 16, 2023
* add nmcli macvlan type

* changelog

* improve docs

* macvlan params

* fix linter and improve module params

* improve_docs

* raise error if type macvlan and macvlan options not set

(cherry picked from commit 29a7d24)
@felixfontein
Copy link
Collaborator

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

felixfontein pushed a commit that referenced this pull request Apr 16, 2023
…type support (#6341)

nmcli: Add macvlan connection type support (#6312)

* add nmcli macvlan type

* changelog

* improve docs

* macvlan params

* fix linter and improve module params

* improve_docs

* raise error if type macvlan and macvlan options not set

(cherry picked from commit 29a7d24)

Co-authored-by: Sergey Putko <mail@psvlan.com>
@ganniterix
Copy link

ganniterix commented Apr 17, 2023

Thanks for this contribution! @psvmcc

@psvmcc psvmcc deleted the nmcli_macvlan branch April 19, 2023 06:36
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Apr 26, 2023
v7.5.0

Minor Changes
-------------

ansible.posix
~~~~~~~~~~~~~

- Add jsonl callback plugin to ansible.posix collection
- firewalld - add `protocol` parameter

cisco.aci
~~~~~~~~~

- Add aci_access_span_dst_group module for fabric access policies span destination group support
- Add aci_access_span_filter_group and aci_access_span_filter_group_entry modules for access span filter group support
- Add aci_config_export_policy module
- Add aci_igmp_interface_policy module
- Add aci_interface_config module for new interface configuration available in ACI v5.2(5)+
- Add aci_interface_policy_spanning_tree  module

cisco.dnac
~~~~~~~~~~

- accesspoint_configuration_details_by_task_id_info - new module
- authentication_policy_servers_info - new module
- credential_to_site_by_siteid_create_v2 - new module
- device_interface_info - attributes `lastInputTime` and `lastOutputTime` were added.
- device_reboot_apreboot_info - new module
- dnac_packages_info - new module
- eox_status_device_info - new module
- eox_status_summary_info - new module
- event_email_config - new module
- event_email_config_info - new module
- event_snmp_config_info - new module
- event_syslog_config - new module
- event_syslog_config_info - new module
- execute_suggested_actions_commands - new module
- global_credential_v2 - new module
- global_credential_v2_info - new module
- integration_settings_instances_itsm - new module
- integration_settings_instances_itsm_info - new module
- lan_automation_log_by_serial_number_info - new module
- network_device_user_defined_field - new module
- network_device_user_defined_field_info - new module
- network_v2 - new module
- network_v2_info - new module
- pnp_device_claim_to_site - attributes `removeInactive` and `hostname` were removed.
- role_permissions_info - new module
- roles_info - new module
- sda_fabric_border_device - attributes `routeDistributionProtocol` and `borderPriority` were added.
- sda_fabric_control_plane_device attribute `routeDistributionProtocol` was added.
- sda_fabric_edge_device - attribute `siteNameHierarchy` was added.
- sda_fabric_site - attribute `fabricType` was added.
- sda_port_assignment_for_user_device - attribute `interfaceNames` was added.
- sda_virtual_network - attribute `vManageVpnId` was added.
- sda_virtual_network_ip_pool - attribute `isBridgeModeVm` was added.
- sda_virtual_network_v2 - attribute `isBridgeModeVm` was added.
- service_provider_v2 - new module
- service_provider_v2_info - new module
- sp_profile_delete_v2 - new module
- user - new module
- user_info - new module
- users_external_servers_info - new module
- wireless_accespoint_configuration - new module
- wireless_accesspoint_configuration_summary_info - new module

cisco.ios
~~~~~~~~~

- ios_bgp_address_family - add option redistribute.ospf.include_connected when redistributing OSPF in IPv6 AFI
- ios_bgp_address_family - add option redistribute.ospf.match.externals.type_1 to allow
- ios_bgp_address_family - add option redistribute.ospf.match.externals.type_2 to allow
- specification of OSPF E1 routes
- specification of OSPF E2 routes

cisco.mso
~~~~~~~~~

- Add ip_data_plane_learning and preferred_group arguments to mso_schema_template_vrf module
- Add module mso_schema_site_anp_epg_bulk_staticport
- Add route_reachability attribute to mso_schema_site_external_epg module

cisco.nxos
~~~~~~~~~~

- `nxos_route_maps` - add support for 'set ip next-hop <>' command in route-maps
- `nxos_vxlan_vtep` - add support for 'advertise virtual-rmac' command under nve interface

community.crypto
~~~~~~~~~~~~~~~~

- get_certificate - add ``asn1_base64`` option to control whether the ASN.1 included in the ``extensions`` return value is binary data or Base64 encoded (ansible-collections/community.crypto#592).

community.general
~~~~~~~~~~~~~~~~~

- cpanm - minor change, use feature from ``ModuleHelper`` (ansible-collections/community.general#6385).
- dconf - be forgiving about boolean values: convert them to GVariant booleans automatically (ansible-collections/community.general#6206).
- dconf - minor refactoring improving parameters and dependencies validation (ansible-collections/community.general#6336).
- deps module utils - add function ``failed()`` providing the ability to check the dependency check result without triggering an exception (ansible-collections/community.general#6383).
- dig lookup plugin - Support multiple domains to be queried as indicated in docs (ansible-collections/community.general#6334).
- gitlab_project - add new option ``topics`` for adding topics to GitLab projects (ansible-collections/community.general#6278).
- homebrew_cask - allows passing ``--greedy`` option to ``upgrade_all`` (ansible-collections/community.general#6267).
- idrac_redfish_command - add ``job_id`` to ``CreateBiosConfigJob`` response (ansible-collections/community.general#5603).
- ipa_hostgroup - add ``append`` parameter for adding a new hosts to existing hostgroups without changing existing hostgroup members (ansible-collections/community.general#6203).
- keycloak_authentication - add flow type option to sub flows to allow the creation of 'form-flow' sub flows like in Keycloak's built-in registration flow (ansible-collections/community.general#6318).
- mksysb - improved the output of the module in case of errors (ansible-collections/community.general#6263).
- nmap inventory plugin - added environment variables for configure ``address`` and ``exclude`` (ansible-collections/community.general#6351).
- nmcli - add ``macvlan`` connection type (ansible-collections/community.general#6312).
- pipx - add ``system_site_packages`` parameter to give application access to system-wide packages (ansible-collections/community.general#6308).
- pipx - ensure ``include_injected`` parameter works with ``state=upgrade`` and ``state=latest`` (ansible-collections/community.general#6212).
- puppet - add new options ``skip_tags`` to exclude certain tagged resources during a puppet agent or apply (ansible-collections/community.general#6293).
- terraform - remove state file check condition and error block, because in the native implementation of terraform will not cause errors due to the non-existent file (ansible-collections/community.general#6296).
- udm_dns_record - minor refactor to the code (ansible-collections/community.general#6382).

community.zabbix
~~~~~~~~~~~~~~~~

- httpapi plugin - updated to work with Zabbix 6.4.
- zabbix_action, zabbix_authentication, zabbix_discovery_rule, zabbix_mediatype, zabbix_user, zabbix_user_directory, zabbix_usergroup - updated to work with Zabbix 6.4.
- zabbix_agent role - Add support for SUSE Linux Enterprise Server for SAP Applications ("SLES_SAP").
- zabbix_host - add missing variants for SNMPv3 authprotocol and privprotocol introduced by Zabbix 6
- zabbix_proxy role - Add variable zabbix_proxy_dbpassword_hash_method to control whether you want postgresql user password to be hashed with md5 or want to use db default. When zabbix_proxy_dbpassword_hash_method is set to anything other than md5 then do not hash the password with md5 so you could use postgresql scram-sha-256 hashing method.
- zabbix_server role - Add variable zabbix_server_dbpassword_hash_method to control whether you want postgresql user password to be hashed with md5 or want to use db default. When zabbix_server_dbpassword_hash_method is set to anything other than md5 then do not hash the password with md5 so you could use postgresql scram-sha-256 hashing method.
- zabbix_usergroup module - userdirectory, hostgroup_rights and templategroup_rights parameters added (Zabbix >= 6.2)
- zabbix_web role - possibility to add custom includes in apache vhost config

dellemc.powerflex
~~~~~~~~~~~~~~~~~

- Info module is enhanced to support the listing of replication pairs.

dellemc.unity
~~~~~~~~~~~~~

- Add synchronous replication support for filesystem.
- Support addition of host from the Host List to NFS Export in nfs module.
- Support enable/disable advanced dedup in volume module.

hetzner.hcloud
~~~~~~~~~~~~~~

- hcloud_image_info - Add cpu architecture field to return value.
- hcloud_image_info - Allow filtering images by cpu architecture.
- hcloud_server - Select matching image for the cpu architecture of the server type on create & rebuild.
- hcloud_server_type_info - Add cpu architecture field to return value.
- inventory plugin - Add cpu architecture to server variables.

netapp.ontap
~~~~~~~~~~~~

- na_ontap_cifs - new options ``browsable`` and ``show_previous_versions`` added in REST.
- na_ontap_cifs - removed default value for ``unix_symlink`` as its not supported with ZAPI.
- na_ontap_cifs - updated documentation and examples for REST.
- na_ontap_file_security_permissions - updated module examples.
- na_ontap_ipspace - improved module fail error message in REST.
- na_ontap_rest_info - improved documentation for ``parameters`` option.
- na_ontap_security_config - updated documentation for ``supported_cipher_suites``.
- na_ontap_user - option ``vserver`` is not required with REST, ignore this option to create cluster scoped user.

netbox.netbox
~~~~~~~~~~~~~

- netbox_aggregate - Add tenant as parameter to module
- netbox_asn - Add module
- netbox_fhrp_group - Add module
- netbox_journal_entry - Add module

purestorage.flashblade
~~~~~~~~~~~~~~~~~~~~~~

- purefb_info - Added `encryption` and `support_keys` information.
- purefb_info - Added bucket quota and safemode information per bucket
- purefb_info - Added security update version for Purity//FB 4.0.2, or higher
- purefb_info - Updated object store account information
- purefb_inventory - Added `part_number` to hardware item information.
- purefb_policy - Added support for multiple rules in snapshot policies
- purefb_proxy - Added new boolean parameter `secure`. Default of true (for backwards compatability) sets the protocol to be `https://`. False sets `http://`
- purefb_s3acc - Added support for default bucket quotas and hard limits
- purefb_s3acc - Added support for object account quota and hard limit

purestorage.fusion
~~~~~~~~~~~~~~~~~~

- added Python package dependency checks in prerequisites.py
- fusion_hap - added missing 'windows' personality type

theforeman.foreman
~~~~~~~~~~~~~~~~~~

- content_export_library, content_export_repository, content_export_version - add ``format`` option to control the export format
- content_view_filter - add support for creating modulemd filters
- content_view_publish role - also accept a list of dicts as the ``content_views`` role for publishing (theforeman/foreman-ansible-modules#1436)
- setting - document how to obtain valid setting names (https://bugzilla.redhat.com/show_bug.cgi?id=2174367)

Deprecated Features
-------------------

cisco.ios
~~~~~~~~~

- ios_bgp_address_family - deprecate redistribute.ospf.match.external with redistribute.ospf.match.externals which enables attributes for OSPF type E1 and E2 routes
- ios_bgp_address_family - deprecate redistribute.ospf.match.nssa_external with redistribute.ospf.match.nssa_externals which enables attributes for OSPF type N1 and N2 routes
- ios_bgp_address_family - deprecate redistribute.ospf.match.type_1 with redistribute.ospf.match.nssa_externals.type_1
- ios_bgp_address_family - deprecate redistribute.ospf.match.type_2 with redistribute.ospf.match.nssa_externals.type_2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request module module net_tools new_contributor Help guide this first time contributor plugins plugin (any type) tests tests unit tests/unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for interfaces of type MACVLAN using nmcli module.
5 participants