Implemented hpenimble Ansible modules for volume and ACR objectset

[ar-india]

Signed-off-by: ar-india alokranjan2005@gmail.com

• hpe_nimble_volume the module is ready and can handle belwo tasks choices:
  • present
  • absent
  • create
  • online
  • offline
  • restore Not supported : Bulk operations are not supported.

hpe_nimble_acr: can handle belwo tasks choices:

• present
• absent
• create

Ansible page for the modules: modules.zip

[ar-india]

How do I render the docs?

As said in demo. There is ansible-doc command. For convenience, i have attached the rendered .html file to this PR. please download the .zip file and then after unzip. please go to html/modules folder. there you will see html files named "hpe_nimble_volume" and hpe_nimble_acr. You can open these in browser. please note it takes sometime to load. All the variables which you see in this html is generated by make-doc command. the source to this is our source code file.

ZIP FILE; nimble-ansible-doc.zip

Can you remove _name from all the user facing parameters and just imply "object", like volume, snapshot etc.

Convention followed for variables name:

1. as mentioned in demo. all those variable which ends with _id has been replaced with _name. for ex: volume object set needs volcoll_id. we are now asking for volcoll_name.

IF we just say volcoll rather than volcoll_name.. will it not be a bit confusing?? Any object set can have multiple properties. Let's say, we want volcoll size, name, limit etc etc. than asking for volcoll_size, volcoll_name makes it more explicit and clear than just asking for volcoll.

also, all the variable names have been taken from the api XML file. for ex: parent_vol_name is taken from XML. I see that you are asking to rename it to parent_volume.

I am ok either way with following whatever naming convention the team decides. Let me know if you still would like me to change the naming as suggested by you.

Are any of the verbiage in the docs generated or is it handcrafted?

As said in demo. There is ansible-doc command which picks the variables as defined here in the "DOCUMENTATION" Tag. in the source file.The verbs are all handicraft and not generated..

[ar-india]

have made changes to the modules as per the review comments ..please take a look

[ar-india]

Have made changes as per Michael’s latest comment. Please take a look

On 10-Jun-2020, at 6:41 AM, Michael Mattsson notifications@github.com wrote:

 @datamattsson requested changes on this pull request.

Please address and I'll do another pass.

In plugins/doc_fragments/hpe_nimble.py:

+requirements:

• • "Nimble OS 5.1.x"
• • "Ansible - 2.9"
• • nimble-sdk >= 1.0.1. Install using 'pip install nimble-sdk'
• • "REST service should be enabled on the Nimble storage array." Quotation is very inconsistent here and is it even needed? Can we somehow list Ansible >2.9, Ansible 2.9+ or something like that. Is there a convention?

Don't confuse users with the REST service. It's always enabled. Where are these requirements enumerated?

In plugins/doc_fragments/hpe_nimble.py:

• • "The storage system IP address."
• required: True
• type: str
• password:
• description:
• • "The storage system password."
• required: True
• type: str
• username:
• description:
• • "The storage system user name." Do these strings need quotation?

In playbook/hpe_nimble_acr_playbook.yml:

@@ -0,0 +1,43 @@ +--- Please keep these examples outside of the repo for now.

In modules/hpe_nimble_volume.py:

• state:
• description:
• • "Choice for Volume operations"
• choices:
• • present
• • absent
• • create
• • online
• • offline
• • restore
• required: True
• type: str Why is there a "four space" indentation here? Can't we stick with the "two space" indentation?

In modules/hpe_nimble_volume.py:

• • "Name of the Source Volume."
• size:
• type: int
• default: 100
• description:
• • "The size of the volume." Remove quotations.

In modules/hpe_nimble_volume.py:

• default: null
• description:
• • Text description of volume.
• perf_policy:
• required: False
• type: str
• default: null
• description:
• • Name of the performance policy. After creating a volume, performance policy for the volume can only be
• changed to another performance policy with same block size.
• limit:
• required: False
• type: int
• default: 100
• description:
• • Limit on the volume's mapped usage, expressed as a percentage of the volume's size Add punctuation.

In modules/hpe_nimble_volume.py:

• type: int
• default: 100
• description:
• • Limit on the volume's mapped usage, expressed as a percentage of the volume's size
• online:
• required: False
• type: bool
• default: True
• description:
• • Online state of volume, available for host initiators to establish connections.
• owned_by_group:
• required: False
• type: str
• default: null
• description:
• • Name of group that currently owns the volume Punctuation.

In modules/hpe_nimble_volume.py:

• • Name of group that currently owns the volume
• multi_initiator:
• required: False
• type: bool
• default: False
• description:
• • For iSCSI Volume Target, this flag indicates whether the volume and its snapshots can be accessed from multiple initiators at the same time.
• iscsi_target_scope:
• required: False
• type: str
• choices:
• • volume
• • group
• default: 'volume'
• description:
• • This indicates whether volume is exported under iSCSI Group Target or iSCSI Volume Target. This attribute is only meaningful to iSCSI system Punctuation.

In modules/hpe_nimble_volume.py:

• default: null
• description:
• • Name of group that currently owns the volume
• multi_initiator:
• required: False
• type: bool
• default: False
• description:
• • For iSCSI Volume Target, this flag indicates whether the volume and its snapshots can be accessed from multiple initiators at the same time.
• iscsi_target_scope:
• required: False
• type: str
• choices:
• • volume
• • group
• default: 'volume' is the quotation needed here?

In modules/hpe_nimble_volume.py:

• type: int
• default: -1
• description:
• • Throughput limit for this volume in MB/s.
• parent_volume:
• required: False
• type: str
• default: null
• description:
• • Name of parent volume
• snapshot:
• required: False
• type: str
• default: null
• description:
• • Base snapshot name. This attribute is required together with name and clone when cloning a volume with the create operation Punctuation.

In modules/hpe_nimble_volume.py:

• if state is create, then create a volume if not present. Fails if already present.

• if state is "present", then create a volume if not present, But Success if it already exists

  Capitalization and punctuations.

If state is "create", then create a volume if not present. Fails if already present.

If state is "present", then create a volume if not present. Succeeds if it already exists.

In modules/hpe_nimble_volume.py:

• type: str
• default: null
• description:
• • Name of parent volume
• snapshot:
• required: False
• type: str
• default: null
• description:
• • Base snapshot name. This attribute is required together with name and clone when cloning a volume with the create operation
• volcoll:
• required: False
• type: str
• default: null
• description:
• • Name of volume collection of which this volume is a member. I see "volume" being capitalized in some places. What is our internal documentation convention for this?

In modules/hpe_nimble_volume.py:

• Create a clone from the given snapshot name.

• if snapshot name is not provided then create a snapshot of source volume

• clone task should only run if parent_volume is specified.snapshot name is optional

• • name: Let's create or refresh a clone!
• hpe_nimble_volume:
• hostname: "{{ hostname }}"
• username: "{{ username }}"
• password: "{{ password }}"
• name: "{{ name }}" # name here is the name of cloned volume
• parent_volume: "{{ parent_volume | mandatory }}"
• snapshot: "{{ snapshot | default(None)}}"
• state: "{{ state | default('present') }}" # fail if exist
• when:
• • parent_volume is defined

    Create a clone from the given snapshot name.

    If snapshot name is not provided then a snapshot is created on the source volume.

    Clone task only run if "parent" is specified. Snapshot is optional.

    -name: Create or refresh a clone

This looks generally much cleaner. I also want to change "parent_volume" to just "parent" as "volume" is already implied since we're in the "hpe_nimble_volume" module.

In modules/hpe_nimble_volume.py:

• Create a clone from the given snapshot name.

• if snapshot name is not provided then create a snapshot of source volume

• clone task should only run if parent_volume is specified.snapshot name is optional

• • name: Let's create or refresh a clone!
• hpe_nimble_volume:
• hostname: "{{ hostname }}"
• username: "{{ username }}"
• password: "{{ password }}"
• name: "{{ name }}" # name here is the name of cloned volume
• parent_volume: "{{ parent_volume | mandatory }}"
• snapshot: "{{ snapshot | default(None)}}"
• state: "{{ state | default('present') }}" # fail if exist
• when:
• • parent_volume is defined
• • name: Let's destroy myvol1 (it's not offline) Destroy Volume (or Destroy volume) depending on the convention we choose.

In modules/hpe_nimble_volume.py:

• • name: Delete volume "{{ name }}" (it's not offline)
• hpe_nimble_volume:
• hostname: "{{ hostname }}"
• username: "{{ username }}"
• password: "{{ password }}"
• name: "{{ name }}"
• state: absent
• • name: Volume move to pool
• hpe_nimble_volume:
• hostname: "{{ hostname }}"
• username: "{{ username }}"
• password: "{{ password }}"
• move: true
• name: "{{ name }}"
• dest_pool: "{{ dest_pool | mandatory }}" Should we just rename this to "destination"? Pool is implied because were already stating move and pool is our only option.

In modules/hpe_nimble_volume.py:

• if no snapshot is given then restore volume to last snapshot, fails if no snapshots exist

• or if snapshot is provided then restore volume from that snapshot

  If no snapshot is given, then restore volume to last snapshot. Fails if no snapshots exist.

  If snapshot is provided, then restore volume from specified snapshot.

  In modules/hpe_nimble_volume.py:

• type: bool

• default: False

• description:

• • Volume is read-only.

• block_size:

• required: False

• type: int

• default: 4096

• description:

• • Size in bytes of blocks in the volume.

• clone:

• required: False

• type: bool

• default: False

• description:

• • Whether this volume is a clone.Use this attribute in combination with name and snapshot to create a clone by setting clone = true. Space between "clone.Use"

In modules/hpe_nimble_volume.py:

• • For iSCSI Volume Target, this flag indicates whether the volume and its snapshots can be accessed from multiple initiators at the same time.
• iscsi_target_scope:
• required: False
• type: str
• choices:
• • volume
• • group
• default: 'volume'
• description:
• • This indicates whether volume is exported under iSCSI Group Target or iSCSI Volume Target. This attribute is only meaningful to iSCSI system
• pool:
• required: False
• type: str
• default: null
• description:
• • Name associated with the pool in the storage pool table. Pool should be capitalized if we capitalize Nimble objects.

In modules/hpe_nimble_volume.py:

+ +ANSIBLE_METADATA = {'metadata_version': '1.1',

• 'status': ['preview'],
• 'supported_by': 'community'}

• +DOCUMENTATION = r''' +--- +author:

• • Alok Ranjan (@ranjanal) +description: "Manage volume on a Nimble Storage group" +module: hpe_nimble_volume +options:
• state:
• description:
• • "Choice for Volume operations" Quotations.

In modules/hpe_nimble_volume.py:

+# Unless required by applicable law or agreed to in writing, software distributed +# under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS +# OF ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. + +# author alok ranjan (alok.ranjan2@hpe.com) + + +from future import absolute_import, division, print_function +metaclass = type + + +ANSIBLE_METADATA = {'metadata_version': '1.1',

• 'status': ['preview'],
• 'supported_by': 'community'}

• double space?

In modules/hpe_nimble_volume.py:

+# file except in compliance with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed +# under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS +# OF ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. + +# author alok ranjan (alok.ranjan2@hpe.com) + + +from future import absolute_import, division, print_function +metaclass = type + + double space?

In modules/hpe_nimble_volume.py:

+#!/usr/bin/python + +# Copyright 2020 Hewlett Packard Enterprise Development LP +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this +# file except in compliance with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed +# under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS +# OF ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. + +# author alok ranjan (alok.ranjan2@hpe.com) + double space?

In modules/hpe_nimble_acr.py:

+ +EXAMPLES = r''' +

• if state is create, create ACL for given volume, fails if it exist or cannot create

• if state is present, create ACL if not present, else success

• • name: Create ACR for volume
• hpe_nimble_acr:
• hostname: "{{ hostname }}"
• username: "{{ username }}"
• password: "{{ password }}"
• volume: "{{ volume }}"
• initiator_group: "{{ initiator_group }}"
• state: "{{ state | default('present') }}" # fail if exist

• Delete the ACL for a given volume name

• • name: Delete ACR for volume Volume?

In modules/hpe_nimble_acr.py:

• type: str
• description:
• • Name for the protocol endpoint this access control record applies to.
• snapshot:
• required: False
• type: str
• description:
• • Name of the snapshot this access control record applies to.
• initiator_group:
• required: False
• type: str
• description:
• • The Initiator Group name.

• +extends_documentation_fragment: hpe_nimble +short_description: "Manages a HPE Nimble Storage ACR" I think we should use "Access Control Record" here.

In modules/hpe_nimble_acr.py:

• if state is create, create ACL for given volume, fails if it exist or cannot create

• if state is present, create ACL if not present, else success

  If state is "create", create ACR for given volume, fails if it exist.

  If state is "present", create ACR if not already present.

  In modules/hpe_nimble_acr.py:

• +DOCUMENTATION = r''' +--- +author:

• • Alok Ranjan (@ranjanal) +description: "On HPE Nimble Storage Array - Create or delete ACL for volume" +module: hpe_nimble_acr

• +options:

• state:

• required: True

• choices:

• • present
• • absent
• • create

• type: 'str' Quotation?

In modules/hpe_nimble_acr.py:

+# author alok ranjan (alok.ranjan2@hpe.com) + +from future import absolute_import, division, print_function +metaclass = type + + +ANSIBLE_METADATA = {'metadata_version': '1.1',

• 'status': ['preview'],
• 'supported_by': 'community'}

• +DOCUMENTATION = r''' +--- +author:

• • Alok Ranjan (@ranjanal) +description: "On HPE Nimble Storage Array - Create or delete ACL for volume" ACR? (and Volume with capital V?)

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or unsubscribe.

[ar-india]

Can you please go over all the user facing exceptions? I can still see a lot of "object_name" when it's more intuitive for a user to just default to a object as "object" without "_name" as name is implied.

Could you also type up in the README how this can be installed with ansible-galaxy? Looking at other modules, I'm not sure this is the correct repo structure, I can be wrong.

Hi Michael, Please review the latest change. have made changes as suggested. We would like to keep playbook as an example. Pls ignore that in your review.

[rgcostea]

I feel we are doing way too much in this one PR. Can we limit the scope and move the galaxy changes into a separate PR? Maybe not now but in the future.

[ar-india]

I strongly suggest removing you example playbooks. I we want to include example playbooks they need to be divided up in sensible tasks if they're user facing. If we think we need playbooks for e2e testing, please indicate such as and remove any reference to your name in the test objects.

ok.. have removed them

[ar-india]

Is there a way we can do a local install from the repo until we get this published for customer consumption?

not really. unfortunately, ansible galaxy does not offer something equivalent to test pypi where one can upload the python package when it is devlopment mode.

As of now i have locally ran the ansible collection command which actually creates the .tar file which eventually will be published to galaxy. Ansible website also has steps on how to test collection locally. Hence, this is what i have thought of.

my plan is to provide a ansible collection which i have build locally on my setup. and share it whoever is interested esp QA. instruction to install the collection is as below: