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: Remove leading zeroes #16817

Conversation

jrajahalme
Copy link
Member

Remove leading zeroes from marks, as 'iptables' is not formatting
them. This allows proper matching of existing rules and avoids
appending duplicate rules.

Backports of the iptables change in #16391 for 1.9 and 1.8 already contain this fix.

Fixes: #16391
Signed-off-by: Jarno Rajahalme jarno@isovalent.com

Remove leading zeroes from marks, as 'iptables' is not formatting
them. This allows proper matching of existing rules and avoids
appending duplicate rules.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme added kind/bug This is a bug in the Cilium logic. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. release-note/misc This PR makes changes that have no direct user impact. needs-backport/1.10 labels Jul 7, 2021
@jrajahalme jrajahalme requested a review from a team July 7, 2021 11:43
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.3 Jul 7, 2021
@jrajahalme jrajahalme requested a review from borkmann July 7, 2021 11:43
@jrajahalme
Copy link
Member Author

test-me-please

Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

This was found by observing duplicate rules in a deployed environment right? Is there a way to detect this kind of problem in future?

@aanm aanm added the kind/regression This functionality worked fine before, but was broken in a newer release of Cilium. label Jul 7, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Backport pending to v1.9 in 1.9.9 Jul 7, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Backport pending to v1.8 in 1.8.11 Jul 7, 2021
@pchaigno pchaigno added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jul 8, 2021
@pchaigno
Copy link
Member

pchaigno commented Jul 8, 2021

This PR needs to be rebased to catch #16767.

@jrajahalme
Copy link
Member Author

known flake #14598 in test-1.21-4.9

@jrajahalme
Copy link
Member Author

None of the test fails are due to this PR, and this PR being a regression fix I'll mark this ready-to-merge without further re-testing.

@jrajahalme jrajahalme added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 12, 2021
@pchaigno
Copy link
Member

this PR being a regression fix

In that case, shouldn't the release-note label say "bug"?

@kkourt
Copy link
Contributor

kkourt commented Jul 13, 2021

Merging this, since the release-note can be changed also after merging.

@kkourt kkourt merged commit d5ff687 into cilium:master Jul 13, 2021
@pchaigno pchaigno removed the release-note/misc This PR makes changes that have no direct user impact. label Jul 13, 2021
@pchaigno pchaigno added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Jul 13, 2021
@jrajahalme jrajahalme removed dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Jul 14, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport done to v1.10 in 1.10.3 Jul 15, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport done to v1.10 in 1.10.3 Jul 15, 2021
@joestringer joestringer added this to Backport pending to v1.9 in 1.9.10 Jul 19, 2021
@joestringer joestringer removed this from Backport pending to v1.9 in 1.9.9 Jul 19, 2021
@joestringer joestringer moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.10 Jul 19, 2021
@joestringer joestringer moved this from Backport pending to v1.8 to Backport done to v1.8 in 1.8.11 Jul 19, 2021
@joestringer
Copy link
Member

@jrajahalme I believe that the backports were already done for this PR for both v1.9 and v1.8, can you confirm?

@joestringer joestringer removed this from Backport done to v1.9 in 1.9.10 Jul 19, 2021
@joestringer joestringer added this to Backport done to v1.9 in 1.9.9 Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. kind/bug This is a bug in the Cilium logic. kind/regression This functionality worked fine before, but was broken in a newer release of Cilium. release-note/bug This PR fixes an issue in a previous release of Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
No open projects
1.10.3
Backport done to v1.10
1.8.11
Backport done to v1.8
1.9.9
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

None yet

6 participants