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

New module to install images on Cisco FTD devices #53467

Open
wants to merge 10 commits into
base: devel
from

Conversation

@annikulin
Copy link
Contributor

annikulin commented Mar 7, 2019

SUMMARY

This PR adds a new ftd_install provisioning module for FTD devices that installs ROMMON image (if needed) and FTD pkg image on hardware devices.

NOTICE: the ftd_install module relies on the kick library that is about to be open-sourced and published on PyPi.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME
  • ftd_install
ADDITIONAL INFORMATION
ansible 2.8.0
@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 7, 2019

@annikulin, just so you are aware we have a dedicated Working Group for network.
You can find other people interested in this in #ansible-network on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@ansibot

This comment was marked as resolved.

Copy link
Contributor

ansibot commented Mar 7, 2019

The test ansible-test sanity --test import --python 2.6 [explain] failed with 2 errors:

lib/ansible/module_utils/network/ftd/device.py:21:0: ImportError: No module named enum
lib/ansible/modules/network/ftd/ftd_install.py:190:0: ImportError: No module named enum

The test ansible-test sanity --test import --python 2.7 [explain] failed with 2 errors:

lib/ansible/module_utils/network/ftd/device.py:21:0: ImportError: No module named enum
lib/ansible/modules/network/ftd/ftd_install.py:190:0: ImportError: No module named enum

The test ansible-test sanity --test shebang [explain] failed with 3 errors:

lib/ansible/module_utils/network/ftd/device.py:0:0: should not have a shebang
test/units/module_utils/network/ftd/test_device.py:1:1: unexpected non-module shebang: b'#!/usr/bin/python'
test/units/modules/network/ftd/test_ftd_install.py:1:1: unexpected non-module shebang: b'#!/usr/bin/python'

The test ansible-test sanity --test validate-modules [explain] failed with 41 errors:

lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.console_ip.type: not a valid value for dictionary value @ data['options']['console_ip']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.console_password.type: not a valid value for dictionary value @ data['options']['console_password']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.console_port.type: not a valid value for dictionary value @ data['options']['console_port']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.console_username.type: not a valid value for dictionary value @ data['options']['console_username']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_gateway.type: not a valid value for dictionary value @ data['options']['device_gateway']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_hostname.type: not a valid value for dictionary value @ data['options']['device_hostname']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_ip.type: not a valid value for dictionary value @ data['options']['device_ip']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_model.type: not a valid value for dictionary value @ data['options']['device_model']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_netmask.type: not a valid value for dictionary value @ data['options']['device_netmask']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_new_password.type: not a valid value for dictionary value @ data['options']['device_new_password']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_password.type: not a valid value for dictionary value @ data['options']['device_password']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_sudo_password.type: not a valid value for dictionary value @ data['options']['device_sudo_password']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_username.type: not a valid value for dictionary value @ data['options']['device_username']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.dns_server.type: not a valid value for dictionary value @ data['options']['dns_server']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.force_install.type: not a valid value for dictionary value @ data['options']['force_install']['type']. Got 'boolean'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.image_file_location.type: not a valid value for dictionary value @ data['options']['image_file_location']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.image_version.type: not a valid value for dictionary value @ data['options']['image_version']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.rommon_file_location.type: not a valid value for dictionary value @ data['options']['rommon_file_location']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.search_domains.type: not a valid value for dictionary value @ data['options']['search_domains']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E319 RETURN.msg.type: not a valid value for dictionary value @ data['msg']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E322 Argument 'force_reinstall' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E323 Argument 'force_install' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'console_ip' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'console_password' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'console_port' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'console_username' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_gateway' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_hostname' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_ip' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_model' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_netmask' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_new_password' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_password' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_sudo_password' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_username' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'dns_server' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'image_file_location' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'image_version' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'rommon_file_location' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'search_domains' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E326 Argument 'device_model' in argument_spec defines choices as (['Cisco ASA5506-X Threat Defense', 'Cisco ASA5508-X Threat Defense', 'Cisco ASA5516-X Threat Defense', 'Cisco Firepower 2110 Threat Defense', 'Cisco Firepower 2120 Threat Defense', 'Cisco Firepower 2130 Threat Defense', 'Cisco Firepower 2140 Threat Defense']) but documentation defines choices as ([])

click here for bot help

@ansibot ansibot added needs_revision and removed core_review labels Mar 7, 2019

@ansibot

This comment was marked as resolved.

Copy link
Contributor

ansibot commented Mar 10, 2019

The test ansible-test sanity --test import --python 2.6 [explain] failed with 2 errors:

lib/ansible/module_utils/network/ftd/device.py:21:0: ImportError: No module named enum
lib/ansible/modules/network/ftd/ftd_install.py:190:0: ImportError: No module named enum

The test ansible-test sanity --test import --python 2.7 [explain] failed with 2 errors:

lib/ansible/module_utils/network/ftd/device.py:21:0: ImportError: No module named enum
lib/ansible/modules/network/ftd/ftd_install.py:190:0: ImportError: No module named enum

The test ansible-test sanity --test shebang [explain] failed with 3 errors:

lib/ansible/module_utils/network/ftd/device.py:0:0: should not have a shebang
test/units/module_utils/network/ftd/test_device.py:1:1: unexpected non-module shebang: b'#!/usr/bin/python'
test/units/modules/network/ftd/test_ftd_install.py:1:1: unexpected non-module shebang: b'#!/usr/bin/python'

The test ansible-test sanity --test validate-modules [explain] failed with 41 errors:

lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.console_ip.type: not a valid value for dictionary value @ data['options']['console_ip']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.console_password.type: not a valid value for dictionary value @ data['options']['console_password']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.console_port.type: not a valid value for dictionary value @ data['options']['console_port']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.console_username.type: not a valid value for dictionary value @ data['options']['console_username']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_gateway.type: not a valid value for dictionary value @ data['options']['device_gateway']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_hostname.type: not a valid value for dictionary value @ data['options']['device_hostname']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_ip.type: not a valid value for dictionary value @ data['options']['device_ip']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_model.type: not a valid value for dictionary value @ data['options']['device_model']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_netmask.type: not a valid value for dictionary value @ data['options']['device_netmask']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_new_password.type: not a valid value for dictionary value @ data['options']['device_new_password']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_password.type: not a valid value for dictionary value @ data['options']['device_password']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_sudo_password.type: not a valid value for dictionary value @ data['options']['device_sudo_password']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.device_username.type: not a valid value for dictionary value @ data['options']['device_username']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.dns_server.type: not a valid value for dictionary value @ data['options']['dns_server']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.force_install.type: not a valid value for dictionary value @ data['options']['force_install']['type']. Got 'boolean'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.image_file_location.type: not a valid value for dictionary value @ data['options']['image_file_location']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.image_version.type: not a valid value for dictionary value @ data['options']['image_version']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.rommon_file_location.type: not a valid value for dictionary value @ data['options']['rommon_file_location']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.search_domains.type: not a valid value for dictionary value @ data['options']['search_domains']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E319 RETURN.msg.type: not a valid value for dictionary value @ data['msg']['type']. Got 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E322 Argument 'force_reinstall' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E323 Argument 'force_install' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'console_ip' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'console_password' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'console_port' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'console_username' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_gateway' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_hostname' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_ip' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_model' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_netmask' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_new_password' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_password' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_sudo_password' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'device_username' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'dns_server' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'image_file_location' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'image_version' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'rommon_file_location' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'search_domains' in argument_spec defines type as 'str' but documentation defines type as 'string'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E326 Argument 'device_model' in argument_spec defines choices as (['Cisco ASA5506-X Threat Defense', 'Cisco ASA5508-X Threat Defense', 'Cisco ASA5516-X Threat Defense', 'Cisco Firepower 2110 Threat Defense', 'Cisco Firepower 2120 Threat Defense', 'Cisco Firepower 2130 Threat Defense', 'Cisco Firepower 2140 Threat Defense']) but documentation defines choices as ([])

click here for bot help

@ansibot ansibot removed the ci_verified label Mar 11, 2019

@ansibot

This comment was marked as resolved.

Copy link
Contributor

ansibot commented Mar 11, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/network/ftd/ftd_install.py:0:0: E305 DOCUMENTATION.options.force_install.type: not a valid value for dictionary value @ data['options']['force_install']['type']. Got 'boolean'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E325 Argument 'force_install' in argument_spec defines type as 'bool' but documentation defines type as 'boolean'
lib/ansible/modules/network/ftd/ftd_install.py:0:0: E326 Argument 'device_model' in argument_spec defines choices as (['Cisco Firepower 2110 Threat Defense', 'Cisco Firepower 2120 Threat Defense', 'Cisco Firepower 2130 Threat Defense', 'Cisco Firepower 2140 Threat Defense', 'Cisco ASA5506-X Threat Defense', 'Cisco ASA5508-X Threat Defense', 'Cisco ASA5516-X Threat Defense', "<bound method FtdModel.supported_models of <class 'ansible.module_utils.network.ftd.device.FtdModel'>>"]) but documentation defines choices as (['Cisco ASA5506-X Threat Defense', 'Cisco ASA5508-X Threat Defense', 'Cisco ASA5516-X Threat Defense', 'Cisco Firepower 2110 Threat Defense', 'Cisco Firepower 2120 Threat Defense', 'Cisco Firepower 2130 Threat Defense', 'Cisco Firepower 2140 Threat Defense'])

click here for bot help

annikulin added some commits Mar 11, 2019

@ansibot ansibot added core_review and removed needs_revision labels Mar 11, 2019

@rcarrillocruz rcarrillocruz requested a review from justjais Mar 13, 2019

FTD_2110 = 'Cisco Firepower 2110 Threat Defense'
FTD_2120 = 'Cisco Firepower 2120 Threat Defense'
FTD_2130 = 'Cisco Firepower 2130 Threat Defense'
FTD_2140 = 'Cisco Firepower 2140 Threat Defense'

This comment has been minimized.

@kbreit

kbreit Mar 17, 2019

Contributor

Does this support 4100s and 9000 series appliances?

This comment has been minimized.

@annikulin

annikulin Mar 18, 2019

Author Contributor

Not as of now. We have tested the ftd_install module to work correctly with 2100/5500 series appliances so far. 4100 and 9000 are next on our backlog list, but their support must be added in a separate PR.

the `local` connection should be used only when the device cannot be accessed via
REST API.
version_added: "2.8"
requirements: [ "python >= 3.5", "kick" ]

This comment has been minimized.

@kbreit

kbreit Mar 17, 2019

Contributor

Does Ansible require support for Python 2.7 as well? If so, how would that impact this module?

This comment has been minimized.

@annikulin

annikulin Mar 19, 2019

Author Contributor

This module depends on the unicon library for connecting to the device and interacting with it. Unfortunately, unicon is available for Python 3.5+ only, so this module cannot be used with Python 2.7.
I checked that other modules (for example, ios_static_route have a Python 3+ requirement, so put it here the same way for now.

This comment has been minimized.

@ganeshrn

ganeshrn Mar 26, 2019

Member

The ios_static_route module works with Python 2.7 version as well. It seems the ipaddress python module is shipped with Python 3.3 version onwards, that's the reason for the explicit mention in the requirement.

Minimum python version supported on the control node is 2.7 https://docs.ansible.com/ansible/latest/installation_guide/intro_installation.html#control-machine-requirements

Generally, the module running on a control node is expected to support Python 2.7 version as well, not sure if currently there is an exception to this requirement.

This comment has been minimized.

@ganeshrn

ganeshrn Mar 26, 2019

Member

Please ignore my above comment.
As per @abadger the syntax of the module should compatible with python-2.7 but can be documented as not running under other versions (when there is a necessary library which doesn't support it)

Also, @jborean93 pointed out that the module can run under a different python on localhost so it's still possible to run Ansible on 2.7 but use a python 3 interpreter for a specific module.
eg:

- mymodule:
  vars:
    ansible_python_interpreter: /usr/bin/python3
requirements: [ "python >= 3.5", "kick" ]
author: "Cisco Systems, Inc. (@annikulin)"
options:
device_hostname:

This comment has been minimized.

@kbreit

kbreit Mar 17, 2019

Contributor

Should some of these parameters be located in the module utility and shared with other FTD modules?

This comment has been minimized.

@annikulin

annikulin Mar 19, 2019

Author Contributor

Good point, but currently we don't have other FTD modules with similar parameters. The rest of FTD modules (e.g., ftd_configuration, ftd_file_download, ftd_file_upload) use the Ansible HTTP API connection with the FTD plugin, so username and password are specified as a part of inventory file.
ftd_install, on the contrary, has them as module params, because it should do image installation even on devices without API (e.g., when the device is broken or the previous image didn't have API exposed). Does it make sense?

This comment has been minimized.

@kbreit

kbreit Mar 19, 2019

Contributor

That does make sense. Thanks.

@kbreit

This comment has been minimized.

Copy link
Contributor

kbreit commented Mar 17, 2019

@dagwieers FTD is a Cisco product. You may want to look into getting these labeled automatically.

@kbreit

This comment has been minimized.

Copy link
Contributor

kbreit commented Mar 17, 2019

+label cisco

@bildastack

This comment has been minimized.

Copy link

bildastack commented Mar 17, 2019

force_install=dict(type='bool', required=False, default=False)
)
module = AnsibleModule(argument_spec=fields)
if not HAS_KICK:

This comment has been minimized.

@justjais

justjais Mar 19, 2019

Contributor

This exception can be handled in module_utils code itself from init method:

Suggested change
if not HAS_KICK:
if not HAS_KICK:
raise Exception('Kick Python module is required to run this module. Please, install it with `pip install kick` command and run the playbook again.')

This comment has been minimized.

@annikulin

annikulin Mar 21, 2019

Author Contributor

Do you mean I should rather throw an exception explicitly instead of calling module.fail_json? I just checked other modules (e.g., github_release or gcp) and followed the same approach.

This comment has been minimized.

@justjais

justjais Mar 25, 2019

Contributor

@annikulin No, from the above comment I meant was that you shall handle respective scenario in module_utils code rather than module code, this way if the same functionality is needed by any future ftd resource module code won't be duplicated (e.g.: nios_api, panos).

This comment has been minimized.

@annikulin

annikulin Mar 25, 2019

Author Contributor

@justjais, ok, that makes sense. I moved the part checking that the library is installed to module_utils. I also passed a module to this method, so that we invoke fail_json and get a short and descriptive error message instead of raising an exception and handling it in every module.

description:
- Provisioning module for FTD devices that installs ROMMON image (if needed) and
FTD pkg image on the firewall.
- Can be used with `httpapi` and `local` connection types. The `httpapi` is preferred,

This comment has been minimized.

@justjais

justjais Mar 19, 2019

Contributor

local connection shall be deprecated in Ansible future release version, so it's better if we can remove it from new module PRs.

This comment has been minimized.

@kbreit

kbreit Mar 19, 2019

Contributor

@justjais Is httpapi the recommended method for requests instead of local in the future?

This comment has been minimized.

@annikulin

annikulin Mar 19, 2019

Author Contributor

@justjais, is delegate_to: localhost supposed to be its replacement when local connection is deprecated (for example, when API is not accessible and httpapi connection cannot be used because of this)?

This comment has been minimized.

@justjais

justjais Mar 20, 2019

Contributor

@kbreit yes, httpapi connection is preferred over local, n that's why in network platform other connection plugins like: network_cli/netconf along with httpapi were introduced.

@annikulin delegate_to: localhost and connection: local with respect to network means the same, also if I understand your example with respect to API not accessible correctly then u meant was if the server is inaccessible and if that's the case then even local connection won't work coz all these connection plugins runs the module locally only.

This comment has been minimized.

@annikulin

annikulin Mar 20, 2019

Author Contributor

@justjais, yes, and in case the API is not accessible, the playbook execution will fail even before executing the first task (when establishing the connection). In this case, we would still like to connect to the device and execute commands via console server.

What type of connection is recommended in this case? Should it be still httpapi with delegate_to: localhost option?

This comment has been minimized.

@ganeshrn

ganeshrn Mar 25, 2019

Member

To add to @justjais's comment, we plan to deprecate connection=local for network platforms that are supported by Ansible Network team (ios, eos, junos, iosxr, nxos, vyos) in favor of first-class network connection plugins (network_cli/httpapi/netconf) support added in Ansible 2.6 version.

For new network platform support added by the community, it is recommended to use first class network connection plugin wherever applicable.

If the connection to console happens over ssh you can use connection=network_cli in that case. This connection creates the pseudo terminal over ssh to talk to remote host Refer network_cli

For all the network connection plugins the module runs locally on control node itself, hence delegate_to: localhost with httpapi connection won't have any effect (unless jumphost is used)

This comment has been minimized.

@annikulin

annikulin Mar 25, 2019

Author Contributor

@ganeshrn, @justjais, thanks for your comments. Consolidating network modules and making them support network connection plugins only sounds reasonable.

As of now, we've used the unicon library developed by Cisco to connect to the device over SSH and execute commands. Having taken a brief look at network_cli connection, the ideas look similar but the approach is a bit different: unicon has a concept of dialogs defining a sequence of commands you want to execute, a sequence of outputs you expect to receive and behavior of what to do when the command fails or times out. I'm pretty sure we can implement similar functionality with the help of network_cli, but it requires thorough investigation and planning together with you guys as owners of this plugin.

Considering that this module is already useful for FTD customers, would it make sense to publish this module as is (with dependency on unicon library and support for local connection type) as a short term solution and plan how to replace it with network_cli connection as a long-term plan and do it as a part of the next release?

This comment has been minimized.

@ganeshrn

ganeshrn Mar 26, 2019

Member

would it make sense to publish this module as is (with dependency on unicon library and support for local connection type) as a short term solution and plan how to replace it with network_cli connection as a long-term plan and do it as a part of the next release?

Yes, that should work IMO. FYI Ansible follows 4 version deprecation cycle that is if the code (options) available in a stable release is deprecated it can be removed after four releases, typical Ansible release cycle is around 4 to 6 months.

msg='Successfully installed FTD image %s on the firewall device.' % module.params["image_version"])


def check_required_params_for_local_connection(module, params):

This comment has been minimized.

@justjais

justjais Mar 19, 2019

Contributor

Similar to local connection comment.

module.fail_json(msg=message)


def get_system_info(resource):

This comment has been minimized.

@justjais

justjais Mar 19, 2019

Contributor

Resource module should be as minimalist as possible, so just wanted to confirm if these following respective functions functionality can be handled from module_utils code? Also, if the same functionality is needed by any other resource modules we'll not duplicate the code efforts.

This comment has been minimized.

@annikulin

annikulin Mar 21, 2019

Author Contributor

Good point. I moved FtdOperations and get_system_info to module_utils.

@dagwieers

This comment has been minimized.

Copy link
Member

dagwieers commented Mar 19, 2019

@kbreit See PR #53654

I added a new label ftd for these modules and I added a $team_ftd to the BOTMETA.yml file. Please let me know if you want to make any edits so I can update my PR. If you prefer firepower as the label/team name let me know.

If you like we can also create a community at https://github.com/ansible/community/wiki for Cisco FTD where information/roadmap etc. can be shared.

@ansibot ansibot added needs_revision and removed core_review labels Mar 19, 2019

@kbreit

This comment has been minimized.

Copy link
Contributor

kbreit commented Mar 19, 2019

@annikulin Do you have a preference whether we call it ftd or firepower for labels?

@annikulin

This comment has been minimized.

Copy link
Contributor Author

annikulin commented Mar 19, 2019

@kbreit, FTD might be a bit better as it emphasises that these modules do not work with FMC that is related to Firepower, but in the end, both are fine with me :)

@kbreit

This comment has been minimized.

Copy link
Contributor

kbreit commented Mar 19, 2019

FTD it is!

@ansibot ansibot removed the stale_ci label Mar 21, 2019

annikulin added some commits Mar 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.