Skip to content

Commit 7244faa

Browse files
ChunyiLyuChunyiLyu
authoredOct 29, 2020
Make TerminationGracePeriodSeconds configurable (#430)
- related to issue #419 - make TerminationGracePeriodSeconds configurable - refactor to use "k8s.io/utils/pointer" library everywhere
·
v2.12.10.49.0
1 parent ada0c87 commit 7244faa

10 files changed

+74
-58
lines changed
 

‎api/v1beta1/rabbitmqcluster_types.go

+6
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ type RabbitmqClusterSpec struct {
6363
// Has no effect if the cluster only consists of one node.
6464
// For more information, see https://www.rabbitmq.com/rabbitmq-queues.8.html#rebalance
6565
SkipPostDeploySteps bool `json:"skipPostDeploySteps,omitempty"`
66+
// TerminationGracePeriodSeconds is the timeout that each rabbitmqcluster pod will have to terminate gracefully.
67+
// It defaults to 604800 seconds ( a week long) to ensure that the container preStop lifecycle hook can finish running.
68+
// For more information, see: https://github.com/rabbitmq/cluster-operator/blob/main/docs/design/20200520-graceful-pod-termination.md
69+
// +kubebuilder:validation:Minimum:=0
70+
// +kubebuilder:default:=604800
71+
TerminationGracePeriodSeconds *int64 `json:"terminationGracePeriodSeconds,omitempty"`
6672
}
6773

6874
type RabbitmqClusterOverrideSpec struct {

‎api/v1beta1/rabbitmqcluster_types_test.go

+10-12
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ import (
2727

2828
var _ = Describe("RabbitmqCluster", func() {
2929

30-
var three int32 = 3
31-
3230
Context("RabbitmqClusterSpec", func() {
3331
It("can be created with a single replica", func() {
3432
created := generateRabbitmqClusterObject("rabbit1")
@@ -42,7 +40,7 @@ var _ = Describe("RabbitmqCluster", func() {
4240

4341
It("can be created with three replicas", func() {
4442
created := generateRabbitmqClusterObject("rabbit2")
45-
created.Spec.Replicas = &three
43+
created.Spec.Replicas = pointer.Int32Ptr(3)
4644
Expect(k8sClient.Create(context.TODO(), created)).To(Succeed())
4745

4846
fetched := &RabbitmqCluster{}
@@ -51,9 +49,8 @@ var _ = Describe("RabbitmqCluster", func() {
5149
})
5250

5351
It("can be created with five replicas", func() {
54-
five := int32(5)
5552
created := generateRabbitmqClusterObject("rabbit3")
56-
created.Spec.Replicas = &five
53+
created.Spec.Replicas = pointer.Int32Ptr(5)
5754
Expect(k8sClient.Create(context.TODO(), created)).To(Succeed())
5855

5956
fetched := &RabbitmqCluster{}
@@ -126,9 +123,8 @@ var _ = Describe("RabbitmqCluster", func() {
126123

127124
It("is validated", func() {
128125
By("checking the replica count", func() {
129-
nOne := int32(-1)
130126
invalidReplica := generateRabbitmqClusterObject("rabbit4")
131-
invalidReplica.Spec.Replicas = &nOne
127+
invalidReplica.Spec.Replicas = pointer.Int32Ptr(-1)
132128
Expect(apierrors.IsInvalid(k8sClient.Create(context.TODO(), invalidReplica))).To(BeTrue())
133129
Expect(k8sClient.Create(context.TODO(), invalidReplica)).To(MatchError(ContainSubstring("spec.replicas in body should be greater than or equal to 0")))
134130
})
@@ -187,9 +183,10 @@ var _ = Describe("RabbitmqCluster", func() {
187183
Namespace: "default",
188184
},
189185
Spec: RabbitmqClusterSpec{
190-
Replicas: &three,
191-
Image: "rabbitmq-image-from-cr",
192-
ImagePullSecrets: []corev1.LocalObjectReference{{Name: "my-super-secret"}},
186+
Replicas: pointer.Int32Ptr(3),
187+
Image: "rabbitmq-image-from-cr",
188+
ImagePullSecrets: []corev1.LocalObjectReference{{Name: "my-super-secret"}},
189+
TerminationGracePeriodSeconds: pointer.Int64Ptr(0),
193190
Service: RabbitmqClusterServiceSpec{
194191
Type: "NodePort",
195192
Annotations: map[string]string{
@@ -476,8 +473,9 @@ func generateRabbitmqClusterObject(clusterName string) *RabbitmqCluster {
476473
Namespace: "default",
477474
},
478475
Spec: RabbitmqClusterSpec{
479-
Replicas: pointer.Int32Ptr(1),
480-
Image: "rabbitmq:3.8.9",
476+
Replicas: pointer.Int32Ptr(1),
477+
Image: "rabbitmq:3.8.9",
478+
TerminationGracePeriodSeconds: pointer.Int64Ptr(604800),
481479
Service: RabbitmqClusterServiceSpec{
482480
Type: "ClusterIP",
483481
},

‎api/v1beta1/zz_generated.deepcopy.go

+5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/rabbitmq.com_rabbitmqclusters.yaml

+9
Original file line numberDiff line numberDiff line change
@@ -3723,6 +3723,15 @@ spec:
37233723
the cluster only consists of one node. For more information, see
37243724
https://www.rabbitmq.com/rabbitmq-queues.8.html#rebalance
37253725
type: boolean
3726+
terminationGracePeriodSeconds:
3727+
default: 604800
3728+
description: 'TerminationGracePeriodSeconds is the timeout that each
3729+
rabbitmqcluster pod will have to terminate gracefully. It defaults
3730+
to 604800 seconds ( a week long) to ensure that the container preStop
3731+
lifecycle hook can finish running. For more information, see: https://github.com/rabbitmq/cluster-operator/blob/main/docs/design/20200520-graceful-pod-termination.md'
3732+
format: int64
3733+
minimum: 0
3734+
type: integer
37263735
tls:
37273736
properties:
37283737
caCertName:

‎controllers/rabbitmqcluster_controller_test.go

+5-8
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ package controllers_test
1313
import (
1414
"context"
1515
"fmt"
16+
"k8s.io/utils/pointer"
1617
"time"
1718

1819
"k8s.io/apimachinery/pkg/util/intstr"
@@ -40,8 +41,7 @@ var _ = Describe("RabbitmqClusterController", func() {
4041

4142
var (
4243
cluster *rabbitmqv1beta1.RabbitmqCluster
43-
one int32 = 1
44-
defaultNamespace = "default"
44+
defaultNamespace = "default"
4545
)
4646

4747
Context("default settings", func() {
@@ -709,7 +709,7 @@ var _ = Describe("RabbitmqClusterController", func() {
709709
Namespace: defaultNamespace,
710710
},
711711
}
712-
cluster.Spec.Replicas = &one
712+
cluster.Spec.Replicas = pointer.Int32Ptr(1)
713713
})
714714

715715
It("exposes ReconcileSuccess condition", func() {
@@ -774,14 +774,13 @@ var _ = Describe("RabbitmqClusterController", func() {
774774
storageClassName = "my-storage-class"
775775
myStorage = k8sresource.MustParse("100Gi")
776776
q, _ = k8sresource.ParseQuantity("10Gi")
777-
ten := int32(10)
778777
cluster = &rabbitmqv1beta1.RabbitmqCluster{
779778
ObjectMeta: metav1.ObjectMeta{
780779
Name: "rabbitmq-sts-override",
781780
Namespace: defaultNamespace,
782781
},
783782
Spec: rabbitmqv1beta1.RabbitmqClusterSpec{
784-
Replicas: &ten,
783+
Replicas: pointer.Int32Ptr(10),
785784
Override: rabbitmqv1beta1.RabbitmqClusterOverrideSpec{
786785
StatefulSet: &rabbitmqv1beta1.StatefulSet{
787786
Spec: &rabbitmqv1beta1.StatefulSetSpec{
@@ -1016,10 +1015,8 @@ var _ = Describe("RabbitmqClusterController", func() {
10161015
})
10171016

10181017
It("updates", func() {
1019-
five := int32(5)
1020-
10211018
Expect(updateWithRetry(cluster, func(r *rabbitmqv1beta1.RabbitmqCluster) {
1022-
cluster.Spec.Override.StatefulSet.Spec.Replicas = &five
1019+
cluster.Spec.Override.StatefulSet.Spec.Replicas = pointer.Int32Ptr(5)
10231020
cluster.Spec.Override.StatefulSet.Spec.Template.Spec.Containers = []corev1.Container{
10241021
{
10251022
Name: "additional-container-2",

‎controllers/reconcile_queue_rebalance_test.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package controllers_test
22

33
import (
44
"k8s.io/apimachinery/pkg/types"
5+
"k8s.io/utils/pointer"
56
"time"
67

78
appsv1 "k8s.io/api/apps/v1"
@@ -15,8 +16,6 @@ import (
1516
var _ = Describe("Reconcile queue Rebalance", func() {
1617
var (
1718
cluster *rabbitmqv1beta1.RabbitmqCluster
18-
one int32 = 1
19-
three int32 = 3
2019
annotations map[string]string
2120
defaultNamespace = "default"
2221
)
@@ -38,7 +37,7 @@ var _ = Describe("Reconcile queue Rebalance", func() {
3837
Namespace: defaultNamespace,
3938
},
4039
Spec: rabbitmqv1beta1.RabbitmqClusterSpec{
41-
Replicas: &three,
40+
Replicas: pointer.Int32Ptr(3),
4241
},
4342
}
4443
Expect(client.Create(ctx, cluster)).To(Succeed())
@@ -119,7 +118,7 @@ var _ = Describe("Reconcile queue Rebalance", func() {
119118
Namespace: defaultNamespace,
120119
},
121120
Spec: rabbitmqv1beta1.RabbitmqClusterSpec{
122-
Replicas: &three,
121+
Replicas: pointer.Int32Ptr(3),
123122
SkipPostDeploySteps: true,
124123
},
125124
}
@@ -183,7 +182,7 @@ var _ = Describe("Reconcile queue Rebalance", func() {
183182
Namespace: defaultNamespace,
184183
},
185184
Spec: rabbitmqv1beta1.RabbitmqClusterSpec{
186-
Replicas: &one,
185+
Replicas: pointer.Int32Ptr(1),
187186
SkipPostDeploySteps: false,
188187
},
189188
}

‎internal/resource/statefulset.go

+9-10
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,9 @@ import (
2929
)
3030

3131
const (
32-
defaultGracePeriodTimeoutSeconds int64 = 60 * 60 * 24 * 7
33-
initContainerCPU string = "100m"
34-
initContainerMemory string = "500Mi"
35-
DeletionMarker string = "skipPreStopChecks"
32+
initContainerCPU string = "100m"
33+
initContainerMemory string = "500Mi"
34+
DeletionMarker string = "skipPreStopChecks"
3635
)
3736

3837
type StatefulSetBuilder struct {
@@ -106,10 +105,9 @@ func (builder *StatefulSetBuilder) Update(object runtime.Object) error {
106105
sts.Spec.Replicas = builder.Instance.Spec.Replicas
107106

108107
//Update Strategy
109-
zero := int32(0)
110108
sts.Spec.UpdateStrategy = appsv1.StatefulSetUpdateStrategy{
111109
RollingUpdate: &appsv1.RollingUpdateStatefulSetStrategy{
112-
Partition: &zero,
110+
Partition: pointer.Int32Ptr(0),
113111
},
114112
Type: appsv1.RollingUpdateStatefulSetStrategyType,
115113
}
@@ -247,8 +245,6 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin
247245
rabbitmqGID := int64(999)
248246
rabbitmqUID := int64(999)
249247

250-
terminationGracePeriod := defaultGracePeriodTimeoutSeconds
251-
252248
volumes := []corev1.Volume{
253249
{
254250
Name: "server-conf",
@@ -512,7 +508,7 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin
512508
RunAsUser: &rabbitmqUID,
513509
},
514510
ImagePullSecrets: builder.Instance.Spec.ImagePullSecrets,
515-
TerminationGracePeriodSeconds: &terminationGracePeriod,
511+
TerminationGracePeriodSeconds: builder.Instance.Spec.TerminationGracePeriodSeconds,
516512
ServiceAccountName: builder.Instance.ChildResourceName(serviceAccountName),
517513
AutomountServiceAccountToken: &automountServiceAccountToken,
518514
Affinity: builder.Instance.Spec.Affinity,
@@ -653,7 +649,10 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin
653649
fmt.Sprintf("if [ ! -z \"$(cat /etc/pod-info/%s)\" ]; then exit 0; fi;", DeletionMarker) +
654650
fmt.Sprintf(" rabbitmq-upgrade await_online_quorum_plus_one -t %d;"+
655651
" rabbitmq-upgrade await_online_synchronized_mirror -t %d;"+
656-
" rabbitmq-upgrade drain -t %d", defaultGracePeriodTimeoutSeconds, defaultGracePeriodTimeoutSeconds, defaultGracePeriodTimeoutSeconds),
652+
" rabbitmq-upgrade drain -t %d",
653+
*builder.Instance.Spec.TerminationGracePeriodSeconds,
654+
*builder.Instance.Spec.TerminationGracePeriodSeconds,
655+
*builder.Instance.Spec.TerminationGracePeriodSeconds),
657656
},
658657
},
659658
},

‎internal/resource/statefulset_test.go

+22-19
Original file line numberDiff line numberDiff line change
@@ -327,10 +327,9 @@ var _ = Describe("StatefulSet", func() {
327327
It("specifies the upgrade policy", func() {
328328
stsBuilder := builder.StatefulSet()
329329
Expect(stsBuilder.Update(statefulSet)).To(Succeed())
330-
zero := int32(0)
331330
updateStrategy := appsv1.StatefulSetUpdateStrategy{
332331
RollingUpdate: &appsv1.RollingUpdateStatefulSetStrategy{
333-
Partition: &zero,
332+
Partition: pointer.Int32Ptr(0),
334333
},
335334
Type: appsv1.RollingUpdateStatefulSetStrategyType,
336335
}
@@ -1022,13 +1021,22 @@ var _ = Describe("StatefulSet", func() {
10221021
}))
10231022
})
10241023

1025-
It("adds the required terminationGracePeriodSeconds", func() {
1024+
It("sets TerminationGracePeriodSeconds in podTemplate as provided in instance spec", func() {
1025+
instance.Spec.TerminationGracePeriodSeconds = pointer.Int64Ptr(10)
1026+
builder = &resource.RabbitmqResourceBuilder{
1027+
Instance: &instance,
1028+
Scheme: scheme,
1029+
}
1030+
10261031
stsBuilder := builder.StatefulSet()
10271032
Expect(stsBuilder.Update(statefulSet)).To(Succeed())
10281033

10291034
gracePeriodSeconds := statefulSet.Spec.Template.Spec.TerminationGracePeriodSeconds
1030-
expectedGracePeriodSeconds := int64(60 * 60 * 24 * 7)
1031-
Expect(gracePeriodSeconds).To(Equal(&expectedGracePeriodSeconds))
1035+
Expect(gracePeriodSeconds).To(Equal(pointer.Int64Ptr(10)))
1036+
1037+
// TerminationGracePeriodSeconds is used to set commands timeouts in the preStop hook
1038+
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"}
1039+
Expect(statefulSet.Spec.Template.Spec.Containers[0].Lifecycle.PreStop.Exec.Command).To(Equal(expectedPreStopCommand))
10321040
})
10331041

10341042
It("checks mirror and querum queue status in preStop hook", func() {
@@ -1110,8 +1118,7 @@ var _ = Describe("StatefulSet", func() {
11101118
})
11111119

11121120
It("sets the replica count of the StatefulSet to the instance value", func() {
1113-
three := int32(3)
1114-
instance.Spec.Replicas = &three
1121+
instance.Spec.Replicas = pointer.Int32Ptr(3)
11151122
builder = &resource.RabbitmqResourceBuilder{
11161123
Instance: &instance,
11171124
Scheme: scheme,
@@ -1182,10 +1189,9 @@ var _ = Describe("StatefulSet", func() {
11821189
})
11831190

11841191
It("overrides statefulSet.spec.replicas", func() {
1185-
ten := int32(10)
11861192
instance.Spec.Override.StatefulSet = &rabbitmqv1beta1.StatefulSet{
11871193
Spec: &rabbitmqv1beta1.StatefulSetSpec{
1188-
Replicas: &ten,
1194+
Replicas: pointer.Int32Ptr(10),
11891195
},
11901196
}
11911197

@@ -1207,13 +1213,12 @@ var _ = Describe("StatefulSet", func() {
12071213
})
12081214

12091215
It("overrides statefulSet.spec.UpdateStrategy", func() {
1210-
one := int32(1)
12111216
instance.Spec.Override.StatefulSet = &rabbitmqv1beta1.StatefulSet{
12121217
Spec: &rabbitmqv1beta1.StatefulSetSpec{
12131218
UpdateStrategy: &appsv1.StatefulSetUpdateStrategy{
12141219
Type: "OnDelete",
12151220
RollingUpdate: &appsv1.RollingUpdateStatefulSetStrategy{
1216-
Partition: &one,
1221+
Partition: pointer.Int32Ptr(1),
12171222
},
12181223
},
12191224
},
@@ -1419,14 +1424,12 @@ var _ = Describe("StatefulSet", func() {
14191424
})
14201425

14211426
It("ensures override takes precedence when same property is set both at the top level and at the override level", func() {
1422-
two := int32(2)
1423-
four := int32(4)
14241427
instance.Spec.Image = "should-be-replaced-image"
1425-
instance.Spec.Replicas = &two
1428+
instance.Spec.Replicas = pointer.Int32Ptr(2)
14261429

14271430
instance.Spec.Override.StatefulSet = &rabbitmqv1beta1.StatefulSet{
14281431
Spec: &rabbitmqv1beta1.StatefulSetSpec{
1429-
Replicas: &four,
1432+
Replicas: pointer.Int32Ptr(4),
14301433
Template: &rabbitmqv1beta1.PodTemplateSpec{
14311434
Spec: &corev1.PodSpec{
14321435
Containers: []corev1.Container{
@@ -1466,16 +1469,16 @@ func extractContainer(containers []corev1.Container, containerName string) corev
14661469

14671470
func generateRabbitmqCluster() rabbitmqv1beta1.RabbitmqCluster {
14681471
storage := k8sresource.MustParse("10Gi")
1469-
one := int32(1)
14701472
return rabbitmqv1beta1.RabbitmqCluster{
14711473
ObjectMeta: v1.ObjectMeta{
14721474
Name: "foo",
14731475
Namespace: "foo-namespace",
14741476
},
14751477
Spec: rabbitmqv1beta1.RabbitmqClusterSpec{
1476-
Replicas: &one,
1477-
Image: "rabbitmq-image-from-cr",
1478-
ImagePullSecrets: []corev1.LocalObjectReference{{Name: "my-super-secret"}},
1478+
Replicas: pointer.Int32Ptr(1),
1479+
Image: "rabbitmq-image-from-cr",
1480+
ImagePullSecrets: []corev1.LocalObjectReference{{Name: "my-super-secret"}},
1481+
TerminationGracePeriodSeconds: pointer.Int64Ptr(604800),
14791482
Service: rabbitmqv1beta1.RabbitmqClusterServiceSpec{
14801483
Type: "this-is-a-service",
14811484
Annotations: map[string]string{},

‎system_tests/system_tests.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ package system_tests
1111

1212
import (
1313
"context"
14+
"k8s.io/utils/pointer"
1415

1516
. "github.com/onsi/ginkgo"
1617
. "github.com/onsi/gomega"
@@ -269,9 +270,8 @@ CONSOLE_LOG=new`
269270
var cluster *rabbitmqv1beta1.RabbitmqCluster
270271

271272
BeforeEach(func() {
272-
three := int32(3)
273273
cluster = generateRabbitmqCluster(namespace, "ha-rabbit")
274-
cluster.Spec.Replicas = &three
274+
cluster.Spec.Replicas = pointer.Int32Ptr(3)
275275

276276
Expect(createRabbitmqCluster(ctx, rmqClusterClient, cluster)).To(Succeed())
277277
waitForRabbitmqRunning(cluster)

0 commit comments

Comments
 (0)
Please sign in to comment.