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.

Sign up for GitHub

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

Jump to bottom

datapath/iptables: de-duplicate program argument construction #14007

Merged
merged 1 commit into from
Nov 16, 2020
Merged

datapath/iptables: de-duplicate program argument construction #14007

merged 1 commit into from
Nov 16, 2020

Conversation

tklauser
Copy link
Member

Merge common code for constructing iptables arguments.

@tklauser tklauser added kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. labels Nov 12, 2020
@tklauser tklauser requested a review from a team November 12, 2020 16:09
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Nov 12, 2020
@tklauser
Copy link
Member Author

test-me-please

qmonnet
qmonnet reviewed Nov 12, 2020
View reviewed changes
Copy link
Member

@qmonnet qmonnet left a comment •
edited

Choose a reason for hiding this comment

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

Nice!
Can't we factorise the "!", "-d", datapath.RemoteSNATDstAddrExclusionCIDR().String(), as well? It looks identical in both cases. Unless it has to be after the -o, but I don't think this is the case?

@tklauser
Copy link
Member Author

Nice!
Can't we factorise the "!", "-d", datapath.RemoteSNATDstAddrExclusionCIDR().String(), as well? It looks identical in both cases. Unless it has to be after the -o, but I don't think this is the case?

Good point. I naïvely assumed that the order of the arguments is important. But it seems for these it's not, so we can factor it out.

@tklauser
datapath/iptables: de-duplicate program argument construction
d5b20d9 
Merge common code for constructing iptables arguments.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@tklauser tklauser force-pushed the pr/tklauser/iptables-deduplicate branch from 3ae2253 to d5b20d9 Compare November 12, 2020 17:40
@tklauser
Copy link
Member Author

tklauser commented Nov 12, 2020 •
edited

test-me-please

previous failure due to dockerhub rate limiting

@tklauser
Copy link
Member Author

retest-gke

@tklauser
Copy link
Member Author

retest-4.9

@tklauser
Copy link
Member Author

retest-4.19

@tklauser
Copy link
Member Author

retest-net-next

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 13, 2020
fristonio
fristonio approved these changes Nov 16, 2020
View reviewed changes
Copy link
Member

@fristonio fristonio left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@fristonio fristonio merged commit 2815b78 into master Nov 16, 2020
@fristonio fristonio deleted the pr/tklauser/iptables-deduplicate branch November 16, 2020 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qmonnet qmonnet qmonnet approved these changes

@fristonio fristonio fristonio approved these changes

Assignees

@qmonnet qmonnet

Labels
kind/cleanup This includes no functional changes. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.10.0
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tklauser @qmonnet @fristonio