Skip to content

Fix missing ownerReferences in pvc metadata#1837

Merged
sunsingerus merged 1 commit into
Altinity:0.25.5from
fkywong:fix-pvc-ownerReferences
Oct 13, 2025
Merged

Fix missing ownerReferences in pvc metadata#1837
sunsingerus merged 1 commit into
Altinity:0.25.5from
fkywong:fix-pvc-ownerReferences

Conversation

@fkywong

@fkywong fkywong commented Oct 2, 2025

Copy link
Copy Markdown
Contributor

Problem

See issue #1799, #1843

Root Cause

By default, the Clickhouse Helm chart deploys the Keeper StatefulSet with PVCs provisioned by the Operator (chk.yaml:33), but the Operator does not set metadata.OwnerReferences on the PVCs.

Without metadata.OwnerReferences correctly set, ArgoCD can't attribute the PVCs to the application installation, and will incorrectly mark the PVCs as OutOfSync, which leads to the following undesirable behaviors/workarounds documented in the aforementioned issue.

The cluster instance's PVCs don't encounter the same issue because the Helm chart chi.yaml doesn't specify spec.defaults.storageManagement.provisioner, so the default StatefulSet provisioner is used:

// Default value
return api.PVCProvisionerStatefulSet

Thus, ArgoCD is able to correctly map the StatefulSet-created PVCs to the CHI StatefulSet resources.

Solution: set metadata.OwnerReferences on PVCs

There's already precedence with doing this on Operator-provisioned StatefulSet and Service k8s resources:

ObjectMeta: meta.ObjectMeta{
Name: c.nm.Name(interfaces.NameStatefulSet, host),
Namespace: host.GetRuntime().GetAddress().GetNamespace(),
Labels: c.macro.Scope(host).Map(c.tagger.Label(interfaces.LabelSTS, host)),
Annotations: c.macro.Scope(host).Map(c.tagger.Annotate(interfaces.AnnotateSTS, host)),
OwnerReferences: c.or.CreateOwnerReferences(c.cr),
},

service.OwnerReferences = ownerReferences
// Combine labels and annotations
service.Labels = macro.Map(util.MergeStringMapsOverwrite(service.Labels, labels))
service.Annotations = macro.Map(util.MergeStringMapsOverwrite(service.Annotations, annotations))

This PR makes code changes in a similar manner to the above code snippets.

Pull Request checklist

  • All commits in the PR are squashed. More info
  • The PR is made into dedicated next-release branch, not into master branch1. More info
  • The PR is signed. More info

Signed-off-by: Felix Wong <fkywong@outlook.com>
@sunsingerus sunsingerus added the planned for review This feature is planned for review label Oct 2, 2025
@sunsingerus sunsingerus merged commit 86bcb4b into Altinity:0.25.5 Oct 13, 2025
2 checks passed
@sunsingerus

Copy link
Copy Markdown
Collaborator

Crushes tests on PVC retain policy. Reverted ATM, will think what to do about it.

@sunsingerus sunsingerus added the work in progress This feautre is not completed yet label Oct 16, 2025
@augustodileo

Copy link
Copy Markdown

Was this delivered into any release? Im on 0.26.2 and not present the ownerReferences in there

@fkywong

fkywong commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

@augustodileo unfortunately due to compatibility issues with tests mentioned earlier, this PR was reverted by a4f5fe1 (>=0.25.5). If you want this functionality, you'll have to cherry-pick the commit and build a custom image of the operator for your workflows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

planned for review This feature is planned for review work in progress This feautre is not completed yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants