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

[WIP] new module ecs_tag #65924

Closed
wants to merge 123 commits into from
Closed

[WIP] new module ecs_tag #65924

wants to merge 123 commits into from

Conversation

@mpechner
Copy link
Contributor

mpechner commented Dec 17, 2019

SUMMARYwrapper around boto3 ecs tag_resource() and untag_resource()
ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

ecs_tag() will tag ecs resources

ADDITIONAL INFORMATION

Started with ec2_tag module code.

mpechner added 2 commits Dec 17, 2019
@mpechner

This comment has been minimized.

Copy link
Contributor Author

mpechner commented Dec 17, 2019

Just looking for comments on API. Will write and check in tests once I receive feedback.

@ansibot

This comment was marked as outdated.

Copy link
Contributor

ansibot commented Dec 17, 2019

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

lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-choices-do-not-match-spec: Argument 'resource_type' in argument_spec defines choices as (['cluster', 'task', 'service', 'task_definition', 'container']) but documentation defines choices as ([])
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-choices-do-not-match-spec: Argument 'state' in argument_spec defines choices as (['present', 'absent', 'list']) but documentation defines choices as ([])
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-default-does-not-match-spec: Argument 'resource_type' in argument_spec defines default as ('cluster') but documentation defines default as (None)
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-default-does-not-match-spec: Argument 'state' in argument_spec defines default as ('present') but documentation defines default as (None)
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-default-does-not-match-spec: Argument 'validate_certs' in argument_spec defines default as (True) but documentation defines default as (False)
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-missing-type: Argument 'aws_access_key' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-missing-type: Argument 'aws_secret_key' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-missing-type: Argument 'cluster_name' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-missing-type: Argument 'ec2_url' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-missing-type: Argument 'profile' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-missing-type: Argument 'region' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-missing-type: Argument 'resource' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-missing-type: Argument 'resource_type' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-missing-type: Argument 'security_token' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-missing-type: Argument 'state' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: doc-required-mismatch: Argument 'cluster_name' in argument_spec is required, but is not documented as being required
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: parameter-type-not-in-doc: Argument 'debug_botocore_endpoint_logs' in argument_spec defines type as 'bool' but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: parameter-type-not-in-doc: Argument 'purge_tags' in argument_spec defines type as 'bool' but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: parameter-type-not-in-doc: Argument 'tags' in argument_spec defines type as 'dict' but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: parameter-type-not-in-doc: Argument 'validate_certs' in argument_spec defines type as 'bool' but documentation doesn't define type
lib/ansible/modules/cloud/amazon/ecs_tag.py:35:7: documentation-syntax-error: DOCUMENTATION is not valid YAML

The test ansible-test sanity --test ansible-doc [explain] failed with the error:

Command "ansible-doc -t module ecs_tag" returned exit status 1.
>>> Standard Error
ERROR! module ecs_tag missing documentation (or could not parse documentation): while parsing a block collection
  in "<unicode string>", line 21, column 7
did not find expected '-' indicator
  in "<unicode string>", line 22, column 7

The test ansible-test sanity --test docs-build [explain] failed with the error:

Command "/usr/bin/python3.6 /root/ansible/test/sanity/code-smell/docs-build.py" returned exit status 1.
>>> Standard Error
Command 'make singlehtmldocs' failed with status code: 2
--> Standard Output
PYTHONPATH=../../lib ../../hacking/build-ansible.py collection-meta --template-file=../templates/collections_galaxy_meta.rst.j2 --output-dir=rst/dev_guide/ ../../lib/ansible/galaxy/data/collections_galaxy_meta.yml
PYTHONPATH=../../lib ../../hacking/build-ansible.py document-config --template-file=../templates/config.rst.j2 --output-dir=rst/reference_appendices/ ../../lib/ansible/config/base.yml
mkdir -p rst/cli
PYTHONPATH=../../lib ../../hacking/build-ansible.py generate-man --template-file=../templates/cli_rst.j2 --output-dir=rst/cli/ --output-format rst ../../lib/ansible/cli/*.py
PYTHONPATH=../../lib ../../hacking/build-ansible.py document-keywords --template-dir=../templates --output-dir=rst/reference_appendices/ ./keyword_desc.yml
PYTHONPATH=../../lib ../../hacking/build-ansible.py document-plugins -t rst --template-dir=../templates --module-dir=../../lib/ansible/modules -o rst/modules/ 
Evaluating module files...
Makefile:99: recipe for target 'modules' failed
--> Standard Error
Traceback (most recent call last):
  File "../../hacking/build-ansible.py", line 92, in <module>
    main()
  File "../../hacking/build-ansible.py", line 81, in main
    retval = command.main(args)
  File "/root/ansible/hacking/build_library/build_ansible/command_plugins/plugin_formatter.py", line 721, in main
    plugin_info, categories = get_plugin_info(args.module_dir, limit_to=args.limit_to, verbose=(args.verbosity > 0))
  File "/root/ansible/hacking/build_library/build_ansible/command_plugins/plugin_formatter.py", line 225, in get_plugin_info
    doc, examples, returndocs, metadata = plugin_docs.get_docstring(module_path, fragment_loader, verbose=verbose)
  File "/root/ansible/lib/ansible/utils/plugin_docs.py", line 124, in get_docstring
    data = read_docstring(filename, verbose=verbose, ignore_errors=ignore_errors)
  File "/root/ansible/lib/ansible/parsing/plugin_docs.py", line 59, in read_docstring
    data[varkey] = AnsibleLoader(child.value.s, file_name=filename).get_single_data()
  File "/usr/local/lib/python3.6/dist-packages/yaml/constructor.py", line 41, in get_single_data
    node = self.get_single_node()
  File "ext/_yaml.pyx", line 707, in _yaml.CParser.get_single_node
  File "ext/_yaml.pyx", line 725, in _yaml.CParser._compose_document
  File "ext/_yaml.pyx", line 776, in _yaml.CParser._compose_node
  File "ext/_yaml.pyx", line 890, in _yaml.CParser._compose_mapping_node
  File "ext/_yaml.pyx", line 776, in _yaml.CParser._compose_node
  File "ext/_yaml.pyx", line 890, in _yaml.CParser._compose_mapping_node
  File "ext/_yaml.pyx", line 776, in _yaml.CParser._compose_node
  File "ext/_yaml.pyx", line 890, in _yaml.CParser._compose_mapping_node
  File "ext/_yaml.pyx", line 774, in _yaml.CParser._compose_node
  File "ext/_yaml.pyx", line 853, in _yaml.CParser._compose_sequence_node
  File "ext/_yaml.pyx", line 905, in _yaml.CParser._parse_next_event
yaml.parser.ParserError: while parsing a block collection
  in "<unicode string>", line 21, column 7
did not find expected '-' indicator
  in "<unicode string>", line 22, column 7
make: *** [modules] Error 1

The test ansible-test sanity --test package-data [explain] failed with the error:

Command "/usr/bin/python3.6 /root/ansible/test/sanity/code-smell/package-data.py" returned exit status 1.
>>> Standard Error
Traceback (most recent call last):
  File "/root/ansible/test/sanity/code-smell/package-data.py", line 383, in <module>
    main()
  File "/root/ansible/test/sanity/code-smell/package-data.py", line 360, in main
    sdist_path = create_sdist(tmp_dir)
  File "/root/ansible/test/sanity/code-smell/package-data.py", line 174, in create_sdist
    raise Exception('make snapshot failed:\n%s' % stderr)
Exception: make snapshot failed:
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
ERROR! module ecs_tag at /tmp/tmpc9xjzh_p/lib/ansible/modules/cloud/amazon/ecs_tag.py has a documentation error formatting or is missing documentation.
Traceback (most recent call last):
  File "packaging/release/changelogs/changelog.py", line 835, in <module>
    main()
  File "packaging/release/changelogs/changelog.py", line 102, in main
    args.func(args)
  File "packaging/release/changelogs/changelog.py", line 132, in command_release
    plugins = load_plugins(version=version, force_reload=reload_plugins)
  File "packaging/release/changelogs/changelog.py", line 184, in load_plugins
    '--json', '--metadata-dump', '-t', plugin_type])
  File "/usr/lib/python3.6/subprocess.py", line 356, in check_output
    **kwargs).stdout
  File "/usr/lib/python3.6/subprocess.py", line 438, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['/tmp/tmpc9xjzh_p/bin/ansible-doc', '--json', '--metadata-dump', '-t', 'module']' returned non-zero exit status 1.
make: *** [changelog] Error 1

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

lib/ansible/modules/cloud/amazon/ecs_tag.py:39:74: W291: trailing whitespace
lib/ansible/modules/cloud/amazon/ecs_tag.py:46:79: W291: trailing whitespace
lib/ansible/modules/cloud/amazon/ecs_tag.py:82:24: W291: trailing whitespace

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

lib/ansible/modules/cloud/amazon/ecs_tag.py:35:7: error: DOCUMENTATION: syntax error: expected <block end>, but found '?'

click here for bot help

pabelanger and others added 6 commits Dec 17, 2019
This allows to group tests by connections, which allow us to disable
local connection testing in collections for the moment.

Signed-off-by: Paul Belanger <pabelanger@redhat.com>
* Use requirements.txt to install prereqs for docs build

* implement feedback
- fetch boto3 clients using module.client()
- remove ec2_argument_spec() use (AnsibleAWSModule adds it automatically)
- remove unused imports
- remove HAS_BOTO3 (AnsibleAWSModule handles it)
…#65335)

* Modified to include plan information.

* Fixed where plan is passed to the virtual machine object.

* Added changelog file

* Update changelogs/fragments/65335-add-plan-to-azure-vmscaleset-module.yaml

Co-Authored-By: Felix Fontein <felix@fontein.de>

* Added plan suboptions with required flag. Removed code block checking plan since suboptions were added.

* Changed true to True. Added space after commas where failed tests indicated.

* Removed extra blank line. Added promotion_code to plan param list.

* Trying to fix indention issue

* Trying to fix indention

* Changed example capacity to trigger build check. Last failure was not due to code.

* Removed property for accepting terms and code block using it..

* Removed extra unneeded spaces.
@ansibot

This comment was marked as outdated.

Copy link
Contributor

ansibot commented Dec 18, 2019

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

lib/ansible/modules/cloud/amazon/ecs_tag.py:0:0: module-incorrect-version-added: version_added should be '2.10'. Currently '2.9'

click here for bot help

@ansibot ansibot added the ci_verified label Dec 18, 2019
@mpechner

This comment has been minimized.

Copy link
Contributor Author

mpechner commented Dec 18, 2019

I'll work on tests this weekend. I wanted this to be reviewed, get feed back on the API I proposed. I am using a version locally that only handles tagging the cluster resource.

@ansibot ansibot added the has_issue label Dec 18, 2019
joej164 and others added 3 commits Dec 17, 2019
##### SUMMARY
<!--- Your description here -->
The links in the file are wrong.  Updating the document name to point to the right document.

https://docs.ansible.com/ansible/latest/modules/aci_access_port_block_to_access_port_module.html

##### ISSUE TYPE
- Docs Pull Request

+label: docsite_pr
* New module zabbix_service (#5)

* new module zabbix_service

* fix type

* fix githubid

* New Zabbix service module (#11)

* new zabbix service module

* fix validate module failure

* Fix algorithm doc and all_childs param name

* Update Ansible version

Co-Authored-By: sky-joker <megane@kurobuti.com>

* remove dump state
mpechner and others added 20 commits Dec 17, 2019
For remove tags, list of keys has to be a list or tupel

Tested:
all 3 states for cluster, service and task_defintion.
No Ansible info for tasks and cluster_instances - or did I miss something. So these resources types have not been tested.
remove unneeded setting

Co-Authored-By: Mark Chappell <mchappel@redhat.com>
false, not no

Co-Authored-By: Mark Chappell <mchappel@redhat.com>
I like the comment change

Co-Authored-By: Mark Chappell <mchappel@redhat.com>
corrected style

Co-Authored-By: Mark Chappell <mchappel@redhat.com>
thanks for the specific

Co-Authored-By: Mark Chappell <mchappel@redhat.com>
thanks for the terminology fix

Co-Authored-By: Mark Chappell <mchappel@redhat.com>
Co-Authored-By: Mark Chappell <mchappel@redhat.com>
Correct, supposed to tag any ECS resource

Co-Authored-By: Mark Chappell <mchappel@redhat.com>
Co-Authored-By: Mark Chappell <mchappel@redhat.com>
Co-Authored-By: Mark Chappell <mchappel@redhat.com>
Co-Authored-By: Mark Chappell <mchappel@redhat.com>
Co-Authored-By: Mark Chappell <mchappel@redhat.com>
Co-Authored-By: Mark Chappell <mchappel@redhat.com>
Co-Authored-By: Mark Chappell <mchappel@redhat.com>
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Dec 29, 2019

@mpechner this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Dec 29, 2019

@mpechner This PR was evaluated as a potentially problematic PR for the following reasons:

  • More than 50 changed files.
  • More than 50 commits.

Such PR can only be merged by human. Contact a Core team member to review this PR on IRC: #ansible-devel on irc.freenode.net

click here for bot help

@opendev-zuul

This comment has been minimized.

Copy link

opendev-zuul bot commented Dec 29, 2019

Build succeeded (third-party-check pipeline).

@mpechner

This comment has been minimized.

Copy link
Contributor Author

mpechner commented Dec 29, 2019

When a rebase does not work, stop and try again.

@mpechner mpechner closed this Dec 29, 2019
@sivel sivel removed the needs_triage label Jan 2, 2020
@plutosrings

This comment has been minimized.

Copy link

plutosrings commented on 663171e Jan 20, 2020

Thanks for posting this fix, the comment (line 30) references: NetworkManager-nmlib, however this should be NetworkManager-libnm as the rest of the module reads.

@ansible ansible locked and limited conversation to collaborators Jan 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
You can’t perform that action at this time.