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

Fix IsNotExist for ip6tables in nft mode #74

Merged
merged 1 commit into from
Mar 20, 2020

Conversation

rhafer
Copy link
Contributor

@rhafer rhafer commented Mar 18, 2020

Don't include the command name when comparing the error messages.
"ip6tables" in nft mode uses "iptables" in it's error messages.

E.g.:
in nft mode:

ip6tables  -D POSTROUTING -s 1100:200::5/24 -j DROP
iptables: Bad rule (does a matching rule exist in that chain?).

same command in legacy mode:

ip6tables -t nat -D POSTROUTING -s 1100:200::5/24 -j DROP
ip6tables: Bad rule (does a matching rule exist in that chain?).

Note: This is a partial revert of 5991869

Signed-off-by: Ralf Haferkamp rhafer@suse.com

Don't include the command name when comparing the error messages.
"ip6tables" in nft mode uses "iptables" in it's error messages.

E.g.:
in nft mode:
ip6tables  -D POSTROUTING -s 1100:200::5/24 -j DROP
iptables: Bad rule (does a matching rule exist in that chain?).

same command in legacy mode:
ip6tables -t nat -D POSTROUTING -s 1100:200::5/24 -j DROP
ip6tables: Bad rule (does a matching rule exist in that chain?).

Note: This is a partial revert of 5991869

Signed-off-by: Ralf Haferkamp <rhafer@suse.com>
@lucab
Copy link
Contributor

lucab commented Mar 20, 2020

LGTM, thanks for fixing that!

@lucab lucab merged commit 521ee6c into coreos:master Mar 20, 2020
@rhafer
Copy link
Contributor Author

rhafer commented Mar 23, 2020

@lucab Thanks a lot for merging the fix so quick. Do you have any plans on cutting a new release?

tlwr added a commit to tlwr/cni-plugins that referenced this pull request Dec 31, 2020
Closes containernetworking#544

The above issue describes a situation where using the bridge plugin
with IPv6 addresses prevented `DEL` from working correctly.

`DEL` seems to be failing in the body of `TeardownIPMasq`

This arises because:

* twice delete postrouting rules: `ipn.String()` `ipn.IP.String()` containernetworking#279
* we are using a version of go-iptables which is bugged for v6

PR github.com/coreos/go-iptables/pull/74 describes why this does
not work. The error message is not being checked correctly.

Using a later version of go-iptables means that
* when the second `ipt.Delete` fails (this is okay)
* we will correctly interpret this as an non-fatal error
* `TeardownIPMasq` will not prematurely exit the method
* `ipt.ClearChain` now can run
* `ipt.DeleteChain` now can run

This explains why this was working for v4 but not v6

Signed-off-by: toby lorne <toby@toby.codes>
tlwr added a commit to tlwr/cni-plugins that referenced this pull request Jan 5, 2021
Closes containernetworking#544

The above issue describes a situation where using the bridge plugin
with IPv6 addresses prevented `DEL` from working correctly.

`DEL` seems to be failing in the body of `TeardownIPMasq`

This arises because:

* twice delete postrouting rules: `ipn.String()` `ipn.IP.String()` containernetworking#279
* we are using a version of go-iptables which is bugged for v6

PR github.com/coreos/go-iptables/pull/74 describes why this does
not work. The error message is not being checked correctly.

Using a later version of go-iptables means that
* when the second `ipt.Delete` fails (this is okay)
* we will correctly interpret this as an non-fatal error
* `TeardownIPMasq` will not prematurely exit the method
* `ipt.ClearChain` now can run
* `ipt.DeleteChain` now can run

This explains why this was working for v4 but not v6

This commit was amended to include v0.5.0 instead of a pseudo-version
v0.4.6-0.20200318170312-12696f5c9108

Signed-off-by: toby lorne <toby@toby.codes>
apreuavar6 added a commit to apreuavar6/cni-plugins that referenced this pull request Aug 11, 2024
Closes #544

The above issue describes a situation where using the bridge plugin
with IPv6 addresses prevented `DEL` from working correctly.

`DEL` seems to be failing in the body of `TeardownIPMasq`

This arises because:

* twice delete postrouting rules: `ipn.String()` `ipn.IP.String()` #279
* we are using a version of go-iptables which is bugged for v6

PR github.com/coreos/go-iptables/pull/74 describes why this does
not work. The error message is not being checked correctly.

Using a later version of go-iptables means that
* when the second `ipt.Delete` fails (this is okay)
* we will correctly interpret this as an non-fatal error
* `TeardownIPMasq` will not prematurely exit the method
* `ipt.ClearChain` now can run
* `ipt.DeleteChain` now can run

This explains why this was working for v4 but not v6

This commit was amended to include v0.5.0 instead of a pseudo-version
v0.4.6-0.20200318170312-12696f5c9108

Signed-off-by: toby lorne <toby@toby.codes>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants