New module for managing NetApp E-Series iSCSI Interfaces #39877

lmprice · 2018-05-08T23:23:47Z

Define a new module for configuring NetApp E-Series iSCSI interfaces.

SUMMARY

Introduce a new module for managing E-Series iSCSI interfaces. Each E-Series system that supports iSCSI will have between 2 and 6 iSCSI interfaces (channels), that can be configured on each controller.

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

netapp_e_iscsi_interface

ANSIBLE VERSION

ansible 2.6.0 (detached HEAD ce8c336d52) last updated 2018/04/18 11:23:03 (GMT -500)
  config file = None
  configured module search path = [u'/home/lorenp/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/lorenp/ansible/lib/ansible
  executable location = /home/lorenp/ansible/bin/ansible
  python version = 2.7.12 (default, Dec  4 2017, 14:50:18) [GCC 5.4.0 20160609]

ADDITIONAL INFORMATION

changed: [testhost] => (item=1) => {"changed": true, "enabled": true, "item": 1, "msg": "The interface settings have been updated."}

ansibot · 2018-05-08T23:33:22Z

cc @amit0701 @broncofan @hulquest @mattclay
click here for bot help

mattclay · 2018-05-09T00:12:22Z

test/integration/integration_config.yml

+netapp_e_ssid: 1
+netapp_e_api_host: 10.113.1.111:8443
+netapp_e_api_username: admin
+netapp_e_api_password: infiniti


Integration tests should not add anything to the shared integration config.

lmprice · 2018-05-10T20:04:57Z

@mattclay That issue should be resolved. Not sure how I can make the request resolved.

Review feedback addressed. I have not reviewed the rest of the changes.

hulquest

Once the question about backward compatibility is answered, this looks good to me.

hulquest · 2018-05-11T20:12:56Z

lib/ansible/modules/storage/netapp/netapp_e_iscsi_interface.py

+    def interfaces(self):
+        ifaces = list()
+        try:
+            (rc, ifaces) = request(self.url + 'storage-systems/%s/graph/xpath-filter?query=/controller/hostInterfaces'


This looks like a newer endpoint. Does the documentation need to have a minimum version of SANtricity OS?

@hulquest We added this back in the WSP 1.3 time-frame (early 2016). While it's probably been in there long enough it doesn't need to be documented (IMO), that does beg the question of how to do so for when it does. I can add a note to the module if that makes sense.

I think a brief sentence is worthy. Nothing is worse than trying something only to figure out the target API on the server isn't compatible. We should try to remember to ask this question of all new modules and substantial changes.

@hulquest I added a note in the documentation. In the next patch I submit I'll see about adding a more lengthy blurb in the doc_fragments about API requirements, etc.

@hulquest @lmprice Is this discussion resolved?

@gundalow Yes, I believe that it has. I added a comment in the notes about what API version is required for this module.

ansibot · 2018-05-18T21:19:55Z

cc @carchi8py @schmots1
click here for bot help

lmprice · 2018-05-22T18:13:05Z

@hulquest @amit0701 @schmots1 @carchi8py Can you give me a review on this patch?

hulquest · 2018-05-24T14:00:09Z

shipit

ansibot · 2018-05-24T14:11:24Z

cc @ndswartz
click here for bot help

ndswartz

shipit

gundalow · 2018-08-28T11:26:07Z

lib/ansible/utils/module_docs_fragments/netapp.py

@@ -136,7 +136,8 @@ class ModuleDocFragment(object):
    required: false


Please remove this file from the PR

gundalow · 2018-08-28T11:26:18Z

test/sanity/validate-modules/ignore.txt

@@ -1133,7 +1133,6 @@ lib/ansible/modules/storage/netapp/na_cdot_volume.py E322
 lib/ansible/modules/storage/netapp/na_cdot_volume.py E324
 lib/ansible/modules/storage/netapp/na_cdot_volume.py E325
 lib/ansible/modules/storage/netapp/netapp_e_amg.py E322
-lib/ansible/modules/storage/netapp/netapp_e_amg.py E325
 lib/ansible/modules/storage/netapp/netapp_e_amg_role.py E322


Please remove this file from the PR

gundalow · 2018-08-28T11:29:52Z

lib/ansible/modules/storage/netapp/netapp_e_iscsi_interface.py

+        description:
+            - The IPv4 address to assign to the interface.
+            - Should be specified in xx.xx.xx.xx form.
+            - Mutually exclusive with I(config_method=dhcp)


Add code to check for this and do fail_json if needed just after argument_spec

gundalow · 2018-08-28T11:29:58Z

lib/ansible/modules/storage/netapp/netapp_e_iscsi_interface.py

+        description:
+            - The subnet mask to utilize for the interface.
+            - Should be specified in xx.xx.xx.xx form.
+            - Mutually exclusive with I(config_method=dhcp)


Add code to check for this and do fail_json if needed just after argument_spec

gundalow · 2018-08-28T11:30:04Z

lib/ansible/modules/storage/netapp/netapp_e_iscsi_interface.py

+        description:
+            - The IPv4 gateway address to utilize for the interface.
+            - Should be specified in xx.xx.xx.xx form.
+            - Mutually exclusive with I(config_method=dhcp)


Add code to check for this and do fail_json if needed just after argument_spec

gundalow · 2018-08-28T11:30:13Z

lib/ansible/modules/storage/netapp/netapp_e_iscsi_interface.py

+    config_method:
+        description:
+            - The configuration method type to use for this interface.
+            - dhcp is mutually exclusive with I(address), I(subnet_mask), and I(gateway).


Add code to check for this and do fail_json if needed just after argument_spec

gundalow · 2018-08-28T11:30:57Z

lib/ansible/modules/storage/netapp/netapp_e_iscsi_interface.py

+    def interfaces(self):
+        ifaces = list()
+        try:
+            (rc, ifaces) = request(self.url + 'storage-systems/%s/graph/xpath-filter?query=/controller/hostInterfaces'


@hulquest @lmprice Is this discussion resolved?

Define a new module for configuring NetApp E-Series iSCSI interfaces.

Restructured integration test to set all iscsi ports to disabled, then defines the ports either statically or with dhcp, next updates the ports with the other definition type (static <-> dhcp), and lastly disables all ports. Each netapp_eseries_iscsi_interface call is verified with the array.

lmprice · 2018-08-28T15:29:45Z

@gundalow The issues should be resolved.

mattclay previously requested changes May 9, 2018

View reviewed changes

ansibot removed the needs_triage Needs a first human triage before being processed. label May 9, 2018

lmprice force-pushed the iscsi_interface branch from dc08d39 to 45e14ca Compare May 9, 2018 19:58

ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels May 10, 2018

hulquest reviewed May 11, 2018

View reviewed changes

lmprice force-pushed the iscsi_interface branch 2 times, most recently from 56b0ebd to fc0965e Compare May 18, 2018 21:17

ansibot added the affects_2.6 This issue/PR affects Ansible v2.6 label May 18, 2018

lmprice force-pushed the iscsi_interface branch from fc0965e to 7ea0f2c Compare May 18, 2018 21:58

carchi8py approved these changes May 22, 2018

View reviewed changes

ansibot added shipit This PR is ready to be merged by Core and removed core_review In order to be merged, this PR must follow the core review workflow. labels May 24, 2018

lmprice force-pushed the iscsi_interface branch from 7ea0f2c to dd74f2b Compare June 4, 2018 16:23

ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed shipit This PR is ready to be merged by Core labels Jun 4, 2018

ndswartz approved these changes Jun 4, 2018

View reviewed changes

ansibot added shipit This PR is ready to be merged by Core and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jun 4, 2018

carchi8py approved these changes Jun 4, 2018

View reviewed changes

mattclay mentioned this pull request Jul 17, 2018

Bot starts and then cancels CI run on PRs ansible/ansibullbot#982

Closed

gundalow requested changes Aug 28, 2018

View reviewed changes

ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed shipit This PR is ready to be merged by Core labels Aug 28, 2018

lmprice and others added 2 commits August 28, 2018 08:53

New module for NTAP E-Series iSCSI Interfaces

d98dc39

Define a new module for configuring NetApp E-Series iSCSI interfaces.

lmprice force-pushed the iscsi_interface branch from dd74f2b to cd44313 Compare August 28, 2018 15:06

ansibot removed the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Aug 28, 2018

gundalow approved these changes Aug 28, 2018

View reviewed changes

gundalow merged commit 97157cf into ansible:devel Aug 28, 2018

ansible locked and limited conversation to collaborators Jul 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New module for managing NetApp E-Series iSCSI Interfaces #39877

New module for managing NetApp E-Series iSCSI Interfaces #39877

lmprice commented May 8, 2018

ansibot commented May 8, 2018

mattclay May 9, 2018

lmprice commented May 10, 2018

hulquest left a comment

hulquest May 11, 2018

lmprice May 15, 2018

hulquest May 15, 2018

lmprice May 18, 2018

gundalow Aug 28, 2018

lmprice Aug 28, 2018

ansibot commented May 18, 2018

lmprice commented May 22, 2018

hulquest commented May 24, 2018

ansibot commented May 24, 2018

ndswartz left a comment

gundalow Aug 28, 2018

gundalow Aug 28, 2018

gundalow Aug 28, 2018

gundalow Aug 28, 2018

gundalow Aug 28, 2018

gundalow Aug 28, 2018

gundalow Aug 28, 2018

lmprice commented Aug 28, 2018

		@@ -136,7 +136,8 @@ class ModuleDocFragment(object):
		required: false

New module for managing NetApp E-Series iSCSI Interfaces #39877

New module for managing NetApp E-Series iSCSI Interfaces #39877

Conversation

lmprice commented May 8, 2018

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ADDITIONAL INFORMATION

ansibot commented May 8, 2018

Choose a reason for hiding this comment

lmprice commented May 10, 2018

hulquest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ansibot commented May 18, 2018

lmprice commented May 22, 2018

hulquest commented May 24, 2018

ansibot commented May 24, 2018

ndswartz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmprice commented Aug 28, 2018