search

kubernetes-sigs / ip-masq-agent

Manage IP masquerade on nodes
Apache License 2.0
217 stars 70 forks source link

Support an optional SNAT target instead of MASQUERADE #57

Closed bjoernrost closed 3 years ago

bjoernrost commented 4 years ago

Adds an optional parameter "SnatTarget". When set to an ip address, ip-masq-agent will create an SNAT rule instead of MASQUERADE for outbound traffic. This fixes #56

k8s-ci-robot commented 4 years ago

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
k8s-ci-robot commented 4 years ago

Welcome @bjoernrost!

It looks like this is your first PR to kubernetes-sigs/ip-masq-agent 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/ip-masq-agent has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. :smiley:

bjoernrost commented 4 years ago

I signed it

bjoernrost commented 4 years ago

/assign @bowei

bjoernrost commented 3 years ago

@bowei @MrHohn do you have an estimate on when you will be able to review this PR? And in the meantime, is there anything that I can do to help this move along?

MrHohn commented 3 years ago

Sorry for the huge delay. I found this to be an interesting use case. I saw a similar use case where a customer wanted to set up 1:1 NAT rule for each pod, which was trickier and we ended up not having a good solution.

Though I do wonder how to use this in practice - wouldn't it invalidate other traffic that is not supposed to be go through the static/LB IP? ip-masq-agent is usually deployed as a daemonset - do we expect outbound traffic from the entire cluster to be SNAT to a static IP? Or is it deploying the agent only to specific nodes to special case them?

Also there are some cases we SNAT to the node IP when bouncing packets from outside to the backend pod that runs on another node. I wonder it this would break those cases.. e.g.:

client -> LB -> node1 -(SNAT to node1 IP)-> node2 -> pod

Adding @varunmar @freehan for inputs as well :) /cc @freehan @varunmar

k8s-ci-robot commented 3 years ago

@MrHohn: GitHub didn't allow me to request PR reviews from the following users: freehan, varunmar.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to [this](https://github.com/kubernetes-sigs/ip-masq-agent/pull/57#issuecomment-725019388): >Sorry for the huge delay. I found this to be an interesting use case. I saw a similar use case where a customer wanted to set up 1:1 NAT rule for each pod, which was trickier and we ended up not having a good solution. > >Though I do wonder how to use this in practice - wouldn't it invalidate other traffic that is not supposed to be go through the static/LB IP? ip-masq-agent is usually deployed as a daemonset - do we expect outbound traffic from the entire cluster to be SNAT to a static IP? Or is it deploying the agent only to specific nodes to special case them? > >Also there are some cases we SNAT to the node IP when bouncing packets from outside to the backend pod that runs on another node. I wonder it this would break those cases.. e.g.: >``` >client -> LB -> node1 -(SNAT to node1 IP)-> node2 -> pod >``` > >Adding @varunmar @freehan for inputs as well :) >/cc @freehan @varunmar Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
bjoernrost commented 3 years ago

Multiple nodes using the same SNAT ip would break external networking for everything but very edge cases. I built this for and tested with single-node "clusters" in mind. I had naively assumed that when using multiple nodes one could have a dedicated LB for each node and set the SNAT ip to the assigned LB. However, that would require two more pieces: 1 - a way to set a different value for the SNAT ip on each node 2 - a mechanism to map a static LB IP to a node and also to manage and create additional LBs as a cluster scales

The second one is a biggie and if/when someone builds that they could likely just add the little bit of iptables logic to that thing.

fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale

fejta-bot commented 3 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten

bowei commented 3 years ago

Some minor comments, otherwise LGTM

@varunmar

k8s-ci-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bjoernrost To complete the pull request process, please ask for approval from bowei after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/kubernetes-sigs/ip-masq-agent/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
bjoernrost commented 3 years ago

/remove-lifecycle rotten

fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale

k8s-triage-robot commented 3 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot commented 3 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot commented 3 years ago

@k8s-triage-robot: Closed this PR.

In response to [this](https://github.com/kubernetes-sigs/ip-masq-agent/pull/57#issuecomment-913237103): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages issues and PRs according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed > >You can: >- Reopen this issue or PR with `/reopen` >- Mark this issue or PR as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.