fix: PersistentVolumeClaim are not cleaned up after cluster deletion

Author: teoincontatto

stackgres-k8s/e2e/utils/kubernetes

@@ -2,7 +2,7 @@
 
 export E2E_ENV="${E2E_ENV:-kind}"
 export KUBECONFIG="${KUBECONFIG:-$HOME/.kube/config}"
-export DEFAULT_K8S_VERSION="1.24"
+export DEFAULT_K8S_VERSION="1.34"
 export K8S_VERSION="${K8S_VERSION:-$DEFAULT_K8S_VERSION}"
 export KUBERNETES_VERSION_NUMBER
 # When DEBUG is set kubectl output debug messages

…tion/IgnorePodReconciliationHandler.java …liation/IgnoreReconciliationHandler.javastackgres-k8s/src/operator/src/main/java/io/stackgres/operator/conciliation/IgnorePodReconciliationHandler.java renamed to stackgres-k8s/src/operator/src/main/java/io/stackgres/operator/conciliation/IgnoreReconciliationHandler.java stackgres-k8s/src/operator/src/main/java/io/stackgres/operator/conciliation/IgnorePodReconciliationHandler.java renamed to stackgres-k8s/src/operator/src/main/java/io/stackgres/operator/conciliation/IgnoreReconciliationHandler.java

@@ -10,15 +10,16 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public abstract class IgnorePodReconciliationHandler<T extends CustomResource<?, ?>>
+public abstract class IgnoreReconciliationHandler<T extends CustomResource<?, ?>>
     implements ReconciliationHandler<T> {
 
   protected static final Logger LOGGER =
-      LoggerFactory.getLogger(IgnorePodReconciliationHandler.class);
+      LoggerFactory.getLogger(IgnoreReconciliationHandler.class);
 
   @Override
   public HasMetadata create(T context, HasMetadata resource) {
-    LOGGER.debug("Skipping creating Pod {}.{}",
+    LOGGER.debug("Skipping creating {} {}.{}",
+        resource.getKind(),
         resource.getMetadata().getNamespace(),
         resource.getMetadata().getName());
     return resource;
@@ -27,30 +28,34 @@ public HasMetadata create(T context, HasMetadata resource) {
   @Override
   public HasMetadata patch(T context, HasMetadata newResource,
       HasMetadata oldResource) {
-    LOGGER.debug("Skipping patching Pod {}.{}",
+    LOGGER.debug("Skipping patching {} {}.{}",
+        oldResource.getKind(),
         oldResource.getMetadata().getNamespace(),
         oldResource.getMetadata().getName());
     return oldResource;
   }
 
   @Override
   public HasMetadata replace(T context, HasMetadata resource) {
-    LOGGER.warn("Skipping replacing Pod {}.{}",
+    LOGGER.warn("Skipping replacing {} {}.{}",
+        resource.getKind(),
         resource.getMetadata().getNamespace(),
         resource.getMetadata().getName());
     return resource;
   }
 
   @Override
   public void delete(T context, HasMetadata resource) {
-    LOGGER.debug("Skipping deleting Pod {}.{}",
+    LOGGER.debug("Skipping deleting {} {}.{}",
+        resource.getKind(),
         resource.getMetadata().getNamespace(),
         resource.getMetadata().getName());
   }
 
   @Override
   public void deleteWithOrphans(T context, HasMetadata resource) {
-    LOGGER.debug("Skipping deleting Pod {}.{}",
+    LOGGER.debug("Skipping deleting {} {}.{}",
+        resource.getKind(),
         resource.getMetadata().getNamespace(),
         resource.getMetadata().getName());
   }

stackgres-k8s/src/operator/src/main/java/io/stackgres/operator/conciliation/cluster/ClusterConciliator.java

@@ -12,6 +12,8 @@
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.ObjectMeta;
+import io.fabric8.kubernetes.api.model.OwnerReference;
+import io.fabric8.kubernetes.api.model.PersistentVolumeClaim;
 import io.fabric8.kubernetes.api.model.Pod;
 import io.fabric8.kubernetes.api.model.apps.StatefulSet;
 import io.fabric8.kubernetes.client.KubernetesClient;
@@ -35,6 +37,7 @@
 import io.stackgres.operator.conciliation.DeployedResource;
 import io.stackgres.operator.conciliation.DeployedResourcesCache;
 import io.stackgres.operator.conciliation.RequiredResourceGenerator;
+import io.stackgres.operatorframework.resource.ResourceUtil;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.inject.Inject;
 
@@ -111,6 +114,9 @@ protected boolean forceChange(HasMetadata requiredResource, StackGresCluster con
             config.getMetadata().getNamespace(),
             config.getMetadata().getName());
       }
+      if (noPrimaryPod) {
+        return true;
+      }
       final boolean anyPodWithWrongOrMissingRole;
       if (!isPatroniOnKubernetes) {
         anyPodWithWrongOrMissingRole = deployedResourcesCache
@@ -126,6 +132,9 @@ protected boolean forceChange(HasMetadata requiredResource, StackGresCluster con
             config.getMetadata().getNamespace(),
             config.getMetadata().getName());
       }
+      if (anyPodWithWrongOrMissingRole) {
+        return true;
+      }
       final boolean anyPodCanRestart;
       if (ClusterRolloutUtil.isRolloutAllowed(config)) {
         anyPodCanRestart = Optional.of(config)
@@ -142,6 +151,9 @@ protected boolean forceChange(HasMetadata requiredResource, StackGresCluster con
             config.getMetadata().getNamespace(),
             config.getMetadata().getName());
       }
+      if (anyPodCanRestart) {
+        return true;
+      }
       final boolean podsCountMismatch = config.getSpec().getInstances()
           != deployedResourcesCache
               .stream()
@@ -154,7 +166,24 @@ protected boolean forceChange(HasMetadata requiredResource, StackGresCluster con
             config.getMetadata().getNamespace(),
             config.getMetadata().getName());
       }
-      return noPrimaryPod || anyPodWithWrongOrMissingRole || anyPodCanRestart || podsCountMismatch;
+      if (podsCountMismatch) {
+        return true;
+      }
+      final OwnerReference clusterOwnerReference = ResourceUtil.getOwnerReference(config);
+      final boolean anyPodOrPvcWithMissingOwner = deployedResourcesCache
+          .stream()
+          .map(DeployedResource::foundDeployed)
+          .anyMatch(foundDeployedResource -> isPodOrPvcWithMissingOwner(
+              foundDeployedResource, clusterOwnerReference));
+      if (anyPodOrPvcWithMissingOwner && LOGGER.isDebugEnabled()) {
+        LOGGER.debug("Will force StatefulSet reconciliation since a pod or pvc is"
+            + " missing owner reference for SGCluster {}.{}",
+            config.getMetadata().getNamespace(),
+            config.getMetadata().getName());
+      }
+      if (anyPodOrPvcWithMissingOwner) {
+        return true;
+      }
     }
     return false;
   }
@@ -191,4 +220,18 @@ private boolean isPodWithWrongOrMissingRole(
         .isPresent();
   }
 
+  private boolean isPodOrPvcWithMissingOwner(
+      HasMetadata foundDeployedResource,
+      OwnerReference clusterOwnerReference) {
+    return (foundDeployedResource instanceof Pod
+        || foundDeployedResource instanceof PersistentVolumeClaim)
+        && !Optional.of(foundDeployedResource.getMetadata())
+        .map(ObjectMeta::getOwnerReferences)
+        .stream()
+        .flatMap(List::stream)
+        .anyMatch(ownerReference -> Objects.equals(
+            clusterOwnerReference,
+            ownerReference));
+  }
+
 }

stackgres-k8s/src/operator/src/main/java/io/stackgres/operator/conciliation/cluster/ClusterDeployedResourceScanner.java

@@ -14,6 +14,7 @@
 import io.fabric8.kubernetes.api.model.Endpoints;
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.KubernetesResourceList;
+import io.fabric8.kubernetes.api.model.PersistentVolumeClaim;
 import io.fabric8.kubernetes.api.model.Pod;
 import io.fabric8.kubernetes.api.model.Secret;
 import io.fabric8.kubernetes.api.model.Service;
@@ -144,6 +145,7 @@ protected KubernetesClient getClient() {
           Map.entry(Endpoints.class, KubernetesClient::endpoints),
           Map.entry(Service.class, KubernetesClient::services),
           Map.entry(Pod.class, client -> client.pods()),
+          Map.entry(PersistentVolumeClaim.class, client -> client.persistentVolumeClaims()),
           Map.entry(Job.class, client -> client.batch().v1().jobs()),
           Map.entry(CronJob.class, client -> client.batch().v1().cronjobs()),
           Map.entry(StatefulSet.class, client -> client.apps().statefulSets()),

stackgres-k8s/src/operator/src/main/java/io/stackgres/operator/conciliation/cluster/ClusterPodReconciliationHandler.java

@@ -6,13 +6,13 @@
 package io.stackgres.operator.conciliation.cluster;
 
 import io.stackgres.common.crd.sgcluster.StackGresCluster;
-import io.stackgres.operator.conciliation.IgnorePodReconciliationHandler;
+import io.stackgres.operator.conciliation.IgnoreReconciliationHandler;
 import io.stackgres.operator.conciliation.ReconciliationScope;
 import jakarta.enterprise.context.ApplicationScoped;
 
 @ReconciliationScope(value = StackGresCluster.class, kind = "Pod")
 @ApplicationScoped
 public class ClusterPodReconciliationHandler
-    extends IgnorePodReconciliationHandler<StackGresCluster> {
+    extends IgnoreReconciliationHandler<StackGresCluster> {
 
 }

stackgres-k8s/src/operator/src/main/java/io/stackgres/operator/conciliation/cluster/ClusterPvcReconciliationHandler.java

@@ -0,0 +1,17 @@
+/*
+ * Copyright (C) 2019 OnGres, Inc.
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+package io.stackgres.operator.conciliation.cluster;
+
+import io.stackgres.common.crd.sgcluster.StackGresCluster;
+import io.stackgres.operator.conciliation.IgnoreReconciliationHandler;
+import io.stackgres.operator.conciliation.ReconciliationScope;
+import jakarta.enterprise.context.ApplicationScoped;
+
+@ReconciliationScope(value = StackGresCluster.class, kind = "PersistentVolumeClaim")
+@ApplicationScoped
+public class ClusterPvcReconciliationHandler
+    extends IgnoreReconciliationHandler<StackGresCluster> {
+}

stackgres-k8s/src/operator/src/main/java/io/stackgres/operator/conciliation/factory/cluster/ClusterStatefulSet.java

@@ -178,6 +178,10 @@ public Stream<HasMetadata> generateResource(StackGresClusterContext context) {
             .build())
         .withServiceName(name)
         .withTemplate(podTemplateSpec.getSpec())
+        .withNewPersistentVolumeClaimRetentionPolicy()
+        .withWhenDeleted("Delete")
+        .withWhenScaled("Retain")
+        .endPersistentVolumeClaimRetentionPolicy()
         .withVolumeClaimTemplates(
             new PersistentVolumeClaimBuilder()
                 .withNewMetadata()