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

iptables lock issue #38106

Closed
wants to merge 3 commits into from
Closed

iptables lock issue #38106

wants to merge 3 commits into from

Conversation

akamac
Copy link

@akamac akamac commented Mar 29, 2018

SUMMARY

When adding multiple rules in a loop it is possible to get lock error.
-w switch fixes this:

--wait -w [seconds] wait for the xtables lock

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

iptables module

ANSIBLE VERSION

ansible 2.6.0 (fix c1aac04495)

ADDITIONAL INFORMATION

Added -w as default parameter.

@ansibot
Copy link
Contributor

ansibot commented Mar 29, 2018

@ansibot ansibot added bug This issue/PR relates to a bug. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Mar 29, 2018
@sebastiendarocha
Copy link
Contributor

Actually, older versions of iptables are incompatible with this option. I think Debian 7 is in this case.

I agree with you, this option important. Maybe we could check the version of iptables, or try without "-w" if the command fails ?

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Mar 29, 2018
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 6, 2018
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 6, 2018
@ansibot
Copy link
Contributor

ansibot commented Apr 6, 2018

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

lib/ansible/modules/system/iptables.py:459:4: using-constant-test Using a conditional statement with a constant value
lib/ansible/modules/system/iptables.py:472:17: undefined-variable Undefined variable 'module'

The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/system/iptables.py:348:0: ImportError: No module named pkg_resources

The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/system/iptables.py:348:0: ImportError: No module named pkg_resources

The test ansible-test sanity --test import --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/system/iptables.py:348:0: ImportError: No module named 'pkg_resources'

The test ansible-test sanity --test import --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/system/iptables.py:348:0: ModuleNotFoundError: No module named 'pkg_resources'

The test ansible-test sanity --test import --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/system/iptables.py:348:0: ModuleNotFoundError: No module named 'pkg_resources'

click here for bot help

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 14, 2018
@mattclay
Copy link
Member

@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Apr 26, 2018
@akamac
Copy link
Author

akamac commented May 10, 2018

@mattclay Seems to be unit tests should be fixed to reflect the new -w switch.

@mattclay
Copy link
Member

@akamac Yes, the unit tests will need to be updated as part of this PR.

@ansibot
Copy link
Contributor

ansibot commented May 18, 2018

@akamac this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented May 18, 2018

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

lib/ansible/modules/system/iptables.py:476:4: using-constant-test Using a conditional statement with a constant value
lib/ansible/modules/system/iptables.py:489:17: undefined-variable Undefined variable 'module'

The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/system/iptables.py:365:0: ImportError: No module named pkg_resources

The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/system/iptables.py:365:0: ImportError: No module named pkg_resources

The test ansible-test sanity --test import --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/system/iptables.py:365:0: ImportError: No module named 'pkg_resources'

The test ansible-test sanity --test import --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/system/iptables.py:365:0: ModuleNotFoundError: No module named 'pkg_resources'

The test ansible-test sanity --test import --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/system/iptables.py:365:0: ModuleNotFoundError: No module named 'pkg_resources'

click here for bot help

@ansibot ansibot added affects_2.6 This issue/PR affects Ansible v2.6 merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html and removed ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels May 18, 2018
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label May 26, 2018
@ansibot ansibot added the system System category label Feb 17, 2019
@bcoca
Copy link
Member

bcoca commented Oct 18, 2019

consider making this a module option, I would not accept it in it's current state as it is both backwards incompatible with older iptables and changes the default behaviour of the module.

needs_info

@ansibot ansibot added the needs_info This issue requires further information. Please answer any outstanding questions. label Oct 18, 2019
@Akasurde
Copy link
Member

Closing in favor of #47877

@Akasurde Akasurde closed this Oct 19, 2019
@ansible ansible locked and limited conversation to collaborators Nov 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.6 This issue/PR affects Ansible v2.6 bug This issue/PR relates to a bug. merge_commit This PR contains at least one merge commit. Please resolve! module This issue/PR relates to a module. needs_info This issue requires further information. Please answer any outstanding questions. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html 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. support:core This issue/PR relates to code supported by the Ansible Engineering Team. system System category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants