Merge pull request rabbitmq#1611 from ViliusS/fix-prestop-hook

Fix graceful shutdown

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

@@ -52,7 +52,7 @@ We run the check in bash because we want the operator to work with upstream Rabb
 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.
 
 ```
-if [ ! -z \"$(cat /etc/pod-info/skipPreStopChecks)\" ]
+if [ ! -z "$(cat /etc/pod-info/skipPreStopChecks)" ]
   then exit 0
 fi
 ```

internal/resource/statefulset.go

@@ -625,8 +625,8 @@ func (builder *StatefulSetBuilder) podTemplateSpec(previousPodAnnotations map[st
 							Exec: &corev1.ExecAction{
 								Command: []string{"/bin/bash", "-c",
 									fmt.Sprintf("if [ ! -z \"$(cat /etc/pod-info/%s)\" ]; then exit 0; fi;", DeletionMarker) +
-										fmt.Sprintf(" rabbitmq-upgrade await_online_quorum_plus_one -t %d;"+
-											" rabbitmq-upgrade await_online_synchronized_mirror -t %d;"+
+										fmt.Sprintf(" rabbitmq-upgrade await_online_quorum_plus_one -t %d &&"+
+											" rabbitmq-upgrade await_online_synchronized_mirror -t %d &&"+
 											" rabbitmq-upgrade drain -t %d",
 											*builder.Instance.Spec.TerminationGracePeriodSeconds,
 											*builder.Instance.Spec.TerminationGracePeriodSeconds,

internal/resource/statefulset_test.go

@@ -1448,15 +1448,15 @@ default_pass = {{ .Data.data.password }}
 			Expect(gracePeriodSeconds).To(Equal(ptr.To(int64(10))))
 
 			// TerminationGracePeriodSeconds is used to set commands timeouts in the preStop hook
-			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"}
+			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"}
 			Expect(statefulSet.Spec.Template.Spec.Containers[0].Lifecycle.PreStop.Exec.Command).To(Equal(expectedPreStopCommand))
 		})
 
 		It("checks mirror and quorum queue status in preStop hook", func() {
 			stsBuilder := builder.StatefulSet()
 			Expect(stsBuilder.Update(statefulSet)).To(Succeed())
 
-			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"}
+			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"}
 
 			Expect(statefulSet.Spec.Template.Spec.Containers[0].Lifecycle.PreStop.Exec.Command).To(Equal(expectedPreStopCommand))
 		})