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

Handle OSError: file exists exception #26580

Closed
wants to merge 3 commits into from
Closed

Handle OSError: file exists exception #26580

wants to merge 3 commits into from

Conversation

cazorla19
Copy link

SUMMARY

I appended the handling of situation where we run multiple inventory cache updates on one host from one user. It's not related to any issue in Github, but it's important anyway.

What's is the issue at all? Let's imagine that I'm running multiple Ansible playbooks on EC2 inventory at a time. There may be used just one node, but might be multiple, it depends on the CI choice. To have my cache fresh, I have to update this running ec2.py --refresh-cache command. When cache will become empty, multiple processes will try to create a new directory and will fail doing this simultaneously.

Traceback (most recent call last):
  File "inventory/ec2.py", line 1326, in <module>
    Ec2Inventory()
  File "inventory/ec2.py", line 163, in __init__
    self.read_settings()
  File "inventory/ec2.py", line 319, in read_settings
    os.makedirs(cache_dir)
  File "/usr/lib64/python2.7/os.py", line 157, in makedirs
    mkdir(name, mode)
OSError: [Errno 17] File exists: '/home/ec2-user/.ansible/tmp/deploy_production_us'

My way to fix this is tested. Please, check the accordance to your code style and merge if everything's fine.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

contrib/inventory/ec2.py

ANSIBLE VERSION
ansible 2.3.0.0
  config file =
  configured module search path = Default w/o overrides
  python version = 2.7.10 (default, Dec  8 2015, 18:25:23) [GCC 4.8.3 20140911 (Red Hat 4.8.3-9)]
ADDITIONAL INFORMATION

To reproduce the error, you can run multiple EC2 inventory update processes. I'm not sure how to make it reproduced every time, because process may try to create the directory anytime and then avoid concurrency issue.

ec2.py --refresh-cache

@ansibot
Copy link
Contributor

ansibot commented Jul 10, 2017

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 aws bugfix_pull_request cloud needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jul 10, 2017
@ansibot
Copy link
Contributor

ansibot commented Jul 10, 2017

The test ansible-test compile --python 3.5 failed with the following error:

contrib/inventory/ec2.py:405:76: SyntaxError: print "Cache directory already exists. Skipping the step..."

The test ansible-test compile --python 3.6 failed with the following error:

contrib/inventory/ec2.py:405:76: SyntaxError: print "Cache directory already exists. Skipping the step..."

The test ansible-test sanity --test pylint failed with the following error:

contrib/inventory/ec2.py:405:0: syntax-error Missing parentheses in call to 'print'

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 10, 2017
@alikins alikins removed the needs_triage Needs a first human triage before being processed. label Jul 10, 2017
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 10, 2017
try:
os.makedirs(cache_dir)
except(OSError):
print('Cache directory already exists. Skipping the step...')
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think printing an informational message is that useful. Just pass here instead

@@ -399,7 +399,10 @@ def read_settings(self):
if self.boto_profile:
cache_dir = os.path.join(cache_dir, 'profile_' + self.boto_profile)
if not os.path.exists(cache_dir):
Copy link
Contributor

Choose a reason for hiding this comment

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

This check is now fairly pointless. If we're going to just try/except, no point checking it first.

Copy link
Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

@cazorla19 It seems like you still need to remove this line.

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jul 11, 2017
@ansibot ansibot added the inventory Inventory category label Jul 12, 2017
@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Jul 20, 2017
@ansibot
Copy link
Contributor

ansibot commented Sep 1, 2017

@ansibot ansibot added c:inventory/contrib_script support:community This issue/PR relates to code supported by the Ansible community. and removed support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 1, 2017
@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Oct 18, 2017
@ansibot ansibot removed the new_contributor This PR is the first contribution by a new community member. label Nov 3, 2017
@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Feb 6, 2018
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 2, 2018
@cazorla19
Copy link
Author

I'm gonna close this as an outdated topic, even me completely lost the track of this issue

@cazorla19 cazorla19 closed this May 22, 2018
@ansible ansible locked and limited conversation to collaborators May 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 aws bug This issue/PR relates to a bug. c:inventory/contrib_script cloud inventory Inventory category needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_contributor This PR is the first contribution by a new community member. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants