Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Added "to_destination" option to forward port to remote host #1617

Closed
wants to merge 1 commit into from
Closed

Added "to_destination" option to forward port to remote host #1617

wants to merge 1 commit into from

Conversation

rtm-ctrlz
Copy link

Example:
iptables -t nat -A port_forward -p tcp -m tcp --dport 2202 -j DNAT --to-destination 10.0.0.2:22

Use case:
On a host with a bunch of containers, ex. LXC.

@gregdek
Copy link
Contributor

gregdek commented Feb 5, 2016

Thanks @rtm-ctrlz. @LinusU please review according to guidelines (http://docs.ansible.com/ansible/developing_modules.html#module-checklist) and comment with text 'shipit' or 'needs_revision' as appropriate.

@sivel
Copy link
Member

sivel commented Feb 23, 2016

============================================================================
./system/iptables.py
============================================================================
ERROR: version_added for new option (to_destination) should be 2.1. Currently 0.0

@robynbergeron
Copy link
Contributor

@rtm-ctrlz A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes.

@LinusU
Copy link
Contributor

LinusU commented Feb 24, 2016

@robynbergeron Seems like he fixed it a day ago, this looks good to me 👍

'shipit'

@gregdek
Copy link
Contributor

gregdek commented Mar 2, 2016

Thanks again to @rtm-ctrlz for this PR, and thanks @LinusU for reviewing. Marking for inclusion.

@resmo
Copy link
Contributor

resmo commented Mar 16, 2016

Thanks @rtm-ctrlz for this PR. Unfortunately, it is not mergeable in its current state due to merge conflicts. Please rebase your PR. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review.

@rtm-ctrlz
Copy link
Author

'ready_for_review'

@LinusU
Copy link
Contributor

LinusU commented Mar 19, 2016

'shipit'

Looking good 👍 I personally would prefer to squash to commits into one though...

@gregdek
Copy link
Contributor

gregdek commented Mar 21, 2016

Thanks again to @rtm-ctrlz for this PR, and thanks @LinusU for reviewing. Marking for inclusion.

@gregdek gregdek added the shipit label Mar 21, 2016
@LinusU
Copy link
Contributor

LinusU commented Mar 22, 2016

132 commits? Something seems to have gone wrong with rebase/merge?

@jimi-c
Copy link
Member

jimi-c commented Mar 23, 2016

Yes, this needs to be cleaned up. This is common when a feature branch isn't used.

I'd create a new branch from origin/devel and cherry-pick the relevant commit(s) over to that, then revert/reset your working devel to match origin/devel. Unfortunately you'll most likely have to resubmit this PR as the source branch will have changed.

@jimi-c
Copy link
Member

jimi-c commented Mar 23, 2016

Another option would be to fork off a branch from your current devel, then fix devel by resetting/rebasing and cherry-pick the commit(s) back to devel. After that, git push devel -f to force push over your previous version on github should clean this up.

This way you should not have to resubmit the PR, but any further merges in your devel branch would mean you'd have to repeat this process (which is why it's always best to send PRs from feature branches).

@rtm-ctrlz rtm-ctrlz closed this Mar 24, 2016
@rtm-ctrlz rtm-ctrlz reopened this Mar 24, 2016
@rtm-ctrlz
Copy link
Author

Sorry for this mess with 130+ commits, fixed it.
Squashed changes to single commit.

As far as I can see this PR is 'ready_for_review'

@LinusU
Copy link
Contributor

LinusU commented Mar 24, 2016

Looks good 'shipit'

@mscherer
Copy link
Contributor

I am not sure the text to describe the option is correct. One can make dnat without specifying the port.

@gregdek
Copy link
Contributor

gregdek commented Mar 24, 2016

Thanks @rtm-ctrlz. To the current maintainers, @LinusU please review according to guidelines (http://docs.ansible.com/ansible/developing_modules.html#module-checklist) and comment with text 'shipit' or 'needs_revision' as appropriate.

@LinusU
Copy link
Contributor

LinusU commented Mar 25, 2016

The code looks good but, as pointed out by @mscherer, the documentation probably needs another look at.

'needs_revision'

@gregdek
Copy link
Contributor

gregdek commented Mar 29, 2016

Thanks @rtm-ctrlz for this PR. This PR requires revisions, either because it fails to build or by reviewer request. Please make the suggested revisions. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review.

[This message brought to you by your friendly Ansibull-bot.]

@gregdek
Copy link
Contributor

gregdek commented Apr 10, 2016

Thanks @rtm-ctrlz for this PR. Unfortunately, it is not mergeable in its current state due to merge conflicts. Please rebase your PR. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review.

[This message brought to you by your friendly Ansibull-bot.]

@rtm-ctrlz
Copy link
Author

Same functionality was pulled with #1971
Thanks a lot

@rtm-ctrlz rtm-ctrlz closed this Apr 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants