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

Document how to run integration tests more clearly #924

Closed
1 task done
mdavis-xyz opened this issue Nov 8, 2021 · 12 comments · Fixed by #1276 or #1278
Closed
1 task done

Document how to run integration tests more clearly #924

mdavis-xyz opened this issue Nov 8, 2021 · 12 comments · Fixed by #1276 or #1278

Comments

@mdavis-xyz
Copy link
Contributor

Summary

I'm trying to write a PR, and run integration tests locally. It's incredibly confusing. The documentation is spread across dozens of pages. I'm really struggling to figure out how to run integration tests, and how to ensure they're run against my clone not the stable release that's already installed. Can you please add something straightforward to the README of this repo?

There are specific points where I get stuck. And I've suggested a new section to put in the README to fix this issue.

Issue Type

Documentation Report

Component Name

docs

Ansible Version

ansible [core 2.11.6] 
  config file = None
  configured module search path = ['/home/ec2-user/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/ec2-user/.pyenv/versions/3.8.11/lib/python3.8/site-packages/ansible
  ansible collection location = /home/ec2-user/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/ec2-user/.pyenv/versions/3.8.11/bin/ansible
  python version = 3.8.11 (default, Nov  7 2021, 23:05:42) [GCC 7.3.1 20180712 (Red Hat 7.3.1-13)]
  jinja version = 3.0.2
  libyaml = True

Collection Versions

# /home/ec2-user/.pyenv/versions/3.8.11/lib/python3.8/site-packages/ansible_collections
Collection                    Version
----------------------------- -------
amazon.aws                    1.5.1  
ansible.netcommon             2.4.0  
ansible.posix                 1.3.0  
ansible.utils                 2.4.2  
ansible.windows               1.7.3  
arista.eos                    2.2.0  
awx.awx                       19.4.0 
azure.azcollection            1.10.0 
check_point.mgmt              2.1.1  
chocolatey.chocolatey         1.1.0  
cisco.aci                     2.1.0  
cisco.asa                     2.1.0  
cisco.intersight              1.0.17 
cisco.ios                     2.5.0  
cisco.iosxr                   2.5.0  
cisco.meraki                  2.5.0  
cisco.mso                     1.2.0  
cisco.nso                     1.0.3  
cisco.nxos                    2.7.0  
cisco.ucs                     1.6.0  
cloudscale_ch.cloud           2.2.0  
community.aws                 1.5.0  
community.azure               1.1.0  
community.crypto              1.9.6  
community.digitalocean        1.11.0 
community.docker              1.10.0 
community.fortios             1.0.0  
community.general             3.8.1  
community.google              1.0.0  
community.grafana             1.2.3  
community.hashi_vault         1.4.1  
community.hrobot              1.2.0  
community.kubernetes          1.2.1  
community.kubevirt            1.0.0  
community.libvirt             1.0.2  
community.mongodb             1.3.1  
community.mysql               2.3.1  
community.network             3.0.0  
community.okd                 1.1.2  
community.postgresql          1.5.0  
community.proxysql            1.3.0  
community.rabbitmq            1.1.0  
community.routeros            1.2.0  
community.skydive             1.0.0  
community.sops                1.1.0  
community.vmware              1.15.0 
community.windows             1.7.0  
community.zabbix              1.5.0  
containers.podman             1.8.1  
cyberark.conjur               1.1.0  
cyberark.pas                  1.0.7  
dellemc.enterprise_sonic      1.1.0  
dellemc.openmanage            3.6.0  
dellemc.os10                  1.1.1  
dellemc.os6                   1.0.7  
dellemc.os9                   1.0.4  
f5networks.f5_modules         1.12.0 
fortinet.fortimanager         2.1.3  
fortinet.fortios              2.1.2  
frr.frr                       1.0.3  
gluster.gluster               1.0.2  
google.cloud                  1.0.2  
hetzner.hcloud                1.6.0  
hpe.nimble                    1.1.3  
ibm.qradar                    1.0.3  
infinidat.infinibox           1.2.4  
inspur.sm                     1.3.0  
junipernetworks.junos         2.6.0  
kubernetes.core               1.2.1  
mellanox.onyx                 1.0.0  
netapp.aws                    21.6.0 
netapp.azure                  21.9.0 
netapp.cloudmanager           21.11.0
netapp.elementsw              21.6.1 
netapp.ontap                  21.12.0
netapp.um_info                21.7.0 
netapp_eseries.santricity     1.2.13 
netbox.netbox                 3.3.0  
ngine_io.cloudstack           2.2.2  
ngine_io.exoscale             1.0.0  
ngine_io.vultr                1.1.0  
openstack.cloud               1.5.1  
openvswitch.openvswitch       2.0.2  
ovirt.ovirt                   1.6.4  
purestorage.flasharray        1.11.0 
purestorage.flashblade        1.7.0  
sensu.sensu_go                1.12.0 
servicenow.servicenow         1.0.6  
splunk.es                     1.0.2  
t_systems_mms.icinga_director 1.23.0 
theforeman.foreman            2.2.0  
vyos.vyos                     2.6.0  
wti.remote                    1.0.1 

Configuration

$ ansible-config dump --only-changed

(empty)

OS / Environment

No response

Additional Information

The problem

For context, I have used this collection, but I've never installed or managed collections directly. I don't think it should be assumed that people wanting to submit PRs to this repo know the semantics of things like COLLECTIONS_PATH.

The top-level README for this repo doesn't say how to run integrations, or where or how to install this collection (from a git checkout). It only explains how to install the stable release, and links to pages saying how to write integration tests.
The contributing file does not say how to run integration tests. It just points to the integration test page of the module development guide. Step 1 of that guide refers to file hacking/env-setup. There is no such file in this repo!

I found another file saying that I can skip this step if ansible-test is installed. Since collections were split off from Ansible core, ansible-test is going to always be installed. So I'm not sure why that's mentioned at all. Can we delete that part? Or link to a different page?

I don't think the docs for this collection link to anything telling me which folder to run ansible-test in.
Pages like this collection testing guide tell me how to write collections, and they tell me what command to use. But they don't tell me which directory to run in, or how to ensure I'm testing my checkout, not the stable release that is installed on the machine already.

When I try to run ansible-test at the top of this repo, I get:

matt:~/environment/community.aws (main) $ ansible-test integration --list-targets
ERROR: The current working directory must be at or below:

 - an Ansible collection: {...}/ansible_collections/{namespace}/{collection}/

Current working directory: /home/ec2-user/environment/community.aws

Note that the only folder on my machine called ansible_collections already has the latest stable release of this collection installed.

I eventually found yet another documentation page telling me to clone into. But the documentation is so dispersed that I'm unable to find it again now while typing this up.

The answer was ~/ansible/collections/ansible_collections/community/aws, and then you put a different directory to that in COLLECTIONS_PATHS. Do Ansible developers really spend all their time nested a hundred folders deep to do work?

Then when I run the unit tests, I get:

WARNING: Excluding tests marked "cloud/aws" which require config (see "/home/ec2-user/.pyenv/versions/3.8.11/lib/python3.8/site-packages/ansible_test/config/cloud-config-aws.ini.template"): s3_lifecycle
WARNING: All targets skipped.

Hmm, that's odd. I'm running this on EC2, so boto should be able to find IAM credentials. Ok, well I open that file and do what it says. I copy /home/ec2-user/.pyenv/versions/3.8.11/lib/python3.8/site-packages/ansible_test/config/cloud-config-aws.ini.template to /home/ec2-user/.pyenv/versions/3.8.11/lib/python3.8/site-packages/ansible_test/config/cloud-config-aws.ini. But what do I put for security_token if I have no security token? Do I leave it as security_token: @SECURITY_TOKEN or have it empty, or '', or null, or delete the line? The file doesn't say.

After doing exactly what the file told me to, I re-run and get the same error. So now I'm stuck.
What do I do after I've edited cloud-config-aws.ini if I still get the same error?

Solution

Add a new section to the README, which says:

  1. Fork the repo
  2. Do not clone the repo into just any folder. You must rename it from community.aws to aws, and save that into a directory heirachy ending in ansible/collections/ansible_collections/community/.
  3. Prepend (don't append) to the COLLECTIONS_PATHS environment variable the collections/ directory which is 3 levels above your clone, with a trailing slash.

For example:

mkdir -p ~/dev/ansible/collections/ansible_collections/community/
cd ~/dev/ansible/collections/ansible_collections/community/
git clone git@github.com:myuser/community.aws.git aws # only exists after forking
cd aws
git remote add upstream git@github.com:ansible-collections/community.aws.git
export COLLECTIONS_PATHS=$HOME/dev/ansible/collections/:$COLLECTIONS_PATHS

This last command is temporary, only for this terminal session. If you want to do this permanently (which will override the stable release of this repo with your checkout), open ~/.bashrc and add

export COLLECTIONS_PATHS=$HOME/dev/ansible/collections/:$COLLECTIONS_PATHS

Now navigate to the top of the repo (e.g. cd ~/dev/ansible/collections/ansible_collections/community/aws).
Run

ansible-test integration --list-targets

This will list all the possible integration tests you can run.

To run one (e.g. x), use:

ansible-test integration x

You will get this error:

WARNING: Excluding tests marked "cloud/aws" which require config (see "something/site-packages/ansible_test/config/cloud-config-aws.ini.template"): s3_lifecycle
WARNING: All targets skipped.

Open up that file and read the instructions there.
You need to copy and rename that file, removing the .template but leaving it in the same directory.

cp something/site-packages/ansible_test/config/cloud-config-aws.ini.template something/site-packages/ansible_test/config/cloud-config-aws.ini

Now edit that new file, to add the relevant IAM credentials.

If you do not need a session token, then TODO: someone tell me what to do with the session token line

Now re-run the previous ansible-test command.
If you still get the same error, then you need to TODO: someone tell me what to do here.

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
ansible-zuul bot referenced this issue in ansible-collections/community.aws Jan 14, 2022
…#794)

Add abort multipart upload and expire obj del markers to s3 lifecycle

Depends-On: ansible/ansible-zuul-jobs#1247
SUMMARY
Fixes #365 #796
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
s3_lifecycle
ADDITIONAL INFORMATION
I have not run integration tests yet because of #793.
I'm unsure about how to name and structure the new arguments.
Do I nest them to match the API, or flatten them to match existing arguments?

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Matthew Davis <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: None <None>
Reviewed-by: Markus Bergholz <git@osuv.de>
patchback bot referenced this issue in ansible-collections/community.aws Jan 14, 2022
…#794)

Add abort multipart upload and expire obj del markers to s3 lifecycle

Depends-On: ansible/ansible-zuul-jobs#1247
SUMMARY
Fixes #365 #796
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
s3_lifecycle
ADDITIONAL INFORMATION
I have not run integration tests yet because of #793.
I'm unsure about how to name and structure the new arguments.
Do I nest them to match the API, or flatten them to match existing arguments?

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Matthew Davis <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: None <None>
Reviewed-by: Markus Bergholz <git@osuv.de>
(cherry picked from commit ed3a7a0)
softwarefactory-project-zuul bot referenced this issue in ansible-collections/community.aws Jan 17, 2022
…#794) (#864)

[PR #794/ed3a7a0b backport][stable-2] Add abort multipart upload and expire obj del markers to s3 lifecycle

This is a backport of PR #794 as merged into main (ed3a7a0).
Depends-On: ansible/ansible-zuul-jobs#1247
SUMMARY
Fixes #365 #796
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
s3_lifecycle
ADDITIONAL INFORMATION
I have not run integration tests yet because of #793.
I'm unsure about how to name and structure the new arguments.
Do I nest them to match the API, or flatten them to match existing arguments?
@Razique
Copy link
Contributor

Razique commented Apr 19, 2022

Agree, things are a bit confusing. It takes some time to understand how pieces fit together.
Regarding usage of credentials, that is, if you get the following message:

WARNING: Excluding tests marked "cloud/aws" which require config
(see "/home/dev/ansible/ansible/test/lib/ansible_test/config/cloud-config-aws.ini.template"): ec2_group

I have found this link that mentions:

In order to run some tests, you must provide access credentials in a file named cloud-config-aws.yml or cloud-config-cs.ini in the test/integration directory. Corresponding .template files are available for for syntax help. The newer AWS tests now use the file test/integration/cloud-config-aws.yml

So you don't need to store the configuration file your virtual environment, but in the integration folder (inside tests), for instance:

λ git issue-628*pwd
/home/dev/ansible_collections/amazon/aws/tests/integration
λ fedora-dev integration → λ git issue-628* → ls -al
total 12
drwxr-xr-x. 1 dev dev  116 Apr 18 19:03 .
drwxr-xr-x. 1 dev dev  126 Apr 18 19:03 ..
-rw-r--r--. 1 dev dev  894 Apr 18 19:00 cloud-config-aws.ini
...

@giom-l
Copy link

giom-l commented May 16, 2022

Damn,
I found this page too late after struggling for the exact same problem.
However I've one thing left but didn't find answer yet :
Is it possible for ansible-test to use credentials provided by aws-vault ?
There months ago that I didn't put a credential in some file and that bother me a little I have to fill something for ansible testing.

@Razique
Copy link
Contributor

Razique commented May 17, 2022

@giom-l I have looked around and didn't find anything. The ansible-test command doesn't support any user input mechanism of reference to supplementary configuration file.
Some some more info here here.

@tremble tremble transferred this issue from ansible-collections/community.aws Jul 10, 2022
@tremble
Copy link
Contributor

tremble commented Jul 10, 2022

I've bumped this issue over to amazon.aws which is where the more comprehensive documentation for amazon.aws and community.aws lives.

@samccann
Copy link
Contributor

samccann commented Oct 7, 2022

Hi folks - we have some generic 'contributing to collections' documentation at https://docs.ansible.com/ansible/latest/community/create_pr_quick_start.html

And some about testing at:
https://docs.ansible.com/ansible/latest/community/collection_contributors/test_index.html

If we link to these from the CONTRIBUTOR file, does this cover a lot of what's been pointed out here as problematic? (things that aren't specific to AWS collection development)...

@mdavis-xyz
Copy link
Contributor Author

mdavis-xyz commented Nov 18, 2022

Ok I'm trying to write another PR, and running into the same issues.

@samccann - Yes that's helpful. I started by going to the README at the top of this repo, and looking at the contribution section. I did not even notice the CONTRIBUTING.md file. Instead I followed where the README pointed and found this, which doesn't work because it refers to a file that no longer exists.

In your doc I'm not sure which directory name_of_test_subdirectory is supposed to be. But overall it is helpful, so we need to link to it from various places. So maybe the contribution section of the README should point to CONTRIBUTING, prior to pointing down any other rabbit hole.

I think an important change would be to modify this warning from ansible-test:

WARNING: Excluding tests marked "cloud/aws" which require config
(see "/home/dev/ansible/ansible/test/lib/ansible_test/config/cloud-config-aws.ini.template"): ec2_group

To say:

WARNING: Excluding tests marked "cloud/aws" which require config
Copy "/home/dev/ansible/ansible/test/lib/ansible_test/config/cloud-config-aws.ini.template" to something/integration/test/cloud-config-aws.ini : ec2_group

And the other change to make things clearer is to update this page to not mention hacking/env-setup because that doesn't exist in this repo any more after the big split.

@mdavis-xyz
Copy link
Contributor Author

Another issue I had is that I can't run ansible-test on it's own unless I remove ~/.aws. Using the --docker option results in an error. On my machine you need to run docker commands with sudo . Most people configure docker to not require that, but that just opens up a security hole because then malicious code can use docker to circumvent sudo protections.
There might be a way to work around that, but that's all too much effort for a one line bug fix that I want to contribute.

@mdavis-xyz
Copy link
Contributor Author

Another issue I just found: Integration tests fail if the account is not empty. The lambda integration tests assume there are no other lambdas in the account. I just swapped my region to one I don't use. But this is just another thing that should be documented in the README or CONTRIBUTING files in this repo.

@mdavis-xyz
Copy link
Contributor Author

After running the integration tests, there is a new file tests/integration/inventory. That seemed to have been created by the test scripts, and contains directories specific to me.

I'll add a .gitignore for that.

@tremble
Copy link
Contributor

tremble commented Nov 18, 2022

Another issue I just found: Integration tests fail if the account is not empty. The lambda integration tests assume there are no other lambdas in the account. I just swapped my region to one I don't use. But this is just another thing that should be documented in the README or CONTRIBUTING files in this repo.

This generally shouldn't be the case, our tests sometimes get run in parallel. Was the error permissions related?

@mdavis-xyz
Copy link
Contributor Author

mdavis-xyz commented Nov 18, 2022

No. The error was for integration testing lambda. It was an assertion error.

  - name: lambda_info | Ensure default query value is 'config' when function name
      omitted
    lambda_info:
    register: lambda_infos_query_config
    check_mode: yes
  - name: lambda_info | Assert successfull retrieval of all information
    assert:
      that:
...
      - lambda_infos_query_config.functions[0].function_name == lambda_function_name
      - 

If the freshly created lambda for this test is not the first lambda in the list (because a pre-existing lambda is), then this assertion fails.

Perhaps it's just that this specific test needs to be made a bit smarter.

@tremble
Copy link
Contributor

tremble commented Nov 18, 2022

Perhaps it's just that this specific test needs to be made a bit smarter.

Yup, that's a bug in that test. Such behaviour is "bad", and will result in our integration tests being flaky when run in CI

softwarefactory-project-zuul bot pushed a commit that referenced this issue Nov 18, 2022
…ts (#1276)

add .gitignore for inventory automatically created by integration tests

SUMMARY
Helps fix #924
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
tests
ADDITIONAL INFORMATION
When running integration tests, a file is generated at tests/integration/inventory. This file contains paths specific to the test machine. I assume this shouldn't be committed. So I've added a .gitignore for it.
The leading slash is to make it not apply recursively. (Stack Overflow)

Reviewed-by: Mark Chappell <None>
@tremble tremble reopened this Nov 18, 2022
softwarefactory-project-zuul bot pushed a commit that referenced this issue Nov 29, 2022
docs: Add more information about running integration tests

SUMMARY
Adds content to CONTRIBUTING.md,
and rearranges some red-herring links.
Fixes: #924
ISSUE TYPE

Docs Pull Request

COMPONENT NAME
CONTRIBUTING.md
ADDITIONAL INFORMATION

Reviewed-by: Mark Chappell <None>
Reviewed-by: Sandra McCann <samccann@redhat.com>
Reviewed-by: Matthew Davis <None>
Reviewed-by: Alina Buzachis <None>
softwarefactory-project-zuul bot pushed a commit to ansible-collections/community.aws that referenced this issue Feb 22, 2023
Update contributing to sync with amazon.aws

Fixes: #1607
SUMMARY
Update CONTRIBUTING.md to sync with amazon.aws and pick up various recent improvements
(See also ansible-collections/amazon.aws#924)
ISSUE TYPE

Docs Pull Request

COMPONENT NAME
CONTRIBUTING.md
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis
kamilturek pushed a commit to kamilturek/community.aws that referenced this issue Mar 19, 2023
Update contributing to sync with amazon.aws

Fixes: ansible-collections#1607
SUMMARY
Update CONTRIBUTING.md to sync with amazon.aws and pick up various recent improvements
(See also ansible-collections/amazon.aws#924)
ISSUE TYPE

Docs Pull Request

COMPONENT NAME
CONTRIBUTING.md
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants