Skip to content

Commit

Permalink
Fix preStop Hooks for ovn-controller pod containers
Browse files Browse the repository at this point in the history
preStop Hooks currently fails with:-
ovs-ctl: exactly one non-option argument required
becaues ";sleep 2" is passed to the stop command which
it do not expect.

To run additional commands we could use "bash -c" but
considering the extra sleep do not help to avoid the
"FailedPreStopHook" event just dropping the extra sleep as
part of this patch.

"FailedPreStopHook" event gets raised even with successful
graceful shutdown because container get's killed due to
these stop commands.

To avoid "FailedPreStopHook" one option is to use wrapper
startup script and handle SIGTERM signals, something like
revert of [1], but leaving that for later.

Also add kuttl test to validate graceful shutdown of
ovn-controller by checking SB DB for the chassis.

[1] openstack-k8s-operators/ovs-operator#53

Resolves: OSPRH-4553
  • Loading branch information
karelyatin committed Mar 15, 2024
1 parent d7d87d3 commit d5db655
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 6 deletions.
9 changes: 3 additions & 6 deletions pkg/ovncontroller/daemonset.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,8 @@ func DaemonSet(
ovsDbArgs = []string{
"--single-child", "--", "/usr/local/bin/container-scripts/start-ovsdb-server.sh",
}
// sleep is required as workaround for https://github.com/kubernetes/kubernetes/issues/39170
ovsDbPreStopCmd = []string{
"/usr/share/openvswitch/scripts/ovs-ctl", "stop", "--no-ovs-vswitchd", ";", "sleep", "2",
"/usr/share/openvswitch/scripts/ovs-ctl", "stop", "--no-ovs-vswitchd",
}

ovsVswitchdLivenessProbe.Exec = &corev1.ExecAction{
Expand All @@ -125,9 +124,8 @@ func DaemonSet(
ovsVswitchdArgs = []string{
"--pidfile", "--mlockall",
}
// sleep is required as workaround for https://github.com/kubernetes/kubernetes/issues/39170
ovsVswitchdPreStopCmd = []string{
"/usr/share/openvswitch/scripts/ovs-ctl", "stop", "--no-ovsdb-server", ";", "sleep", "2",
"/usr/share/openvswitch/scripts/ovs-ctl", "stop", "--no-ovsdb-server",
}

ovnControllerCmd = []string{
Expand All @@ -142,9 +140,8 @@ func DaemonSet(
" ",
),
}
// sleep is required as workaround for https://github.com/kubernetes/kubernetes/issues/39170
ovnControllerPreStopCmd = []string{
"/usr/share/ovn/scripts/ovn-ctl", "stop_controller", ";", "sleep", "2",
"/usr/share/ovn/scripts/ovn-ctl", "stop_controller",
}

envVars := map[string]env.Setter{}
Expand Down
12 changes: 12 additions & 0 deletions tests/kuttl/common/assert_sample_deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -330,3 +330,15 @@ commands:
done
exit 0
---
# check that the chassis is registered in sb db
apiVersion: kuttl.dev/v1beta1
kind: TestAssert
commands:
- script: |
controller_pod=$(oc get pod -n $NAMESPACE -l service=ovn-controller -o name|head -1)
host=$(oc rsh -n $NAMESPACE ${controller_pod} ovs-vsctl get open . external_ids:hostname)
sb_pod=$(oc get pod -n $NAMESPACE -l service=ovsdbserver-sb -o name|head -1)
oc rsh -n $NAMESPACE ${sb_pod} ovn-sbctl list chassis | grep -q ${host} || exit 1
exit 0
13 changes: 13 additions & 0 deletions tests/kuttl/tests/ovn_config/03-assert.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#
# Check for:
#
# - chassis unregistered in sb db

apiVersion: kuttl.dev/v1beta1
kind: TestAssert
timeout: 30
commands:
- script: |
sb_pod=$(oc get pod -n $NAMESPACE -l service=ovsdbserver-sb -o name|head -1)
oc rsh -n $NAMESPACE ${sb_pod} ovn-sbctl list chassis | grep -q hostname && exit 1
exit 0
5 changes: 5 additions & 0 deletions tests/kuttl/tests/ovn_config/03-remove-ovncontroller.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- script: |
oc patch OVNController -n $NAMESPACE ovncontroller-sample --type='json' -p='[{"op": "replace", "path": "/spec/nodeSelector", "value":{"node": "non-existing-node-name"}}]'

0 comments on commit d5db655

Please sign in to comment.