Skip to content

[WIP] Start host openvswitch using systemctl #477

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

Closed
mccv1r0 wants to merge 2 commits into from
Closed

[WIP] Start host openvswitch using systemctl #477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a78f9d
Add hostPath type: Directory where needed for OVS host directories
mccv1r0 Apr 9, 2020
7895421
ovnkube-master.yaml /run/openvswitch change from emptyDir to hostPath
mccv1r0 Apr 22, 2020
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
77 changes: 73 additions & 4 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 @@ -42,6 +42,18 @@ spec:
- |
#!/bin/bash
set -euo pipefail
set -x
Copy link
Copy Markdown
Contributor

@dcbw dcbw Apr 2, 2020

Choose a reason for hiding this comment

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

@mccv1r0 let's remove this for the final one

Copy link
Copy Markdown
Contributor Author

@mccv1r0 mccv1r0 Apr 2, 2020

Choose a reason for hiding this comment

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

Agreed

if [[ -f /usr/bin/id ]]; then
Copy link
Copy Markdown
Contributor

@rbbratta rbbratta May 5, 2020

Choose a reason for hiding this comment

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

-x for executable?

I don't understand all these checks, they are not atomic and thus worse than redundant because they hid the real error message.

/usr/bin/id openvswitch || true

Copy link
Copy Markdown
Contributor

@rbbratta rbbratta May 5, 2020

Choose a reason for hiding this comment

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

or no || true with the set -e

Copy link
Copy Markdown
Contributor

@rbbratta rbbratta May 5, 2020

Choose a reason for hiding this comment

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

or I guess it's not clear in which environment id is not present?

Copy link
Copy Markdown
Contributor Author

@mccv1r0 mccv1r0 May 5, 2020

Choose a reason for hiding this comment

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

I don't understand all these checks, they are not atomic and thus worse than redundant because they hid the real error message.

Most (if not all) of these checks were added to debug and will not exist in the final PR.

/usr/bin/id openvswitch
else
echo "id command not found"
fi
if [[ -d /run/openvswitch ]]; then
ls -al /run/openvswitch
Copy link
Copy Markdown
Contributor

@rbbratta rbbratta May 5, 2020

Choose a reason for hiding this comment

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

ls -al /run/openvswitch/ || true trailing / forces treating it as a directory.

Copy link
Copy Markdown
Contributor

@rbbratta rbbratta May 5, 2020

Choose a reason for hiding this comment

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

I guess question here as well, when is /var/run not a symlink to /run ?

fi
if [[ -d /var/run/openvswitch ]]; then
ls -al /var/run/openvswitch
fi
if [[ -f "/env/${K8S_NODE}" ]]; then
set -o allexport
source "/env/${K8S_NODE}"
Expand All @@ -50,16 +62,56 @@ spec:
echo "$(date -Iseconds) - starting ovs-daemons"
chown -R openvswitch:openvswitch /run/openvswitch
chown -R openvswitch:openvswitch /etc/openvswitch
if [[ -d /run/openvswitch ]]; then
ls -al /run/openvswitch
fi
if [[ -d /var/run/openvswitch ]]; then
ls -al /var/run/openvswitch
fi
if [[ -f /var/run/openvswitch/ovs-vswitchd.pid ]] ; then
Copy link
Copy Markdown
Contributor

@rbbratta rbbratta May 5, 2020

Choose a reason for hiding this comment

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

-r for readable.

cat /var/run/openvswitch/ovs-vswitchd.pid
fi
if [[ -f /var/run/openvswitch/ovsdb-server.pid ]] ; then
cat /var/run/openvswitch/ovsdb-server.pid
fi
function quit {
Copy link
Copy Markdown
Contributor

@squeed squeed Jun 16, 2020

Choose a reason for hiding this comment

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

Yikes. This needs to be moved to the "non-systemd case"

Copy link
Copy Markdown
Contributor Author

@mccv1r0 mccv1r0 Jun 16, 2020

Choose a reason for hiding this comment

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

This is all debug code

Copy link
Copy Markdown
Contributor

@squeed squeed Jun 16, 2020

Choose a reason for hiding this comment

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

not the kill -9...

# 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
if [[ -S /var/run/openvswitch/db.sock ]]; then
echo "Found /var/run/openvswitch/db.sock"
else
echo "File /var/run/openvswitch/db.sock was not found"
fi
if [[ -S /run/openvswitch/db.sock ]]; then
echo "Found /run/openvswitch/db.sock"
else
echo "File /run/openvswitch/db.sock was not found"
fi
export SYSTEMD_IGNORE_CHROOT=yes
journalctl -xeu ovsdb-server --no-pager
#systemctl is-active --quiet ovsdb-server
journalctl -xeu openvswitch --no-pager
if (modprobe openvswitch) ; then
echo "modprobe openvswitch success?"
else
echo "modprobe openvswitch fail?"
fi
lsmod | grep openvswitch
Comment thread
mccv1r0 marked this conversation as resolved.
if (systemctl is-active --quiet openvswitch) ; then
echo "OVS started by RHCOS"
systemctl is-active openvswitch
journalctl -xeu openvswitch --no-pager
Copy link
Copy Markdown
Contributor

@squeed squeed Jun 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Probably just want to exec journalctl -f -u openvswitch ovsdb-server and be done with the script.

else
Comment thread
mccv1r0 marked this conversation as resolved.
Outdated
echo "OVS started by ovs-node container, NOT RHCOS"
# Need container up v0.0 just to see if host starts ovs and friends
Copy link
Copy Markdown
Contributor

@squeed squeed Jun 16, 2020

Choose a reason for hiding this comment

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

Can you fix the indentation here?

Copy link
Copy Markdown
Contributor

@trozet trozet Jun 17, 2020

Choose a reason for hiding this comment

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

I don't really understand why we need to start OVS from this container. Shouldn't systemd OVS be configured as a service with restart=Always? Why does a pod need to start it?

Copy link
Copy Markdown
Contributor

@dcbw dcbw Jun 18, 2020

Choose a reason for hiding this comment

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

This might have been left-over from before the MCO was going to start OVS. Our first pass was starting system OVS from a pod via systemctl without MCO, but we moved to MCO because Clayton told us to.

/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
fi
echo "$(date -Iseconds) - ovs-daemons running"

tail -F --pid=$(cat /var/run/openvswitch/ovs-vswitchd.pid) /var/log/openvswitch/ovs-vswitchd.log &
Expand All @@ -82,9 +134,15 @@ spec:
name: host-sys
readOnly: true
- mountPath: /run/openvswitch
name: run-openvswitch
name: host-run-openvswitch
- mountPath: /run/systemd
name: host-run-systemd
- mountPath: /etc/openvswitch
name: etc-openvswitch
- mountPath: /var/log/openvswitch
name: host-var-log-openvswitch
- mountPath: /sys/fs/cgroup
name: sys-fs-cgroup
- mountPath: /var/lib/openvswitch
name: var-lib-openvswitch
- mountPath: /env
Expand Down Expand Up @@ -120,22 +178,33 @@ spec:
- key: network.operator.openshift.io/external-openvswitch
operator: DoesNotExist
volumes:
# used for iptables wrapper scripts
- name: host-modules
hostPath:
path: /lib/modules
- name: var-lib-openvswitch
hostPath:
path: /var/lib/openvswitch/data
- name: host-run-openvswitch
hostPath:
path: /run/openvswitch
type: Directory
- name: host-run-systemd
hostPath:
path: /run/systemd
type: Directory
- name: etc-openvswitch
hostPath:
path: /var/lib/openvswitch/etc
- name: run-openvswitch
- name: host-var-log-openvswitch
hostPath:
path: /run/openvswitch
path: /var/log/openvswitch
type: Directory
- name: host-sys
hostPath:
path: /sys
- name: sys-fs-cgroup
hostPath:
path: /sys/fs/cgroup
- name: env-overrides
configMap:
name: env-overrides
Expand Down
4 changes: 3 additions & 1 deletion bindata/network/ovn-kubernetes/ovnkube-master.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,9 @@ spec:
hostPath:
path: /var/lib/ovn/data
- name: run-openvswitch
emptyDir: {}
hostPath:
path: /run/openvswitch
type: Directory
- name: run-ovn
hostPath:
path: /var/run/ovn
Expand Down
100 changes: 95 additions & 5 deletions bindata/network/ovn-kubernetes/ovnkube-node.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,56 @@ spec:
- -c
- |
set -e
set -x
if [[ -f /usr/bin/id ]]; then
/usr/bin/id openvswitch
else
echo "id command not found"
fi
if [[ -d /run/openvswitch ]]; then
ls -al /run/openvswitch
fi
if [[ -d /var/run/openvswitch ]]; then
ls -al /var/run/openvswitch
fi
if [[ -f /var/run/openvswitch/ovs-vswitchd.pid ]] ; then
cat /var/run/openvswitch/ovs-vswitchd.pid
fi
if [[ -f /var/run/openvswitch/ovsdb-server.pid ]] ; then
cat /var/run/openvswitch/ovsdb-server.pid
fi
if [[ -d /run/ovn ]]; then
ls -al /run/ovn
else
echo "/run/ovn doesn't exist"
fi
if [[ -d /var/run/ovn ]]; then
ls -al /var/run/ovn
else
echo "/var/run/ovn doesn't exist"
fi
if [[ -f "/env/${K8S_NODE}" ]]; then
set -o allexport
source "/env/${K8S_NODE}"
set +o allexport
fi
echo "$(date -Iseconds) - starting ovn-controller"
if (modprobe openvswitch) ; then
echo "modprobe openvswitch success?"
else
echo "modprobe openvswitch fail?"
fi
lsmod | grep openvswitch
if [[ -S /var/run/openvswitch/db.sock ]]; then
echo "Found /var/run/openvswitch/db.sock"
else
echo "File /var/run/openvswitch/db.sock was not found"
fi
if [[ -S /run/openvswitch/db.sock ]]; then
echo "Found /run/openvswitch/db.sock"
else
echo "File /run/openvswitch/db.sock was not found"
fi
exec ovn-controller unix:/var/run/openvswitch/db.sock -vfile:off \
--no-chdir --pidfile=/var/run/ovn/ovn-controller.pid \
-p /ovn-cert/tls.key -c /ovn-cert/tls.crt -C /ovn-ca/ca-bundle.crt \
Expand All @@ -62,7 +106,7 @@ spec:
fieldPath: spec.nodeName
volumeMounts:
- mountPath: /run/openvswitch
name: run-openvswitch
name: host-run-openvswitch
- mountPath: /run/ovn/
name: run-ovn
- mountPath: /etc/openvswitch
Expand Down Expand Up @@ -91,12 +135,56 @@ spec:
- -c
- |
set -xe
if [[ -f /usr/bin/id ]]; then
/usr/bin/id openvswitch
else
echo "id command not found"
fi
if [[ -d /run/openvswitch ]]; then
ls -al /run/openvswitch
fi
if [[ -d /var/run/openvswitch ]]; then
ls -al /var/run/openvswitch
fi
if [[ -f /var/run/openvswitch/ovs-vswitchd.pid ]] ; then
cat /var/run/openvswitch/ovs-vswitchd.pid
fi
if [[ -f /var/run/openvswitch/ovsdb-server.pid ]] ; then
cat /var/run/openvswitch/ovsdb-server.pid
fi
if [[ -d /run/ovn ]]; then
ls -al /run/ovn
else
echo "/run/ovn doesn't exist"
fi
if [[ -d /var/run/ovn ]]; then
ls -al /var/run/ovn
else
echo "/var/run/ovn doesn't exist"
fi
if [[ -f "/env/${K8S_NODE}" ]]; then
set -o allexport
source "/env/${K8S_NODE}"
set +o allexport
fi
echo "I$(date "+%m%d %H:%M:%S.%N") - waiting for db_ip addresses"
if (modprobe openvswitch) ; then
echo "modprobe openvswitch success?"
else
echo "modprobe openvswitch fail?"
fi
lsmod | grep openvswitch
if [[ -S /var/run/openvswitch/db.sock ]]; then
echo "Found /var/run/openvswitch/db.sock"
else
echo "File /var/run/openvswitch/db.sock was not found"
fi
if [[ -S /run/openvswitch/db.sock ]]; then
echo "Found /run/openvswitch/db.sock"
else
echo "File /run/openvswitch/db.sock was not found"
fi
mkdir -p /run/ovn-kubernetes
cp -f /usr/libexec/cni/ovn-k8s-cni-overlay /cni-bin-dir/
ovn_config_namespace=openshift-ovn-kubernetes
retries=0
Expand Down Expand Up @@ -158,7 +246,7 @@ spec:
# for the iptables wrapper
- mountPath: /host
name: host-slash
readOnly: true
readOnly: false
# for the CNI server socket
- mountPath: /run/ovn-kubernetes/
name: host-run-ovn-kubernetes
Expand All @@ -177,7 +265,7 @@ spec:
- mountPath: /var/lib/cni/networks/ovn-k8s-cni-overlay
name: host-var-lib-cni-networks-ovn-kubernetes
- mountPath: /run/openvswitch
name: run-openvswitch
name: host-run-openvswitch
- mountPath: /run/ovn/
name: run-ovn
- mountPath: /etc/openvswitch
Expand Down Expand Up @@ -214,6 +302,7 @@ spec:
- name: host-slash
hostPath:
path: /
type: Directory
- name: host-run-netns
hostPath:
path: /run/netns
Expand All @@ -223,9 +312,10 @@ spec:
- name: etc-openvswitch
hostPath:
path: /var/lib/openvswitch/etc
- name: run-openvswitch
- name: host-run-openvswitch
hostPath:
path: /var/run/openvswitch
path: /run/openvswitch
type: Directory
- name: run-ovn
hostPath:
path: /var/run/ovn
Expand Down