Added Support for Termination Grace Period in crdbcluster (#1078)

* Fix the development workflow in make dev/build. Removed rules_k8s bazel dependency as it is deprecated and not supported with rules_oci. * [CRDB-43619] Added support for terminationGracePeriod in crdbcluster CR * fix: unit tests * update: modify TerminationGracePeriodSecs data type to int64 to remove implicit conversion * fix: resolve PR reviews --------- Co-authored-by: @NishanthNalluri
cockroachdb · Feb 4, 2025 · 286ae86 · 286ae86
1 parent fddacd4
commit 286ae86
Show file tree

Hide file tree

Showing 21 changed files with 48 additions and 4 deletions.
diff --git a/WORKSPACE b/WORKSPACE
@@ -57,12 +57,14 @@ load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies")
 
 go_rules_dependencies()
 
+# gazelle:repository_macro hack/build/repos.bzl%_go_dependencies
+go_dependencies()
+
 go_register_toolchains(version = "1.17")
 
 gazelle_dependencies()
 
-# gazelle:repository_macro hack/build/repos.bzl%_go_dependencies
-go_dependencies()
+
 
 ################################
 # begin rules_oci dependencies #

diff --git a/apis/v1alpha1/cluster_types.go b/apis/v1alpha1/cluster_types.go
@@ -145,6 +145,11 @@ type CrdbClusterSpec struct {
 	// Default: false
 	// +optional
 	AutomountServiceAccountToken bool `json:"automountServiceAccountToken,omitempty"`
+	// (Optional) The grace period in seconds prior to the container being forcibly terminated
+	// when marked for deletion or restarted.
+	// Default : 300
+	// +optional
+	TerminationGracePeriodSecs int64 `json:"terminationGracePeriodSecs,omitempty"`
 }
 
 // +k8s:openapi-gen=true

diff --git a/config/crd/bases/crdb.cockroachlabs.com_crdbclusters.yaml b/config/crd/bases/crdb.cockroachlabs.com_crdbclusters.yaml
@@ -1393,6 +1393,12 @@ spec:
                 description: '(Optional) The SQL Port number Default: 26257'
                 format: int32
                 type: integer
+              terminationGracePeriodSecs:
+                description: '(Optional) The grace period in seconds prior to the
+                  container being forcibly terminated when marked for deletion or
+                  restarted. Default : 300'
+                format: int64
+                type: integer
               tlsEnabled:
                 description: (Optional) TLSEnabled determines if TLS is enabled for
                   your CockroachDB Cluster

diff --git a/e2e/upgrades/upgrades_test.go b/e2e/upgrades/upgrades_test.go
@@ -221,6 +221,7 @@ func TestUpgradesMinorVersionThenRollback(t *testing.T) {
 
 	builder := testutil.NewBuilder("crdb").
 		WithAutomountServiceAccountToken(true).
+		WithTerminationGracePeriodSeconds(5).
 		WithNodeCount(3).
 		WithTLS().
 		WithImage(e2e.MinorVersion1).

diff --git a/hack/dev.sh b/hack/dev.sh
@@ -21,7 +21,7 @@ APP_VERSION="${APP_VERSION:-v$(cat version.txt)}"
 CLUSTER_NAME="dev"
 NODE_IMAGE="rancher/k3s:v1.23.3-k3s1"
 REGISTRY_NAME="registry.localhost"
-REGISTRY_PORT=5000
+REGISTRY_PORT=5002
 DEV_REGISTRY="${REGISTRY_NAME}:${REGISTRY_PORT}"
 
 main() {

diff --git a/install/crds.yaml b/install/crds.yaml
@@ -1391,6 +1391,12 @@ spec:
                 description: '(Optional) The SQL Port number Default: 26257'
                 format: int32
                 type: integer
+              terminationGracePeriodSecs:
+                description: '(Optional) The grace period in seconds prior to the
+                  container being forcibly terminated when marked for deletion or
+                  restarted. Default : 300'
+                format: int64
+                type: integer
               tlsEnabled:
                 description: (Optional) TLSEnabled determines if TLS is enabled for
                   your CockroachDB Cluster

diff --git a/pkg/resource/cluster.go b/pkg/resource/cluster.go
@@ -330,6 +330,13 @@ func (cluster Cluster) SecureMode() string {
 	return "--insecure"
 }
 
+func (cluster Cluster) GetTerminationGracePeriod() int64 {
+	if cluster.Spec().TerminationGracePeriodSecs == 0 {
+		return terminationGracePeriodSecs
+	}
+	return cluster.Spec().TerminationGracePeriodSecs
+}
+
 func (cluster Cluster) LoggingConfiguration(fetcher Fetcher) (string, error) {
 	if cluster.Spec().LogConfigMap != "" {
 		cm := &corev1.ConfigMap{

diff --git a/pkg/resource/statefulset.go b/pkg/resource/statefulset.go
@@ -209,7 +209,7 @@ func (b StatefulSetBuilder) makePodTemplate() corev1.PodTemplateSpec {
 				RunAsUser: ptr.Int64(1000581000),
 				FSGroup:   ptr.Int64(1000581000),
 			},
-			TerminationGracePeriodSeconds: ptr.Int64(terminationGracePeriodSecs),
+			TerminationGracePeriodSeconds: ptr.Int64(b.GetTerminationGracePeriod()),
 			Containers:                    b.MakeContainers(),
 			AutomountServiceAccountToken:  ptr.Bool(b.Spec().AutomountServiceAccountToken),
 			ServiceAccountName:            b.ServiceAccountName(),

diff --git a/pkg/resource/testdata/TestStatefulSetBuilder/automount_sa.golden b/pkg/resource/testdata/TestStatefulSetBuilder/automount_sa.golden
@@ -26,6 +26,7 @@ spec:
         car: koenigsegg
     spec:
       automountServiceAccountToken: true
+      terminationGracePeriodSecs: 300
       containers:
       - command:
         - /bin/bash

diff --git a/pkg/resource/testdata/TestStatefulSetBuilder/automount_sa_in.yaml b/pkg/resource/testdata/TestStatefulSetBuilder/automount_sa_in.yaml
@@ -20,6 +20,7 @@ metadata:
   namespace: test-ns
 spec:
   automountServiceAccountToken: true
+  terminationGracePeriodSecs: 300
   dataStore:
     pvc:
       spec:

diff --git a/pkg/resource/testdata/TestStatefulSetBuilder/default_insecure_statefulset.golden b/pkg/resource/testdata/TestStatefulSetBuilder/default_insecure_statefulset.golden
@@ -24,6 +24,7 @@ spec:
         app.kubernetes.io/name: cockroachdb
     spec:
       automountServiceAccountToken: false
+      terminationGracePeriodSecs: 300
       containers:
       - command:
         - /bin/bash

diff --git a/pkg/resource/testdata/TestStatefulSetBuilder/default_insecure_statefulset_in.yaml b/pkg/resource/testdata/TestStatefulSetBuilder/default_insecure_statefulset_in.yaml
@@ -19,6 +19,7 @@ metadata:
   name: test-cluster
   namespace: test-ns
 spec:
+  terminationGracePeriodSecs: 300
   dataStore:
     pvc:
       spec:

diff --git a/pkg/resource/testdata/TestStatefulSetBuilder/default_secure.golden b/pkg/resource/testdata/TestStatefulSetBuilder/default_secure.golden
@@ -26,6 +26,7 @@ spec:
         car: koenigsegg
     spec:
       automountServiceAccountToken: false
+      terminationGracePeriodSecs: 300
       containers:
       - command:
         - /bin/bash

diff --git a/pkg/resource/testdata/TestStatefulSetBuilder/default_secure_in.yaml b/pkg/resource/testdata/TestStatefulSetBuilder/default_secure_in.yaml
@@ -19,6 +19,7 @@ metadata:
   name: test-cluster
   namespace: test-ns
 spec:
+  terminationGracePeriodSecs: 300
   dataStore:
     pvc:
       spec:

diff --git a/pkg/resource/testdata/TestStatefulSetBuilder/insecure_statefulset_cli_args.golden b/pkg/resource/testdata/TestStatefulSetBuilder/insecure_statefulset_cli_args.golden
@@ -39,6 +39,7 @@ spec:
               topologyKey: kubernetes.io/hostname
             weight: 100
       automountServiceAccountToken: false
+      terminationGracePeriodSecs: 300
       containers:
       - command:
         - /bin/bash

diff --git a/pkg/resource/testdata/TestStatefulSetBuilder/insecure_statefulset_cli_args_in.yaml b/pkg/resource/testdata/TestStatefulSetBuilder/insecure_statefulset_cli_args_in.yaml
@@ -58,4 +58,5 @@ spec:
       effect: "NoSchedule"
   additionalAnnotations:
     key: "test-value"
+  terminationGracePeriodSecs: 300
 status: {}
diff --git a/pkg/resource/testdata/TestStatefulSetBuilder/insecure_statefulset_cli_args_with_join.golden b/pkg/resource/testdata/TestStatefulSetBuilder/insecure_statefulset_cli_args_with_join.golden
@@ -39,6 +39,7 @@ spec:
               topologyKey: kubernetes.io/hostname
             weight: 100
       automountServiceAccountToken: false
+      terminationGracePeriodSecs: 300
       containers:
       - command:
         - /bin/bash

diff --git a/pkg/resource/testdata/TestStatefulSetBuilder/insecure_statefulset_cli_args_with_join_in.yaml b/pkg/resource/testdata/TestStatefulSetBuilder/insecure_statefulset_cli_args_with_join_in.yaml
@@ -58,4 +58,5 @@ spec:
       effect: "NoSchedule"
   additionalAnnotations:
     key: "test-value"
+  terminationGracePeriodSecs: 300
 status: {}
diff --git a/pkg/resource/testdata/TestStatefulSetBuilder/insecure_statefulset_with_resources.golden b/pkg/resource/testdata/TestStatefulSetBuilder/insecure_statefulset_with_resources.golden
@@ -24,6 +24,7 @@ spec:
         app.kubernetes.io/name: cockroachdb
     spec:
       automountServiceAccountToken: false
+      terminationGracePeriodSecs: 300
       containers:
       - command:
         - /bin/bash

diff --git a/pkg/resource/testdata/TestStatefulSetBuilder/insecure_statefulset_with_resources_in.yaml b/pkg/resource/testdata/TestStatefulSetBuilder/insecure_statefulset_with_resources_in.yaml
@@ -41,4 +41,5 @@ spec:
   topology:
     zones:
       - locality: ""
+  terminationGracePeriodSecs: 300
 status: {}
diff --git a/pkg/testutil/builder.go b/pkg/testutil/builder.go
@@ -56,6 +56,11 @@ func (b ClusterBuilder) WithAutomountServiceAccountToken(mount bool) ClusterBuil
 	return b
 }
 
+func (b ClusterBuilder) WithTerminationGracePeriodSeconds(s int64) ClusterBuilder {
+	b.cluster.Spec.TerminationGracePeriodSecs = s
+	return b
+}
+
 func (b ClusterBuilder) WithUID(uid string) ClusterBuilder {
 	b.cluster.ObjectMeta.UID = amtypes.UID(uid)
 	return b