Skip to content

Commit f404474

Browse files
committed
Fix graceful shutdown
Previously all actions in preStop hook were performed independently of each other. For example, drain would kick in even if queue sinchronization returned an error. This resulted in cases where graceful shutdown was assumed just because last action in the script, i.e. `rabbitmq-upgrade drain`, is a success. Now all actions in the hook actually waits for the previous action to complete successfully. The only exception is checking for the deletion marker file. Script will still exit with 0 (and shutdown will continue) if the marker is present without executing other actions.
1 parent 59c0b45 commit f404474

File tree

3 files changed

+5
-5
lines changed

3 files changed

+5
-5
lines changed

docs/design/20200520-graceful-pod-termination.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ We run the check in bash because we want the operator to work with upstream Rabb
5252
When deleting the RabbitMQ Custom Resource, we don't want to have to wait on anything. We assume that if the user chooses to run `kubectl delete rabbitmqclusers my-cluster`, then they don't care about queue sync. We also don't want a situation where a final node can't be deleted because the check concludes the obvious but irrelevant fact that you will lose quorum. Our Custom Resource is configured with a [finalizer](https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#finalizers) so that our Operator reconciles on CR deletion. In the deletion loop, we set a label on the StatefulSet. The label updates a file mounted in the RabbitMQ container via the [Kubernetes DownwardAPI](https://kubernetes.io/docs/tasks/inject-data-application/downward-api-volume-expose-pod-information/). In turn, this file is checked inside the PreStop hook to exit before the `rabbitmq-queues` CLI checks. We then remove the finalizer from the Custom Resource so it can be garbage collected and we avoid blocking.
5353

5454
```
55-
if [ ! -z \"$(cat /etc/pod-info/skipPreStopChecks)\" ]
55+
if [ ! -z "$(cat /etc/pod-info/skipPreStopChecks)" ]
5656
then exit 0
5757
fi
5858
```

internal/resource/statefulset.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -625,8 +625,8 @@ func (builder *StatefulSetBuilder) podTemplateSpec(previousPodAnnotations map[st
625625
Exec: &corev1.ExecAction{
626626
Command: []string{"/bin/bash", "-c",
627627
fmt.Sprintf("if [ ! -z \"$(cat /etc/pod-info/%s)\" ]; then exit 0; fi;", DeletionMarker) +
628-
fmt.Sprintf(" rabbitmq-upgrade await_online_quorum_plus_one -t %d;"+
629-
" rabbitmq-upgrade await_online_synchronized_mirror -t %d;"+
628+
fmt.Sprintf(" rabbitmq-upgrade await_online_quorum_plus_one -t %d &&"+
629+
" rabbitmq-upgrade await_online_synchronized_mirror -t %d &&"+
630630
" rabbitmq-upgrade drain -t %d",
631631
*builder.Instance.Spec.TerminationGracePeriodSeconds,
632632
*builder.Instance.Spec.TerminationGracePeriodSeconds,

internal/resource/statefulset_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1448,15 +1448,15 @@ default_pass = {{ .Data.data.password }}
14481448
Expect(gracePeriodSeconds).To(Equal(ptr.To(int64(10))))
14491449

14501450
// TerminationGracePeriodSeconds is used to set commands timeouts in the preStop hook
1451-
expectedPreStopCommand := []string{"/bin/bash", "-c", "if [ ! -z \"$(cat /etc/pod-info/skipPreStopChecks)\" ]; then exit 0; fi; rabbitmq-upgrade await_online_quorum_plus_one -t 10; rabbitmq-upgrade await_online_synchronized_mirror -t 10; rabbitmq-upgrade drain -t 10"}
1451+
expectedPreStopCommand := []string{"/bin/bash", "-c", "if [ ! -z \"$(cat /etc/pod-info/skipPreStopChecks)\" ]; then exit 0; fi; rabbitmq-upgrade await_online_quorum_plus_one -t 10 && rabbitmq-upgrade await_online_synchronized_mirror -t 10 && rabbitmq-upgrade drain -t 10"}
14521452
Expect(statefulSet.Spec.Template.Spec.Containers[0].Lifecycle.PreStop.Exec.Command).To(Equal(expectedPreStopCommand))
14531453
})
14541454

14551455
It("checks mirror and quorum queue status in preStop hook", func() {
14561456
stsBuilder := builder.StatefulSet()
14571457
Expect(stsBuilder.Update(statefulSet)).To(Succeed())
14581458

1459-
expectedPreStopCommand := []string{"/bin/bash", "-c", "if [ ! -z \"$(cat /etc/pod-info/skipPreStopChecks)\" ]; then exit 0; fi; rabbitmq-upgrade await_online_quorum_plus_one -t 604800; rabbitmq-upgrade await_online_synchronized_mirror -t 604800; rabbitmq-upgrade drain -t 604800"}
1459+
expectedPreStopCommand := []string{"/bin/bash", "-c", "if [ ! -z \"$(cat /etc/pod-info/skipPreStopChecks)\" ]; then exit 0; fi; rabbitmq-upgrade await_online_quorum_plus_one -t 604800 && rabbitmq-upgrade await_online_synchronized_mirror -t 604800 && rabbitmq-upgrade drain -t 604800"}
14601460

14611461
Expect(statefulSet.Spec.Template.Spec.Containers[0].Lifecycle.PreStop.Exec.Command).To(Equal(expectedPreStopCommand))
14621462
})

0 commit comments

Comments
 (0)