Skip to content

openshift-sdn, ovn-kubernetes: adopt systemd-managed openvswitch if present #672

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.

Sign up for GitHub

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

Merged
openshift-merge-robot merged 1 commit into from
Jun 23, 2020
Merged

openshift-sdn, ovn-kubernetes: adopt systemd-managed openvswitch if present #672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 96 additions & 95 deletions bindata/network/openshift-sdn/sdn-ovs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,131 +34,128 @@ spec:
- |
#!/bin/bash
set -euo pipefail
chown -R openvswitch:openvswitch /var/run/openvswitch
chown -R openvswitch:openvswitch /etc/openvswitch

# if another process is listening on the cni-server socket, wait until it exits
retries=0
while true; do
if /usr/share/openvswitch/scripts/ovs-ctl status &>/dev/null; then
echo "warning: Another process is currently managing OVS, waiting 15s ..." 2>&1
sleep 15 & wait
(( retries += 1 ))
else
break
fi
if [[ "${retries}" -gt 40 ]]; then
echo "error: Another process is currently managing OVS, exiting" 2>&1
exit 1
export SYSTEMD_IGNORE_CHROOT=yes
# Check to see if ovs is provided by the node:
if systemctl status ovsdb-server; then
Comment thread
squeed marked this conversation as resolved.
echo "openvswitch is running in systemd"
# In some very strange corner cases, the owner for /run/openvswitch
# can be wrong, so we need to clean up and restart.
ovs_uid=$(chroot /host id -u openvswitch)
ovs_gid=$(chroot /host id -g openvswitch)
chown -R "${ovs_uid}:${ovs_gid}" /run/openvswitch

@mccv1r0 mccv1r0 Jun 22, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doesn't this just move the problem?

When MCO and CNO have different uid/gid for the openvswitch user/group the CNO got permission denied for things created by MCO. The result was OVN didn't come up. Doing chown -R to set the uid/gid to what MCO uses doesn't change the fact that the CNO container is using a different uid/gid for the openvswitch user/group. When a CNO container tries to access these files using a different uid/gid are you sure that CNO will not hit the same permission denied problem(s)?

@squeed squeed Jun 22, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Everything except openvswitch runs as root. So, yes, we should be fine for permissions. We're also not writing anything to this directory from a container anymore.

@trozet trozet Jun 22, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

my understanding here is that we are not trying to set the permissions for this container, we are trying to fix the permissions on the host for the ovs directory because it may have a conflict with kubelet. We had similar problems in openstack between OVS and nova. I guess you could argue that this really isn't this container's job and the problem should be fixed by MCO?

On the other hand while this container will not issue any ovs commands, the ovn-k8s container will. Does the uid/gid matter in that case to just call the binaries?

@squeed squeed Jun 23, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I should have been more specific. If openvswitch is a systemd service, no containerized processes will write to /run/openvswitch. They'll only read. So we won't have permissions issues.

This horrible hack is to clean up something that is unlikely, namely, kubelet starting this before /run/openvswitch exists. I think it can't happen anymore, but I saw it in CI a while back.

@mccv1r0 mccv1r0 Jun 23, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There were issues with br-int.mgmt being created but didn't have permission. Is this OVS or OVN?

@squeed squeed Jun 23, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Where did you see these issues? In CI? Or in your PR?

if [[ ! -S /run/openvswitch/db.sock ]]; then
systemctl restart ovsdb-server
fi
done

function quit {
# Save the flows
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Saving flows ..." 2>&1
bridges=$(ovs-vsctl -- --real list-br)
TMPDIR=/var/run/openvswitch /usr/share/openvswitch/scripts/ovs-save save-flows $bridges > /var/run/openvswitch/flows.sh
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Saved flows" 2>&1
# Don't need to worry about restoring flows; this can only change if we've rebooted
exec tail -F /host/var/log/openvswitch/ovs-vswitchd.log /host/var/log/openvswitch/ovsdb-server.log
else

# Don't allow ovs-vswitchd to clear datapath flows on exit
kill -9 $(cat /var/run/openvswitch/ovs-vswitchd.pid 2>/dev/null) 2>/dev/null || true
kill $(cat /var/run/openvswitch/ovsdb-server.pid 2>/dev/null) 2>/dev/null || true
exit 0
}
trap quit SIGTERM
echo "openvswitch is running in container"
# if another process is listening on the cni-server socket, wait until it exits
retries=0
while true; do
if /usr/share/openvswitch/scripts/ovs-ctl status &>/dev/null; then
echo "warning: Another process is currently managing OVS, waiting 15s ..." 2>&1
sleep 15 & wait
(( retries += 1 ))
else
break
fi
if [[ "${retries}" -gt 40 ]]; then
echo "error: Another process is currently managing OVS, exiting" 2>&1
exit 1
fi
done

function deletePid {
rm /var/run/openvswitch/ovs-vswitchd.pid
rm /var/run/openvswitch/ovsdb-server.pid
}
trap deletePid EXIT
function quit {
# Save the flows
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Saving flows ..." 2>&1
bridges=$(ovs-vsctl -- --real list-br)
TMPDIR=/var/run/openvswitch /usr/share/openvswitch/scripts/ovs-save save-flows $bridges > /var/run/openvswitch/flows.sh
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Saved flows" 2>&1

# launch OVS
# Start the ovsdb so that we can prep it before we start the ovs-vswitchd
/usr/share/openvswitch/scripts/ovs-ctl start --ovs-user=openvswitch:openvswitch --no-ovs-vswitchd --system-id=random --no-monitor
# Don't allow ovs-vswitchd to clear datapath flows on exit
kill -9 $(cat /var/run/openvswitch/ovs-vswitchd.pid 2>/dev/null) 2>/dev/null || true
kill $(cat /var/run/openvswitch/ovsdb-server.pid 2>/dev/null) 2>/dev/null || true
exit 0
}
trap quit SIGTERM

# Set the flow-restore-wait to true so ovs-vswitchd will wait till flows are restored
ovs-vsctl --no-wait set Open_vSwitch . other_config:flow-restore-wait=true

# Restrict the number of pthreads ovs-vswitchd creates to reduce the
# amount of RSS it uses on hosts with many cores
# https://bugzilla.redhat.com/show_bug.cgi?id=1571379
# https://bugzilla.redhat.com/show_bug.cgi?id=1572797
if [[ `nproc` -gt 12 ]]; then
ovs-vsctl --no-wait set Open_vSwitch . other_config:n-revalidator-threads=4
ovs-vsctl --no-wait set Open_vSwitch . other_config:n-handler-threads=10
fi
function deletePid {
rm /var/run/openvswitch/ovs-vswitchd.pid
rm /var/run/openvswitch/ovsdb-server.pid
}
trap deletePid EXIT

# And finally start the ovs-vswitchd now the DB is prepped
/usr/share/openvswitch/scripts/ovs-ctl start --ovs-user=openvswitch:openvswitch --no-ovsdb-server --system-id=random --no-monitor
chown -R openvswitch:openvswitch /var/run/openvswitch
chown -R openvswitch:openvswitch /etc/openvswitch

# Load any flows that we saved
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Loading previous flows ..." 2>&1
if [[ -f /var/run/openvswitch/flows.sh ]]; then
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Adding br0 if it doesn't exist ..." 2>&1
/usr/bin/ovs-vsctl --may-exist add-br br0 -- set Bridge br0 fail_mode=secure protocols=OpenFlow13
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Created br0, now adding flows ..." 2>&1
sh -x /var/run/openvswitch/flows.sh
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Done restoring the existing flows ..." 2>&1
fi

echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Remove other config ..." 2>&1
ovs-vsctl --no-wait --if-exists remove Open_vSwitch . other_config flow-restore-wait=true
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Removed other config ..." 2>&1
# launch OVS
# Start the ovsdb so that we can prep it before we start the ovs-vswitchd
/usr/share/openvswitch/scripts/ovs-ctl start --ovs-user=openvswitch:openvswitch --no-ovs-vswitchd --system-id=random --no-monitor

# Set the flow-restore-wait to true so ovs-vswitchd will wait till flows are restored
ovs-vsctl --no-wait set Open_vSwitch . other_config:flow-restore-wait=true

# Restrict the number of pthreads ovs-vswitchd creates to reduce the
# amount of RSS it uses on hosts with many cores
# https://bugzilla.redhat.com/show_bug.cgi?id=1571379
# https://bugzilla.redhat.com/show_bug.cgi?id=1572797
if [[ `nproc` -gt 12 ]]; then
ovs-vsctl --no-wait set Open_vSwitch . other_config:n-revalidator-threads=4
ovs-vsctl --no-wait set Open_vSwitch . other_config:n-handler-threads=10
fi

# And finally start the ovs-vswitchd now the DB is prepped
/usr/share/openvswitch/scripts/ovs-ctl start --ovs-user=openvswitch:openvswitch --no-ovsdb-server --system-id=random --no-monitor

# Load any flows that we saved
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Loading previous flows ..." 2>&1
if [[ -f /var/run/openvswitch/flows.sh ]]; then
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Adding br0 if it doesn't exist ..." 2>&1
/usr/bin/ovs-vsctl --may-exist add-br br0 -- set Bridge br0 fail_mode=secure protocols=OpenFlow13
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Created br0, now adding flows ..." 2>&1
sh -x /var/run/openvswitch/flows.sh
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Done restoring the existing flows ..." 2>&1
fi

echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Remove other config ..." 2>&1
ovs-vsctl --no-wait --if-exists remove Open_vSwitch . other_config flow-restore-wait=true
echo "$(date -u "+%Y-%m-%d %H:%M:%S") info: Removed other config ..." 2>&1

tail -F --pid=$(cat /var/run/openvswitch/ovs-vswitchd.pid) /var/log/openvswitch/ovs-vswitchd.log &
tail -F --pid=$(cat /var/run/openvswitch/ovsdb-server.pid) /var/log/openvswitch/ovsdb-server.log &
wait
tail -F --pid=$(cat /var/run/openvswitch/ovs-vswitchd.pid) /var/log/openvswitch/ovs-vswitchd.log &
tail -F --pid=$(cat /var/run/openvswitch/ovsdb-server.pid) /var/log/openvswitch/ovsdb-server.log &
wait
fi
securityContext:
privileged: true
volumeMounts:
- mountPath: /lib/modules
name: host-modules
readOnly: true
- mountPath: /run/openvswitch
name: host-run-ovs
- mountPath: /var/run/openvswitch
name: host-run-ovs
- mountPath: /run
name: host-run
- mountPath: /sys
name: host-sys
readOnly: true
- mountPath: /etc/openvswitch
name: host-config-openvswitch
- mountPath: /host
name: host-slash
readOnly: true
resources:
requests:
cpu: 100m
memory: 400Mi
terminationMessagePolicy: FallbackToLogsOnError
livenessProbe:
exec:
command:
# Validate that ovs-vswitchd and ovsdb are running. These three commands check
# 1. that the pids in the pid file are up
# 2. that ovs-vswitchd is responding to queries
# 3. that ovsdb is responding to queries
# 4. if br0 is configured, that its management process is responding
- /bin/bash
- -c
- |
#!/bin/bash
/usr/share/openvswitch/scripts/ovs-ctl status > /dev/null &&
/usr/bin/ovs-appctl -T 5 ofproto/list > /dev/null &&
/usr/bin/ovs-vsctl -t 5 show > /dev/null &&
if /usr/bin/ovs-vsctl -t 5 br-exists br0; then /usr/bin/ovs-ofctl -t 5 -O OpenFlow13 probe br0; else true; fi
initialDelaySeconds: 15
periodSeconds: 5
readinessProbe:
exec:
command:
# The same validation as above, but without checking for br0
# This is because it's created by openshift-sdn, so looking for it
# won't be very meaningful.
- /bin/bash
- -c
- |
#!/bin/bash
/usr/share/openvswitch/scripts/ovs-ctl status > /dev/null &&
/usr/bin/ovs-appctl -T 5 ofproto/list > /dev/null &&
/usr/bin/ovs-vsctl -t 5 show > /dev/null
initialDelaySeconds: 15
periodSeconds: 5
Expand All @@ -176,15 +173,19 @@ spec:
- name: host-modules
hostPath:
path: /lib/modules
- name: host-run-ovs
- name: host-run
hostPath:
path: /run/openvswitch
path: /run
- name: host-sys
hostPath:
path: /sys
- name: host-config-openvswitch
hostPath:
path: /var/lib/openvswitch
type: DirectoryOrCreate
- name: host-slash
hostPath:
path: /
tolerations:
- operator: "Exists"
{{- end}}
81 changes: 50 additions & 31 deletions bindata/network/ovn-kubernetes/006-ovs-node.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,24 +47,40 @@ spec:
source "/env/${K8S_NODE}"
set +o allexport
fi
echo "$(date -Iseconds) - starting ovs-daemons"
chown -R openvswitch:openvswitch /run/openvswitch
chown -R openvswitch:openvswitch /etc/openvswitch
function quit {
# Don't allow ovs-vswitchd to clear datapath flows on exit
kill -9 $(cat /var/run/openvswitch/ovs-vswitchd.pid 2>/dev/null) 2>/dev/null || true
kill $(cat /var/run/openvswitch/ovsdb-server.pid 2>/dev/null) 2>/dev/null || true
exit 0
}
trap quit SIGTERM
/usr/share/openvswitch/scripts/ovs-ctl start --ovs-user=openvswitch:openvswitch --system-id=random
ovs-appctl vlog/set "file:${OVS_LOG_LEVEL}"
/usr/share/openvswitch/scripts/ovs-ctl --protocol=udp --dport=6081 enable-protocol
echo "$(date -Iseconds) - ovs-daemons running"
export SYSTEMD_IGNORE_CHROOT=yes
# Check to see if ovs is provided by the node:
if systemctl status ovsdb-server; then
Comment thread
squeed marked this conversation as resolved.
echo "openvswitch is running in systemd"
# In some very strange corner cases, the owner for /run/openvswitch
# can be wrong, so we need to clean up and restart.
ovs_uid=$(chroot /host id -u openvswitch)
ovs_gid=$(chroot /host id -g openvswitch)
chown -R "${ovs_uid}:${ovs_gid}" /run/openvswitch
if [[ ! -S /run/openvswitch/db.sock ]]; then
systemctl restart ovsdb-server
fi
# Don't need to worry about restoring flows; this can only change if we've rebooted
exec tail -F /host/var/log/openvswitch/ovs-vswitchd.log /host/var/log/openvswitch/ovsdb-server.log
else
echo "$(date -Iseconds) - starting ovs-daemons"
chown -R openvswitch:openvswitch /run/openvswitch
chown -R openvswitch:openvswitch /etc/openvswitch
function quit {
# Don't allow ovs-vswitchd to clear datapath flows on exit
kill -9 $(cat /var/run/openvswitch/ovs-vswitchd.pid 2>/dev/null) 2>/dev/null || true
kill $(cat /var/run/openvswitch/ovsdb-server.pid 2>/dev/null) 2>/dev/null || true
exit 0
}
trap quit SIGTERM
/usr/share/openvswitch/scripts/ovs-ctl start --ovs-user=openvswitch:openvswitch --system-id=random
ovs-appctl vlog/set "file:${OVS_LOG_LEVEL}"
/usr/share/openvswitch/scripts/ovs-ctl --protocol=udp --dport=6081 enable-protocol
echo "$(date -Iseconds) - ovs-daemons running"

tail -F --pid=$(cat /var/run/openvswitch/ovs-vswitchd.pid) /var/log/openvswitch/ovs-vswitchd.log &
tail -F --pid=$(cat /var/run/openvswitch/ovsdb-server.pid) /var/log/openvswitch/ovsdb-server.log &
wait
tail -F --pid=$(cat /var/run/openvswitch/ovs-vswitchd.pid) /var/log/openvswitch/ovs-vswitchd.log &
tail -F --pid=$(cat /var/run/openvswitch/ovsdb-server.pid) /var/log/openvswitch/ovsdb-server.log &
wait
fi
env:
- name: OVS_LOG_LEVEL
value: info
Expand All @@ -81,35 +97,33 @@ spec:
- mountPath: /sys
name: host-sys
readOnly: true
- mountPath: /run/openvswitch
name: run-openvswitch
- mountPath: /run
name: host-run
- mountPath: /etc/openvswitch
name: etc-openvswitch
- mountPath: /var/lib/openvswitch
name: var-lib-openvswitch
- mountPath: /env
name: env-overrides
- mountPath: /host
name: host-slash
readOnly: true
resources:
requests:
cpu: 100m
memory: 300Mi
terminationMessagePolicy: FallbackToLogsOnError
livenessProbe:
exec:
command:
- /usr/share/openvswitch/scripts/ovs-ctl
- status
initialDelaySeconds: 15
periodSeconds: 5
readinessProbe:
exec:
command:
- /usr/share/openvswitch/scripts/ovs-ctl
- status
- /bin/bash
- -c
- |
#!/bin/bash
/usr/bin/ovs-vsctl -t 5 show > /dev/null
initialDelaySeconds: 15
periodSeconds: 5
terminationGracePeriodSeconds: 10

nodeSelector:
beta.kubernetes.io/os: "linux"
affinity:
Expand All @@ -127,15 +141,20 @@ spec:
- name: var-lib-openvswitch
hostPath:
path: /var/lib/openvswitch/data
type: DirectoryOrCreate
- name: etc-openvswitch
hostPath:
path: /var/lib/openvswitch/etc
- name: run-openvswitch
type: DirectoryOrCreate
- name: host-run
hostPath:
path: /run/openvswitch
path: /run
- name: host-sys
hostPath:
path: /sys
- name: host-slash
hostPath:
path: /
- name: env-overrides
configMap:
name: env-overrides
Expand Down