SigNoz/charts

Missing reclaim policy for PVC

srikanthccv opened this issue · 4 comments

The default reclaim policy for PVC from the operator is Delete https://github.com/Altinity/clickhouse-operator/blob/689d35c207b4b189adfa7d3171b9a606b6cfa44e/pkg/model/volumer.go#L39-L53. We don't have any reclaim policy set here

{{- if and (.Values.persistence.enabled) (not .Values.persistence.existingClaim) }}
volumeClaimTemplates:
- name: data-volumeclaim-template
spec:
accessModes:
{{- toYaml .Values.persistence.accessModes | nindent 12 }}
resources:
requests:
storage: {{ .Values.persistence.size }}
{{- $storageClass := default .Values.persistence.storageClass .Values.global.storageClass -}}
{{- if $storageClass -}}
{{- if (eq "-" $storageClass) }}
storageClassName: ""
{{- else }}
storageClassName: {{ $storageClass }}
{{- end }}
{{- end }}
{{- end }}
?

They say Use reclaimPolicy: Retain in all your cluster resource definitions. here https://altinity.com/blog/preventing-clickhouse-storage-deletion-with-the-altinity-kubernetes-operator-reclaimpolicy. Why do use the default Delete policy instead of Retain for PVCs also?

The retain policy of the PV falls back the retain policy of the used storage class.

Nevertheless, I think it's good to include the changes.

@srikanthccv do review the linked PR and let me know.

I am confused by your usage of PV and PVC here/Slack/Notion etc and points have been incoherent. We are trying to address the PVC policy. This doesn't affect the PV policies in any way.

We are trying to address the PVC policy. This doesn't affect the PV policies in any way.

yes, you are right. And the linked PR helps in that.
But I was talking wrt our previous environment, this didn't have much change in behaviour.

Let me share details about the same with you on DM.

PR with the changes are merged and part of clickhouse-23.12.3 release.

Will include this from next signoz chart release