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 'efs_tag' #608

Merged
merged 2 commits into from
Aug 13, 2021

Conversation

zeten30
Copy link
Contributor

@zeten30 zeten30 commented Jun 24, 2021

SUMMARY

New module - efs_tag

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

efs_tag

ADDITIONAL INFORMATION

There's a similar 'ec2_tag' module amazon.aws.ec2_tag that allows tagging on various EC2 resources. I'm adding a module that allows all EFS resource tagging (both filesystem and access point)

- name: Ensure tags are present on a resource
  community.aws.efs_tag:
    resource: fsap-123456ab
    state: present
    tags:
      Name: MyEFSAccessPoint
      Env: Production

@ansibullbot
Copy link

@ansibullbot ansibullbot added WIP Work in progress integration tests/integration module module needs_triage new_module New module new_plugin New plugin plugins plugin (any type) tests tests labels Jun 24, 2021
@goneri goneri closed this Jun 28, 2021
@goneri goneri reopened this Jun 28, 2021
@zeten30
Copy link
Contributor Author

zeten30 commented Jul 2, 2021

@goneri Hi, any idea why checks are failing? It looks like a check problem, not the new module code.

The task includes an option with an undefined variable. The error was: '_hashed_targets' is undefined

The error appears to be in '/var/lib/zuul/builds/e8ae5b2a68be41ca958f877439a2b3e3/untrusted/project_0/github.com/ansible/ansible-zuul-jobs/roles/ansible-test/tasks/split_targets.yaml': line 20, column 3, but may
be elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:

  loop: '{{ ansible_test_targets.stdout_lines }}'
- set_fact:
  ^ here

@goneri
Copy link
Member

goneri commented Jul 2, 2021

@goneri Hi, any idea why checks are failing? It looks like a check problem, not the new module code.

The task includes an option with an undefined variable. The error was: '_hashed_targets' is undefined

The error appears to be in '/var/lib/zuul/builds/e8ae5b2a68be41ca958f877439a2b3e3/untrusted/project_0/github.com/ansible/ansible-zuul-jobs/roles/ansible-test/tasks/split_targets.yaml': line 20, column 3, but may
be elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:

  loop: '{{ ansible_test_targets.stdout_lines }}'
- set_fact:
  ^ here

Hi @zeten30, Sorry for the situation. This should not happen anymore now.

@goneri
Copy link
Member

goneri commented Jul 2, 2021

recheck

@zeten30 zeten30 changed the title WIP: new module 'efs_tag' New module 'efs_tag' Jul 4, 2021
@ansibullbot ansibullbot added community_review and removed WIP Work in progress labels Jul 4, 2021
Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @zeten30

@ansibullbot
Copy link

Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tags look good. You also need to explicitly enable the AWS Retries when you make the client calls. You're actually passed a decorated copy of the boto3 client where most calls have an extra "aws_retry" parameter.

Copy link
Member

@markuman markuman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work. Looks great so far.
You need also a changelog/fragments file.

@zeten30 zeten30 requested a review from tremble July 13, 2021 07:48
@zeten30 zeten30 changed the title New module 'efs_tag' WIP: New module 'efs_tag' Jul 13, 2021
@ansibullbot ansibullbot added the WIP Work in progress label Jul 13, 2021
@zeten30
Copy link
Contributor Author

zeten30 commented Jul 13, 2021

Making as WIP again. I'm going to create a complete set of integration tests, remove module from 'unsupported' and apply all suggested fixes.

@zeten30
Copy link
Contributor Author

zeten30 commented Jul 13, 2021

BTW: Do you know why ansible/gate Expected — Waiting for status to be reported is hanging in this state more that a day?

@markuman
Copy link
Member

markuman commented Jul 13, 2021

BTW: Do you know why ansible/gate Expected — Waiting for status to be reported is hanging in this state more that a day?

A PR needs two reviewer that confirms the PR.
...and the ansible/gate is waiting for it before the PR is merging it automagically.

@zeten30
Copy link
Contributor Author

zeten30 commented Jul 14, 2021

Getting ready for the integration tests - mattclay/aws-terminator#157

I assume the next check will fail:) We're missing some EFS: actions ^.

@zeten30
Copy link
Contributor Author

zeten30 commented Jul 19, 2021

All the EFS related problems should be fixed. Integration test are failing on unrelated module:

ERROR: The 1 integration test(s) listed below (out of 20) failed. See error output above for details:
ec2_vpc_endpoint_service_info
Cleaning up temporary python directory: /tmp/python-01sgnf9j-ansible

Can we merge this? Thanks in advance.

@zeten30
Copy link
Contributor Author

zeten30 commented Jul 20, 2021

recheck

@zeten30
Copy link
Contributor Author

zeten30 commented Jul 20, 2021

And another error: ERROR: Command "ansible-playbook ec2_vpc_endpoint_service_info-ox_pgtf5.yml -i ../../../../../../../venv/lib/python3.6/site-packages/ansible_test/_data/inventory --skip-tags False --diff -vvvvvv" returned exit status 4.

@goneri Any idea what is wrong here? Thank you very much in advance.

@markuman
Copy link
Member

markuman commented Jul 20, 2021

It looks like all last pipeline runs are affected.
But I've no idea atm. Locally the efs integrationtest runs successful for me.

ansible-test integration -v --python 3.7 efs --docker
...
localhost                  : ok=58   changed=16   unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

@markuman
Copy link
Member

recheck

@zeten30
Copy link
Contributor Author

zeten30 commented Jul 20, 2021

It looks like all last pipeline runs are affected.
But I've no idea atm. Locally the efs integrationtest runs successful for me.

ansible-test integration -v --python 3.7 efs --docker
...
localhost                  : ok=58   changed=16   unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

yes, EFS works. This is now the problem:

ERROR: The 1 integration test(s) listed below (out of 9) failed. See error output above for details:
ec2_vpc_endpoint_service_info

And test timeout sometimes. Timeout needs to be increased or split the test even more.

@markuman
Copy link
Member

markuman commented Aug 3, 2021

recheck

1 similar comment
@markuman
Copy link
Member

markuman commented Aug 3, 2021

recheck

@markuman
Copy link
Member

markuman commented Aug 9, 2021

recheck

1 similar comment
@markuman
Copy link
Member

markuman commented Aug 9, 2021

recheck

@zeten30
Copy link
Contributor Author

zeten30 commented Aug 9, 2021

Really weird. Failed with:

botocore.exceptions.ClientError: An error occurred (ValidationException) when calling the DescribeFileSystems operation: 1 validation error detected: Value 'ansible-test-44911258-fedora-34-1vcpu-vexxhost-sjc1-0001719877-efs' at 'creationToken' failed to satisfy constraint: Member must have length less than or equal to 64

Playbook: tests/integration/targets/efs/playbooks/version_fail.yml

- hosts: localhost
  connection: local
  environment: "{{ ansible_test.environment }}"
  vars:
     resource_prefix: 'test-'

  tasks:
    - name: 'efs graceful failure tests'
      collections:
        - amazon.aws
      module_defaults:
        group/aws:
          aws_access_key: '{{ aws_access_key }}'
          aws_secret_key: '{{ aws_secret_key }}'
          security_token: '{{ security_token | default(omit) }}'
          region: '{{ aws_region }}'
      block:

        - name: create efs with provisioned_throughput options (fails gracefully)
          efs:
            state: present
            name: "{{ resource_prefix }}-efs"
            throughput_mode: 'provisioned'
            provisioned_throughput_in_mibps: 8.0
          register: efs_provisioned_throughput_creation
          ignore_errors: yes

        - name: check that graceful error message is returned when creation with throughput_mode and old botocore
          assert:
            that:
              - efs_provisioned_throughput_creation.failed
              - 'efs_provisioned_throughput_creation.msg == "throughput_mode parameter requires botocore >= 1.10.57"'

@zeten30 zeten30 requested a review from markuman August 9, 2021 14:10
@markuman
Copy link
Member

markuman commented Aug 9, 2021

recheck

@markuman
Copy link
Member

New day new random errors?

"assertion": "efs_provisioned_throughput_creation.msg == \"throughput_mode parameter requires botocore >= 1.10.57\"",

That's also not affected by this PR

@markuman
Copy link
Member

recheck

@zeten30
Copy link
Contributor Author

zeten30 commented Aug 10, 2021

@tremble 👍 thx for help with integration tests cleanup.
@markuman I hope that the last commit will finally solve our CI issues.

@tremble
Copy link
Contributor

tremble commented Aug 10, 2021

test failure is unrelated.

#675 is running through gating and will likely generate a merge conflict I'll rebase this for you once the gating completes

@tremble tremble force-pushed the new_module_efs_tag branch from 409696d to 7a43d67 Compare August 10, 2021 13:24
@zeten30
Copy link
Contributor Author

zeten30 commented Aug 11, 2021

recheck

@zeten30
Copy link
Contributor Author

zeten30 commented Aug 11, 2021

\o/ - a successful ansible/check finally

Copy link
Member

@markuman markuman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@tremble tremble added the gate label Aug 11, 2021
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tremble
Copy link
Contributor

tremble commented Aug 13, 2021

recheck

@ansible-zuul ansible-zuul bot merged commit 7802551 into ansible-collections:main Aug 13, 2021
@zeten30 zeten30 deleted the new_module_efs_tag branch August 18, 2021 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community_review integration tests/integration module module new_module New module new_plugin New plugin plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants