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

Bug 1726045: skip OPENSHIFT-MASQ for traffic already marked for masquerade #13

Merged
merged 1 commit into from Jul 24, 2019

Conversation

squeed
Copy link
Contributor

@squeed squeed commented Jul 24, 2019

If a packet has already been marked by other kube-proxy rules for masquerade, don't run it through the OPENSHIFT-MASQUERADE chain for further twiddling.

Most notably, this chain is used for Egress IPs.

This change fixes a bug where egress IPs can't access services via their ExternalIP. (bz 1726045)

If a packet has already been marked by other kube-proxy rules for
masquerade, don't run it through the OPENSHIFT-MASQUERADE chain for
further twiddling.

Most notably, this chain is used for Egress IPs.

This change fixes a bug where egress IPs can't access services via their
ExternalIP. (bz 1726045)
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 24, 2019
@squeed
Copy link
Contributor Author

squeed commented Jul 24, 2019

@danwinship you already LGTM'd this - but now it's in the correct repo.

@squeed squeed changed the title skip OPENSHIFT-MASQ for traffic already marked for masquerade Bug 1726045: skip OPENSHIFT-MASQ for traffic already marked for masquerade Jul 24, 2019
@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Jul 24, 2019
@openshift-ci-robot
Copy link
Contributor

@squeed: This pull request references a valid Bugzilla bug. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Bug 1726045: skip OPENSHIFT-MASQ for traffic already marked for masquerade

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@danwinship
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 24, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, squeed

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit a1c1a66 into openshift:master Jul 24, 2019
@openshift-ci-robot
Copy link
Contributor

@squeed: All pull requests linked via external trackers have merged. The Bugzilla bug has been moved to the MODIFIED state.

In response to this:

Bug 1726045: skip OPENSHIFT-MASQ for traffic already marked for masquerade

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@squeed squeed deleted the no-egressip-masq branch July 25, 2019 12:14
squeed added a commit to squeed/openshift-sdn that referenced this pull request Jul 29, 2019
PR openshift#13 changed the source rule for OPENSHIFT-MASQUERADE. However,
rolling out source-rule changes on a running system isn't clean. Easier
to leave the source-rule unchanged, and just add a rule first in the
chain that gets us where we want to be.

So, -j RETURN for traffic that's already marked for MASQ.
openshift-merge-robot added a commit that referenced this pull request Jul 29, 2019
iptables: partial #13 revert, skip masq in chain
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants