Skip to content

[ CC-32596 ] Add WAL failover custom path support for helm chart of cockroachdb operator deployments and handle the migration changes #557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[ CC-32596 ] Add WAL failover custom path support for helm chart of cockroachdb operator deployments and handle the migration changes #557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3c69dc
Add WAL failover custom path support for helm chart migration
shreyaskm623 Sep 10, 2025
c1ab842
Fix syntax errors and formatting issues after merge conflict resolution
shreyaskm623 Oct 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions cockroachdb-parent/charts/cockroachdb/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,11 @@ cockroachdb:
#
# walFailoverSpec captures the configuration for WAL Failover.
walFailoverSpec: {}
# name determines the prefix of the pvc name that we want to provide
# for the wal failover. This field was introduced during the migration process
# from the statefulset to the operator.
# name: failover-dir
#
# status determines the possible values to WAL Failover configuration.
# It has 3 possible values: "", "enable" and "disable".
# status: ""
Expand All @@ -277,6 +282,15 @@ cockroachdb:
# If not set, the default provisioner will be chosen (gp2 on AWS, standard on GKE).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per our discussion it is not true. When left empty in CockroachDB operator it doesn't pickup the default storage class. So we have to take an input from user if enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prafull01 In case of a fresh deployment - if the storageClassName is not provided , it picks up the default provisioner , only in case of migration , since the storageClassName would already be present , we not giving the className in the spec would cause problems. So populating the storageClassName is important during migration and not mandatory for fresh deployments.

# storageClassName: ""
#
# path is the absolute mount path inside the CockroachDB container to use for WAL failover side disk.
# Examples:
# - /cockroach/cockroach-failover
# - /cockroach/custom/path
# IMPORTANT: Set it only for fresh deployments. Changing it on upgrades might cause issues with the deployment.
# TODO: Add validation in the operator to prevent changing this value on upgrades.
#
# path: "/cockroach/cockroach-failover"
#
# podLabels captures additional labels to apply to CockroachDB pods.
podLabels:
app.kubernetes.io/component: cockroachdb
Expand Down
2 changes: 1 addition & 1 deletion cockroachdb-parent/charts/operator/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ image:
# pullPolicy specifies the image pull policy.
pullPolicy: IfNotPresent
# tag is the image tag.
tag: "6f62639e9fee99d99b0387a9dccda84daa1a489b592b008f2f354ec57eae09ac"
tag: "496a3d0b4b4a38637d97299aec52646c41eb32380c56b425376c8dd262c8b882"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your Image is not correct, that is why test is not working. Check the registry and repository as well

# certificate defines the certificate settings for the Operator.
certificate:
# validForDays specifies the number of days the certificate is valid for.
Expand Down
8 changes: 8 additions & 0 deletions pkg/migrate/build-manifests.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ func (m *Manifest) FromHelmChart() error {
}

input.pcrSpec = detectPCRFromInitJob(m.clientset, sts.Name, m.namespace)

// Cluster level WalSpec
var clusterWalSpec *v1alpha1.CrdbWalFailoverSpec
for nodeIdx := int32(0); nodeIdx < *sts.Spec.Replicas; nodeIdx++ {
podName := fmt.Sprintf("%s-%d", sts.Name, nodeIdx)
pod, err := m.clientset.CoreV1().Pods(m.namespace).Get(ctx, podName, metav1.GetOptions{})
Expand All @@ -183,6 +186,11 @@ func (m *Manifest) FromHelmChart() error {
return errors.Newf("pod %s isn't scheduled to a node", podName)
}

// Build the walFailoverSpec if applicable
buildWalFailoverSpec(ctx, m.clientset, sts, pod.Spec.NodeName, nodeIdx, &input)
if input.walFailoverSpec != nil && clusterWalSpec == nil {
clusterWalSpec = input.walFailoverSpec
}
nodeSpec := buildNodeSpecFromHelm(sts, pod.Spec.NodeName, input)
crdbNode := v1alpha1.CrdbNode{
TypeMeta: metav1.TypeMeta{
Expand Down
Loading
Loading