Skip to content

K8SPXC-1688 feat: add support for extraPVCs #2128

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
eminaktas wants to merge 3 commits into from
Closed

K8SPXC-1688 feat: add support for extraPVCs #2128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7baf735
feat: add support for extraPVCs
eminaktas Jul 23, 2025
2b1a171
Merge branch 'main' into feat/add-extra-pvcs
hors Sep 22, 2025
d351086
Merge branch 'main' into feat/add-extra-pvcs
hors Nov 17, 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
525 changes: 525 additions & 0 deletions config/crd/bases/pxc.percona.com_perconaxtradbclusters.yaml
View file
Open in desktop

Large diffs are not rendered by default.

525 changes: 525 additions & 0 deletions deploy/bundle.yaml
View file
Open in desktop

Large diffs are not rendered by default.

525 changes: 525 additions & 0 deletions deploy/crd.yaml
View file
Open in desktop

Large diffs are not rendered by default.

525 changes: 525 additions & 0 deletions deploy/cw-bundle.yaml
View file
Open in desktop

Large diffs are not rendered by default.

91 changes: 75 additions & 16 deletions pkg/apis/pxc/v1/pxc_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,22 +549,31 @@ type PodSpec struct {
// Deprecated: Use ServiceExpose.Labels instead
ReplicasServiceLabels map[string]string `json:"replicasServiceLabels,omitempty"`

SchedulerName string `json:"schedulerName,omitempty"`
ReadinessInitialDelaySeconds *int32 `json:"readinessDelaySec,omitempty"`
ReadinessProbes corev1.Probe `json:"readinessProbes,omitempty"`
LivenessInitialDelaySeconds *int32 `json:"livenessDelaySec,omitempty"`
LivenessProbes corev1.Probe `json:"livenessProbes,omitempty"`
PodSecurityContext *corev1.PodSecurityContext `json:"podSecurityContext,omitempty"`
ContainerSecurityContext *corev1.SecurityContext `json:"containerSecurityContext,omitempty"`
ServiceAccountName string `json:"serviceAccountName,omitempty"`
ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"`
Sidecars []corev1.Container `json:"sidecars,omitempty"`
SidecarVolumes []corev1.Volume `json:"sidecarVolumes,omitempty"`
SidecarPVCs []corev1.PersistentVolumeClaim `json:"sidecarPVCs,omitempty"`
RuntimeClassName *string `json:"runtimeClassName,omitempty"`
HookScript string `json:"hookScript,omitempty"`
Lifecycle corev1.Lifecycle `json:"lifecycle,omitempty"`
TopologySpreadConstraints []corev1.TopologySpreadConstraint `json:"topologySpreadConstraints,omitempty"`
SchedulerName string `json:"schedulerName,omitempty"`
ReadinessInitialDelaySeconds *int32 `json:"readinessDelaySec,omitempty"`
ReadinessProbes corev1.Probe `json:"readinessProbes,omitempty"`
LivenessInitialDelaySeconds *int32 `json:"livenessDelaySec,omitempty"`
LivenessProbes corev1.Probe `json:"livenessProbes,omitempty"`
PodSecurityContext *corev1.PodSecurityContext `json:"podSecurityContext,omitempty"`
ContainerSecurityContext *corev1.SecurityContext `json:"containerSecurityContext,omitempty"`
ServiceAccountName string `json:"serviceAccountName,omitempty"`
ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"`
Sidecars []corev1.Container `json:"sidecars,omitempty"`
SidecarVolumes []corev1.Volume `json:"sidecarVolumes,omitempty"`
// +kubebuilder:validation:items:XEmbeddedResource
SidecarPVCs []corev1.PersistentVolumeClaim `json:"sidecarPVCs,omitempty"`
RuntimeClassName *string `json:"runtimeClassName,omitempty"`
HookScript string `json:"hookScript,omitempty"`
Lifecycle corev1.Lifecycle `json:"lifecycle,omitempty"`
TopologySpreadConstraints []corev1.TopologySpreadConstraint `json:"topologySpreadConstraints,omitempty"`
ExtraPVCs []ExtraPVC `json:"extraPVCs,omitempty"`
}

type ExtraPVC struct {
// +kubebuilder:validation:EmbeddedResource
VolumeClaimTemplate corev1.PersistentVolumeClaim `json:"volumeClaimTemplate,omitempty"`
MountPath string `json:"mountPath,omitempty"`
ReadOnly *bool `json:"readOnly,omitempty"`
}

func (spec *PodSpec) HasSidecarInternalSecret(secret *corev1.Secret) bool {
Expand Down Expand Up @@ -1569,6 +1578,56 @@ func AddSidecarPVCs(log logr.Logger, existing, sidecarPVCs []corev1.PersistentVo
return existing
}

func AddExtraVolumeMounts(log logr.Logger, existing []corev1.VolumeMount, extraPVCs []ExtraPVC) []corev1.VolumeMount {
if len(extraPVCs) == 0 {
return existing
}

names := make(map[string]struct{}, len(existing))
for _, v := range existing {
names[v.Name] = struct{}{}
}

for _, v := range extraPVCs {
name := v.VolumeClaimTemplate.GetName()
if _, ok := names[name]; ok {
log.Info("Volume name already exists, it is skipped", "volumeName", name)
continue
}

existing = append(existing, corev1.VolumeMount{
Name: name,
MountPath: v.MountPath,
ReadOnly: *v.ReadOnly,
})
}

return existing
}

func AddExtraPVCs(log logr.Logger, existing []corev1.PersistentVolumeClaim, extraPVCs []ExtraPVC) []corev1.PersistentVolumeClaim {
if len(extraPVCs) == 0 {
return existing
}

names := make(map[string]struct{}, len(existing))
for _, p := range existing {
names[p.Name] = struct{}{}
}

for _, p := range extraPVCs {
name := p.VolumeClaimTemplate.GetName()
if _, ok := names[name]; ok {
log.Info("PVC name already exists, it is skipped", "PVCName", name)
continue
}

existing = append(existing, p.VolumeClaimTemplate)
}

return existing
}

func (cr *PerconaXtraDBCluster) ProxySQLUnreadyServiceNamespacedName() types.NamespacedName {
return types.NamespacedName{
Name: cr.Name + "-proxysql-unready",
Expand Down
28 changes: 28 additions & 0 deletions pkg/apis/pxc/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion pkg/controller/pxcrestore/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ func TestOperatorRestart(t *testing.T) {
ctx := context.Background()

const clusterName = "test-cluster"
const datadirPvcName = "datadir-" + clusterName + "-pxc-0"
const namespace = "namespace"
const backupName = clusterName + "-backup"
const restoreName = clusterName + "-restore"
Expand Down Expand Up @@ -436,7 +437,7 @@ func TestOperatorRestart(t *testing.T) {
cr.Status.State = state
objects := append(tt.objects, tt.bcp, cr, cluster, crSecret, &corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "pvc1",
Name: datadirPvcName,
Namespace: namespace,
Labels: statefulset.NewNode(cluster).Labels(),
},
Expand Down
14 changes: 11 additions & 3 deletions pkg/k8s/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func PauseClusterWithWait(ctx context.Context, cl client.Client, cr *api.Percona
}
}

err = waitForPVCShutdown(ctx, cl, ls, cr.Namespace)
err = waitForPVCShutdown(ctx, cl, ls, cr.Namespace, pvcNameTemplate)
if err != nil {
return errors.Wrap(err, "shutdown pvc")
}
Expand Down Expand Up @@ -172,7 +172,7 @@ func waitForPodsShutdown(ctx context.Context, cl client.Client, ls map[string]st

const waitLimitSec int64 = 300

func waitForPVCShutdown(ctx context.Context, cl client.Client, ls map[string]string, namespace string) error {
func waitForPVCShutdown(ctx context.Context, cl client.Client, ls map[string]string, namespace, pvcNameTemplate string) error {
for i := int64(0); i < waitLimitSec; i++ {
pvcs := corev1.PersistentVolumeClaimList{}

Expand All @@ -188,7 +188,15 @@ func waitForPVCShutdown(ctx context.Context, cl client.Client, ls map[string]str
return errors.Wrap(err, "get pvc list")
}

if len(pvcs.Items) == 1 {
filtered := []corev1.PersistentVolumeClaim{}
for _, pvc := range pvcs.Items {
// check prefix to filter out extraPVCs
if strings.HasPrefix(pvc.Name, pvcNameTemplate) {
filtered = append(filtered, pvc)
}
}

if len(filtered) == 1 {
return nil
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/pxc/statefulset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ func StatefulSet(
return nil, errors.Wrap(err, "app container")
}

appC.VolumeMounts = api.AddExtraVolumeMounts(log, appC.VolumeMounts, podSpec.ExtraPVCs)

pmmC, err := sfs.PMMContainer(ctx, cl, cr.Spec.PMM, secret, cr)
if err != nil {
log.Info(`"pmm container error"`, "secrets", cr.Spec.SecretsName, "internalSecrets", "internal-"+cr.Name, "error", err)
Expand Down Expand Up @@ -138,6 +140,7 @@ func StatefulSet(
obj.Spec.VolumeClaimTemplates = sfsVolume.PVCs
}
obj.Spec.VolumeClaimTemplates = api.AddSidecarPVCs(log, obj.Spec.VolumeClaimTemplates, podSpec.SidecarPVCs)
obj.Spec.VolumeClaimTemplates = api.AddExtraPVCs(log, obj.Spec.VolumeClaimTemplates, podSpec.ExtraPVCs)
Copy link
Contributor

@gkech gkech Nov 5, 2025

Choose a reason for hiding this comment

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

VolumeClaimTemplates in a StatefulSet cannot be modified after creation. If we try to add extra PVCs to an existing StatefulSet, we will get an error like:

The StatefulSet "xyz" is invalid: spec: Forbidden: updates to statefulset spec for fields other ... are forbidden

This means that adding extra pvcs on a running cluster will not work and only fresh cluster will be able to utilize this feature, so we have a major limitation by approaching it like that.

Copy link
Contributor Author

@eminaktas eminaktas Nov 5, 2025

Choose a reason for hiding this comment

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

Can we consider cascading deletion here? In the code, there is an existing example here.

Ref: https://kubernetes.io/docs/tasks/administer-cluster/use-cascading-deletion/#set-orphan-deletion-policy

Copy link
Contributor

@gkech gkech Nov 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Given the use case you shared on the GH issue you originally opened,

Use-Case
We have a scenario where our application stores certain domain-specific information into a shared volume that DB needs access to. This data is not part of the database itself, but is used in conjunction with queries and procedures. For example, it may contain reference files, large binary objects, or externally generated lookup tables.

I believe we should approach this in a slightly different angle. We can only consider adding as extra pvcs, actual pvcs that are created externally and contain the data we want already. Then these PVCs are going to be available for all sts pods with either read or write permissions (accepting all the individual storage type limitations and potential risks of the shared storage). The implementation for that is different from what we have right now available on this PR, but it is more suitable for the use case you are describing and indeed is valid.

For having the operator create additional PVCs and mount them, as you suggested an approach is the cascade deletion, but this can have various other implications that maybe are out of the context of this issue we are describing here.

Copy link
Collaborator

@hors hors Nov 13, 2025

Choose a reason for hiding this comment

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

Hi @eminaktas , do you have time to finish it? We want to include it in PXCO 1.19.0.

Copy link
Contributor Author

@eminaktas eminaktas Nov 13, 2025

Choose a reason for hiding this comment

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

Hi @hors, I am a bit tight with my works currently. If someone wish to take over, I’d be great but I can work with my best effort.

Copy link
Contributor Author

@eminaktas eminaktas Nov 14, 2025

Choose a reason for hiding this comment

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

@gkech To be clear, what is your suggestion?

Also, We should consider sidecarPVCs which works in the similar way. I implemented this feature using exinsting sidecarPVCs implementation as an example. Can you also help me understand that part as well?

Copy link
Contributor

@gkech gkech Nov 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@gkech To be clear, what is your suggestion?

For now to only implement the configuration and mounting of an existing PVC that was created independently of the operator workflow. So you have a single PVC with all the data you need, PVC which all the database pod can access and use. That would solve the need you are describing in the Github Issue you opened, right?

Copy link
Contributor

@gkech gkech Dec 2, 2025

Choose a reason for hiding this comment

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

Since we would like to introduce the related to the extra pvcs change to the next operator release, we prepared this PR: #2276, let us know if it works for you

Copy link
Contributor Author

@eminaktas eminaktas Dec 2, 2025

Choose a reason for hiding this comment

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

@gkech Thanks for taking ownership of this — I wasn’t able to get back to it earlier. The changes in this PR work for us as well.

We also have a use case where we want to attach local-disk PVCs to MySQL instances on each host. I see that this isn’t covered here, which makes sense since my original PR focused on that part. We can address that use case separately later on if it’s something that would be useful for the project as well.

Copy link
Contributor

@gkech gkech Dec 9, 2025

Choose a reason for hiding this comment

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

We also have a use case where we want to attach local-disk PVCs to MySQL instances on each host. I see that this isn’t covered here, which makes sense since my original PR focused on that part. We can address that use case separately later on if it’s something that would be useful for the project as well.

@eminaktas we created a separate ticket to address the case: https://perconadev.atlassian.net/browse/K8SPXC-1764

We will close this PR for now since the original issue described was addressed with the extra PVCs fix. If you feel that you've got the time to address the new ticket, please, by all means, go for it and let us know :)


return obj, nil
}
Expand Down
Loading