[mariadb-operator] Add post-delete job to remove PVCs #1553
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughA new Helm template is added for MySQL that defines a post-delete cleanup workflow. The template includes a Job, ServiceAccount, Role, and RoleBinding configured to delete orphaned PVCs labeled with the release instance during Helm deletion. Changes
Sequence DiagramsequenceDiagram
participant User
participant Helm
participant K8s as Kubernetes API
participant Job
participant Kubelet
User->>Helm: helm delete <release>
Helm->>K8s: Delete release resources
Helm->>K8s: Trigger post-delete hooks<br/>(hook-weight, hook-delete-policy)
K8s->>Job: Create cleanup Job
Kubelet->>Job: Execute Job pod
Job->>K8s: List PVCs with<br/>app.kubernetes.io/instance=<release>
Job->>K8s: Delete matched PVCs
K8s->>K8s: PVCs deleted
Job-->>Helm: Job completes
Helm->>K8s: Delete hook Job<br/>(based on hook-delete-policy)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🪛 YAMLlint (1.37.1)packages/apps/mysql/templates/hooks/cleanup-pvc.yaml[error] 5-5: syntax error: expected , but found '' (syntax) 🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @IvanHunters, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Kubernetes PersistentVolumeClaim (PVC) template designed to streamline storage provisioning for MariaDB applications. By integrating this template, MariaDB deployments can now automatically request and manage their persistent storage without requiring an external operator, simplifying the deployment process and ensuring consistent storage allocation within the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds a template for a PersistentVolumeClaim. However, there are a couple of significant issues with the implementation. First, the namespace is hardcoded, which limits the chart's reusability. Second, it creates a single PVC for what is configured as a multi-replica application by default, which will not work with ReadWriteOnce access mode. This seems to conflict with the MariaDB operator's own storage management, which would typically handle PVC creation for each replica via a StatefulSet.
| apiVersion: v1 | ||
| kind: PersistentVolumeClaim | ||
| metadata: | ||
| name: storage-{{ .Release.Name }} |
There was a problem hiding this comment.
The namespace is hardcoded to tenant-whmcs. This prevents the chart from being deployed in any namespace other than tenant-whmcs. To make the chart reusable, the namespace should be templated to use the release's namespace.
namespace: {{ .Release.Namespace }}| --- | ||
| apiVersion: v1 | ||
| kind: PersistentVolumeClaim | ||
| metadata: | ||
| name: storage-{{ .Release.Name }} | ||
| namespace: tenant-whmcs | ||
| labels: | ||
| app.kubernetes.io/instance: {{ .Release.Name }} | ||
| app.kubernetes.io/name: mariadb | ||
| pvc.k8s.mariadb.com/role: storage | ||
| spec: | ||
| accessModes: | ||
| - ReadWriteOnce | ||
| resources: | ||
| requests: | ||
| storage: {{ .Values.size }} | ||
| storageClassName: {{ .Values.storageClass }} |
There was a problem hiding this comment.
This template defines a single PersistentVolumeClaim with accessModes: ReadWriteOnce. The chart defaults to replicas: 2, but a ReadWriteOnce volume cannot be shared between pods. This will cause failures in a multi-replica setup.
The MariaDB operator already manages PVC creation for its replicas. This standalone PVC conflicts with the operator's functionality and is not suitable for the default multi-replica configuration.
8745ce0 to
a99ace7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml (2)
4-10: Add explicit namespace to RBAC resources for consistency.The ServiceAccount and Role omit namespace from their metadata, while the Job explicitly specifies it (line 48). For clarity and consistency, add
namespace: {{ .Release.Namespace }}to the SA and Role metadata.This ensures all resources are unambiguously scoped to the release namespace, improving maintainability and reducing confusion.
Apply this diff to the ServiceAccount metadata:
metadata: name: {{ .Release.Name }}-cleanup + namespace: {{ .Release.Namespace }} labels:Apply this diff to the Role metadata:
metadata: name: {{ .Release.Name }}-cleanup + namespace: {{ .Release.Namespace }} labels:Also applies to: 14-20, 28-34, 48-48
65-65: Consider improved error logging despite error suppression.The
|| trueon line 71 silences all errors to prevent hook failure, which is appropriate to avoid cascading deletion failures. However, consider logging errors before suppressing them for observability:- kubectl delete pvc -n {{ .Release.Namespace }} -l app.kubernetes.io/instance={{ .Release.Name }} || true + kubectl delete pvc -n {{ .Release.Namespace }} -l app.kubernetes.io/instance={{ .Release.Name }} || echo "Warning: PVC deletion encountered errors (non-fatal)"This preserves error suppression while improving debuggability for hook troubleshooting.
Also applies to: 71-71
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml
[error] 5-5: syntax error: expected , but found ''
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml (1)
35-42: RoleBinding correctly references both Role and ServiceAccount.The RoleBinding properly links the ServiceAccount to the Role with explicit namespace specification for the subject. The roleRef correctly references the Role by name.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml (1)
54-62: AddactiveDeadlineSecondsto Job spec for timeout protection.The Job lacks a timeout mechanism. If the cleanup container hangs or encounters issues, the Job could remain active indefinitely, potentially blocking the release deletion flow.
Apply this diff to add a timeout:
spec: template: metadata: labels: app.kubernetes.io/instance: {{ .Release.Name }} role: cleanup spec: serviceAccountName: {{ .Release.Name }}-cleanup + activeDeadlineSeconds: 300 restartPolicy: Never containers:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml
[error] 5-5: syntax error: expected , but found ''
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (2)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml (2)
1-72: Clean approach to orphaned PVC cleanup with post-delete hook.The overall design is sound: using a post-delete hook with appropriate RBAC and a Job to clean up PVCs labeled with the release instance. The use of
|| trueto suppress errors prevents hook failures, and thebefore-hook-creation,hook-succeededdelete policy ensures cleanup resources are removed after their task completes.Once the namespace metadata is added and the third-party image choice is verified, this should be ready. Please also confirm in the PR description or commit message that this cleanup strategy is intentional and what problem it solves (e.g., PVCs orphaned by the MariaDB operator).
65-65: I'll verify the review comment by searching for information about the clastix/kubectl image and available alternatives.Update the image to registry.k8s.io-hosted official Kubernetes client container image or verify clastix/kubectl's production readiness.
The clastix/kubectl repository shows recent maintenance activity as of June 2025, and the Dockerfile downloads kubectl from the official Kubernetes release registry (dl.k8s.io) and uses the distroless base image, which demonstrates security-conscious practices. However, official Kubernetes container images are deployed to registry.k8s.io—consider verifying whether a containerized kubectl is available there for direct use, or evaluate whether this third-party wrapper aligns with your security and maintenance policies. If clastix/kubectl continues to be the chosen option, document the maintenance commitment and vulnerability scanning processes in use.
| apiVersion: v1 | ||
| kind: ServiceAccount | ||
| metadata: | ||
| name: {{ .Release.Name }}-cleanup |
There was a problem hiding this comment.
Add explicit namespace field to ServiceAccount, Role, and RoleBinding metadata.
While Helm defaults to the release namespace when not specified, the Job on line 48 explicitly declares namespace: {{ .Release.Namespace }}. For consistency and clarity, all namespace-scoped resources should explicitly declare their namespace in metadata. This prevents potential deployment surprises and makes the template's intent unmistakable.
Apply this diff to add namespace declarations:
apiVersion: v1
kind: ServiceAccount
metadata:
name: {{ .Release.Name }}-cleanup
+ namespace: {{ .Release.Namespace }}
labels:
app.kubernetes.io/instance: {{ .Release.Name }}apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: {{ .Release.Name }}-cleanup
+ namespace: {{ .Release.Namespace }}
labels:
app.kubernetes.io/instance: {{ .Release.Name }}apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: {{ .Release.Name }}-cleanup
+ namespace: {{ .Release.Namespace }}
labels:
app.kubernetes.io/instance: {{ .Release.Name }}Also applies to: 15-15, 29-29
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: syntax error: expected , but found ''
(syntax)
🤖 Prompt for AI Agents
In packages/apps/mysql/templates/hooks/cleanup-pvc.yaml around lines 5, 15, and
29, the ServiceAccount, Role, and RoleBinding metadata blocks lack an explicit
namespace field; add namespace: {{ .Release.Namespace }} under each resource's
metadata to match the Job's explicit namespace and ensure all namespace-scoped
resources declare the same release namespace consistently.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml (1)
45-51: Add explicitnamespacefield to Job metadata.For consistency with best practices and to match the pattern for other namespace-scoped resources, add an explicit
namespacefield to the Job's metadata section.metadata: name: {{ .Release.Name }}-cleanup + namespace: {{ .Release.Namespace }} labels: app.kubernetes.io/instance: {{ .Release.Name }} annotations: "helm.sh/hook": post-delete "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
🧹 Nitpick comments (1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml (1)
62-69: Consider adding image pull policy and resource limits.The Job uses a fixed image version (v1.32), which is good for reproducibility. However, consider adding
imagePullPolicy: IfNotPresentfor predictability and resource requests/limits to prevent resource hogging during cleanup.- name: cleanup image: docker.io/clastix/kubectl:v1.32 + imagePullPolicy: IfNotPresent + resources: + requests: + cpu: 100m + memory: 128Mi + limits: + cpu: 500m + memory: 256Mi command: - /bin/sh - -c - | echo "Deleting orphaned PVCs for {{ .Release.Name }}..." kubectl delete pvc -n {{ .Release.Namespace }} -l app.kubernetes.io/instance={{ .Release.Name }} || true echo "PVC cleanup complete."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml
[error] 5-5: syntax error: expected , but found ''
(syntax)
| metadata: | ||
| name: {{ .Release.Name }}-cleanup | ||
| labels: | ||
| app.kubernetes.io/instance: {{ .Release.Name }} | ||
| annotations: | ||
| "helm.sh/hook": post-delete | ||
| "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded |
There was a problem hiding this comment.
Add explicit namespace field to ServiceAccount, Role, and RoleBinding metadata (unresolved prior feedback).
The previous review flagged this concern, but it remains unaddressed. While Helm defaults to the release namespace, all namespace-scoped resources should explicitly declare their namespace in metadata for clarity and consistency. This prevents deployment surprises and makes the template intent unmistakable.
Apply this diff to add namespace declarations:
apiVersion: v1
kind: ServiceAccount
metadata:
name: {{ .Release.Name }}-cleanup
+ namespace: {{ .Release.Namespace }}
labels:
app.kubernetes.io/instance: {{ .Release.Name }}
annotations:
"helm.sh/hook": post-delete
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: {{ .Release.Name }}-cleanup
+ namespace: {{ .Release.Namespace }}
labels:
app.kubernetes.io/instance: {{ .Release.Name }}
annotations:
"helm.sh/hook": post-delete
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: {{ .Release.Name }}-cleanup
+ namespace: {{ .Release.Namespace }}
labels:
app.kubernetes.io/instance: {{ .Release.Name }}
annotations:
"helm.sh/hook": post-delete
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeededAlso applies to: 14-20, 28-34
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: syntax error: expected , but found ''
(syntax)
🤖 Prompt for AI Agents
In packages/apps/mysql/templates/hooks/cleanup-pvc.yaml around lines 4-10 (and
also apply to lines 14-20 and 28-34), add an explicit namespace field under the
metadata block for the ServiceAccount, Role, and RoleBinding by setting
metadata.namespace: {{ .Release.Namespace }} so each namespace-scoped resource
declares the release namespace explicitly; update the three resource metadata
sections accordingly to include the namespace template value.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml (2)
73-74: Consider adding explicitnamespaceto RoleBinding subjects for consistency.While the ServiceAccount reference in subjects (line 73–74) defaults to the same namespace as the RoleBinding when not specified, explicitly declaring the namespace improves clarity, especially in audit trails and debugging. Add
namespace: {{ .Release.Namespace }}to the subject entry:subjects: - kind: ServiceAccount name: {{ .Release.Name }}-cleanup + namespace: {{ .Release.Namespace }}
4-11: Add explicitnamespaceto Job metadata for consistency with other resources.For consistency with the other namespace-scoped resources (once namespace fields are added to ServiceAccount, Role, and RoleBinding per earlier comments), the Job's metadata should also explicitly declare the namespace:
apiVersion: batch/v1 kind: Job metadata: name: {{ .Release.Name }}-cleanup + namespace: {{ .Release.Namespace }} labels: app.kubernetes.io/instance: {{ .Release.Name }} annotations: "helm.sh/hook": post-delete "helm.sh/hook-weight": "10" "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml
[error] 5-5: syntax error: expected , but found ''
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml (1)
35-35: Ignore this review comment—it contradicts established Helm best practices.The current code correctly omits
namespacefields from ServiceAccount, Role, and RoleBinding metadata. Kubernetes and Helm best practices explicitly discourage embedding namespace in resource metadata because hardcoding namespaces breaks chart flexibility, overrides, and downstream tooling like post-processors, kustomize, and GitOps pipelines. The recommended pattern is to omit the namespace field entirely and let Helm install into the namespace supplied by the user.Adding the proposed diffs would introduce an anti-pattern and reduce chart portability.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml (3)
34-41: Add explicitnamespacefield to ServiceAccount metadata (unresolved prior feedback).This issue was flagged in previous reviews but remains unaddressed. The ServiceAccount should explicitly declare
namespace: {{ .Release.Namespace }}under metadata for consistency with the Job and to prevent deployment surprises.Apply this diff:
metadata: name: {{ .Release.Name }}-cleanup + namespace: {{ .Release.Namespace }} labels: app.kubernetes.io/instance: {{ .Release.Name }} annotations: - "helm.sh/hook": pre-delete + "helm.sh/hook": post-delete helm.sh/hook-weight: "0"Note: Also change
pre-deletetopost-deleteas flagged in the Job review.
45-52: Add explicitnamespacefield to Role metadata (unresolved prior feedback).This issue was flagged in previous reviews. The Role should explicitly declare
namespace: {{ .Release.Namespace }}under metadata.Apply this diff:
metadata: name: {{ .Release.Name }}-cleanup + namespace: {{ .Release.Namespace }} labels: app.kubernetes.io/instance: {{ .Release.Name }} annotations: - "helm.sh/hook": pre-delete + "helm.sh/hook": post-delete "helm.sh/hook-weight": "5"Note: Also change
pre-deletetopost-deleteas flagged in the Job review.
60-67: Add explicitnamespacefield to RoleBinding metadata (unresolved prior feedback).This issue was flagged in previous reviews. The RoleBinding should explicitly declare
namespace: {{ .Release.Namespace }}under metadata.Apply this diff:
metadata: name: {{ .Release.Name }}-cleanup + namespace: {{ .Release.Namespace }} labels: app.kubernetes.io/instance: {{ .Release.Name }} annotations: - "helm.sh/hook": pre-delete + "helm.sh/hook": post-delete helm.sh/hook-weight: "5"Note: Also change
pre-deletetopost-deleteas flagged in the Job review.
🧹 Nitpick comments (2)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml (2)
29-29: Line exceeds 80-character limit.Line 29 is 118 characters, exceeding the 80-character limit flagged by YAMLlint in previous reviews.
Apply this diff to split the line:
- kubectl delete pvc -n {{ .Release.Namespace }} -l app.kubernetes.io/instance={{ .Release.Name }} || true + kubectl delete pvc -n {{ .Release.Namespace }} \ + -l app.kubernetes.io/instance={{ .Release.Name }} || true
72-74: Add explicitnamespaceto ServiceAccount subject.For cross-namespace clarity and completeness, the ServiceAccount subject should explicitly declare its namespace, even though it's in the same namespace as the RoleBinding.
Apply this diff:
subjects: - kind: ServiceAccount name: {{ .Release.Name }}-cleanup + namespace: {{ .Release.Namespace }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hack/e2e-apps/mysql.bats(1 hunks)packages/apps/mysql/templates/hooks/cleanup-pvc.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml
[error] 5-5: syntax error: expected , but found ''
(syntax)
🔇 Additional comments (1)
packages/apps/mysql/templates/hooks/cleanup-pvc.yaml (1)
5-5: YAMLlint syntax error is a false positive.The YAMLlint error at line 5 occurs because the linter doesn't understand Helm template syntax (
{{ .Release.Name }}). This is expected for Helm templates and should be ignored. The template expressions are correctly formatted with single spaces.
hack/e2e-apps/mysql.bats
Outdated
| sleep 10 | ||
| kubectl -n tenant-test get pvc |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add assertion to verify PVC cleanup instead of just listing.
The test adds a 10-second sleep and lists PVCs, but doesn't validate that cleanup actually occurred. With the pre-delete hook, the timing window is critical and 10 seconds may be insufficient for the Job to complete. Consider asserting that expected PVCs are absent or verifying the cleanup Job succeeded.
Apply this diff to verify PVC cleanup:
kubectl -n tenant-test delete mysqls.apps.cozystack.io $name
- sleep 10
- kubectl -n tenant-test get pvc
+ # Wait for cleanup hook to complete
+ timeout 60 sh -ec "until ! kubectl -n tenant-test get pvc -l app.kubernetes.io/instance=mysql-$name 2>/dev/null | grep -q 'mysql-$name'; do sleep 5; done"
+ # Verify PVCs are gone
+ kubectl -n tenant-test get pvc -l app.kubernetes.io/instance=mysql-$name --no-headers | wc -l | grep -q '^0$'Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In hack/e2e-apps/mysql.bats around lines 46-47, the test currently sleeps 10s
then runs `kubectl -n tenant-test get pvc` without asserting cleanup; change
this to poll for the expected PVCs to be removed (or for the cleanup Job to
complete) with a reasonable timeout instead of a fixed 10s sleep: implement a
loop that checks `kubectl -n tenant-test get pvc <pvc-name>` (or `kubectl -n
tenant-test get job <cleanup-job> -o jsonpath=...` to verify succeeded) until
the resource is absent/Job status is Complete, failing the test if the timeout
is reached, and log the last observed state for debugging.
| metadata: | ||
| name: {{ .Release.Name }}-cleanup | ||
| labels: | ||
| app.kubernetes.io/instance: {{ .Release.Name }} | ||
| annotations: | ||
| "helm.sh/hook": pre-delete | ||
| "helm.sh/hook-weight": "10" | ||
| "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded |
There was a problem hiding this comment.
Add explicit namespace field to Job metadata (unresolved prior feedback).
Previous reviews flagged the missing namespace declarations on ServiceAccount, Role, and RoleBinding. The same issue applies to the Job. While Helm defaults to the release namespace, explicit declaration prevents deployment surprises and improves clarity.
Apply this diff:
metadata:
name: {{ .Release.Name }}-cleanup
+ namespace: {{ .Release.Namespace }}
labels:
app.kubernetes.io/instance: {{ .Release.Name }}🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: syntax error: expected , but found ''
(syntax)
🤖 Prompt for AI Agents
In packages/apps/mysql/templates/hooks/cleanup-pvc.yaml around lines 4 to 11,
the Job metadata is missing an explicit namespace field; add a namespace: {{
.Release.Namespace }} entry under metadata (matching the
ServiceAccount/Role/RoleBinding) so the Job is explicitly bound to the release
namespace and avoids Helm/cluster surprises.
This patch adds a Helm post-delete hook job that removes PersistentVolumeClaims left behind after Helm release deletion. The MariaDB Operator currently does not handle PVC cleanup, so this job ensures proper resource removal. ```release-note [mariadb-operator] Add a post-delete hook job to clean up PVCs left after Helm release deletion. ``` Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
992eb8b to
0f286ee
Compare
<!-- Thank you for making a contribution! Here are some tips for you: - Start the PR title with the [label] of Cozystack component: - For system components: [platform], [system], [linstor], [cilium], [kube-ovn], [dashboard], [cluster-api], etc. - For managed apps: [apps], [tenant], [kubernetes], [postgres], [virtual-machine] etc. - For development and maintenance: [tests], [ci], [docs], [maintenance]. - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does [mariadb-operator] Add post-delete job to remove PVCs This patch adds a Helm post-delete hook job that removes PersistentVolumeClaims left behind after Helm release deletion. The MariaDB Operator currently does not handle PVC cleanup, so this job ensures proper resource removal. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same [label] as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note [mariadb-operator] Add a post-delete hook job to clean up PVCs left after Helm release deletion. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Persistent storage volumes are now automatically cleaned up when the MySQL application is deleted, preventing orphaned storage resources from accumulating in your cluster. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
What this PR does
[mariadb-operator] Add post-delete job to remove PVCs
This patch adds a Helm post-delete hook job that removes PersistentVolumeClaims
left behind after Helm release deletion. The MariaDB Operator currently does not
handle PVC cleanup, so this job ensures proper resource removal.
Release note
Summary by CodeRabbit