OCPBUGS-33656: Remove ip xfrm state when IPsec is disabled #2372
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
pperiyasamy
force-pushed
the
remove-ipsec-state
branch
3 times, most recently
from
05e8f36 to
dbebf6c
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
pperiyasamy
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@pperiyasamy: This pull request references Jira Issue OCPBUGS-33656, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-bug
|
/retest |
| # entries created for ovs. Since it's cleaning up whole entries, | ||
| # it may cause slight interruption in case of external ipsec | ||
| # configuration. | ||
| echo "remove ovs ipsec state if present" |
There was a problem hiding this comment.
problem is, this also removes the N-S state, not just ovs
so need to verify OVNIPsecEnable actually means both?
There was a problem hiding this comment.
yes, it also does ipsec service restart and wouldn't this get ipsec state for NS back into the system ?
Moreover this logic is similar to what ovn-ipsec container does here and same thing happens for container restart scenarios as well.
The OVNIPsecEnable flag is used only to identify if EW IPsec is enabled or not.
The IPsec mode is changed in following order Full -> Disabled -> External, EW ipsec connections in openshift.conf occasionally not cleaned up even after IPsec disabled, then external mode gets back those connections active again. we may delete only openshift.conf file to handle this case, but not sure about the behavior for Full -> External . Hence this change is introduced to handle all the cases.
| ip x p flush | ||
| rm -f /etc/ipsec.d/openshift.conf || true | ||
| # if pluto is running, we need to restart it after the flush | ||
| chroot /proc/1/root ipsec restart || true |
There was a problem hiding this comment.
@bengal I remember there used to be a NM issue around this, not sure if fixed?
There was a problem hiding this comment.
AFAIK, restarting ipsec while there are active connections created by NM-libreswan would break them. You need to reactivate such connections manually to restore connectivity.
There was a problem hiding this comment.
doesnt feels very production grade.
if pluto daemon restart, for any reason (error? crash? etc)
all the connections will die and wont even attempt to restart
There was a problem hiding this comment.
Right, in case of crashes that's a problem. Is this issue already filed somewhere? If not please do, so that we can plan how to improve this in NM-libreswan.
openshift-merge-robot
added
the
needs-rebase
| # entries created for ovs. Since it's cleaning up whole entries, | ||
| # it may cause slight interruption in case of external ipsec | ||
| # configuration. | ||
| echo "remove ovs ipsec state if present" |
There was a problem hiding this comment.
another important issue with this, is that AFAIK, it'll create connectivity issues for workloads!
removing the state, forces ipsec off, but if the other node is not updated at exact the same moment, it'll still try to do ipsec (according to it's policy)
I dont think there's a good solution for this besides leaving ipsec on until next reboot
pperiyasamy
force-pushed
the
remove-ipsec-state
branch
from
dbebf6c to
2f6f0d2
Compare
openshift-merge-robot
removed
the
needs-rebase
pperiyasamy
force-pushed
the
remove-ipsec-state
branch
from
2f6f0d2 to
b1d59fe
Compare
|
/test e2e-aws-ovn-ipsec-upgrade |
openshift-merge-robot
added
the
needs-rebase
openshift-ci
bot
added
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcaamano, pperiyasamy, trozet The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/assign @huiran0826 @anuragthehatter |
|
The ipsec test failures with |
|
Pre-merge tested this PR, after changing ipsec mode from full to external. I checked below items
/label qe-approved |
openshift-ci
bot
added
the
qe-approved
openshift-ci-robot
added
jira/invalid-bug
|
@pperiyasamy: This pull request references Jira Issue OCPBUGS-33656, which is invalid:
Comment In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@pperiyasamy: This pull request references Jira Issue OCPBUGS-33656, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/test e2e-metal-ipi-ovn-ipv6-ipsec |
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/label acknowledge-critical-fixes-only |
openshift-ci
bot
added
the
acknowledge-critical-fixes-only
|
/retest |
openshift-merge-bot
bot
merged commit 95f4792
into
openshift:master
|
@pperiyasamy: Jira Issue OCPBUGS-33656: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-33656 has been moved to the MODIFIED state. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@pperiyasamy: The following tests failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
|
[ART PR BUILD NOTIFIER] Distgit: cluster-network-operator |
|
/cherry-pick release-4.18 |
|
@pperiyasamy: new pull request created: #2592 In response to this:
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-sigs/prow repository. |
The ovn-ipsec-host daemonset doesn't remove ipsec state when IPsec for east west traffic is disabled. This might still keep running data plane traffic with ipsec encrypted. So this PR cleans up ipsec state so that east west traffic restored with only geneve header.