From fb144bf0d1699f858c7c0be1baff359c99ba5a69 Mon Sep 17 00:00:00 2001 From: Michael Morello Date: Tue, 11 Jun 2019 15:13:31 +0200 Subject: [PATCH 1/2] standard may not be the default --- operators/pkg/controller/elasticsearch/pvc/pvc.go | 9 ++++----- .../pkg/controller/elasticsearch/pvc/pvc_test.go | 14 +++++++------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/operators/pkg/controller/elasticsearch/pvc/pvc.go b/operators/pkg/controller/elasticsearch/pvc/pvc.go index eb99fc45db..4805ef3ecd 100644 --- a/operators/pkg/controller/elasticsearch/pvc/pvc.go +++ b/operators/pkg/controller/elasticsearch/pvc/pvc.go @@ -18,7 +18,6 @@ import ( var ( log = logf.Log.WithName("pvc") - standardStorageClassname = "standard" ErrNotNodeNameLabelNotFound = errors.New("node name not found as a label on the PVC") ) @@ -107,11 +106,11 @@ func compareResources(claim, candidate *corev1.PersistentVolumeClaim) bool { } func compareStorageClass(claim, candidate *corev1.PersistentVolumeClaim) bool { - if claim.Spec.StorageClassName != nil { - return reflect.DeepEqual(claim.Spec.StorageClassName, candidate.Spec.StorageClassName) + if claim.Spec.StorageClassName == nil { + // volumeClaimTemplate has no storageClass set + return true } - // No storage class name in the claim, only match if the claim is a standard storage class - return standardStorageClassname == *candidate.Spec.StorageClassName + return reflect.DeepEqual(claim.Spec.StorageClassName, candidate.Spec.StorageClassName) } // compare two maps but ignore the label.PodNameLabelName key diff --git a/operators/pkg/controller/elasticsearch/pvc/pvc_test.go b/operators/pkg/controller/elasticsearch/pvc/pvc_test.go index 4200ecd000..119f99a95d 100644 --- a/operators/pkg/controller/elasticsearch/pvc/pvc_test.go +++ b/operators/pkg/controller/elasticsearch/pvc/pvc_test.go @@ -203,14 +203,14 @@ func TestOrphanedPersistentVolumeClaims_FindOrphanedVolumeClaim(t *testing.T) { "elasticsearch-sample-es-6bw9qkw77k-"+volume.ElasticsearchDataVolumeName, sampleLabels1, "1Gi", - &standardStorageClassname, + nil, ), *newPVC( "elasticsearch-sample-es-6qg4hmd9dj", "elasticsearch-sample-es-6qg4hmd9dj-"+volume.ElasticsearchDataVolumeName, sampleLabels1, "1Gi", - &standardStorageClassname, + nil, ), }}, args: args{ @@ -230,7 +230,7 @@ func TestOrphanedPersistentVolumeClaims_FindOrphanedVolumeClaim(t *testing.T) { "elasticsearch-sample-es-6bw9qkw77k-"+volume.ElasticsearchDataVolumeName, sampleLabels1, "1Gi", - &standardStorageClassname, + nil, ), }, { name: "Labels mismatch", @@ -241,14 +241,14 @@ func TestOrphanedPersistentVolumeClaims_FindOrphanedVolumeClaim(t *testing.T) { "elasticsearch-sample-es-6bw9qkw77k-"+volume.ElasticsearchDataVolumeName, sampleLabels2, "1Gi", - &standardStorageClassname, + nil, ), *newPVC( "elasticsearch-sample-es-6qg4hmd9dj", "elasticsearch-sample-es-6qg4hmd9dj-"+volume.ElasticsearchDataVolumeName, sampleLabels2, "1Gi", - &standardStorageClassname, + nil, ), }}, args: args{ @@ -313,14 +313,14 @@ func TestOrphanedPersistentVolumeClaims_FindOrphanedVolumeClaim(t *testing.T) { "elasticsearch-sample-es-6bw9qkw77k-"+volume.ElasticsearchDataVolumeName, sampleLabels1, "1Gi", - &standardStorageClassname, + nil, ), *newPVC( "elasticsearch-sample-es-6qg4hmd9dj", "elasticsearch-sample-es-6qg4hmd9dj-"+volume.ElasticsearchDataVolumeName, sampleLabels1, "1Gi", - &standardStorageClassname, + nil, ), }}, args: args{ From 80dbe03676a7007eb46eb9d6142dc6dcbfcf2d5a Mon Sep 17 00:00:00 2001 From: Michael Morello Date: Wed, 12 Jun 2019 16:52:08 +0200 Subject: [PATCH 2/2] change from review --- operators/pkg/controller/elasticsearch/pvc/pvc.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/operators/pkg/controller/elasticsearch/pvc/pvc.go b/operators/pkg/controller/elasticsearch/pvc/pvc.go index 4805ef3ecd..a05eda97cf 100644 --- a/operators/pkg/controller/elasticsearch/pvc/pvc.go +++ b/operators/pkg/controller/elasticsearch/pvc/pvc.go @@ -107,7 +107,9 @@ func compareResources(claim, candidate *corev1.PersistentVolumeClaim) bool { func compareStorageClass(claim, candidate *corev1.PersistentVolumeClaim) bool { if claim.Spec.StorageClassName == nil { - // volumeClaimTemplate has no storageClass set + // volumeClaimTemplate has no storageClass set: it should use the k8s cluster default + // since we don't know that default, we fallback to reusing any available volume + // from the same cluster (whatever the storage class actually is) return true } return reflect.DeepEqual(claim.Spec.StorageClassName, candidate.Spec.StorageClassName)