kubevirt
kubevirt copied to clipboard
kubevirt
masquerade: enable iptables masquerade working on low kernel
What this PR does / why we need it:
The PR is try to fix nft masquerade mode not work on low kernel < 3.18, on kernel < 3.18 will try to set rules with iptables.
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #6620,#7006
Special notes for your reviewer: using masquerade mode on kernel < 3.18 is not work. Release note:
NONE
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by:
To complete the pull request process, please assign alonakaplan after the PR has been reviewed.
You can assign the PR to them by writing /assign @alonakaplan in a comment when ready.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Hi @tgfree7. Thanks for your PR.
I'm waiting for a kubevirt member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
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.
@tgfree7 , I am unsure if we as a project want to support such old nodes. It seems to me like a heavy maintenance burden to take.
There was also a motion towards announcing the deprecation of iptables for the same reason (reduce maintenance), especially after most distributions have moved to nftables.
/cc @AlonaKaplan
@tgfree7 , I am unsure if we as a project want to support such old nodes. It seems to me like a heavy maintenance burden to take.
There was also a motion towards announcing the deprecation of
iptablesfor the same reason (reduce maintenance), especially after most distributions have moved tonftables./cc @AlonaKaplan
I totally agree with @EdDev. We are planning to declare iptables as deprecated. @davidvossel what is the oldest kernel version we should support?
@tgfree7 , I am unsure if we as a project want to support such old nodes. It seems to me like a heavy maintenance burden to take.
There was also a motion towards announcing the deprecation of
iptablesfor the same reason (reduce maintenance), especially after most distributions have moved tonftables./cc @AlonaKaplan
@EdDev Thanks for reply, if kubevirt not supprt old nodes, may add a requirement for Kernel version on docs or somewhere?
@davidvossel what is the oldest kernel version we should support?
Practically, we only guarantee support for whatever kernel our CI tests use. Everything else is best effort simply because we are only testing one kernel release series in CI. That doesn't mean we shouldn't continue to accept bug fixes that impact older or newer kernels.
For this iptables issue, as long as we have the code in the code base we should accept fixes. If people are encountering issues here, that means these code paths are actually being used.
We should have a discussion about how we approach deprecating features so we can begin planning how to properly transition away from maintaining untested features like this long term.
@davidvossel what is the oldest kernel version we should support?
Practically, we only guarantee support for whatever kernel our CI tests use. Everything else is best effort simply because we are only testing one kernel release series in CI. That doesn't mean we shouldn't continue to accept bug fixes that impact older or newer kernels.
For this iptables issue, as long as we have the code in the code base we should accept fixes. If people are encountering issues here, that means these code paths are actually being used.
Our CI is not using iptables for at least a year. Our first choice is always nft and only if it doesn't exist iptables are used. In the specific case of this PR the iptable code path wasn't invoked (if it was we wouldn't need the fix:)). Network team will send a mail to the community notifying iptables is going to be deprecated and then a PR to remove iptables from the code base.
We should have a discussion about how we approach deprecating features so we can begin planning how to properly transition away from maintaining untested features like this long term.
@tgfree7: PR needs rebase.
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.
Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
/lifecycle stale
Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
/lifecycle rotten
Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
/close
@kubevirt-bot: Closed this PR.
In response to this:
Rotten issues close after 30d of inactivity. Reopen the issue with
/reopen. Mark the issue as fresh with/remove-lifecycle rotten./close
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.