From 6c1eb47d7a5493ec4030f442c35822583761f39f Mon Sep 17 00:00:00 2001 From: Michael Burman Date: Thu, 2 May 2024 15:02:55 +0300 Subject: [PATCH 1/3] Add Spec.MindReadySeconds to allow setting a non-default value for MinReadySeconds in StatefulSets. Set the default MinReadySeconds to 5 in StatefulSets to prevent sudden double restarts from statefulset controller. --- CHANGELOG.md | 2 ++ apis/cassandra/v1beta1/cassandradatacenter_types.go | 4 ++++ apis/cassandra/v1beta1/zz_generated.deepcopy.go | 5 +++++ .../bases/cassandra.datastax.com_cassandradatacenters.yaml | 6 ++++++ pkg/reconciliation/construct_statefulset.go | 5 +++++ 5 files changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 519d786d..84aa0d47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ Changelog for Cass Operator, new PRs should update the `main / unreleased` secti ## unreleased +* [ENHANCEMENT] [#648](https://github.com/k8ssandra/cass-operator/issues/648) Make MinReadySeconds configurable value in the Spec. + ## v1.21.1 * [BUGFIX] [#665](https://github.com/k8ssandra/cass-operator/issues/665) The RequiresUpdate and ObservedGenerations were updated too often, even when the reconcile was not finished. diff --git a/apis/cassandra/v1beta1/cassandradatacenter_types.go b/apis/cassandra/v1beta1/cassandradatacenter_types.go index fc4a76c4..35f27b36 100644 --- a/apis/cassandra/v1beta1/cassandradatacenter_types.go +++ b/apis/cassandra/v1beta1/cassandradatacenter_types.go @@ -258,6 +258,10 @@ type CassandraDatacenterSpec struct { // Use cautiously. // +optional DatacenterName string `json:"datacenterName,omitempty"` + + // MinReadySeconds sets the minimum number of seconds for which a newly created pod should be ready without any of its containers crashing, for it to be considered available. Defaults to 5 seconds and is set in the StatefulSet spec. + // Setting to 0 might cause multiple Cassandra pods to restart at the same time despite PodDisruptionBudget settings. + MinReadySeconds *int32 `json:"minReadySeconds,omitempty"` } type NetworkingConfig struct { diff --git a/apis/cassandra/v1beta1/zz_generated.deepcopy.go b/apis/cassandra/v1beta1/zz_generated.deepcopy.go index 63c077c8..ad4af351 100644 --- a/apis/cassandra/v1beta1/zz_generated.deepcopy.go +++ b/apis/cassandra/v1beta1/zz_generated.deepcopy.go @@ -357,6 +357,11 @@ func (in *CassandraDatacenterSpec) DeepCopyInto(out *CassandraDatacenterSpec) { *out = new(CDCConfiguration) (*in).DeepCopyInto(*out) } + if in.MinReadySeconds != nil { + in, out := &in.MinReadySeconds, &out.MinReadySeconds + *out = new(int32) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CassandraDatacenterSpec. diff --git a/config/crd/bases/cassandra.datastax.com_cassandradatacenters.yaml b/config/crd/bases/cassandra.datastax.com_cassandradatacenters.yaml index 65605344..74558566 100644 --- a/config/crd/bases/cassandra.datastax.com_cassandradatacenters.yaml +++ b/config/crd/bases/cassandra.datastax.com_cassandradatacenters.yaml @@ -348,6 +348,12 @@ spec: - serverSecretName type: object type: object + minReadySeconds: + description: |- + MinReadySeconds sets the minimum number of seconds for which a newly created pod should be ready without any of its containers crashing, for it to be considered available. Defaults to 5 seconds and is set in the StatefulSet spec. + Setting to 0 might cause multiple Cassandra pods to restart at the same time despite PodDisruptionBudget settings. + format: int32 + type: integer networking: properties: hostNetwork: diff --git a/pkg/reconciliation/construct_statefulset.go b/pkg/reconciliation/construct_statefulset.go index a9b1ca81..f9227710 100644 --- a/pkg/reconciliation/construct_statefulset.go +++ b/pkg/reconciliation/construct_statefulset.go @@ -145,6 +145,7 @@ func newStatefulSetForCassandraDatacenter( PodManagementPolicy: appsv1.ParallelPodManagement, Template: *template, VolumeClaimTemplates: volumeClaimTemplates, + MinReadySeconds: 5, }, } @@ -169,6 +170,10 @@ func newStatefulSetForCassandraDatacenter( result.Spec.UpdateStrategy = strategy } + if dc.Spec.MinReadySeconds != nil { + result.Spec.MinReadySeconds = *dc.Spec.MinReadySeconds + } + // add a hash here to facilitate checking if updates are needed utils.AddHashAnnotation(result) From 1a903e55ec8b94d4c9aee52f30875d2dbd32ca09 Mon Sep 17 00:00:00 2001 From: Michael Burman Date: Thu, 2 May 2024 15:55:46 +0300 Subject: [PATCH 2/3] Modify unit test to ensure value is set --- pkg/reconciliation/construct_statefulset_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/reconciliation/construct_statefulset_test.go b/pkg/reconciliation/construct_statefulset_test.go index 3db9d14e..bfb96939 100644 --- a/pkg/reconciliation/construct_statefulset_test.go +++ b/pkg/reconciliation/construct_statefulset_test.go @@ -420,6 +420,7 @@ func Test_newStatefulSetForCassandraDatacenterWithAdditionalVolumes(t *testing.T assert.Equal(t, 1, len(got.Spec.Template.Spec.InitContainers[1].VolumeMounts)) assert.Equal(t, "server-config", got.Spec.Template.Spec.InitContainers[1].VolumeMounts[0].Name) assert.Equal(t, "/config", got.Spec.Template.Spec.InitContainers[1].VolumeMounts[0].MountPath) + assert.Equal(t, int32(5), got.Spec.MinReadySeconds) } } From 3c8c0582e37359c59ac773532b51a98d8d70f0b8 Mon Sep 17 00:00:00 2001 From: Michael Burman Date: Thu, 20 Jun 2024 11:11:28 +0300 Subject: [PATCH 3/3] Add additional test that the change is actually applied --- .../construct_statefulset_test.go | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/pkg/reconciliation/construct_statefulset_test.go b/pkg/reconciliation/construct_statefulset_test.go index bfb96939..45e02997 100644 --- a/pkg/reconciliation/construct_statefulset_test.go +++ b/pkg/reconciliation/construct_statefulset_test.go @@ -712,3 +712,35 @@ func TestPodTemplateSpecHashAnnotationChanges(t *testing.T) { updatedHash = sts.Annotations[utils.ResourceHashAnnotationKey] assert.NotEqual(currentHash, updatedHash, "expected hash to change when PodTemplateSpec labels change") } + +func TestMinReadySecondsChange(t *testing.T) { + assert := assert.New(t) + dc := &api.CassandraDatacenter{ + Spec: api.CassandraDatacenterSpec{ + ClusterName: "test", + ServerType: "cassandra", + ServerVersion: "4.0.7", + StorageConfig: api.StorageConfig{ + CassandraDataVolumeClaimSpec: &corev1.PersistentVolumeClaimSpec{}, + }, + Racks: []api.Rack{ + { + Name: "testrack", + }, + }, + PodTemplateSpec: &corev1.PodTemplateSpec{}, + }, + } + + sts, err := newStatefulSetForCassandraDatacenter(nil, dc.Spec.Racks[0].Name, dc, 3) + assert.NoError(err, "failed to build statefulset") + + assert.Equal(int32(5), sts.Spec.MinReadySeconds) + + dc.Spec.MinReadySeconds = ptr.To[int32](10) + + sts, err = newStatefulSetForCassandraDatacenter(nil, dc.Spec.Racks[0].Name, dc, 3) + assert.NoError(err, "failed to build statefulset") + + assert.Equal(int32(10), sts.Spec.MinReadySeconds) +}