CORENET-5972: Consume openvswitch-ipsec systemd service for OVN IPsec deployment - #2662
Conversation
|
/testwith openshift/cluster-network-operator/master/e2e-aws-ovn-ipsec-upgrade openshift/machine-config-operator#4854 openshift/os#1718 openshift/machine-config-operator#4878 openshift/ovn-kubernetes#2472 |
2 similar comments
|
/testwith openshift/cluster-network-operator/master/e2e-aws-ovn-ipsec-upgrade openshift/machine-config-operator#4854 openshift/os#1718 openshift/machine-config-operator#4878 openshift/ovn-kubernetes#2472 |
|
/testwith openshift/cluster-network-operator/master/e2e-aws-ovn-ipsec-upgrade openshift/machine-config-operator#4854 openshift/os#1718 openshift/machine-config-operator#4878 openshift/ovn-kubernetes#2472 |
|
The |
|
@pperiyasamy I agree, we should get OVS 3.5 first into rhcos / ovn-k / microshift. We can install openvswitch3.5-ipsec at the same time, it should not be a problem since the service is disabled until CNO activates it. We need OVS 3.5 either way for other purposes (rhel 10 support, for example). Once we have OVS 3.5 and the openvswitch-ipsec service we can more easily test CNO and other changes. |
pperiyasamy
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@pperiyasamy: This pull request references SDN-5330 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. DetailsIn 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. |
51fb402 to
70f121e
Compare
|
/testwith openshift/cluster-network-operator/master/e2e-aws-ovn-ipsec-upgrade openshift/machine-config-operator#4854 openshift/os#1718 openshift/machine-config-operator#4878 openshift/ovn-kubernetes#2472 |
|
/testwith openshift/cluster-network-operator/master/e2e-aws-ovn-ipsec-upgrade openshift/os#1718 openshift/machine-config-operator#4878 openshift/ovn-kubernetes#2472 |
2933e60 to
45b106a
Compare
|
/testwith openshift/cluster-network-operator/master/e2e-aws-ovn-ipsec-upgrade openshift/ovn-kubernetes#2472 openshift/machine-config-operator#4878 |
|
/testwith openshift/cluster-network-operator/master/e2e-aws-ovn-ipsec-serial openshift/ovn-kubernetes#2472 openshift/machine-config-operator#4878 |
|
/testwith openshift/cluster-network-operator/master/e2e-aws-ovn-ipsec-upgrade openshift/ovn-kubernetes#2472 openshift/machine-config-operator#4878 |
|
/testwith openshift/cluster-network-operator/master/e2e-aws-ovn-ipsec-serial openshift/ovn-kubernetes#2472 openshift/machine-config-operator#4878 |
45b106a to
555d31c
Compare
|
/testwith openshift/cluster-network-operator/master/e2e-aws-ovn-ipsec-upgrade openshift/ovn-kubernetes#2472 openshift/machine-config-operator#4878 |
|
/testwith openshift/cluster-network-operator/master/e2e-aws-ovn-ipsec-serial openshift/ovn-kubernetes#2472 openshift/machine-config-operator#4878 |
|
/assign @anuragthehatter @huiran0826 |
555d31c to
5b0839a
Compare
|
Scheduling required tests: Scheduling tests matching the |
|
/testwith openshift/ovn-kubernetes/main/5.0-upgrade-from-stable-4.22/e2e-aws-ovn-upgrade-ipsec openshift/ovn-kubernetes#3254 openshift/machine-config-operator#4878 |
|
/testwith openshift/cluster-network-operator/master/e2e-aws-ovn-serial-ipsec openshift/machine-config-operator#4878 |
|
@pperiyasamy I didn't test this code and I'm not an expert in pod lifecycles, but the change seems correct to me. There is a ton of fallback code for the old behavior, which I hope can be removed in 5.1 (users can't upgrade to 5.1 without upgrading to 5.0 first, right?). Otherwise, |
| # Store initial PID for liveness probe to detect service restarts | ||
| cp /var/run/openvswitch/ovs-monitor-ipsec.pid /tmp/ovs-monitor-ipsec-base.pid |
There was a problem hiding this comment.
I guess I would do something like this:
# Capture PID and InvocationID atomically, verify no restart occurred
invocation_id=$(chroot /proc/1/root systemctl show -p InvocationID --value openvswitch-ipsec)
main_pid=$(chroot /proc/1/root systemctl show -p MainPID --value openvswitch-ipsec)
file_pid=$(cat /var/run/openvswitch/ovs-monitor-ipsec.pid 2>/dev/null)
if [ "$main_pid" != "$file_pid" ]; then
echo "openvswitch-ipsec PID mismatch: systemd reports $main_pid, PID file has $file_pid"
exit 1
fi
echo "$main_pid" > /tmp/ovs-monitor-ipsec-base.pid
while true; do
chroot /proc/1/root journalctl -f "_SYSTEMD_INVOCATION_ID=$invocation_id" -n all
echo "journalctl exited unexpectedly, retrying..."
sleep 2
done
So we capture invocation_id and pid right away, to frame what service run we are monitoring.
0417732 to
b757a12
Compare
jcaamano
left a comment
jcaamano
left a comment
There was a problem hiding this comment.
small nit, otherwise lgtm
| file_pid=$(cat /var/run/openvswitch/ovs-monitor-ipsec.pid 2>/dev/null) | ||
| main_pid=$(cat /tmp/ovs-monitor-ipsec-main.pid) | ||
| if [ "$main_pid" != "$file_pid" ]; then | ||
| echo "openvswitch-ipsec PID mismatch: systemd reports $main_pid, PID file has $file_pid" |
There was a problem hiding this comment.
I would keep the original message you had here
b757a12 to
cd84c22
Compare
| - mountPath: /usr/sbin | ||
| name: usr-sbin | ||
| - mountPath: /usr/libexec | ||
| name: usr-libexec |
There was a problem hiding this comment.
Hmm. The code still supports both modes - systemd on the host or ovs-monitor-ipsec daemon inside the container. But if we do not mount these, then ovs-monitor-ipsec inside the container will use ipsec command from the inside of the container that is potentially incompatible with the pluto running on the host. Is that right? Should the mounts be preserved until the fallback code is removed?
There was a problem hiding this comment.
I guess this comment is inline with the comment here
Also if we manage to keep the fallback working with commits Remove /usr/sbin host directory dependency from ovn-ipsec-host pod and Remove /usr/libexec mount and avoid GLIBC compatibility issues I don't understand why we need anything else.
I talked with @pperiyasamy offline and what I understood is this is picking between two evils: either we live RHEL10 glibc problems or we live with our host/container protocol problems and he chose the latter as the most unlikely. Please @pperiyasamy confirm.
There was a problem hiding this comment.
yes, mounting /usr/sbin also broke with chroot command (which pod uses it heavily) due to glibc compatibility issues. so had to get rid of mounting host /usr/sbin directory completely.
There was a problem hiding this comment.
And I guess this would be the only thing that would work in an hypothetical RHEL9 -> RHEL10 upgrade case, which I am not sure its a thing.
There was a problem hiding this comment.
Yeah, I see, it's hard to actually have both cases 100% working without doing terrible things inside the container like building a separate chroot or something. Since RHEL10 is TP in 5.0, I suppose, sacrificing the 4.x RHEL 9 -> 5.0 RHEL 10 upgrade path is more reasonable than having breakages in common cases. May need some documentation around that, but otherwise sounds fine to me.
The consumption of openvswitch-ipsec systemd service from the host for the ovn-ipsec-host deployment helps to avoid using any host binaries to be running from ovn-ipsec container, so it solves protocol, glibc and hardware compatibility issues when RHEL10 is supported for OCP platforms. Hence this commit does the following: 1. Stop spawning ovs-monitor-ipsec as foreground process in the ovn-ipsec container. Instead setup required IPsec configuration parameters in the /etc/sysconfig/openvswitch file, enable and start the openvswitch-ipsec service on the host. This is done at the time when ovn-ipsec-host pod is coming up for the first time. For the pod restart scenarios, it just checks openvswitch-ipsec service is running on the host, otherwise exit from the container with error. 2. Keep running an ovn-ipsec container that waits for the ovs-monitor-ipsec.log file to be created, monitors the openvswitch-ipsec service health, and redirects /var/log/openvswitch/ovs-monitor-ipsec.log to the ovn-ipsec container's stdout console. The container fails if the systemd service stops unexpectedly, ensuring proper pod lifecycle management. 3. There is no necessity of doing ipsec state and policy cleanup in ovn-ipsec-cleanup container when OVN IPsec is handled via openvswitch-ipsec systemd service. 4. During the OCP upgrade, the new ipsec os extension takes a while to deploy with openvswitch3.5-ipsec package, so by the time ovn-ipsec-host daemonset is rendered, we need to handle that scenario by running ovs-monitor-ipsec in the container. So this commit is also considering the transition phase of the process that is moving from container to host. 5. The ovn-keys init container configures ovs with IPsec certificate paths, so the container uses same host directory path to store and configure ovs with certificates because the ovs-monitor-ipsec process is running on the host now. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
This commit removes the mount of /usr/sbin from the host to eliminate need for host binaries. it would make chroot to use container binary which avoids GLIBC compatibility issue. The 'ipsec --checknss' check is removed, it doesn't run during upgrade until openvswitch3.5-ipsec is deployed to the node anyway, so it's safe to skip during that transition period. The liveness probe now uses 'ovs-appctl -t ovs-monitor-ipsec ipsec/status' instead of 'ipsec whack --trafficstatus' to check for configured IPsec tunnels without requiring host binaries. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
During OCP upgrade the new ovn-ipsec-host pod is rendered before the machine config rolls out openvswitch3.5-ipsec package to node. This causes the container to fall back to running ovs-monitor-ipsec in the container, but mounting host's /usr/libexec shadows container binaries and causes GLIBC compatibility errors (GLIBC_ABI_DT_RELR not found) when executing binaries like addconn and platform-python. - Remove /usr/libexec host mount entirely as it's no longer needed. - Use container's /usr/libexec/platform-python binary to run ovs-monitor-ipsec script. - Remove addconn validation as /etc/ipsec.conf is managed by ovs-monitor-ipsec and wait-for-ipsec-connect service, so it is always syntactically correct. - Remove _stackmanager check as it's obsolete (removed in libreswan 5.3+) and OCP must be using this version since 4.19. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
cd84c22 to
9de83fa
Compare
|
/lgtm |
|
Scheduling required tests: Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: igsilya, jcaamano, pperiyasamy The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/retest |
|
/retest-required |
|
/verified by CI |
openshift-ci-robot
added
the
verified
|
@mkowalski: This PR has been marked as verified by DetailsIn 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. DetailsInstructions 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. |
|
/override-sticky ci/prow/e2e-aws-ovn-upgrade-ipsec |
|
@mkowalski: Overrode contexts on behalf of mkowalski: ci/prow/e2e-aws-ovn-fdp-qe, ci/prow/e2e-aws-ovn-upgrade-ipsec, ci/prow/e2e-metal-ipi-ovn-dualstack-bgp-local-gw These overrides will persist across retests on the current HEAD SHA. Pushing a new commit will clear them. Use DetailsIn 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. |
7ca03ed
into
openshift:master
9 participants
This ipsec machine config extension is now installing
openvswitch3.5-ipsecpackage on the node, so this PR consumes that package to configure, enable and startopenvswitch-ipsecsystemd service which basically moves away running ovs-monitor-ipsec process from container to host.It fixes following issues.
addconnwhich depends onglibcwhich creates version incompatibility issues between host and container.auto=startparameter into each IPsec connections which is introduced by the PR OCPBUGS-52280, SDN-5330: Add ipsec connect wait service machine-config-operator#4854.In order to consume openvswitch-ipsec systemd service, this PR does the following:
Stop spawning ovs-monitor-ipsec as foreground process in the ovn-ipsec container. Instead setup required IPsec configuration parameters in the /etc/sysconfig/openvswitch file, enable and start the openvswitch-ipsec service
on the host. This is done at the time when ovn-ipsec-host pod is coming up for the first time. For the pod restart scenarios, it just checks openvswitch-ipsec service is running on the host, otherwise exit from the container with error.
Keep running an ovn-ipsec container that waits for the ovs-monitor-ipsec.log file to be created, monitors the openvswitch-ipsec service health, and redirects /var/log/openvswitch/ovs-monitor-ipsec.log to the ovn-ipsec container's stdout console. The container fails if the systemd service stops unexpectedly, ensuring
proper pod lifecycle management.
There is no necessity of doing ipsec state and policy cleanup in ovn-ipsec-cleanup container when OVN IPsec is handled via openvswitch-ipsec systemd service.
During the OCP upgrade, the new ipsec os extension takes a while to deploy with openvswitch3.5-ipsec package, so by the time ovn-ipsec-host daemonset is rendered, we need to handle that scenario by running ovs-monitor-ipsec in the container. So this PR is also considering the transition phase of the process that is moving
from container to host.
The ovn-keys init container configures ovs with IPsec certificate paths, so the container uses same host directory path to store and configure ovs with certificates because the ovs-monitor-ipsec process is running on the host now.
/assign @igsilya
Summary by CodeRabbit
Release Notes
Bug Fixes
Refactor