diff --git a/pkg/controller/elasticsearch/nodespec/resources.go b/pkg/controller/elasticsearch/nodespec/resources.go index 26c61220b6f..36a16c6d4c2 100644 --- a/pkg/controller/elasticsearch/nodespec/resources.go +++ b/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) diff --git a/pkg/controller/elasticsearch/nodespec/resources_test.go b/pkg/controller/elasticsearch/nodespec/resources_test.go index 4964a2c8ed1..6c012be765c 100644 --- a/pkg/controller/elasticsearch/nodespec/resources_test.go +++ b/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) + } + }) + } +} diff --git a/pkg/controller/elasticsearch/nodespec/statefulset.go b/pkg/controller/elasticsearch/nodespec/statefulset.go index 2a5c209bdd1..99630417596 100644 --- a/pkg/controller/elasticsearch/nodespec/statefulset.go +++ b/pkg/controller/elasticsearch/nodespec/statefulset.go @@ -16,7 +16,6 @@ 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" @@ -24,6 +23,10 @@ import ( "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) {