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

feat(linkerd-cni): add support for plain iptables commands #449

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

alpeb
Copy link
Member

@alpeb alpeb commented Dec 10, 2024

Currently the iptables-mode for linkerd-cni admits the values legacy and nft, which make the plugin use the iptables-legacy[-save] and iptables-nft[-save] commands respectively.

This assumes those commands are available in the node environment, given that linkerd-cni is triggered by the kubelet.

We have found that not to be the case for RHEL, where by default only iptables[-save] is available, which is equivalent to the iptables-nft[-save] command in other enviroments.

To address this case, this change adds a new possible value iptables-mode: default that makes the plugin use the iptables[-save] commands.

This has been tested successfully using RKE2 deployed in RHEL 8.10.

Currently the `iptables-mode` for linkerd-cni admits the values `legacy`
and `default`, which make the plugin use the `iptables-legacy[-save]`
and `iptables-nft[-save]` commands respectively.

This assumes those commands are available in the node environment, given
that linkerd-cni is triggered by the kubelet.

We have found that not to be the case for RHEL, where by default only
`iptables[-save]` is available, which is equivalent to the
`iptables-nft[-save]` command in other enviroments.

To address this case, this change adds a new possible value
`iptables-mode: default` that makes the plugin use the `iptables[-save]`
commands.

This has been tested successfully using RKE2 deployed in RHEL 8.10.
@alpeb alpeb requested a review from a team as a code owner December 10, 2024 16:21
alpeb added a commit to linkerd/linkerd2 that referenced this pull request Dec 10, 2024
This goes along with linkerd/linkerd2-proxy-init#449, that adds a new
value for the linkerd2-cni chart's `iptableMode` config. Only a doc
change.
Copy link
Member

@olix0r olix0r left a comment

Choose a reason for hiding this comment

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

Is the "default" mode actually the default? It looks like if the mode is unset, the default behavior is dependent on the value of FirewallBinPath, but if that's not set then we fail with an error?

My feeling is that we should not call this mode "default"--it's not the default behavior, and the default behavior can change in the future to a different mode! Is this more accurately called "infer?" Or should we explicitly call this "rhel"?

@alpeb
Copy link
Member Author

alpeb commented Dec 10, 2024

The --iptables-mode flag supersedes the --firewall-bin-path one. If neither is set we default to the legacy mode (iptables-legacy[-save]).
You're right, I was hesitant about using "default" for the new value. I wouldn't use "rhel" as this behavior might be found elsewhere. "infer" doesn't ring a bell either because we're not deducing about one command or another... Other options that come to mind: bare, flat, plain.
Let me know what you think.

@olix0r
Copy link
Member

olix0r commented Dec 11, 2024

"plain" sounds okay to me ¯_(ツ)_/¯

@alpeb alpeb merged commit ca9c460 into main Dec 11, 2024
7 checks passed
@alpeb alpeb deleted the alpeb/rhel-8-support branch December 11, 2024 21:15
alpeb added a commit to linkerd/linkerd2 that referenced this pull request Dec 12, 2024
* feat(linkerd-cni): add support for plain iptables commands

This goes along with linkerd/linkerd2-proxy-init#449, that adds a new
value for the linkerd2-cni chart's `iptableMode` config. Only a doc
change.

* s/default/plain
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