From 8bb4f603e65b4c6f327dccc8f8c93fa56ec60cce Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Thu, 21 May 2020 11:22:22 -0600 Subject: [PATCH] ceph: default the device set template name to data In 1.3 we added support for the metadata pvcs, thus requiring the name to be set on the templates to differentiate between data and metadata. For backward compatibility we need to assume the template is for data if it is not set. Signed-off-by: Travis Nielsen --- pkg/operator/ceph/cluster/osd/deviceSet.go | 9 ++- .../ceph/cluster/osd/deviceset_test.go | 65 +++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 pkg/operator/ceph/cluster/osd/deviceset_test.go diff --git a/pkg/operator/ceph/cluster/osd/deviceSet.go b/pkg/operator/ceph/cluster/osd/deviceSet.go index ec6dab286..27ee94686 100644 --- a/pkg/operator/ceph/cluster/osd/deviceSet.go +++ b/pkg/operator/ceph/cluster/osd/deviceSet.go @@ -40,6 +40,11 @@ func (c *Cluster) prepareStorageClassDeviceSets(config *provisionConfig) []rookv continue } for _, pvcTemplate := range storageClassDeviceSet.VolumeClaimTemplates { + if pvcTemplate.Name == "" { + // For backward compatibility a blank name must be treated as a data volume + pvcTemplate.Name = bluestorePVCBlock + } + pvc, err := c.createStorageClassDeviceSetPVC(storageClassDeviceSet.Name, pvcTemplate, i) if err != nil { config.addError("failed to create osd for storageClassDeviceSet %q for count %d. %v", storageClassDeviceSet.Name, i, err) @@ -73,7 +78,7 @@ func (c *Cluster) createStorageClassDeviceSetPVC(storageClassDeviceSetName strin pvc := makeStorageClassDeviceSetPVC(storageClassDeviceSetName, pvcStorageClassDeviceSetPVCId, setIndex, pvcTemplate) oldPresentPVCs, err := c.context.Clientset.CoreV1().PersistentVolumeClaims(c.Namespace).List(metav1.ListOptions{LabelSelector: pvcStorageClassDeviceSetPVCIdLabelSelector}) if err != nil { - return nil, errors.Wrapf(err, "failed to create pvc %s for storageClassDeviceSet %s", pvc.GetGenerateName(), storageClassDeviceSetName) + return nil, errors.Wrapf(err, "failed to list pvc %s for storageClassDeviceSet %s", pvcStorageClassDeviceSetPVCIdLabelSelector, storageClassDeviceSetName) } // return old labelled pvc, if we find any @@ -87,7 +92,7 @@ func (c *Cluster) createStorageClassDeviceSetPVC(storageClassDeviceSetName strin pvc = makeStorageClassDeviceSetPVC(storageClassDeviceSetName, pvcStorageClassDeviceSetPVCId, setIndex, pvcTemplate) presentPVCs, err := c.context.Clientset.CoreV1().PersistentVolumeClaims(c.Namespace).List(metav1.ListOptions{LabelSelector: pvcStorageClassDeviceSetPVCIdLabelSelector}) if err != nil { - return nil, errors.Wrapf(err, "failed to create pvc %s for storageClassDeviceSet %s", pvc.GetGenerateName(), storageClassDeviceSetName) + return nil, errors.Wrapf(err, "failed to list pvc %s for storageClassDeviceSet %s", pvcStorageClassDeviceSetPVCIdLabelSelector, storageClassDeviceSetName) } presentPVCsNum := len(presentPVCs.Items) diff --git a/pkg/operator/ceph/cluster/osd/deviceset_test.go b/pkg/operator/ceph/cluster/osd/deviceset_test.go new file mode 100644 index 000000000..bb026291f --- /dev/null +++ b/pkg/operator/ceph/cluster/osd/deviceset_test.go @@ -0,0 +1,65 @@ +/* +Copyright 2020 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package osd + +import ( + "testing" + + rookv1 "github.com/rook/rook/pkg/apis/rook.io/v1" + "github.com/rook/rook/pkg/clusterd" + testexec "github.com/rook/rook/pkg/operator/test" + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestPrepareDeviceSets(t *testing.T) { + clientset := testexec.New(t, 1) + context := &clusterd.Context{ + Clientset: clientset, + } + storageClass := "mysource" + claim := v1.PersistentVolumeClaim{Spec: v1.PersistentVolumeClaimSpec{ + StorageClassName: &storageClass, + }} + deviceSet := rookv1.StorageClassDeviceSet{ + Name: "mydata", + Count: 1, + Portable: true, + VolumeClaimTemplates: []v1.PersistentVolumeClaim{claim}, + } + desired := rookv1.StorageScopeSpec{StorageClassDeviceSets: []rookv1.StorageClassDeviceSet{deviceSet}} + cluster := &Cluster{ + context: context, + DesiredStorage: desired, + Namespace: "testns", + } + + config := &provisionConfig{} + volumeSources := cluster.prepareStorageClassDeviceSets(config) + assert.Equal(t, 1, len(volumeSources)) + assert.Equal(t, 0, len(config.errorMessages)) + assert.Equal(t, "mydata", volumeSources[0].Name) + assert.Equal(t, "data", volumeSources[0].Type) + assert.True(t, volumeSources[0].Portable) + + // Verify that the PVC has the expected generated name with the default of "data" in the name + pvcs, err := clientset.CoreV1().PersistentVolumeClaims(cluster.Namespace).List(metav1.ListOptions{}) + assert.NoError(t, err) + assert.Equal(t, 1, len(pvcs.Items)) + assert.Equal(t, "mydata-data-0-", pvcs.Items[0].GenerateName) + assert.Equal(t, cluster.Namespace, pvcs.Items[0].Namespace) +}