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: Cloud Volume for AWS FileSystems #61343

Merged
merged 3 commits into from Aug 29, 2019

Conversation

@carchi8py
Copy link
Contributor

commented Aug 26, 2019

SUMMARY

new module: Cloud Volume for AWS FileSystems

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME
  • aws_netapp_cvs_FileSystems.py
ADDITIONAL INFORMATION

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Aug 26, 2019

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

click here for bot help

@thedoubl3j

This comment has been minimized.

Copy link
Contributor

commented Aug 27, 2019

bot_status

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Aug 27, 2019

Components

lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py
support: community
maintainers:

lib/ansible/plugins/doc_fragments/netapp.py
support: community
maintainers:

test/units/modules/cloud/amazon/test_aws_netapp_cvs_FileSystems.py
support: core
maintainers:

Metadata

waiting_on: carchi8py
changes_requested_by: null
needs_info: False
needs_revision: True
needs_rebase: True
merge_commits: []
too many files or commits: False
mergeable_state: dirty
shippable_status: failure
maintainer_shipits (module maintainers): False
community_shipits (namespace maintainers): False
ansible_shipits (core team members): False
shipit_actors (maintainer or core team member): None
shipit_actors_other:
automerge: automerge shipit test failed

click here for bot help

@carchi8py carchi8py force-pushed the carchi8py:1836 branch from cc8b5fa to 5dd1d12 Aug 27, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Aug 27, 2019

The test ansible-test sanity --test future-import-boilerplate [explain] failed with 1 error:

test/units/modules/cloud/amazon/test_aws_netapp_cvs_FileSystems.py:0:0: missing: from __future__ import (absolute_import, division, print_function)

The test ansible-test sanity --test import --python 3.8 [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:252:0: SyntaxWarning: "is" with a literal. Did you mean "=="?
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:265:0: SyntaxWarning: "is" with a literal. Did you mean "=="?
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:277:0: SyntaxWarning: "is" with a literal. Did you mean "=="?

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

test/units/modules/cloud/amazon/test_aws_netapp_cvs_FileSystems.py:0:0: missing: __metaclass__ = type

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

lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E312 No RETURN provided
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E325 Argument 'allowedClients' in argument_spec found in exportPolicy -> rules defines type as 'str' but documentation defines type as 'list'
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E325 Argument 'unixReadOnly' in argument_spec found in exportPolicy -> rules defines type as 'int' but documentation defines type as 'bool'
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E325 Argument 'unixReadWrite' in argument_spec found in exportPolicy -> rules defines type as 'int' but documentation defines type as 'bool'
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E337 Argument 'exportPolicy' in argument_spec defines type as 'dict' but documentation doesn't define type
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E337 Argument 'rules' in argument_spec found in exportPolicy defines type as 'list' but documentation doesn't define type
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E338 Argument 'serviceLevel' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E338 Argument 'state' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_cluster_ha.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_firmware_upgrade.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_info.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_motd.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_ndmp.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_object_store.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_ports.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_qos_adaptive_policy_group.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_qtree.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_quotas.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_volume_autosize.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_volume_clone.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_vscan.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_vserver_cifs_security.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_facts.py:0:0: E337 Argument 'api_password' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_facts.py:0:0: E337 Argument 'api_url' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_facts.py:0:0: E337 Argument 'api_username' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_facts.py:0:0: E337 Argument 'ssid' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_volume_copy.py:0:0: E338 Argument 'api_password' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_volume_copy.py:0:0: E338 Argument 'api_url' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_volume_copy.py:0:0: E338 Argument 'api_username' in argument_spec uses default type ('str') but documentation doesn't define type

click here for bot help

@ansibot ansibot removed the needs_rebase label Aug 27, 2019

@thedoubl3j

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

@carchi8py CI still failing

@lonico
Copy link
Contributor

left a comment

The logic around cd_action needs to be revisited.

def get_filesystemId(self):
# Check given FileSystem is exists
# Return fileSystemId is found, None otherwise
list_filesystem, error = self.restApi.get('FileSystems')

This comment has been minimized.

Copy link
@lonico

lonico Aug 28, 2019

Contributor

since we are only interested in the creationToken field, is it possible to scope the query to only return this field?

I'm concerned this can generate a lot of traffic if all attributes are returned.

Can be delayed.

if response['jobs'][0]['jobId'] is not None:
jobId = response['jobs'][0]['jobId']
if self.restApi.get_state(jobId) is 'done':
return response

This comment has been minimized.

Copy link
@lonico

lonico Aug 28, 2019

Contributor

not used by the caller.

The error processing is weak. What if we are getting something different than done?

We'll to validate how much is reported in 'error', and whether we need to support async and sync operations.

(I confirmed there are sync operations, we are waiting for the command to complete)


fileSystemId = self.get_filesystemId()

cd_action = self.na_helper.get_cd_action(fileSystem, self.parameters)

This comment has been minimized.

Copy link
@lonico

lonico Aug 28, 2019

Contributor

fileSystem is not set yet, so cd_action always believes the fileSystem does not exist.

I confirmed this prevents 'delete' to work correctly.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

The test ansible-test sanity --test future-import-boilerplate [explain] failed with 1 error:

test/units/modules/cloud/amazon/test_aws_netapp_cvs_FileSystems.py:0:0: missing: from __future__ import (absolute_import, division, print_function)

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

test/units/modules/cloud/amazon/test_aws_netapp_cvs_FileSystems.py:0:0: missing: __metaclass__ = type

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

lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E312 No RETURN provided
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E325 Argument 'allowedClients' in argument_spec found in exportPolicy -> rules defines type as 'str' but documentation defines type as 'list'
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E325 Argument 'unixReadOnly' in argument_spec found in exportPolicy -> rules defines type as 'int' but documentation defines type as 'bool'
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E325 Argument 'unixReadWrite' in argument_spec found in exportPolicy -> rules defines type as 'int' but documentation defines type as 'bool'
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E337 Argument 'exportPolicy' in argument_spec defines type as 'dict' but documentation doesn't define type
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E337 Argument 'rules' in argument_spec found in exportPolicy defines type as 'list' but documentation doesn't define type
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E338 Argument 'serviceLevel' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/cloud/amazon/aws_netapp_cvs_FileSystems.py:0:0: E338 Argument 'state' in argument_spec uses default type ('str') but documentation doesn't define type

click here for bot help

@ansibot ansibot added the ci_verified label Aug 28, 2019

@lonico
lonico approved these changes Aug 28, 2019
Copy link
Contributor

left a comment

shipit

@thedoubl3j thedoubl3j self-assigned this Aug 28, 2019

@thedoubl3j
Copy link
Contributor

left a comment

CI is green, docs look good and tests included. lgtm. shipit.

@thedoubl3j thedoubl3j merged commit 4879cf8 into ansible:devel Aug 29, 2019

1 check passed

Shippable Run 140684 status is SUCCESS.
Details

@sivel sivel removed the needs_triage label Aug 29, 2019

adharshsrivatsr added a commit to adharshsrivatsr/ansible that referenced this pull request Sep 3, 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.