diff --git a/.github/workflows/test-E2E.yml b/.github/workflows/test-E2E.yml index 321e1c46..0540590e 100644 --- a/.github/workflows/test-E2E.yml +++ b/.github/workflows/test-E2E.yml @@ -280,6 +280,143 @@ jobs: # Check events kubectl get events -n $DB_NS --sort-by='.lastTimestamp' + - name: Verify mount options are set by PV controller + run: | + echo "Verifying PV mount options are set by the PV controller..." + + # Get the PVC and PV names from the existing cluster + pvc_name=$(kubectl -n ${{ env.DB_NS }} get pvc -l cnpg.io/cluster=${{ env.DB_NAME }} -o jsonpath='{.items[0].metadata.name}') + pv_name=$(kubectl -n ${{ env.DB_NS }} get pvc $pvc_name -o jsonpath='{.spec.volumeName}') + + echo "PVC name: $pvc_name" + echo "PV name: $pv_name" + + if [ -z "$pv_name" ]; then + echo "❌ Failed to find PV bound to PVC $pvc_name" + exit 1 + fi + + # Get mount options from PV + mount_options=$(kubectl get pv $pv_name -o jsonpath='{.spec.mountOptions}') + echo "PV mount options: $mount_options" + + # Check for security mount options (nodev, nosuid, noexec) + if echo "$mount_options" | grep -q "nodev" && \ + echo "$mount_options" | grep -q "nosuid" && \ + echo "$mount_options" | grep -q "noexec"; then + echo "✓ PV mount options (nodev, nosuid, noexec) are set correctly" + else + echo "❌ PV mount options are missing. Expected nodev, nosuid, noexec" + exit 1 + fi + + - name: Test PV reclaim policy default and explicit Delete + shell: bash + run: | + echo "Testing PV reclaim policy - default (Retain) and explicit Delete..." + + # Test 1: Verify default policy is Retain on the existing cluster + echo "=== Test 1: Verify default PV reclaim policy is Retain ===" + + # Get the PVC and PV names from the existing cluster + pvc_name=$(kubectl -n ${{ env.DB_NS }} get pvc -l cnpg.io/cluster=${{ env.DB_NAME }} -o jsonpath='{.items[0].metadata.name}') + pv_name=$(kubectl -n ${{ env.DB_NS }} get pvc $pvc_name -o jsonpath='{.spec.volumeName}') + + echo "PVC name: $pvc_name" + echo "PV name: $pv_name" + + if [ -z "$pv_name" ]; then + echo "❌ Failed to find PV bound to PVC $pvc_name" + exit 1 + fi + + # Verify default PV reclaim policy is Retain + current_policy=$(kubectl get pv $pv_name -o jsonpath='{.spec.persistentVolumeReclaimPolicy}') + echo "Current PV reclaim policy: $current_policy" + + if [ "$current_policy" != "Retain" ]; then + echo "❌ Expected default PV reclaim policy to be 'Retain', but got '$current_policy'" + exit 1 + fi + echo "✓ Default PV reclaim policy is correctly set to Retain" + + # Test 2: Change policy to Delete and verify PV is deleted with cluster + echo "" + echo "=== Test 2: Change policy to Delete and verify PV cleanup ===" + + # Patch the existing DocumentDB to set persistentVolumeReclaimPolicy to Delete + echo "Patching DocumentDB to set persistentVolumeReclaimPolicy to Delete..." + kubectl -n ${{ env.DB_NS }} patch documentdb ${{ env.DB_NAME }} --type=merge \ + -p '{"spec":{"resource":{"storage":{"persistentVolumeReclaimPolicy":"Delete"}}}}' + + # Wait for PV controller to update the PV reclaim policy + echo "Waiting for PV reclaim policy to be updated to Delete..." + MAX_RETRIES=30 + SLEEP_INTERVAL=5 + ITER=0 + while [ $ITER -lt $MAX_RETRIES ]; do + new_policy=$(kubectl get pv $pv_name -o jsonpath='{.spec.persistentVolumeReclaimPolicy}') + if [ "$new_policy" == "Delete" ]; then + echo "✓ PV reclaim policy updated to Delete" + break + else + echo "PV reclaim policy is still '$new_policy'. Waiting..." + sleep $SLEEP_INTERVAL + fi + ((++ITER)) + done + + if [ "$new_policy" != "Delete" ]; then + echo "❌ PV reclaim policy was not updated to Delete within expected time" + exit 1 + fi + + # Delete the DocumentDB cluster + echo "Deleting DocumentDB cluster to test PV cleanup with Delete policy..." + kubectl -n ${{ env.DB_NS }} delete documentdb ${{ env.DB_NAME }} --wait=false + + # Wait for DocumentDB to be deleted + echo "Waiting for DocumentDB to be deleted..." + MAX_RETRIES=30 + SLEEP_INTERVAL=10 + ITER=0 + while [ $ITER -lt $MAX_RETRIES ]; do + db_exists=$(kubectl -n ${{ env.DB_NS }} get documentdb ${{ env.DB_NAME }} --ignore-not-found) + if [ -z "$db_exists" ]; then + echo "✓ DocumentDB deleted successfully." + break + else + echo "DocumentDB still exists. Waiting..." + sleep $SLEEP_INTERVAL + fi + ((++ITER)) + done + + # Verify no PVsRetained warning event was emitted (since policy is Delete) + events=$(kubectl -n ${{ env.DB_NS }} get events --field-selector reason=PVsRetained,involvedObject.name=${{ env.DB_NAME }} --ignore-not-found -o jsonpath='{.items}') + if [ -z "$events" ] || [ "$events" == "[]" ]; then + echo "✓ No PVsRetained warning event emitted (expected for Delete policy)" + else + echo "⚠️ Unexpected PVsRetained event found for Delete policy cluster" + fi + + # Wait a bit for PV to be deleted (the storage class handles actual deletion) + echo "Waiting for PV to be deleted..." + sleep 30 + + # Verify PV was deleted (because reclaim policy is Delete) + pv_exists=$(kubectl get pv $pv_name --ignore-not-found 2>/dev/null) + if [ -z "$pv_exists" ]; then + echo "✓ PV $pv_name was deleted as expected (Delete policy)" + else + pv_status=$(kubectl get pv $pv_name -o jsonpath='{.status.phase}') + echo "⚠️ PV $pv_name still exists with status: $pv_status" + echo "Note: PV deletion depends on the storage provisioner. The reclaim policy was correctly set to Delete." + fi + + echo "" + echo "✓ PV reclaim policy test completed successfully" + - name: Collect comprehensive logs on failure if: failure() uses: ./.github/actions/collect-logs diff --git a/.github/workflows/test-backup-and-restore.yml b/.github/workflows/test-backup-and-restore.yml index 4949b4df..60058a14 100644 --- a/.github/workflows/test-backup-and-restore.yml +++ b/.github/workflows/test-backup-and-restore.yml @@ -354,4 +354,206 @@ jobs: ((++ITER)) done echo "❌ Expired backup was not cleaned up within expected time." - exit 1 \ No newline at end of file + exit 1 + + - name: Test PV retention after DocumentDB deletion + id: test-pv-retention + shell: bash + run: | + echo "Testing PV retention after DocumentDB deletion..." + + # Get the PVC name and PV name before deleting the DocumentDB + # PVCs are created by CNPG and labeled with cnpg.io/cluster + pvc_name=$(kubectl -n ${{ env.DB_NS }} get pvc -l cnpg.io/cluster=${{ env.DB_RESTORE_NAME }} -o jsonpath='{.items[0].metadata.name}') + echo "PVC name: $pvc_name" + + if [ -z "$pvc_name" ]; then + echo "❌ Failed to find PVC for cluster ${{ env.DB_RESTORE_NAME }}" + exit 1 + fi + + # Get the PV name bound to this PVC + pv_name=$(kubectl -n ${{ env.DB_NS }} get pvc $pvc_name -o jsonpath='{.spec.volumeName}') + echo "PV name: $pv_name" + + if [ -z "$pv_name" ]; then + echo "❌ Failed to find PV bound to PVC $pvc_name" + exit 1 + fi + + # Check current PV reclaim policy - should be Retain by default + current_policy=$(kubectl get pv $pv_name -o jsonpath='{.spec.persistentVolumeReclaimPolicy}') + echo "Current PV reclaim policy: $current_policy" + + if [ "$current_policy" != "Retain" ]; then + echo "❌ Expected PV reclaim policy to be 'Retain' (default), but got '$current_policy'" + exit 1 + fi + echo "✓ PV reclaim policy is correctly set to Retain (default)" + + # Delete the restored DocumentDB cluster + kubectl -n ${{ env.DB_NS }} delete documentdb ${{ env.DB_RESTORE_NAME }} --wait=false + + # Wait for DocumentDB to be deleted + echo "Waiting for DocumentDB to be deleted..." + MAX_RETRIES=30 + SLEEP_INTERVAL=10 + ITER=0 + while [ $ITER -lt $MAX_RETRIES ]; do + db_exists=$(kubectl -n ${{ env.DB_NS }} get documentdb ${{ env.DB_RESTORE_NAME }} --ignore-not-found) + if [ -z "$db_exists" ]; then + echo "✓ DocumentDB deleted successfully." + break + else + echo "DocumentDB still exists. Waiting..." + sleep $SLEEP_INTERVAL + fi + ((++ITER)) + done + + # Verify PV still exists (because reclaim policy is Retain) + pv_exists=$(kubectl get pv $pv_name --ignore-not-found) + if [ -n "$pv_exists" ]; then + echo "✓ PV $pv_name retained after DocumentDB deletion" + else + echo "❌ PV $pv_name was deleted unexpectedly" + exit 1 + fi + + # Store PV name for later steps using GitHub Actions output (more robust than temp files) + echo "pv_name=$pv_name" >> $GITHUB_OUTPUT + + - name: Restore DocumentDB from retained PV + shell: bash + run: | + pv_name="${{ steps.test-pv-retention.outputs.pv_name }}" + echo "Restoring DocumentDB from retained PV: $pv_name" + + # Check the PV status - it should be in "Released" state after PVC deletion + pv_status=$(kubectl get pv $pv_name -o jsonpath='{.status.phase}') + echo "PV status: $pv_status" + + # Clear the claimRef from the PV so a new PVC can bind to it + # When a PV is in "Released" state, it still has a claimRef to the old deleted PVC + echo "Clearing claimRef from PV $pv_name to allow new PVC binding..." + kubectl patch pv $pv_name --type=json -p='[{"op": "remove", "path": "/spec/claimRef"}]' + + # Verify PV is now Available + pv_status=$(kubectl get pv $pv_name -o jsonpath='{.status.phase}') + echo "PV status after clearing claimRef: $pv_status" + + # Create a new PVC that binds to the retained PV + new_pvc_name="recovered-pvc-from-pv" + echo "Creating new PVC $new_pvc_name to bind to retained PV $pv_name" + + # Get the storage capacity from the PV + pv_capacity=$(kubectl get pv $pv_name -o jsonpath='{.spec.capacity.storage}') + echo "PV capacity: $pv_capacity" + + cat </dev/null || true + rm -f /tmp/pf_pid + fi + + # Clean up output log + rm -f /tmp/pf_output.log + + - name: Collect logs on failure + if: failure() + uses: ./.github/actions/collect-logs + with: + architecture: ${{ matrix.architecture }} + operator-namespace: ${{ env.OPERATOR_NS }} + db-namespace: ${{ env.DB_NS }} + db-cluster-name: ${{ env.DB_NAME }} + cert-manager-namespace: ${{ env.CERT_MANAGER_NS }} diff --git a/docs/operator-public-documentation/preview/advanced-configuration/README.md b/docs/operator-public-documentation/preview/advanced-configuration/README.md index cda1cacd..4aa1bc67 100644 --- a/docs/operator-public-documentation/preview/advanced-configuration/README.md +++ b/docs/operator-public-documentation/preview/advanced-configuration/README.md @@ -257,6 +257,111 @@ kubectl patch documentdb -n --type='json' \ -p='[{"op": "replace", "path": "/spec/storage/size", "value":"200Gi"}]' ``` +### PersistentVolume Security + +The DocumentDB operator automatically applies security-hardening mount options to all PersistentVolumes associated with DocumentDB clusters: + +| Mount Option | Description | +|--------------|-------------| +| `nodev` | Prevents device files from being interpreted on the filesystem | +| `nosuid` | Prevents setuid/setgid bits from taking effect | +| `noexec` | Prevents execution of binaries on the filesystem | + +These options are automatically applied by the PV controller and require no additional configuration. + +### Disk Encryption + +Encryption at rest is essential for protecting sensitive database data. Here's how to configure disk encryption for each cloud provider: + +#### Azure Kubernetes Service (AKS) + +AKS encrypts all managed disks by default using Azure Storage Service Encryption (SSE) with platform-managed keys. No additional configuration is required. + +For customer-managed keys (CMK), use Azure Disk Encryption: + +```yaml +apiVersion: storage.k8s.io/v1 +kind: StorageClass +metadata: + name: managed-csi-encrypted +provisioner: disk.csi.azure.com +parameters: + skuName: Premium_LRS + # For customer-managed keys, specify the disk encryption set + diskEncryptionSetID: /subscriptions//resourceGroups//providers/Microsoft.Compute/diskEncryptionSets/ +reclaimPolicy: Delete +volumeBindingMode: WaitForFirstConsumer +allowVolumeExpansion: true +``` + +#### Google Kubernetes Engine (GKE) + +GKE encrypts all persistent disks by default using Google-managed encryption keys. No additional configuration is required. + +For customer-managed encryption keys (CMEK): + +```yaml +apiVersion: storage.k8s.io/v1 +kind: StorageClass +metadata: + name: pd-ssd-encrypted +provisioner: pd.csi.storage.gke.io +parameters: + type: pd-ssd + # For CMEK, specify the key + disk-encryption-kms-key: projects//locations//keyRings//cryptoKeys/ +reclaimPolicy: Delete +volumeBindingMode: WaitForFirstConsumer +allowVolumeExpansion: true +``` + +#### Amazon Elastic Kubernetes Service (EKS) + +**Important**: Unlike AKS and GKE, EBS volumes on EKS are **not encrypted by default**. You must explicitly enable encryption in the StorageClass: + +```yaml +apiVersion: storage.k8s.io/v1 +kind: StorageClass +metadata: + name: ebs-sc-encrypted +provisioner: ebs.csi.aws.com +parameters: + type: gp3 + encrypted: "true" # Required for encryption + # Optional: specify a KMS key for customer-managed encryption + # kmsKeyId: arn:aws:kms:::key/ +reclaimPolicy: Delete +volumeBindingMode: WaitForFirstConsumer +allowVolumeExpansion: true +``` + +To use the encrypted storage class with DocumentDB: + +```yaml +apiVersion: documentdb.io/preview +kind: DocumentDB +metadata: + name: my-cluster + namespace: default +spec: + environment: eks + resource: + storage: + pvcSize: 100Gi + storageClass: ebs-sc-encrypted # Use the encrypted storage class + # ... other configuration +``` + +### Encryption Summary + +| Provider | Default Encryption | Customer-Managed Keys | +|----------|-------------------|----------------------| +| AKS | ✅ Enabled (SSE) | Optional via DiskEncryptionSet | +| GKE | ✅ Enabled (Google-managed) | Optional via CMEK | +| EKS | ❌ **Not enabled** | Required: set `encrypted: "true"` in StorageClass | + +**Recommendation**: For production deployments on EKS, always create a StorageClass with `encrypted: "true"` to ensure data at rest is protected. + --- ## Resource Management diff --git a/docs/operator-public-documentation/preview/backup-and-restore.md b/docs/operator-public-documentation/preview/backup-and-restore.md index 43de4743..2e006fe4 100644 --- a/docs/operator-public-documentation/preview/backup-and-restore.md +++ b/docs/operator-public-documentation/preview/backup-and-restore.md @@ -288,4 +288,113 @@ spec: - Changing `DocumentDB.spec.backup.retentionDays` doesn’t retroactively update existing backups. - Failed backups still expire (timer starts at creation). - Deleting the cluster does NOT delete its Backup objects immediately—they still wait for expiration. -- No "keep forever" mode—export externally if you need permanent archival. \ No newline at end of file +- No "keep forever" mode—export externally if you need permanent archival. + +## PersistentVolume Retention and Recovery + +The DocumentDB operator supports retaining PersistentVolumes (PVs) after cluster deletion, allowing you to recover data by creating a new cluster from the retained PV. + +### Configuring PV Retention + +By default, PVs are deleted when the DocumentDB cluster is deleted. To retain the PV, set the `persistentVolumeReclaimPolicy` to `Retain`: + +```yaml +apiVersion: documentdb.io/preview +kind: DocumentDB +metadata: + name: my-cluster + namespace: default +spec: + resource: + storage: + pvcSize: 10Gi + persistentVolumeReclaimPolicy: Retain # Keep PV after cluster deletion + # ... other configuration +``` + +### How PV Retention Works + +1. When you set `persistentVolumeReclaimPolicy: Retain`, the operator updates the underlying PersistentVolume's reclaim policy +2. When the DocumentDB cluster is deleted, the PVC is deleted but the PV is retained in a "Released" state +3. The retained PV contains all the database data and can be used to recover the cluster + +### Recovering from a Retained PV + +To restore a DocumentDB cluster from a retained PV: + +**Step 1: Identify the retained PV** + +```bash +# List PVs in Released state +kubectl get pv | grep Released +``` + +**Step 2: Clear the claimRef to make the PV available** + +The PV in "Released" state still has a reference to the old deleted PVC. Clear it: + +```bash +kubectl patch pv --type=json -p='[{"op": "remove", "path": "/spec/claimRef"}]' +``` + +Verify the PV is now "Available": + +```bash +kubectl get pv +``` + +**Step 3: Create a new PVC bound to the retained PV** + +```yaml +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: recovered-pvc + namespace: default +spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 10Gi # Must match the PV capacity + storageClassName: managed-csi # Must match the PV's storage class + volumeName: # Specify the retained PV name +``` + +Apply and verify the PVC is bound: + +```bash +kubectl apply -f recovered-pvc.yaml +kubectl get pvc recovered-pvc +``` + +**Step 4: Create a new DocumentDB cluster with PVC recovery** + +```yaml +apiVersion: documentdb.io/preview +kind: DocumentDB +metadata: + name: my-recovered-cluster + namespace: default +spec: + nodeCount: 1 + instancesPerNode: 1 + resource: + storage: + pvcSize: 10Gi + storageClass: managed-csi + exposeViaService: + serviceType: ClusterIP + bootstrap: + recovery: + pvc: + name: recovered-pvc # Reference the PVC bound to the retained PV +``` + +### Important Notes for PV Recovery + +- The new cluster must be in the same namespace as the PVC +- Storage size and class should match the original configuration +- You cannot specify both `backup` and `pvc` recovery at the same time +- PVC recovery preserves all data including users, roles, and collections +- After successful recovery, consider setting up regular backups for the new cluster diff --git a/operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml b/operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml index b5e41e80..2f575552 100644 --- a/operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml +++ b/operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml @@ -79,7 +79,23 @@ spec: required: - name type: object + pvc: + description: |- + PVC specifies the source PVC to restore from. + Cannot be used together with Backup. + properties: + name: + description: Name of the referent. + type: string + required: + - name + type: object type: object + x-kubernetes-validations: + - message: cannot specify both backup and pvc recovery at the + same time + rule: '!(has(self.backup) && self.backup.name != '''' && has(self.pvc) + && self.pvc.name != '''')' type: object clusterReplication: description: ClusterReplication configures cross-cluster replication @@ -198,6 +214,28 @@ spec: storage: description: Storage configuration for DocumentDB persistent volumes. properties: + persistentVolumeReclaimPolicy: + default: Retain + description: |- + PersistentVolumeReclaimPolicy controls what happens to the PersistentVolume when + the DocumentDB cluster is deleted. + + When a DocumentDB cluster is deleted, the following chain of deletions occurs: + DocumentDB deletion → CNPG Cluster deletion → PVC deletion → PV deletion (based on this policy) + + Options: + - Retain (default): The PV is preserved after cluster deletion, allowing manual + data recovery or forensic analysis. Use for production workloads where data + safety is critical. Orphaned PVs must be manually deleted when no longer needed. + - Delete: The PV is automatically deleted when the PVC is deleted. Use for development, + testing, or ephemeral environments where data persistence is not required. + + WARNING: Setting this to "Delete" means all data will be permanently lost when + the DocumentDB cluster is deleted. This cannot be undone. + enum: + - Retain + - Delete + type: string pvcSize: description: PvcSize is the size of the persistent volume claim for DocumentDB storage (e.g., "10Gi"). diff --git a/operator/documentdb-helm-chart/templates/05_clusterrole.yaml b/operator/documentdb-helm-chart/templates/05_clusterrole.yaml index bc8393e0..77a345c4 100644 --- a/operator/documentdb-helm-chart/templates/05_clusterrole.yaml +++ b/operator/documentdb-helm-chart/templates/05_clusterrole.yaml @@ -56,3 +56,7 @@ rules: - apiGroups: ["snapshot.storage.k8s.io"] resources: ["volumesnapshotclasses"] verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] +# PersistentVolume permissions for PV controller +- apiGroups: [""] + resources: ["persistentvolumes"] + verbs: ["get", "list", "watch", "update", "patch"] diff --git a/operator/src/api/preview/documentdb_types.go b/operator/src/api/preview/documentdb_types.go index 2ed37a0e..d87001b8 100644 --- a/operator/src/api/preview/documentdb_types.go +++ b/operator/src/api/preview/documentdb_types.go @@ -86,10 +86,16 @@ type BootstrapConfiguration struct { } // RecoveryConfiguration defines backup recovery settings. +// +kubebuilder:validation:XValidation:rule="!(has(self.backup) && self.backup.name != ” && has(self.pvc) && self.pvc.name != ”)",message="cannot specify both backup and pvc recovery at the same time" type RecoveryConfiguration struct { // Backup specifies the source backup to restore from. // +optional Backup cnpgv1.LocalObjectReference `json:"backup,omitempty"` + + // PVC specifies the source PVC to restore from. + // Cannot be used together with Backup. + // +optional + PVC cnpgv1.LocalObjectReference `json:"pvc,omitempty"` } // BackupConfiguration defines backup settings for DocumentDB. @@ -115,6 +121,27 @@ type StorageConfiguration struct { // StorageClass specifies the storage class for DocumentDB persistent volumes. // If not specified, the cluster's default storage class will be used. StorageClass string `json:"storageClass,omitempty"` + + // PersistentVolumeReclaimPolicy controls what happens to the PersistentVolume when + // the DocumentDB cluster is deleted. + // + // When a DocumentDB cluster is deleted, the following chain of deletions occurs: + // DocumentDB deletion → CNPG Cluster deletion → PVC deletion → PV deletion (based on this policy) + // + // Options: + // - Retain (default): The PV is preserved after cluster deletion, allowing manual + // data recovery or forensic analysis. Use for production workloads where data + // safety is critical. Orphaned PVs must be manually deleted when no longer needed. + // - Delete: The PV is automatically deleted when the PVC is deleted. Use for development, + // testing, or ephemeral environments where data persistence is not required. + // + // WARNING: Setting this to "Delete" means all data will be permanently lost when + // the DocumentDB cluster is deleted. This cannot be undone. + // + // +kubebuilder:validation:Enum=Retain;Delete + // +kubebuilder:default=Retain + // +optional + PersistentVolumeReclaimPolicy string `json:"persistentVolumeReclaimPolicy,omitempty"` } type ClusterReplication struct { diff --git a/operator/src/api/preview/zz_generated.deepcopy.go b/operator/src/api/preview/zz_generated.deepcopy.go index 07a84559..3e55f1a3 100644 --- a/operator/src/api/preview/zz_generated.deepcopy.go +++ b/operator/src/api/preview/zz_generated.deepcopy.go @@ -430,6 +430,7 @@ func (in *ProvidedTLS) DeepCopy() *ProvidedTLS { func (in *RecoveryConfiguration) DeepCopyInto(out *RecoveryConfiguration) { *out = *in in.Backup.DeepCopyInto(&out.Backup) + in.PVC.DeepCopyInto(&out.PVC) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RecoveryConfiguration. diff --git a/operator/src/cmd/main.go b/operator/src/cmd/main.go index e0370c12..bf5f2ddf 100644 --- a/operator/src/cmd/main.go +++ b/operator/src/cmd/main.go @@ -239,6 +239,13 @@ func main() { os.Exit(1) } + if err = (&controller.PersistentVolumeReconciler{ + Client: mgr.GetClient(), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "PersistentVolume") + os.Exit(1) + } + // +kubebuilder:scaffold:builder if metricsCertWatcher != nil { diff --git a/operator/src/config/crd/bases/documentdb.io_dbs.yaml b/operator/src/config/crd/bases/documentdb.io_dbs.yaml index b5e41e80..2f575552 100644 --- a/operator/src/config/crd/bases/documentdb.io_dbs.yaml +++ b/operator/src/config/crd/bases/documentdb.io_dbs.yaml @@ -79,7 +79,23 @@ spec: required: - name type: object + pvc: + description: |- + PVC specifies the source PVC to restore from. + Cannot be used together with Backup. + properties: + name: + description: Name of the referent. + type: string + required: + - name + type: object type: object + x-kubernetes-validations: + - message: cannot specify both backup and pvc recovery at the + same time + rule: '!(has(self.backup) && self.backup.name != '''' && has(self.pvc) + && self.pvc.name != '''')' type: object clusterReplication: description: ClusterReplication configures cross-cluster replication @@ -198,6 +214,28 @@ spec: storage: description: Storage configuration for DocumentDB persistent volumes. properties: + persistentVolumeReclaimPolicy: + default: Retain + description: |- + PersistentVolumeReclaimPolicy controls what happens to the PersistentVolume when + the DocumentDB cluster is deleted. + + When a DocumentDB cluster is deleted, the following chain of deletions occurs: + DocumentDB deletion → CNPG Cluster deletion → PVC deletion → PV deletion (based on this policy) + + Options: + - Retain (default): The PV is preserved after cluster deletion, allowing manual + data recovery or forensic analysis. Use for production workloads where data + safety is critical. Orphaned PVs must be manually deleted when no longer needed. + - Delete: The PV is automatically deleted when the PVC is deleted. Use for development, + testing, or ephemeral environments where data persistence is not required. + + WARNING: Setting this to "Delete" means all data will be permanently lost when + the DocumentDB cluster is deleted. This cannot be undone. + enum: + - Retain + - Delete + type: string pvcSize: description: PvcSize is the size of the persistent volume claim for DocumentDB storage (e.g., "10Gi"). diff --git a/operator/src/config/rbac/role.yaml b/operator/src/config/rbac/role.yaml index fbc26477..3c0b4b79 100644 --- a/operator/src/config/rbac/role.yaml +++ b/operator/src/config/rbac/role.yaml @@ -7,11 +7,22 @@ rules: - apiGroups: - "" resources: + - persistentvolumeclaims - secrets verbs: - get - list - watch +- apiGroups: + - "" + resources: + - persistentvolumes + verbs: + - get + - list + - patch + - update + - watch - apiGroups: - cert-manager.io resources: diff --git a/operator/src/internal/cnpg/cnpg_cluster.go b/operator/src/internal/cnpg/cnpg_cluster.go index 5761160b..bb667df2 100644 --- a/operator/src/internal/cnpg/cnpg_cluster.go +++ b/operator/src/internal/cnpg/cnpg_cluster.go @@ -8,6 +8,7 @@ import ( cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/pointer" @@ -113,18 +114,44 @@ func getInheritedMetadataLabels(appName string) *cnpgv1.EmbeddedObjectMetadata { } func getBootstrapConfiguration(documentdb *dbpreview.DocumentDB, isPrimaryRegion bool, log logr.Logger) *cnpgv1.BootstrapConfiguration { - if isPrimaryRegion && documentdb.Spec.Bootstrap != nil && documentdb.Spec.Bootstrap.Recovery != nil && documentdb.Spec.Bootstrap.Recovery.Backup.Name != "" { - backupName := documentdb.Spec.Bootstrap.Recovery.Backup.Name - log.Info("DocumentDB cluster will be bootstrapped from backup", "backupName", backupName) - return &cnpgv1.BootstrapConfiguration{ - Recovery: &cnpgv1.BootstrapRecovery{ - Backup: &cnpgv1.BackupSource{ - LocalObjectReference: cnpgv1.LocalObjectReference{Name: backupName}, + if isPrimaryRegion && documentdb.Spec.Bootstrap != nil && documentdb.Spec.Bootstrap.Recovery != nil { + recovery := documentdb.Spec.Bootstrap.Recovery + + // Handle backup recovery + if recovery.Backup.Name != "" { + backupName := recovery.Backup.Name + log.Info("DocumentDB cluster will be bootstrapped from backup", "backupName", backupName) + return &cnpgv1.BootstrapConfiguration{ + Recovery: &cnpgv1.BootstrapRecovery{ + Backup: &cnpgv1.BackupSource{ + LocalObjectReference: recovery.Backup, + }, }, - }, + } + } + + // Handle PVC recovery + if recovery.PVC.Name != "" { + pvcName := recovery.PVC.Name + log.Info("DocumentDB cluster will be bootstrapped from PVC", "pvcName", pvcName) + return &cnpgv1.BootstrapConfiguration{ + Recovery: &cnpgv1.BootstrapRecovery{ + VolumeSnapshots: &cnpgv1.DataSource{ + Storage: corev1.TypedLocalObjectReference{ + Name: pvcName, + Kind: "PersistentVolumeClaim", + APIGroup: pointer.String(""), + }, + }, + }, + } } } + return getDefaultBootstrapConfiguration() +} + +func getDefaultBootstrapConfiguration() *cnpgv1.BootstrapConfiguration { return &cnpgv1.BootstrapConfiguration{ InitDB: &cnpgv1.BootstrapInitDB{ PostInitSQL: []string{ diff --git a/operator/src/internal/cnpg/cnpg_cluster_test.go b/operator/src/internal/cnpg/cnpg_cluster_test.go new file mode 100644 index 00000000..a9a0899b --- /dev/null +++ b/operator/src/internal/cnpg/cnpg_cluster_test.go @@ -0,0 +1,302 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package cnpg + +import ( + cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/utils/pointer" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + dbpreview "github.com/documentdb/documentdb-operator/api/preview" +) + +var _ = Describe("getBootstrapConfiguration", func() { + var log = zap.New(zap.WriteTo(GinkgoWriter)) + + It("returns default bootstrap when no bootstrap is configured", func() { + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{}, + } + + result := getBootstrapConfiguration(documentdb, true, log) + Expect(result).ToNot(BeNil()) + Expect(result.InitDB).ToNot(BeNil()) + Expect(result.InitDB.PostInitSQL).To(HaveLen(3)) + Expect(result.InitDB.PostInitSQL[0]).To(Equal("CREATE EXTENSION documentdb CASCADE")) + Expect(result.Recovery).To(BeNil()) + }) + + It("returns default bootstrap when not primary region", func() { + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + Bootstrap: &dbpreview.BootstrapConfiguration{ + Recovery: &dbpreview.RecoveryConfiguration{ + Backup: cnpgv1.LocalObjectReference{ + Name: "my-backup", + }, + }, + }, + }, + } + + result := getBootstrapConfiguration(documentdb, false, log) + Expect(result).ToNot(BeNil()) + Expect(result.InitDB).ToNot(BeNil()) + Expect(result.Recovery).To(BeNil()) + }) + + It("returns default bootstrap when recovery is not configured", func() { + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + Bootstrap: &dbpreview.BootstrapConfiguration{}, + }, + } + + result := getBootstrapConfiguration(documentdb, true, log) + Expect(result).ToNot(BeNil()) + Expect(result.InitDB).ToNot(BeNil()) + Expect(result.Recovery).To(BeNil()) + }) + + It("returns backup recovery when backup name is specified", func() { + backupName := "my-backup" + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + Bootstrap: &dbpreview.BootstrapConfiguration{ + Recovery: &dbpreview.RecoveryConfiguration{ + Backup: cnpgv1.LocalObjectReference{ + Name: backupName, + }, + }, + }, + }, + } + + result := getBootstrapConfiguration(documentdb, true, log) + Expect(result).ToNot(BeNil()) + Expect(result.Recovery).ToNot(BeNil()) + Expect(result.Recovery.Backup).ToNot(BeNil()) + Expect(result.Recovery.Backup.LocalObjectReference.Name).To(Equal(backupName)) + Expect(result.Recovery.VolumeSnapshots).To(BeNil()) + Expect(result.InitDB).To(BeNil()) + }) + + It("returns PVC recovery when PVC name is specified", func() { + pvcName := "my-pvc" + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + Bootstrap: &dbpreview.BootstrapConfiguration{ + Recovery: &dbpreview.RecoveryConfiguration{ + PVC: cnpgv1.LocalObjectReference{ + Name: pvcName, + }, + }, + }, + }, + } + + result := getBootstrapConfiguration(documentdb, true, log) + Expect(result).ToNot(BeNil()) + Expect(result.Recovery).ToNot(BeNil()) + Expect(result.Recovery.VolumeSnapshots).ToNot(BeNil()) + Expect(result.Recovery.VolumeSnapshots.Storage.Name).To(Equal(pvcName)) + Expect(result.Recovery.VolumeSnapshots.Storage.Kind).To(Equal("PersistentVolumeClaim")) + Expect(result.Recovery.VolumeSnapshots.Storage.APIGroup).To(Equal(pointer.String(""))) + Expect(result.Recovery.Backup).To(BeNil()) + Expect(result.InitDB).To(BeNil()) + }) + + It("returns default bootstrap when backup name is empty", func() { + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + Bootstrap: &dbpreview.BootstrapConfiguration{ + Recovery: &dbpreview.RecoveryConfiguration{ + Backup: cnpgv1.LocalObjectReference{ + Name: "", + }, + }, + }, + }, + } + + result := getBootstrapConfiguration(documentdb, true, log) + Expect(result).ToNot(BeNil()) + Expect(result.InitDB).ToNot(BeNil()) + Expect(result.Recovery).To(BeNil()) + }) + + It("returns default bootstrap when PVC name is empty", func() { + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + Bootstrap: &dbpreview.BootstrapConfiguration{ + Recovery: &dbpreview.RecoveryConfiguration{ + PVC: cnpgv1.LocalObjectReference{ + Name: "", + }, + }, + }, + }, + } + + result := getBootstrapConfiguration(documentdb, true, log) + Expect(result).ToNot(BeNil()) + Expect(result.InitDB).ToNot(BeNil()) + Expect(result.Recovery).To(BeNil()) + }) +}) + +var _ = Describe("getDefaultBootstrapConfiguration", func() { + It("returns a bootstrap configuration with InitDB", func() { + result := getDefaultBootstrapConfiguration() + Expect(result).ToNot(BeNil()) + Expect(result.InitDB).ToNot(BeNil()) + Expect(result.Recovery).To(BeNil()) + }) + + It("includes required PostInitSQL statements", func() { + result := getDefaultBootstrapConfiguration() + Expect(result.InitDB.PostInitSQL).To(HaveLen(3)) + Expect(result.InitDB.PostInitSQL).To(ContainElement("CREATE EXTENSION documentdb CASCADE")) + Expect(result.InitDB.PostInitSQL).To(ContainElement("CREATE ROLE documentdb WITH LOGIN PASSWORD 'Admin100'")) + Expect(result.InitDB.PostInitSQL).To(ContainElement("ALTER ROLE documentdb WITH SUPERUSER CREATEDB CREATEROLE REPLICATION BYPASSRLS")) + }) +}) + +var _ = Describe("GetCnpgClusterSpec", func() { + var log = zap.New(zap.WriteTo(GinkgoWriter)) + + It("creates a CNPG cluster spec with default bootstrap", func() { + req := ctrl.Request{} + req.Name = "test-cluster" + req.Namespace = "default" + + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + InstancesPerNode: 3, + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PvcSize: "10Gi", + }, + }, + }, + } + + result := GetCnpgClusterSpec(req, documentdb, "postgres:16", "test-sa", "standard", true, log) + Expect(result).ToNot(BeNil()) + Expect(result.Name).To(Equal("test-cluster")) + Expect(result.Namespace).To(Equal("default")) + Expect(int(result.Spec.Instances)).To(Equal(3)) + Expect(result.Spec.Bootstrap).ToNot(BeNil()) + Expect(result.Spec.Bootstrap.InitDB).ToNot(BeNil()) + }) + + It("creates a CNPG cluster spec with backup recovery", func() { + req := ctrl.Request{} + req.Name = "test-cluster" + req.Namespace = "default" + + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + InstancesPerNode: 3, + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PvcSize: "10Gi", + }, + }, + Bootstrap: &dbpreview.BootstrapConfiguration{ + Recovery: &dbpreview.RecoveryConfiguration{ + Backup: cnpgv1.LocalObjectReference{ + Name: "test-backup", + }, + }, + }, + }, + } + + result := GetCnpgClusterSpec(req, documentdb, "postgres:16", "test-sa", "standard", true, log) + Expect(result).ToNot(BeNil()) + Expect(result.Spec.Bootstrap).ToNot(BeNil()) + Expect(result.Spec.Bootstrap.Recovery).ToNot(BeNil()) + Expect(result.Spec.Bootstrap.Recovery.Backup).ToNot(BeNil()) + Expect(result.Spec.Bootstrap.Recovery.Backup.LocalObjectReference.Name).To(Equal("test-backup")) + }) + + It("creates a CNPG cluster spec with PVC recovery", func() { + req := ctrl.Request{} + req.Name = "test-cluster" + req.Namespace = "default" + + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + InstancesPerNode: 3, + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PvcSize: "10Gi", + }, + }, + Bootstrap: &dbpreview.BootstrapConfiguration{ + Recovery: &dbpreview.RecoveryConfiguration{ + PVC: cnpgv1.LocalObjectReference{ + Name: "test-pvc", + }, + }, + }, + }, + } + + result := GetCnpgClusterSpec(req, documentdb, "postgres:16", "test-sa", "standard", true, log) + Expect(result).ToNot(BeNil()) + Expect(result.Spec.Bootstrap).ToNot(BeNil()) + Expect(result.Spec.Bootstrap.Recovery).ToNot(BeNil()) + Expect(result.Spec.Bootstrap.Recovery.VolumeSnapshots).ToNot(BeNil()) + Expect(result.Spec.Bootstrap.Recovery.VolumeSnapshots.Storage.Name).To(Equal("test-pvc")) + Expect(result.Spec.Bootstrap.Recovery.VolumeSnapshots.Storage.Kind).To(Equal("PersistentVolumeClaim")) + }) + + It("uses specified storage class", func() { + req := ctrl.Request{} + req.Name = "test-cluster" + req.Namespace = "default" + + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + InstancesPerNode: 3, + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PvcSize: "10Gi", + }, + }, + }, + } + + result := GetCnpgClusterSpec(req, documentdb, "postgres:16", "test-sa", "premium-storage", true, log) + Expect(result).ToNot(BeNil()) + Expect(result.Spec.StorageConfiguration.StorageClass).ToNot(BeNil()) + Expect(*result.Spec.StorageConfiguration.StorageClass).To(Equal("premium-storage")) + }) + + It("uses nil storage class when empty string is provided", func() { + req := ctrl.Request{} + req.Name = "test-cluster" + req.Namespace = "default" + + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + InstancesPerNode: 3, + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PvcSize: "10Gi", + }, + }, + }, + } + + result := GetCnpgClusterSpec(req, documentdb, "postgres:16", "test-sa", "", true, log) + Expect(result).ToNot(BeNil()) + Expect(result.Spec.StorageConfiguration.StorageClass).To(BeNil()) + }) +}) diff --git a/operator/src/internal/cnpg/suite_test.go b/operator/src/internal/cnpg/suite_test.go new file mode 100644 index 00000000..287c2cd4 --- /dev/null +++ b/operator/src/internal/cnpg/suite_test.go @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package cnpg + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestCNPG(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "CNPG Suite") +} diff --git a/operator/src/internal/controller/certificate_controller_test.go b/operator/src/internal/controller/certificate_controller_test.go new file mode 100644 index 00000000..96620a3a --- /dev/null +++ b/operator/src/internal/controller/certificate_controller_test.go @@ -0,0 +1,143 @@ +package controller + +import ( + "context" + "testing" + "time" + + cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + dbpreview "github.com/documentdb/documentdb-operator/api/preview" + util "github.com/documentdb/documentdb-operator/internal/utils" +) + +// helper to build TLS reconciler with objects +func buildCertificateReconciler(t *testing.T, objs ...runtime.Object) *CertificateReconciler { + scheme := runtime.NewScheme() + require.NoError(t, dbpreview.AddToScheme(scheme)) + require.NoError(t, cmapi.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + builder := fake.NewClientBuilder().WithScheme(scheme) + if len(objs) > 0 { + builder = builder.WithRuntimeObjects(objs...) + clientObjs := make([]client.Object, 0, len(objs)) + for _, obj := range objs { + if co, ok := obj.(client.Object); ok { + clientObjs = append(clientObjs, co) + } + } + if len(clientObjs) > 0 { + builder = builder.WithStatusSubresource(clientObjs...) + } + } + c := builder.Build() + return &CertificateReconciler{Client: c, Scheme: scheme} +} + +func baseDocumentDB(name, ns string) *dbpreview.DocumentDB { + return &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns}, + Spec: dbpreview.DocumentDBSpec{ + NodeCount: 1, + InstancesPerNode: 1, + Resource: dbpreview.Resource{Storage: dbpreview.StorageConfiguration{PvcSize: "1Gi"}}, + DocumentDBImage: "test-image", + ExposeViaService: dbpreview.ExposeViaService{ServiceType: "ClusterIP"}, + }, + } +} + +func TestEnsureProvidedSecret(t *testing.T) { + ctx := context.Background() + ddb := baseDocumentDB("ddb-prov", "default") + ddb.Spec.TLS = &dbpreview.TLSConfiguration{Gateway: &dbpreview.GatewayTLS{Mode: "Provided", Provided: &dbpreview.ProvidedTLS{SecretName: "mycert"}}} + // Secret missing first + r := buildCertificateReconciler(t, ddb) + res, err := r.reconcileCertificates(ctx, ddb) + require.NoError(t, err) + require.Equal(t, RequeueAfterShort, res.RequeueAfter) + require.False(t, ddb.Status.TLS.Ready, "Should not be ready until secret exists") + + // Create secret with required keys then reconcile again + secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "mycert", Namespace: "default"}, Data: map[string][]byte{"tls.crt": []byte("crt"), "tls.key": []byte("key")}} + require.NoError(t, r.Client.Create(ctx, secret)) + res, err = r.reconcileCertificates(ctx, ddb) + require.NoError(t, err) + require.Zero(t, res.RequeueAfter) + require.True(t, ddb.Status.TLS.Ready, "Provided secret should mark TLS ready") + require.Equal(t, "mycert", ddb.Status.TLS.SecretName) +} + +func TestEnsureCertManagerManagedCert(t *testing.T) { + ctx := context.Background() + ddb := baseDocumentDB("ddb-cm", "default") + ddb.Spec.TLS = &dbpreview.TLSConfiguration{Gateway: &dbpreview.GatewayTLS{Mode: "CertManager", CertManager: &dbpreview.CertManagerTLS{IssuerRef: dbpreview.IssuerRef{Name: "test-issuer", Kind: "Issuer"}, DNSNames: []string{"custom.example"}}}} + ddb.Status.TLS = &dbpreview.TLSStatus{} + issuer := &cmapi.Issuer{ObjectMeta: metav1.ObjectMeta{Name: "test-issuer", Namespace: "default"}, Spec: cmapi.IssuerSpec{IssuerConfig: cmapi.IssuerConfig{SelfSigned: &cmapi.SelfSignedIssuer{}}}} + r := buildCertificateReconciler(t, ddb, issuer) + + // Call certificate ensure twice to mimic reconcile loops + res, err := r.reconcileCertificates(ctx, ddb) + require.NoError(t, err) + require.Equal(t, RequeueAfterShort, res.RequeueAfter) + res, err = r.reconcileCertificates(ctx, ddb) + require.NoError(t, err) + require.Equal(t, RequeueAfterShort, res.RequeueAfter) + + cert := &cmapi.Certificate{} + // fetch certificate (self-created by reconcile). If not found, run reconcile again once. + require.NoError(t, r.Client.Get(ctx, types.NamespacedName{Name: "ddb-cm-gateway-cert", Namespace: "default"}, cert)) + // Debug: list all certificates to ensure store functioning + certList := &cmapi.CertificateList{} + _ = r.Client.List(ctx, certList) + for _, c := range certList.Items { + t.Logf("Found certificate: %s/%s secret=%s", c.Namespace, c.Name, c.Spec.SecretName) + } + require.Contains(t, cert.Spec.DNSNames, "custom.example") + // Should include service DNS names + serviceBase := util.DOCUMENTDB_SERVICE_PREFIX + ddb.Name + require.Contains(t, cert.Spec.DNSNames, serviceBase) + + // Simulate readiness condition then invoke ensure again (mimic reconcile loop) + cert.Status.Conditions = append(cert.Status.Conditions, cmapi.CertificateCondition{Type: cmapi.CertificateConditionReady, Status: cmmeta.ConditionTrue, LastTransitionTime: &metav1.Time{Time: time.Now()}}) + require.NoError(t, r.Client.Update(ctx, cert)) + res, err = r.reconcileCertificates(ctx, ddb) + require.NoError(t, err) + require.Zero(t, res.RequeueAfter) + require.True(t, ddb.Status.TLS.Ready, "Cert-manager managed cert should mark ready after condition true") + require.NotEmpty(t, ddb.Status.TLS.SecretName) +} + +func TestEnsureSelfSignedCert(t *testing.T) { + ctx := context.Background() + ddb := baseDocumentDB("ddb-ss", "default") + ddb.Spec.TLS = &dbpreview.TLSConfiguration{Gateway: &dbpreview.GatewayTLS{Mode: "SelfSigned"}} + ddb.Status.TLS = &dbpreview.TLSStatus{} + r := buildCertificateReconciler(t, ddb) + + // First call should create issuer and certificate + res, err := r.reconcileCertificates(ctx, ddb) + require.NoError(t, err) + require.Equal(t, RequeueAfterShort, res.RequeueAfter) + + // Certificate should exist + cert := &cmapi.Certificate{} + require.NoError(t, r.Client.Get(ctx, types.NamespacedName{Name: "ddb-ss-gateway-cert", Namespace: "default"}, cert)) + + // Simulate ready condition and call again + cert.Status.Conditions = append(cert.Status.Conditions, cmapi.CertificateCondition{Type: cmapi.CertificateConditionReady, Status: cmmeta.ConditionTrue, LastTransitionTime: &metav1.Time{Time: time.Now()}}) + require.NoError(t, r.Client.Update(ctx, cert)) + res, err = r.reconcileCertificates(ctx, ddb) + require.NoError(t, err) + require.Zero(t, res.RequeueAfter) + require.True(t, ddb.Status.TLS.Ready) + require.NotEmpty(t, ddb.Status.TLS.SecretName) +} diff --git a/operator/src/internal/controller/documentdb_controller.go b/operator/src/internal/controller/documentdb_controller.go index cb6d8010..90aff359 100644 --- a/operator/src/internal/controller/documentdb_controller.go +++ b/operator/src/internal/controller/documentdb_controller.go @@ -23,10 +23,12 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" + "k8s.io/client-go/tools/record" "k8s.io/client-go/tools/remotecommand" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -39,6 +41,12 @@ import ( const ( RequeueAfterShort = 10 * time.Second RequeueAfterLong = 30 * time.Second + + // documentDBFinalizer ensures we can emit PV retention warnings before deletion completes + documentDBFinalizer = "documentdb.io/pv-retention-finalizer" + + // CNPG label used to identify PVCs belonging to a cluster + cnpgClusterLabel = "cnpg.io/cluster" ) // DocumentDBReconciler reconciles a DocumentDB object @@ -47,6 +55,7 @@ type DocumentDBReconciler struct { Scheme *runtime.Scheme Config *rest.Config Clientset kubernetes.Interface + Recorder record.EventRecorder } var reconcileMutex sync.Mutex @@ -54,6 +63,7 @@ var reconcileMutex sync.Mutex // +kubebuilder:rbac:groups=documentdb.io,resources=dbs,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=documentdb.io,resources=dbs/status,verbs=get;update;patch // +kubebuilder:rbac:groups=documentdb.io,resources=dbs/finalizers,verbs=update +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch func (r *DocumentDBReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { reconcileMutex.Lock() defer reconcileMutex.Unlock() @@ -76,6 +86,22 @@ func (r *DocumentDBReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } + // Handle deletion with finalizer + if !documentdb.ObjectMeta.DeletionTimestamp.IsZero() { + return r.handleDeletion(ctx, documentdb) + } + + // Ensure finalizer is present for non-deleting resources + if !controllerutil.ContainsFinalizer(documentdb, documentDBFinalizer) { + controllerutil.AddFinalizer(documentdb, documentDBFinalizer) + if err := r.Update(ctx, documentdb); err != nil { + logger.Error(err, "Failed to add finalizer") + return ctrl.Result{}, err + } + logger.Info("Added finalizer to DocumentDB") + return ctrl.Result{Requeue: true}, nil + } + replicationContext, err := util.GetReplicationContext(ctx, r.Client, *documentdb) if err != nil { logger.Error(err, "Failed to determine replication context") @@ -285,6 +311,94 @@ func (r *DocumentDBReconciler) cleanupResources(ctx context.Context, req ctrl.Re return nil } +// handleDeletion processes DocumentDB deletion, emitting warnings about retained PVs if needed +func (r *DocumentDBReconciler) handleDeletion(ctx context.Context, documentdb *dbpreview.DocumentDB) (ctrl.Result, error) { + logger := log.FromContext(ctx) + + if !controllerutil.ContainsFinalizer(documentdb, documentDBFinalizer) { + // Finalizer already removed, nothing to do + return ctrl.Result{}, nil + } + + // Check if PVs will be retained and emit warning + if r.shouldWarnAboutRetainedPVs(documentdb) { + if err := r.emitPVRetentionWarning(ctx, documentdb); err != nil { + // Log but don't block deletion + logger.Error(err, "Failed to emit PV retention warning, continuing with deletion") + } + } + + // Remove finalizer to allow deletion to proceed + controllerutil.RemoveFinalizer(documentdb, documentDBFinalizer) + if err := r.Update(ctx, documentdb); err != nil { + logger.Error(err, "Failed to remove finalizer") + return ctrl.Result{}, err + } + + logger.Info("Removed finalizer, deletion will proceed") + return ctrl.Result{}, nil +} + +// shouldWarnAboutRetainedPVs returns true if the reclaim policy is Retain (explicitly or by default) +func (r *DocumentDBReconciler) shouldWarnAboutRetainedPVs(documentdb *dbpreview.DocumentDB) bool { + policy := documentdb.Spec.Resource.Storage.PersistentVolumeReclaimPolicy + // Default is Retain, so warn unless explicitly set to Delete + return policy == "" || policy == reclaimPolicyRetain +} + +// emitPVRetentionWarning emits a warning event listing PVs that will be retained after deletion +func (r *DocumentDBReconciler) emitPVRetentionWarning(ctx context.Context, documentdb *dbpreview.DocumentDB) error { + logger := log.FromContext(ctx) + + if r.Recorder == nil { + logger.Info("Event recorder not configured, skipping PV retention warning") + return nil + } + + // Find PVs associated with this DocumentDB + pvNames, err := r.findPVsForDocumentDB(ctx, documentdb) + if err != nil { + return fmt.Errorf("failed to find PVs: %w", err) + } + + if len(pvNames) == 0 { + logger.V(1).Info("No PVs found for DocumentDB") + return nil + } + + // Emit actionable warning event + message := fmt.Sprintf( + "PersistentVolumes retained after cluster deletion (policy=Retain). "+ + "To delete when no longer needed: kubectl delete pv %s", + strings.Join(pvNames, " ")) + + r.Recorder.Event(documentdb, corev1.EventTypeWarning, "PVsRetained", message) + logger.Info("Emitted PV retention warning", "pvCount", len(pvNames), "pvNames", pvNames) + + return nil +} + +// findPVsForDocumentDB finds all PV names associated with a DocumentDB cluster using CNPG labels +func (r *DocumentDBReconciler) findPVsForDocumentDB(ctx context.Context, documentdb *dbpreview.DocumentDB) ([]string, error) { + // CNPG cluster name matches DocumentDB name + pvcList := &corev1.PersistentVolumeClaimList{} + if err := r.List(ctx, pvcList, + client.InNamespace(documentdb.Namespace), + client.MatchingLabels{cnpgClusterLabel: documentdb.Name}, + ); err != nil { + return nil, err + } + + pvNames := make([]string, 0, len(pvcList.Items)) + for _, pvc := range pvcList.Items { + if pvc.Status.Phase == corev1.ClaimBound && pvc.Spec.VolumeName != "" { + pvNames = append(pvNames, pvc.Spec.VolumeName) + } + } + + return pvNames, nil +} + func (r *DocumentDBReconciler) EnsureServiceAccountRoleAndRoleBinding(ctx context.Context, documentdb *dbpreview.DocumentDB, namespace string) error { log := log.FromContext(ctx) diff --git a/operator/src/internal/controller/documentdb_controller_test.go b/operator/src/internal/controller/documentdb_controller_test.go index 96620a3a..378eccf2 100644 --- a/operator/src/internal/controller/documentdb_controller_test.go +++ b/operator/src/internal/controller/documentdb_controller_test.go @@ -1,143 +1,612 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + package controller import ( "context" - "testing" - "time" - cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" - cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" - "github.com/stretchr/testify/require" + cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" + "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" dbpreview "github.com/documentdb/documentdb-operator/api/preview" - util "github.com/documentdb/documentdb-operator/internal/utils" ) -// helper to build TLS reconciler with objects -func buildCertificateReconciler(t *testing.T, objs ...runtime.Object) *CertificateReconciler { - scheme := runtime.NewScheme() - require.NoError(t, dbpreview.AddToScheme(scheme)) - require.NoError(t, cmapi.AddToScheme(scheme)) - require.NoError(t, corev1.AddToScheme(scheme)) - builder := fake.NewClientBuilder().WithScheme(scheme) - if len(objs) > 0 { - builder = builder.WithRuntimeObjects(objs...) - clientObjs := make([]client.Object, 0, len(objs)) - for _, obj := range objs { - if co, ok := obj.(client.Object); ok { - clientObjs = append(clientObjs, co) - } - } - if len(clientObjs) > 0 { - builder = builder.WithStatusSubresource(clientObjs...) - } - } - c := builder.Build() - return &CertificateReconciler{Client: c, Scheme: scheme} -} - -func baseDocumentDB(name, ns string) *dbpreview.DocumentDB { - return &dbpreview.DocumentDB{ - ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns}, - Spec: dbpreview.DocumentDBSpec{ - NodeCount: 1, - InstancesPerNode: 1, - Resource: dbpreview.Resource{Storage: dbpreview.StorageConfiguration{PvcSize: "1Gi"}}, - DocumentDBImage: "test-image", - ExposeViaService: dbpreview.ExposeViaService{ServiceType: "ClusterIP"}, - }, - } -} - -func TestEnsureProvidedSecret(t *testing.T) { - ctx := context.Background() - ddb := baseDocumentDB("ddb-prov", "default") - ddb.Spec.TLS = &dbpreview.TLSConfiguration{Gateway: &dbpreview.GatewayTLS{Mode: "Provided", Provided: &dbpreview.ProvidedTLS{SecretName: "mycert"}}} - // Secret missing first - r := buildCertificateReconciler(t, ddb) - res, err := r.reconcileCertificates(ctx, ddb) - require.NoError(t, err) - require.Equal(t, RequeueAfterShort, res.RequeueAfter) - require.False(t, ddb.Status.TLS.Ready, "Should not be ready until secret exists") - - // Create secret with required keys then reconcile again - secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "mycert", Namespace: "default"}, Data: map[string][]byte{"tls.crt": []byte("crt"), "tls.key": []byte("key")}} - require.NoError(t, r.Client.Create(ctx, secret)) - res, err = r.reconcileCertificates(ctx, ddb) - require.NoError(t, err) - require.Zero(t, res.RequeueAfter) - require.True(t, ddb.Status.TLS.Ready, "Provided secret should mark TLS ready") - require.Equal(t, "mycert", ddb.Status.TLS.SecretName) -} - -func TestEnsureCertManagerManagedCert(t *testing.T) { - ctx := context.Background() - ddb := baseDocumentDB("ddb-cm", "default") - ddb.Spec.TLS = &dbpreview.TLSConfiguration{Gateway: &dbpreview.GatewayTLS{Mode: "CertManager", CertManager: &dbpreview.CertManagerTLS{IssuerRef: dbpreview.IssuerRef{Name: "test-issuer", Kind: "Issuer"}, DNSNames: []string{"custom.example"}}}} - ddb.Status.TLS = &dbpreview.TLSStatus{} - issuer := &cmapi.Issuer{ObjectMeta: metav1.ObjectMeta{Name: "test-issuer", Namespace: "default"}, Spec: cmapi.IssuerSpec{IssuerConfig: cmapi.IssuerConfig{SelfSigned: &cmapi.SelfSignedIssuer{}}}} - r := buildCertificateReconciler(t, ddb, issuer) - - // Call certificate ensure twice to mimic reconcile loops - res, err := r.reconcileCertificates(ctx, ddb) - require.NoError(t, err) - require.Equal(t, RequeueAfterShort, res.RequeueAfter) - res, err = r.reconcileCertificates(ctx, ddb) - require.NoError(t, err) - require.Equal(t, RequeueAfterShort, res.RequeueAfter) - - cert := &cmapi.Certificate{} - // fetch certificate (self-created by reconcile). If not found, run reconcile again once. - require.NoError(t, r.Client.Get(ctx, types.NamespacedName{Name: "ddb-cm-gateway-cert", Namespace: "default"}, cert)) - // Debug: list all certificates to ensure store functioning - certList := &cmapi.CertificateList{} - _ = r.Client.List(ctx, certList) - for _, c := range certList.Items { - t.Logf("Found certificate: %s/%s secret=%s", c.Namespace, c.Name, c.Spec.SecretName) - } - require.Contains(t, cert.Spec.DNSNames, "custom.example") - // Should include service DNS names - serviceBase := util.DOCUMENTDB_SERVICE_PREFIX + ddb.Name - require.Contains(t, cert.Spec.DNSNames, serviceBase) - - // Simulate readiness condition then invoke ensure again (mimic reconcile loop) - cert.Status.Conditions = append(cert.Status.Conditions, cmapi.CertificateCondition{Type: cmapi.CertificateConditionReady, Status: cmmeta.ConditionTrue, LastTransitionTime: &metav1.Time{Time: time.Now()}}) - require.NoError(t, r.Client.Update(ctx, cert)) - res, err = r.reconcileCertificates(ctx, ddb) - require.NoError(t, err) - require.Zero(t, res.RequeueAfter) - require.True(t, ddb.Status.TLS.Ready, "Cert-manager managed cert should mark ready after condition true") - require.NotEmpty(t, ddb.Status.TLS.SecretName) -} - -func TestEnsureSelfSignedCert(t *testing.T) { - ctx := context.Background() - ddb := baseDocumentDB("ddb-ss", "default") - ddb.Spec.TLS = &dbpreview.TLSConfiguration{Gateway: &dbpreview.GatewayTLS{Mode: "SelfSigned"}} - ddb.Status.TLS = &dbpreview.TLSStatus{} - r := buildCertificateReconciler(t, ddb) - - // First call should create issuer and certificate - res, err := r.reconcileCertificates(ctx, ddb) - require.NoError(t, err) - require.Equal(t, RequeueAfterShort, res.RequeueAfter) - - // Certificate should exist - cert := &cmapi.Certificate{} - require.NoError(t, r.Client.Get(ctx, types.NamespacedName{Name: "ddb-ss-gateway-cert", Namespace: "default"}, cert)) - - // Simulate ready condition and call again - cert.Status.Conditions = append(cert.Status.Conditions, cmapi.CertificateCondition{Type: cmapi.CertificateConditionReady, Status: cmmeta.ConditionTrue, LastTransitionTime: &metav1.Time{Time: time.Now()}}) - require.NoError(t, r.Client.Update(ctx, cert)) - res, err = r.reconcileCertificates(ctx, ddb) - require.NoError(t, err) - require.Zero(t, res.RequeueAfter) - require.True(t, ddb.Status.TLS.Ready) - require.NotEmpty(t, ddb.Status.TLS.SecretName) -} +var _ = Describe("DocumentDB Controller", func() { + const ( + documentDBName = "test-documentdb" + documentDBNamespace = "default" + ) + + var ( + ctx context.Context + scheme *runtime.Scheme + recorder *record.FakeRecorder + ) + + BeforeEach(func() { + ctx = context.Background() + scheme = runtime.NewScheme() + recorder = record.NewFakeRecorder(10) + Expect(dbpreview.AddToScheme(scheme)).To(Succeed()) + Expect(cnpgv1.AddToScheme(scheme)).To(Succeed()) + Expect(corev1.AddToScheme(scheme)).To(Succeed()) + }) + + Describe("shouldWarnAboutRetainedPVs", func() { + var reconciler *DocumentDBReconciler + + BeforeEach(func() { + fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build() + reconciler = &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + } + }) + + It("returns true when policy is empty (default Retain)", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + }, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PvcSize: "10Gi", + PersistentVolumeReclaimPolicy: "", // empty = default Retain + }, + }, + }, + } + Expect(reconciler.shouldWarnAboutRetainedPVs(documentdb)).To(BeTrue()) + }) + + It("returns true when policy is explicitly Retain", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + }, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PvcSize: "10Gi", + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + } + Expect(reconciler.shouldWarnAboutRetainedPVs(documentdb)).To(BeTrue()) + }) + + It("returns false when policy is Delete", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + }, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PvcSize: "10Gi", + PersistentVolumeReclaimPolicy: "Delete", + }, + }, + }, + } + Expect(reconciler.shouldWarnAboutRetainedPVs(documentdb)).To(BeFalse()) + }) + }) + + Describe("findPVsForDocumentDB", func() { + It("returns PV names for bound PVCs with matching cluster label", func() { + pvc1 := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName + "-1", + Namespace: documentDBNamespace, + Labels: map[string]string{ + cnpgClusterLabel: documentDBName, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: "pv-abc123", + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + }, + } + pvc2 := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName + "-2", + Namespace: documentDBNamespace, + Labels: map[string]string{ + cnpgClusterLabel: documentDBName, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: "pv-def456", + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(pvc1, pvc2). + Build() + + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + } + + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + }, + } + + pvNames, err := reconciler.findPVsForDocumentDB(ctx, documentdb) + Expect(err).ToNot(HaveOccurred()) + Expect(pvNames).To(HaveLen(2)) + Expect(pvNames).To(ContainElements("pv-abc123", "pv-def456")) + }) + + It("excludes PVCs that are not bound", func() { + boundPVC := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName + "-bound", + Namespace: documentDBNamespace, + Labels: map[string]string{ + cnpgClusterLabel: documentDBName, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: "pv-bound", + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + }, + } + pendingPVC := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName + "-pending", + Namespace: documentDBNamespace, + Labels: map[string]string{ + cnpgClusterLabel: documentDBName, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: "pv-pending", + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimPending, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(boundPVC, pendingPVC). + Build() + + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + } + + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + }, + } + + pvNames, err := reconciler.findPVsForDocumentDB(ctx, documentdb) + Expect(err).ToNot(HaveOccurred()) + Expect(pvNames).To(HaveLen(1)) + Expect(pvNames).To(ContainElement("pv-bound")) + }) + + It("excludes PVCs from different clusters", func() { + matchingPVC := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName + "-match", + Namespace: documentDBNamespace, + Labels: map[string]string{ + cnpgClusterLabel: documentDBName, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: "pv-match", + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + }, + } + otherClusterPVC := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-cluster-pvc", + Namespace: documentDBNamespace, + Labels: map[string]string{ + cnpgClusterLabel: "other-cluster", + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: "pv-other", + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(matchingPVC, otherClusterPVC). + Build() + + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + } + + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + }, + } + + pvNames, err := reconciler.findPVsForDocumentDB(ctx, documentdb) + Expect(err).ToNot(HaveOccurred()) + Expect(pvNames).To(HaveLen(1)) + Expect(pvNames).To(ContainElement("pv-match")) + }) + + It("returns empty slice when no PVCs exist", func() { + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + } + + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + }, + } + + pvNames, err := reconciler.findPVsForDocumentDB(ctx, documentdb) + Expect(err).ToNot(HaveOccurred()) + Expect(pvNames).To(BeEmpty()) + }) + }) + + Describe("emitPVRetentionWarning", func() { + It("emits warning event with PV names when PVCs exist", func() { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName + "-1", + Namespace: documentDBNamespace, + Labels: map[string]string{ + cnpgClusterLabel: documentDBName, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: "pv-test123", + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(pvc). + Build() + + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + } + + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + }, + } + + err := reconciler.emitPVRetentionWarning(ctx, documentdb) + Expect(err).ToNot(HaveOccurred()) + + // Check that an event was recorded + Eventually(recorder.Events).Should(Receive(ContainSubstring("PVsRetained"))) + }) + + It("does not emit event when no PVCs exist", func() { + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + } + + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + }, + } + + err := reconciler.emitPVRetentionWarning(ctx, documentdb) + Expect(err).ToNot(HaveOccurred()) + + // No event should be recorded + Consistently(recorder.Events).ShouldNot(Receive()) + }) + + It("does not panic when Recorder is nil", func() { + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: nil, // No recorder + } + + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + }, + } + + err := reconciler.emitPVRetentionWarning(ctx, documentdb) + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Describe("handleDeletion", func() { + It("removes finalizer and allows deletion when finalizer is present", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + Finalizers: []string{documentDBFinalizer}, + }, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PvcSize: "10Gi", + PersistentVolumeReclaimPolicy: "Delete", // No warning should be emitted + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb). + Build() + + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + } + + // Call handleDeletion - it checks for finalizer, not DeletionTimestamp + result, err := reconciler.handleDeletion(ctx, documentdb) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + + // Verify finalizer was removed + updated := &dbpreview.DocumentDB{} + Expect(fakeClient.Get(ctx, types.NamespacedName{Name: documentDBName, Namespace: documentDBNamespace}, updated)).To(Succeed()) + Expect(controllerutil.ContainsFinalizer(updated, documentDBFinalizer)).To(BeFalse()) + }) + + It("emits warning event when policy is Retain and PVCs exist", func() { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName + "-1", + Namespace: documentDBNamespace, + Labels: map[string]string{ + cnpgClusterLabel: documentDBName, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: "pv-retained", + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + }, + } + + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + Finalizers: []string{documentDBFinalizer}, + }, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PvcSize: "10Gi", + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb, pvc). + Build() + + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + } + + result, err := reconciler.handleDeletion(ctx, documentdb) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + + // Verify warning event was emitted + Eventually(recorder.Events).Should(Receive(ContainSubstring("pv-retained"))) + + // Verify finalizer was removed + updated := &dbpreview.DocumentDB{} + Expect(fakeClient.Get(ctx, types.NamespacedName{Name: documentDBName, Namespace: documentDBNamespace}, updated)).To(Succeed()) + Expect(controllerutil.ContainsFinalizer(updated, documentDBFinalizer)).To(BeFalse()) + }) + + It("returns without action when finalizer is not present", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + Finalizers: []string{}, // No finalizer + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb). + Build() + + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + } + + result, err := reconciler.handleDeletion(ctx, documentdb) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + + // Verify object still exists (no Update was called) + existing := &dbpreview.DocumentDB{} + Expect(fakeClient.Get(ctx, types.NamespacedName{Name: documentDBName, Namespace: documentDBNamespace}, existing)).To(Succeed()) + }) + + It("does not emit warning when policy is Delete", func() { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName + "-1", + Namespace: documentDBNamespace, + Labels: map[string]string{ + cnpgClusterLabel: documentDBName, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: "pv-will-be-deleted", + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + }, + } + + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + Finalizers: []string{documentDBFinalizer}, + }, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PvcSize: "10Gi", + PersistentVolumeReclaimPolicy: "Delete", + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb, pvc). + Build() + + // Create a new recorder to ensure it's empty + localRecorder := record.NewFakeRecorder(10) + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: localRecorder, + } + + result, err := reconciler.handleDeletion(ctx, documentdb) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + + // Verify NO warning event was emitted (policy is Delete) + Consistently(localRecorder.Events).ShouldNot(Receive()) + }) + }) + + Describe("Finalizer management in Reconcile", func() { + It("adds finalizer to new DocumentDB resource", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + }, + Spec: dbpreview.DocumentDBSpec{ + NodeCount: 1, + InstancesPerNode: 1, + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PvcSize: "10Gi", + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb). + Build() + + // Verify resource starts without finalizer + Expect(controllerutil.ContainsFinalizer(documentdb, documentDBFinalizer)).To(BeFalse()) + + // Add finalizer like the controller does + controllerutil.AddFinalizer(documentdb, documentDBFinalizer) + Expect(fakeClient.Update(ctx, documentdb)).To(Succeed()) + + // Verify finalizer was added + updated := &dbpreview.DocumentDB{} + Expect(fakeClient.Get(ctx, types.NamespacedName{Name: documentDBName, Namespace: documentDBNamespace}, updated)).To(Succeed()) + Expect(controllerutil.ContainsFinalizer(updated, documentDBFinalizer)).To(BeTrue()) + }) + }) +}) diff --git a/operator/src/internal/controller/pv_controller.go b/operator/src/internal/controller/pv_controller.go new file mode 100644 index 00000000..2b70a88f --- /dev/null +++ b/operator/src/internal/controller/pv_controller.go @@ -0,0 +1,407 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package controller + +import ( + "context" + "slices" + "sort" + "strings" + + cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + dbpreview "github.com/documentdb/documentdb-operator/api/preview" +) + +const ( + // cnpgAPIVersionPrefix is the prefix for CNPG API versions + cnpgAPIVersionPrefix = "postgresql.cnpg.io" + + // ownerRefKindCluster is the Kind for CNPG Cluster owner references + ownerRefKindCluster = "Cluster" + + // ownerRefKindDocumentDB is the Kind for DocumentDB owner references + ownerRefKindDocumentDB = "DocumentDB" + + // reclaimPolicyRetain is the string value for Retain policy in DocumentDB spec + reclaimPolicyRetain = "Retain" + + // reclaimPolicyDelete is the string value for Delete policy in DocumentDB spec + reclaimPolicyDelete = "Delete" +) + +// securityMountOptions defines the mount options applied to PVs for security hardening: +// - nodev: Prevents device files from being interpreted on the filesystem +// - nosuid: Prevents setuid/setgid bits from taking effect +// - noexec: Prevents execution of binaries on the filesystem +// +// NOTE: These mount options are compatible with most CSI drivers and storage providers. +// However, some storage classes may not support these options, which could cause PV +// binding issues or pod startup failures. If you encounter issues with PV binding after +// deploying the operator, verify your storage provider supports these mount options. +// Common compatible providers: Azure Disk, AWS EBS, GCE PD, most NFS implementations. +var securityMountOptions = []string{"nodev", "noexec", "nosuid"} + +// PersistentVolumeReconciler reconciles PersistentVolume objects +// to set their ReclaimPolicy and mount options based on the associated DocumentDB configuration +type PersistentVolumeReconciler struct { + client.Client +} + +// +kubebuilder:rbac:groups="",resources=persistentvolumes,verbs=get;list;watch;update;patch +// +kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list;watch + +func (r *PersistentVolumeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + logger := log.FromContext(ctx) + + // Fetch the PersistentVolume + pv := &corev1.PersistentVolume{} + if err := r.Get(ctx, req.NamespacedName, pv); err != nil { + if errors.IsNotFound(err) { + return ctrl.Result{}, nil + } + logger.Error(err, "Failed to get PersistentVolume") + return ctrl.Result{}, err + } + + // Skip if PV is not bound to a PVC + if pv.Spec.ClaimRef == nil { + logger.V(1).Info("PV has no claimRef, skipping", "pv", pv.Name) + return ctrl.Result{}, nil + } + + // Find the associated DocumentDB through the ownership chain: + // PV -> PVC -> CNPG Cluster -> DocumentDB + documentdb, err := r.findDocumentDBForPV(ctx, pv) + if err != nil { + logger.Error(err, "Failed to find DocumentDB for PV") + return ctrl.Result{}, err + } + + if documentdb == nil { + logger.V(1).Info("PV is not associated with a DocumentDB cluster, skipping", "pv", pv.Name) + return ctrl.Result{}, nil + } + + // Apply desired configuration to PV + needsUpdate := r.applyDesiredPVConfiguration(ctx, pv, documentdb) + + if needsUpdate { + if err := r.Update(ctx, pv); err != nil { + logger.Error(err, "Failed to update PV") + return ctrl.Result{}, err + } + + logger.Info("Successfully updated PV", + "pv", pv.Name, + "reclaimPolicy", pv.Spec.PersistentVolumeReclaimPolicy, + "mountOptions", pv.Spec.MountOptions) + } + + return ctrl.Result{}, nil +} + +// applyDesiredPVConfiguration applies the desired reclaim policy and mount options to a PV. +// Returns true if any changes were made. +func (r *PersistentVolumeReconciler) applyDesiredPVConfiguration(ctx context.Context, pv *corev1.PersistentVolume, documentdb *dbpreview.DocumentDB) bool { + logger := log.FromContext(ctx) + needsUpdate := false + + // Check if reclaim policy needs update + desiredPolicy := r.getDesiredReclaimPolicy(documentdb) + if pv.Spec.PersistentVolumeReclaimPolicy != desiredPolicy { + logger.Info("PV reclaim policy needs update", + "pv", pv.Name, + "currentPolicy", pv.Spec.PersistentVolumeReclaimPolicy, + "desiredPolicy", desiredPolicy, + "documentdb", documentdb.Name) + pv.Spec.PersistentVolumeReclaimPolicy = desiredPolicy + needsUpdate = true + } + + // Check if mount options need update + if !containsAllMountOptions(pv.Spec.MountOptions, securityMountOptions) { + logger.Info("PV mount options need update", + "pv", pv.Name, + "currentMountOptions", pv.Spec.MountOptions, + "desiredMountOptions", securityMountOptions) + pv.Spec.MountOptions = mergeMountOptions(pv.Spec.MountOptions, securityMountOptions) + needsUpdate = true + } + + return needsUpdate +} + +// containsAllMountOptions checks if all desired mount options are present in current options +func containsAllMountOptions(current, desired []string) bool { + for _, opt := range desired { + if !slices.Contains(current, opt) { + return false + } + } + return true +} + +// mergeMountOptions merges desired mount options into current, avoiding duplicates. +// Returns a sorted slice for deterministic output. +func mergeMountOptions(current, desired []string) []string { + optSet := make(map[string]struct{}, len(current)+len(desired)) + for _, opt := range current { + optSet[opt] = struct{}{} + } + for _, opt := range desired { + optSet[opt] = struct{}{} + } + + result := make([]string, 0, len(optSet)) + for opt := range optSet { + result = append(result, opt) + } + sort.Strings(result) + return result +} + +// findDocumentDBForPV traverses the ownership chain to find the DocumentDB +// associated with a PersistentVolume: +// PV.claimRef -> PVC -> (ownerRef) CNPG Cluster -> (ownerRef) DocumentDB +func (r *PersistentVolumeReconciler) findDocumentDBForPV(ctx context.Context, pv *corev1.PersistentVolume) (*dbpreview.DocumentDB, error) { + logger := log.FromContext(ctx) + + // Step 1: Get the PVC from PV's claimRef + if pv.Spec.ClaimRef == nil { + return nil, nil + } + + pvc := &corev1.PersistentVolumeClaim{} + pvcKey := types.NamespacedName{ + Name: pv.Spec.ClaimRef.Name, + Namespace: pv.Spec.ClaimRef.Namespace, + } + if err := r.Get(ctx, pvcKey, pvc); err != nil { + if errors.IsNotFound(err) { + logger.V(1).Info("PVC not found for PV", "pvc", pvcKey, "pv", pv.Name) + return nil, nil + } + return nil, err + } + + // Step 2: Find CNPG Cluster that owns the PVC + cnpgCluster := r.findCNPGClusterOwner(ctx, pvc) + if cnpgCluster == nil { + logger.V(1).Info("No CNPG Cluster owner found for PVC", "pvc", pvc.Name) + return nil, nil + } + + // Step 3: Find DocumentDB that owns the CNPG Cluster + documentdb := r.findDocumentDBOwner(ctx, cnpgCluster) + if documentdb == nil { + logger.V(1).Info("No DocumentDB owner found for CNPG Cluster", "cluster", cnpgCluster.Name) + return nil, nil + } + + logger.V(1).Info("Found DocumentDB for PV", + "pv", pv.Name, + "pvc", pvc.Name, + "cluster", cnpgCluster.Name, + "documentdb", documentdb.Name) + + return documentdb, nil +} + +// findCNPGClusterOwner finds the CNPG Cluster that owns the given PVC +func (r *PersistentVolumeReconciler) findCNPGClusterOwner(ctx context.Context, pvc *corev1.PersistentVolumeClaim) *cnpgv1.Cluster { + logger := log.FromContext(ctx) + + for _, ownerRef := range pvc.OwnerReferences { + if !isCNPGClusterOwnerRef(ownerRef) { + continue + } + + cluster := &cnpgv1.Cluster{} + if err := r.Get(ctx, types.NamespacedName{ + Name: ownerRef.Name, + Namespace: pvc.Namespace, + }, cluster); err != nil { + if !errors.IsNotFound(err) { + logger.Error(err, "Failed to get CNPG Cluster", "name", ownerRef.Name) + } + continue + } + return cluster + } + + return nil +} + +// findDocumentDBOwner finds the DocumentDB that owns the given CNPG Cluster +func (r *PersistentVolumeReconciler) findDocumentDBOwner(ctx context.Context, cluster *cnpgv1.Cluster) *dbpreview.DocumentDB { + logger := log.FromContext(ctx) + + for _, ownerRef := range cluster.OwnerReferences { + if ownerRef.Kind != ownerRefKindDocumentDB { + continue + } + + documentdb := &dbpreview.DocumentDB{} + if err := r.Get(ctx, types.NamespacedName{ + Name: ownerRef.Name, + Namespace: cluster.Namespace, + }, documentdb); err != nil { + if !errors.IsNotFound(err) { + logger.Error(err, "Failed to get DocumentDB", "name", ownerRef.Name) + } + continue + } + return documentdb + } + + return nil +} + +// isCNPGClusterOwnerRef checks if an owner reference refers to a CNPG Cluster +func isCNPGClusterOwnerRef(ownerRef metav1.OwnerReference) bool { + return ownerRef.Kind == ownerRefKindCluster && strings.Contains(ownerRef.APIVersion, cnpgAPIVersionPrefix) +} + +// isOwnedByDocumentDB checks if a CNPG Cluster is owned by a specific DocumentDB +func isOwnedByDocumentDB(cluster *cnpgv1.Cluster, documentdbName string) bool { + for _, ownerRef := range cluster.OwnerReferences { + if ownerRef.Kind == ownerRefKindDocumentDB && ownerRef.Name == documentdbName { + return true + } + } + return false +} + +// getDesiredReclaimPolicy returns the reclaim policy based on DocumentDB configuration +func (r *PersistentVolumeReconciler) getDesiredReclaimPolicy(documentdb *dbpreview.DocumentDB) corev1.PersistentVolumeReclaimPolicy { + switch documentdb.Spec.Resource.Storage.PersistentVolumeReclaimPolicy { + case reclaimPolicyRetain: + return corev1.PersistentVolumeReclaimRetain + case reclaimPolicyDelete: + return corev1.PersistentVolumeReclaimDelete + default: + // Default to Retain if not specified - safer for database workloads + return corev1.PersistentVolumeReclaimRetain + } +} + +// pvPredicate filters PV events to only process bound PVs +func pvPredicate() predicate.Predicate { + return predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + pv, ok := e.Object.(*corev1.PersistentVolume) + if !ok { + return false + } + // Only process PVs that are bound and have a claimRef + return pv.Status.Phase == corev1.VolumeBound && pv.Spec.ClaimRef != nil + }, + UpdateFunc: func(e event.UpdateEvent) bool { + newPV, ok := e.ObjectNew.(*corev1.PersistentVolume) + if !ok { + return false + } + // Process when PV becomes bound or when claimRef changes + return newPV.Status.Phase == corev1.VolumeBound && newPV.Spec.ClaimRef != nil + }, + DeleteFunc: func(e event.DeleteEvent) bool { + // No need to reconcile deleted PVs + return false + }, + GenericFunc: func(e event.GenericEvent) bool { + pv, ok := e.Object.(*corev1.PersistentVolume) + if !ok { + return false + } + return pv.Status.Phase == corev1.VolumeBound && pv.Spec.ClaimRef != nil + }, + } +} + +// SetupWithManager sets up the controller with the Manager +func (r *PersistentVolumeReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + // Apply pvPredicate only to PersistentVolume events, not globally + For(&corev1.PersistentVolume{}, builder.WithPredicates(pvPredicate())). + // Watch DocumentDB changes and trigger reconciliation of associated PVs + Watches( + &dbpreview.DocumentDB{}, + handler.EnqueueRequestsFromMapFunc(r.findPVsForDocumentDB), + builder.WithPredicates(documentDBReclaimPolicyPredicate()), + ). + Named("pv-controller"). + Complete(r) +} + +// documentDBReclaimPolicyPredicate only triggers when the reclaim policy field changes +func documentDBReclaimPolicyPredicate() predicate.Predicate { + return predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + oldDB, ok := e.ObjectOld.(*dbpreview.DocumentDB) + if !ok { + return false + } + newDB, ok := e.ObjectNew.(*dbpreview.DocumentDB) + if !ok { + return false + } + return oldDB.Spec.Resource.Storage.PersistentVolumeReclaimPolicy != newDB.Spec.Resource.Storage.PersistentVolumeReclaimPolicy + }, + CreateFunc: func(e event.CreateEvent) bool { return false }, + DeleteFunc: func(e event.DeleteEvent) bool { return false }, + GenericFunc: func(e event.GenericEvent) bool { return false }, + } +} + +// findPVsForDocumentDB finds all PVs associated with a DocumentDB and returns reconcile requests for them. +// Uses CNPG's cluster label (cnpg.io/cluster) for efficient PVC filtering instead of listing all resources. +func (r *PersistentVolumeReconciler) findPVsForDocumentDB(ctx context.Context, obj client.Object) []reconcile.Request { + logger := log.FromContext(ctx) + documentdb, ok := obj.(*dbpreview.DocumentDB) + if !ok { + return nil + } + + // Use CNPG's cluster label to efficiently find PVCs belonging to this DocumentDB. + // CNPG automatically labels PVCs with "cnpg.io/cluster" set to the cluster name, + // and CNPG cluster name matches DocumentDB name by convention. + pvcList := &corev1.PersistentVolumeClaimList{} + if err := r.List(ctx, pvcList, + client.InNamespace(documentdb.Namespace), + client.MatchingLabels{cnpgClusterLabel: documentdb.Name}, + ); err != nil { + logger.Error(err, "Failed to list PVCs for DocumentDB") + return nil + } + + var requests []reconcile.Request + for _, pvc := range pvcList.Items { + if pvc.Spec.VolumeName != "" { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: pvc.Spec.VolumeName, + }, + }) + } + } + + logger.Info("Found PVs to reconcile for DocumentDB update", + "documentdb", documentdb.Name, + "pvCount", len(requests)) + + return requests +} diff --git a/operator/src/internal/controller/pv_controller_test.go b/operator/src/internal/controller/pv_controller_test.go new file mode 100644 index 00000000..7f346314 --- /dev/null +++ b/operator/src/internal/controller/pv_controller_test.go @@ -0,0 +1,1346 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package controller + +import ( + "context" + "fmt" + + cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + + dbpreview "github.com/documentdb/documentdb-operator/api/preview" +) + +var _ = Describe("PersistentVolume Controller", func() { + const ( + pvName = "test-pv" + pvcName = "test-pvc" + clusterName = "test-cluster" + documentdbName = "test-documentdb" + testNamespace = "default" + ) + + var ( + ctx context.Context + scheme *runtime.Scheme + ) + + BeforeEach(func() { + ctx = context.Background() + scheme = runtime.NewScheme() + Expect(dbpreview.AddToScheme(scheme)).To(Succeed()) + Expect(cnpgv1.AddToScheme(scheme)).To(Succeed()) + Expect(corev1.AddToScheme(scheme)).To(Succeed()) + }) + + Describe("containsAllMountOptions", func() { + It("returns true when all desired options are present", func() { + current := []string{"nodev", "nosuid", "noexec", "rw"} + desired := []string{"nodev", "nosuid", "noexec"} + Expect(containsAllMountOptions(current, desired)).To(BeTrue()) + }) + + It("returns false when some desired options are missing", func() { + current := []string{"nodev", "rw"} + desired := []string{"nodev", "nosuid", "noexec"} + Expect(containsAllMountOptions(current, desired)).To(BeFalse()) + }) + + It("returns true when desired is empty", func() { + current := []string{"nodev", "rw"} + desired := []string{} + Expect(containsAllMountOptions(current, desired)).To(BeTrue()) + }) + + It("returns false when current is empty but desired is not", func() { + current := []string{} + desired := []string{"nodev"} + Expect(containsAllMountOptions(current, desired)).To(BeFalse()) + }) + }) + + Describe("mergeMountOptions", func() { + It("merges options without duplicates", func() { + current := []string{"rw", "nodev"} + desired := []string{"nodev", "nosuid", "noexec"} + result := mergeMountOptions(current, desired) + Expect(result).To(HaveLen(4)) + Expect(result).To(ContainElements("rw", "nodev", "nosuid", "noexec")) + }) + + It("returns sorted result for deterministic output", func() { + current := []string{"zz", "aa"} + desired := []string{"mm", "bb"} + result := mergeMountOptions(current, desired) + Expect(result).To(Equal([]string{"aa", "bb", "mm", "zz"})) + }) + + It("handles empty current slice", func() { + current := []string{} + desired := []string{"nodev", "nosuid"} + result := mergeMountOptions(current, desired) + Expect(result).To(Equal([]string{"nodev", "nosuid"})) + }) + + It("handles empty desired slice", func() { + current := []string{"rw", "nodev"} + desired := []string{} + result := mergeMountOptions(current, desired) + Expect(result).To(Equal([]string{"nodev", "rw"})) + }) + }) + + Describe("isCNPGClusterOwnerRef", func() { + It("returns true for valid CNPG Cluster owner reference", func() { + ownerRef := metav1.OwnerReference{ + APIVersion: "postgresql.cnpg.io/v1", + Kind: "Cluster", + Name: "test-cluster", + } + Expect(isCNPGClusterOwnerRef(ownerRef)).To(BeTrue()) + }) + + It("returns false for non-Cluster kind", func() { + ownerRef := metav1.OwnerReference{ + APIVersion: "postgresql.cnpg.io/v1", + Kind: "Backup", + Name: "test-backup", + } + Expect(isCNPGClusterOwnerRef(ownerRef)).To(BeFalse()) + }) + + It("returns false for non-CNPG API version", func() { + ownerRef := metav1.OwnerReference{ + APIVersion: "apps/v1", + Kind: "Cluster", + Name: "test-cluster", + } + Expect(isCNPGClusterOwnerRef(ownerRef)).To(BeFalse()) + }) + }) + + Describe("isOwnedByDocumentDB", func() { + It("returns true when cluster is owned by the specified DocumentDB", func() { + cluster := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "documentdb.io/v1", + Kind: "DocumentDB", + Name: documentdbName, + }, + }, + }, + } + Expect(isOwnedByDocumentDB(cluster, documentdbName)).To(BeTrue()) + }) + + It("returns false when cluster is owned by a different DocumentDB", func() { + cluster := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "documentdb.io/v1", + Kind: "DocumentDB", + Name: "other-documentdb", + }, + }, + }, + } + Expect(isOwnedByDocumentDB(cluster, documentdbName)).To(BeFalse()) + }) + + It("returns false when cluster has no owner references", func() { + cluster := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: testNamespace, + }, + } + Expect(isOwnedByDocumentDB(cluster, documentdbName)).To(BeFalse()) + }) + }) + + Describe("getDesiredReclaimPolicy", func() { + var reconciler *PersistentVolumeReconciler + + BeforeEach(func() { + fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build() + reconciler = &PersistentVolumeReconciler{Client: fakeClient} + }) + + It("returns Retain when spec specifies Retain", func() { + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + } + Expect(reconciler.getDesiredReclaimPolicy(documentdb)).To(Equal(corev1.PersistentVolumeReclaimRetain)) + }) + + It("returns Delete when spec specifies Delete", func() { + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Delete", + }, + }, + }, + } + Expect(reconciler.getDesiredReclaimPolicy(documentdb)).To(Equal(corev1.PersistentVolumeReclaimDelete)) + }) + + It("returns Retain when spec is empty (default)", func() { + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{}, + }, + }, + } + Expect(reconciler.getDesiredReclaimPolicy(documentdb)).To(Equal(corev1.PersistentVolumeReclaimRetain)) + }) + + It("returns Retain for unknown policy value", func() { + documentdb := &dbpreview.DocumentDB{ + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Unknown", + }, + }, + }, + } + Expect(reconciler.getDesiredReclaimPolicy(documentdb)).To(Equal(corev1.PersistentVolumeReclaimRetain)) + }) + }) + + Describe("applyDesiredPVConfiguration", func() { + var reconciler *PersistentVolumeReconciler + + BeforeEach(func() { + fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build() + reconciler = &PersistentVolumeReconciler{Client: fakeClient} + }) + + It("returns true and updates PV when reclaim policy differs", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimDelete, + MountOptions: []string{"nodev", "noexec", "nosuid"}, + }, + } + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName}, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + } + + needsUpdate := reconciler.applyDesiredPVConfiguration(ctx, pv, documentdb) + Expect(needsUpdate).To(BeTrue()) + Expect(pv.Spec.PersistentVolumeReclaimPolicy).To(Equal(corev1.PersistentVolumeReclaimRetain)) + }) + + It("returns true and updates PV when mount options are missing", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimDelete, + MountOptions: []string{"rw"}, + }, + } + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName}, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Delete", + }, + }, + }, + } + + needsUpdate := reconciler.applyDesiredPVConfiguration(ctx, pv, documentdb) + Expect(needsUpdate).To(BeTrue()) + Expect(pv.Spec.MountOptions).To(ContainElements("nodev", "noexec", "nosuid", "rw")) + }) + + It("returns false when no changes are needed", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain, + MountOptions: []string{"nodev", "noexec", "nosuid"}, + }, + } + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName}, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + } + + needsUpdate := reconciler.applyDesiredPVConfiguration(ctx, pv, documentdb) + Expect(needsUpdate).To(BeFalse()) + }) + }) + + Describe("Reconcile", func() { + It("skips PV without claimRef", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimDelete, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(pv). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result, err := reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: types.NamespacedName{Name: pvName}, + }) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + + // Verify PV was not modified + updatedPV := &corev1.PersistentVolume{} + Expect(fakeClient.Get(ctx, types.NamespacedName{Name: pvName}, updatedPV)).To(Succeed()) + Expect(updatedPV.Spec.MountOptions).To(BeEmpty()) + }) + + It("skips PV not associated with DocumentDB", func() { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: testNamespace, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: pvName, + }, + } + + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimDelete, + ClaimRef: &corev1.ObjectReference{ + Name: pvcName, + Namespace: testNamespace, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(pv, pvc). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result, err := reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: types.NamespacedName{Name: pvName}, + }) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + }) + + It("updates PV when associated with DocumentDB and changes needed", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentdbName, + Namespace: testNamespace, + UID: "documentdb-uid", + }, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + } + + trueVal := true + cluster := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: testNamespace, + UID: "cluster-uid", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "documentdb.io/v1", + Kind: "DocumentDB", + Name: documentdbName, + UID: "documentdb-uid", + Controller: &trueVal, + }, + }, + }, + } + + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "postgresql.cnpg.io/v1", + Kind: "Cluster", + Name: clusterName, + UID: "cluster-uid", + Controller: &trueVal, + }, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: pvName, + }, + } + + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimDelete, + ClaimRef: &corev1.ObjectReference{ + Name: pvcName, + Namespace: testNamespace, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb, cluster, pvc, pv). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result, err := reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: types.NamespacedName{Name: pvName}, + }) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + + // Verify PV was updated + updatedPV := &corev1.PersistentVolume{} + Expect(fakeClient.Get(ctx, types.NamespacedName{Name: pvName}, updatedPV)).To(Succeed()) + Expect(updatedPV.Spec.PersistentVolumeReclaimPolicy).To(Equal(corev1.PersistentVolumeReclaimRetain)) + Expect(updatedPV.Spec.MountOptions).To(ContainElements("nodev", "noexec", "nosuid")) + }) + + It("returns empty result when PV not found", func() { + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result, err := reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: types.NamespacedName{Name: "non-existent-pv"}, + }) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + }) + + It("returns error when Get PV fails with non-NotFound error", func() { + expectedErr := fmt.Errorf("api server unavailable") + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*corev1.PersistentVolume); ok { + return expectedErr + } + return client.Get(ctx, key, obj, opts...) + }, + }). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result, err := reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: types.NamespacedName{Name: pvName}, + }) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("api server unavailable")) + Expect(result).To(Equal(ctrl.Result{})) + }) + + It("returns error when PV update fails", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentdbName, + Namespace: testNamespace, + UID: "documentdb-uid", + }, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + } + + trueVal := true + cluster := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: testNamespace, + UID: "cluster-uid", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "documentdb.io/v1", + Kind: "DocumentDB", + Name: documentdbName, + UID: "documentdb-uid", + Controller: &trueVal, + }, + }, + }, + } + + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "postgresql.cnpg.io/v1", + Kind: "Cluster", + Name: clusterName, + UID: "cluster-uid", + Controller: &trueVal, + }, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: pvName, + }, + } + + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimDelete, + ClaimRef: &corev1.ObjectReference{ + Name: pvcName, + Namespace: testNamespace, + }, + }, + } + + expectedErr := fmt.Errorf("update conflict") + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb, cluster, pvc, pv). + WithInterceptorFuncs(interceptor.Funcs{ + Update: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + if _, ok := obj.(*corev1.PersistentVolume); ok { + return expectedErr + } + return client.Update(ctx, obj, opts...) + }, + }). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result, err := reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: types.NamespacedName{Name: pvName}, + }) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("update conflict")) + Expect(result).To(Equal(ctrl.Result{})) + }) + + It("returns error when findDocumentDBForPV fails due to PVC Get error", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimDelete, + ClaimRef: &corev1.ObjectReference{ + Name: pvcName, + Namespace: testNamespace, + }, + }, + } + + expectedErr := fmt.Errorf("permission denied") + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(pv). + WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*corev1.PersistentVolumeClaim); ok { + return expectedErr + } + return client.Get(ctx, key, obj, opts...) + }, + }). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result, err := reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: types.NamespacedName{Name: pvName}, + }) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("permission denied")) + Expect(result).To(Equal(ctrl.Result{})) + }) + }) + + Describe("findPVsForDocumentDB", func() { + It("returns reconcile requests for PVs associated with DocumentDB using CNPG cluster label", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentdbName, + Namespace: testNamespace, + UID: "documentdb-uid", + }, + } + + // PVC with CNPG cluster label (this is how CNPG labels its PVCs) + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: testNamespace, + Labels: map[string]string{ + "cnpg.io/cluster": documentdbName, // CNPG cluster name matches DocumentDB name + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: pvName, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb, pvc). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + requests := reconciler.findPVsForDocumentDB(ctx, documentdb) + Expect(requests).To(HaveLen(1)) + Expect(requests[0].Name).To(Equal(pvName)) + }) + + It("returns empty when no PVCs have matching CNPG cluster label", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentdbName, + Namespace: testNamespace, + }, + } + + // PVC without CNPG cluster label or with different label value + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: testNamespace, + Labels: map[string]string{ + "cnpg.io/cluster": "different-cluster", + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: pvName, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb, pvc). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + requests := reconciler.findPVsForDocumentDB(ctx, documentdb) + Expect(requests).To(BeEmpty()) + }) + + It("returns empty when object is not DocumentDB", func() { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: testNamespace, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + requests := reconciler.findPVsForDocumentDB(ctx, pvc) + Expect(requests).To(BeNil()) + }) + + It("returns nil when listing PVCs fails", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentdbName, + Namespace: testNamespace, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb). + WithInterceptorFuncs(interceptor.Funcs{ + List: func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + if _, ok := list.(*corev1.PersistentVolumeClaimList); ok { + return fmt.Errorf("pvc list error") + } + return client.List(ctx, list, opts...) + }, + }). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + requests := reconciler.findPVsForDocumentDB(ctx, documentdb) + Expect(requests).To(BeNil()) + }) + + It("skips PVCs without volume name", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentdbName, + Namespace: testNamespace, + }, + } + + // PVC with CNPG cluster label but no volume name yet (not bound) + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: testNamespace, + Labels: map[string]string{ + "cnpg.io/cluster": documentdbName, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: "", // Not yet bound + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb, pvc). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + requests := reconciler.findPVsForDocumentDB(ctx, documentdb) + Expect(requests).To(BeEmpty()) + }) + }) + + Describe("pvPredicate", func() { + var pred predicate.Predicate + + BeforeEach(func() { + pred = pvPredicate() + }) + + Describe("CreateFunc", func() { + It("returns true for bound PV with claimRef", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + ClaimRef: &corev1.ObjectReference{Name: pvcName, Namespace: testNamespace}, + }, + Status: corev1.PersistentVolumeStatus{ + Phase: corev1.VolumeBound, + }, + } + e := event.CreateEvent{Object: pv} + Expect(pred.Create(e)).To(BeTrue()) + }) + + It("returns false for unbound PV", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + ClaimRef: &corev1.ObjectReference{Name: pvcName, Namespace: testNamespace}, + }, + Status: corev1.PersistentVolumeStatus{ + Phase: corev1.VolumeAvailable, + }, + } + e := event.CreateEvent{Object: pv} + Expect(pred.Create(e)).To(BeFalse()) + }) + + It("returns false for PV without claimRef", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Status: corev1.PersistentVolumeStatus{ + Phase: corev1.VolumeBound, + }, + } + e := event.CreateEvent{Object: pv} + Expect(pred.Create(e)).To(BeFalse()) + }) + + It("returns false for non-PV object", func() { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: pvcName, Namespace: testNamespace}, + } + e := event.CreateEvent{Object: pvc} + Expect(pred.Create(e)).To(BeFalse()) + }) + }) + + Describe("UpdateFunc", func() { + It("returns true for bound PV with claimRef", func() { + oldPV := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Status: corev1.PersistentVolumeStatus{ + Phase: corev1.VolumeAvailable, + }, + } + newPV := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + ClaimRef: &corev1.ObjectReference{Name: pvcName, Namespace: testNamespace}, + }, + Status: corev1.PersistentVolumeStatus{ + Phase: corev1.VolumeBound, + }, + } + e := event.UpdateEvent{ObjectOld: oldPV, ObjectNew: newPV} + Expect(pred.Update(e)).To(BeTrue()) + }) + + It("returns false for non-PV object", func() { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: pvcName, Namespace: testNamespace}, + } + e := event.UpdateEvent{ObjectOld: pvc, ObjectNew: pvc} + Expect(pred.Update(e)).To(BeFalse()) + }) + }) + + Describe("DeleteFunc", func() { + It("always returns false", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + } + e := event.DeleteEvent{Object: pv} + Expect(pred.Delete(e)).To(BeFalse()) + }) + }) + }) + + Describe("documentDBReclaimPolicyPredicate", func() { + var pred predicate.Predicate + + BeforeEach(func() { + pred = documentDBReclaimPolicyPredicate() + }) + + Describe("UpdateFunc", func() { + It("returns true when reclaim policy changes", func() { + oldDB := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Delete", + }, + }, + }, + } + newDB := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + } + e := event.UpdateEvent{ObjectOld: oldDB, ObjectNew: newDB} + Expect(pred.Update(e)).To(BeTrue()) + }) + + It("returns false when reclaim policy unchanged", func() { + oldDB := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + } + newDB := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + } + e := event.UpdateEvent{ObjectOld: oldDB, ObjectNew: newDB} + Expect(pred.Update(e)).To(BeFalse()) + }) + + It("returns false for non-DocumentDB objects", func() { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: pvcName, Namespace: testNamespace}, + } + e := event.UpdateEvent{ObjectOld: pvc, ObjectNew: pvc} + Expect(pred.Update(e)).To(BeFalse()) + }) + }) + + Describe("CreateFunc", func() { + It("always returns false", func() { + db := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + } + e := event.CreateEvent{Object: db} + Expect(pred.Create(e)).To(BeFalse()) + }) + }) + + Describe("DeleteFunc", func() { + It("always returns false", func() { + db := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + } + e := event.DeleteEvent{Object: db} + Expect(pred.Delete(e)).To(BeFalse()) + }) + }) + }) + + Describe("findDocumentDBForPV", func() { + It("returns nil when PV has no claimRef", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result, err := reconciler.findDocumentDBForPV(ctx, pv) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("returns nil when PVC is not found", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + ClaimRef: &corev1.ObjectReference{ + Name: "non-existent-pvc", + Namespace: testNamespace, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result, err := reconciler.findDocumentDBForPV(ctx, pv) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("returns nil when PVC has no CNPG Cluster owner", func() { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: testNamespace, + }, + } + + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + ClaimRef: &corev1.ObjectReference{ + Name: pvcName, + Namespace: testNamespace, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(pvc). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result, err := reconciler.findDocumentDBForPV(ctx, pv) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("returns DocumentDB when full ownership chain exists", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentdbName, + Namespace: testNamespace, + UID: "documentdb-uid", + }, + } + + trueVal := true + cluster := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: testNamespace, + UID: "cluster-uid", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "documentdb.io/v1", + Kind: "DocumentDB", + Name: documentdbName, + UID: "documentdb-uid", + Controller: &trueVal, + }, + }, + }, + } + + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "postgresql.cnpg.io/v1", + Kind: "Cluster", + Name: clusterName, + UID: "cluster-uid", + Controller: &trueVal, + }, + }, + }, + } + + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName}, + Spec: corev1.PersistentVolumeSpec{ + ClaimRef: &corev1.ObjectReference{ + Name: pvcName, + Namespace: testNamespace, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb, cluster, pvc). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result, err := reconciler.findDocumentDBForPV(ctx, pv) + Expect(err).ToNot(HaveOccurred()) + Expect(result).ToNot(BeNil()) + Expect(result.Name).To(Equal(documentdbName)) + }) + }) + + Describe("findCNPGClusterOwner", func() { + It("returns nil when PVC has no owner references", func() { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: testNamespace, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result := reconciler.findCNPGClusterOwner(ctx, pvc) + Expect(result).To(BeNil()) + }) + + It("returns nil when owner is not a CNPG Cluster", func() { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "apps/v1", + Kind: "StatefulSet", + Name: "some-statefulset", + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result := reconciler.findCNPGClusterOwner(ctx, pvc) + Expect(result).To(BeNil()) + }) + + It("returns cluster when PVC is owned by CNPG Cluster", func() { + cluster := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: testNamespace, + UID: "cluster-uid", + }, + } + + trueVal := true + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "postgresql.cnpg.io/v1", + Kind: "Cluster", + Name: clusterName, + UID: "cluster-uid", + Controller: &trueVal, + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(cluster). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result := reconciler.findCNPGClusterOwner(ctx, pvc) + Expect(result).ToNot(BeNil()) + Expect(result.Name).To(Equal(clusterName)) + }) + + It("returns nil and continues when Get CNPG Cluster fails with non-NotFound error", func() { + trueVal := true + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "postgresql.cnpg.io/v1", + Kind: "Cluster", + Name: clusterName, + UID: "cluster-uid", + Controller: &trueVal, + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*cnpgv1.Cluster); ok { + return fmt.Errorf("api timeout") + } + return client.Get(ctx, key, obj, opts...) + }, + }). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + // Should return nil and continue (error is logged but not returned) + result := reconciler.findCNPGClusterOwner(ctx, pvc) + Expect(result).To(BeNil()) + }) + }) + + Describe("findDocumentDBOwner", func() { + It("returns nil when cluster has no owner references", func() { + cluster := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: testNamespace, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result := reconciler.findDocumentDBOwner(ctx, cluster) + Expect(result).To(BeNil()) + }) + + It("returns nil when owner is not DocumentDB", func() { + cluster := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "apps/v1", + Kind: "Deployment", + Name: "some-deployment", + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result := reconciler.findDocumentDBOwner(ctx, cluster) + Expect(result).To(BeNil()) + }) + + It("returns DocumentDB when cluster is owned by DocumentDB", func() { + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentdbName, + Namespace: testNamespace, + UID: "documentdb-uid", + }, + } + + trueVal := true + cluster := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "documentdb.io/v1", + Kind: "DocumentDB", + Name: documentdbName, + UID: "documentdb-uid", + Controller: &trueVal, + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + result := reconciler.findDocumentDBOwner(ctx, cluster) + Expect(result).ToNot(BeNil()) + Expect(result.Name).To(Equal(documentdbName)) + }) + + It("returns nil and continues when Get DocumentDB fails with non-NotFound error", func() { + trueVal := true + cluster := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "documentdb.io/v1", + Kind: "DocumentDB", + Name: documentdbName, + UID: "documentdb-uid", + Controller: &trueVal, + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*dbpreview.DocumentDB); ok { + return fmt.Errorf("api timeout") + } + return client.Get(ctx, key, obj, opts...) + }, + }). + Build() + + reconciler := &PersistentVolumeReconciler{Client: fakeClient} + + // Should return nil and continue (error is logged but not returned) + result := reconciler.findDocumentDBOwner(ctx, cluster) + Expect(result).To(BeNil()) + }) + }) +})