Set BlockOwnerDeletion to false on PVCs

pkg/controller/elasticsearch/nodespec/resources.go

@@ -37,7 +37,12 @@ func (l ResourcesList) StatefulSets() sset.StatefulSetList {
 	return ssetList
 }
 
-func BuildExpectedResources(es v1beta1.Elasticsearch, keystoreResources *keystore.Resources, scheme *runtime.Scheme, certResources *certificates.CertificateResources) (ResourcesList, error) {
+func BuildExpectedResources(
+	es v1beta1.Elasticsearch,
+	keystoreResources *keystore.Resources,
+	scheme *runtime.Scheme,
+	certResources *certificates.CertificateResources,
+) (ResourcesList, error) {
 	nodesResources := make(ResourcesList, 0, len(es.Spec.NodeSets))
 
 	ver, err := version.Parse(es.Spec.Version)

pkg/controller/elasticsearch/nodespec/resources_test.go

@@ -8,7 +8,13 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1beta1"
 	"github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/sset"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes/scheme"
 )
 
 func TestResourcesList_MasterNodesNames(t *testing.T) {
@@ -43,3 +49,69 @@ func TestResourcesList_MasterNodesNames(t *testing.T) {
 		})
 	}
 }
+
+func TestSetVolumeClaimsControllerReference(t *testing.T) {
+	es := v1beta1.Elasticsearch{
+		ObjectMeta: v1.ObjectMeta{
+			Name:      "es1",
+			Namespace: "default",
+			UID:       "ABCDEF",
+		},
+	}
+	require.NoError(t, v1beta1.AddToScheme(scheme.Scheme))
+	type args struct {
+		volumeClaims []corev1.PersistentVolumeClaim
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    []string
+		wantErr bool
+	}{
+		{
+			name: "Simple test case",
+			args: args{
+				volumeClaims: []corev1.PersistentVolumeClaim{
+					{ObjectMeta: v1.ObjectMeta{Name: "elasticsearch-data"}},
+				},
+			},
+			want: []string{"elasticsearch-data"},
+		},
+		{
+			name: "With a user volume",
+			args: args{
+				volumeClaims: []corev1.PersistentVolumeClaim{
+					{ObjectMeta: v1.ObjectMeta{Name: "elasticsearch-data"}},
+					{ObjectMeta: v1.ObjectMeta{Name: "user-volume"}},
+				},
+			},
+			want: []string{"elasticsearch-data", "user-volume"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := setVolumeClaimsControllerReference(tt.args.volumeClaims, es, scheme.Scheme)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("BuildExpectedResources() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			assert.Equal(t, len(tt.want), len(got))
+
+			// Extract PVC names
+			actualPVCs := make([]string, len(got))
+			for i := range got {
+				actualPVCs[i] = got[i].Name
+			}
+			// Check the number of PVCs we got
+			assert.ElementsMatch(t, tt.want, actualPVCs)
+
+			// Check that VolumeClaimTemplates have an owner with the right settings
+			for _, pvc := range got {
+				assert.Equal(t, 1, len(pvc.OwnerReferences))
+				ownerRef := pvc.OwnerReferences[0]
+				require.False(t, *ownerRef.BlockOwnerDeletion)
+				assert.Equal(t, es.UID, ownerRef.UID)
+			}
+		})
+	}
+}

pkg/controller/elasticsearch/nodespec/statefulset.go

@@ -16,14 +16,17 @@ import (
 	"github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/settings"
 	esvolume "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/volume"
 	"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
-
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 )
 
+var (
+	f = false
+)
+
 // HeadlessServiceName returns the name of the headless service for the given StatefulSet.
 func HeadlessServiceName(ssetName string) string {
 	// just use the sset name
@@ -75,14 +78,9 @@ func BuildStatefulSet(
 		ssetLabels[k] = v
 	}
 
-	// set the owner reference of all volume claims to the ES resource,
-	// so PVC get deleted automatically upon Elasticsearch resource deletion
-	claims := make([]corev1.PersistentVolumeClaim, 0, len(nodeSet.VolumeClaimTemplates))
-	for _, claim := range nodeSet.VolumeClaimTemplates {
-		if err := controllerutil.SetControllerReference(&es, &claim, scheme); err != nil {
-			return appsv1.StatefulSet{}, err
-		}
-		claims = append(claims, claim)
+	claims, err := setVolumeClaimsControllerReference(nodeSet.VolumeClaimTemplates, es, scheme)
+	if err != nil {
+		return appsv1.StatefulSet{}, err
 	}
 
 	sset := appsv1.StatefulSet{
@@ -118,6 +116,30 @@ func BuildStatefulSet(
 	return sset, nil
 }
 
+func setVolumeClaimsControllerReference(
+	persistentVolumeClaims []corev1.PersistentVolumeClaim,
+	es v1beta1.Elasticsearch,
+	scheme *runtime.Scheme,
+) ([]corev1.PersistentVolumeClaim, error) {
+	// set the owner reference of all volume claims to the ES resource,
+	// so PVC get deleted automatically upon Elasticsearch resource deletion
+	claims := make([]corev1.PersistentVolumeClaim, 0, len(persistentVolumeClaims))
+	for _, claim := range persistentVolumeClaims {
+		if err := controllerutil.SetControllerReference(&es, &claim, scheme); err != nil {
+			return nil, err
+		}
+		// Set block owner deletion to false as the statefulset controller might not be able to do that if it cannot
+		// set finalizers on the resource.
+		// See https://github.com/elastic/cloud-on-k8s/issues/1884
+		refs := claim.OwnerReferences
+		for i := range refs {
+			refs[i].BlockOwnerDeletion = &f
+		}
+		claims = append(claims, claim)
+	}
+	return claims, nil
+}
+
 // UpdateReplicas updates the given StatefulSet with the given replicas,
 // and modifies the template hash label accordingly.
 func UpdateReplicas(statefulSet *appsv1.StatefulSet, replicas *int32) {