From c4e18bf42d53c614d07b564909097356cc15192f Mon Sep 17 00:00:00 2001 From: Brandon Palm Date: Mon, 6 Jan 2025 13:53:15 -0600 Subject: [PATCH] storage: adjust return consistency --- pkg/storage/objectbucketclaim.go | 12 ++++++++---- pkg/storage/ocs.go | 4 ++-- pkg/storage/pv.go | 4 ++-- pkg/storage/pvc.go | 29 ++++++++++++----------------- pkg/storage/pvc_test.go | 8 ++++---- 5 files changed, 28 insertions(+), 29 deletions(-) diff --git a/pkg/storage/objectbucketclaim.go b/pkg/storage/objectbucketclaim.go index b67f98bb9..99b58684d 100644 --- a/pkg/storage/objectbucketclaim.go +++ b/pkg/storage/objectbucketclaim.go @@ -59,12 +59,16 @@ func NewObjectBucketClaimBuilder( glog.V(100).Infof("The name of the objectBucketClaim is empty") builder.errorMsg = "objectBucketClaim 'name' cannot be empty" + + return builder } if nsname == "" { glog.V(100).Infof("The nsname of the objectBucketClaim is empty") builder.errorMsg = "objectBucketClaim 'nsname' cannot be empty" + + return builder } return builder @@ -88,7 +92,7 @@ func PullObjectBucketClaim(apiClient *clients.Settings, name, nsname string) (*O return nil, err } - builder := ObjectBucketClaimBuilder{ + builder := &ObjectBucketClaimBuilder{ apiClient: apiClient.Client, Definition: &noobaav1alpha1.ObjectBucketClaim{ ObjectMeta: metav1.ObjectMeta{ @@ -116,7 +120,7 @@ func PullObjectBucketClaim(apiClient *clients.Settings, name, nsname string) (*O builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Get returns objectBucketClaim object if found. @@ -138,7 +142,7 @@ func (builder *ObjectBucketClaimBuilder) Get() (*noobaav1alpha1.ObjectBucketClai return nil, err } - return objectBucketClaimObj, err + return objectBucketClaimObj, nil } // Create makes a objectBucketClaim in the cluster and stores the created object in struct. @@ -226,7 +230,7 @@ func (builder *ObjectBucketClaimBuilder) Update() (*ObjectBucketClaimBuilder, er builder.Object = builder.Definition - return builder, err + return builder, nil } // WithStorageClassName sets the objectBucketClaim operator's storageClassName configuration. diff --git a/pkg/storage/ocs.go b/pkg/storage/ocs.go index c7423d436..b060e4a58 100644 --- a/pkg/storage/ocs.go +++ b/pkg/storage/ocs.go @@ -92,7 +92,7 @@ func PullStorageCluster(apiClient *clients.Settings, name, namespace string) (*S return nil, err } - builder := StorageClusterBuilder{ + builder := &StorageClusterBuilder{ apiClient: apiClient.Client, Definition: &ocsoperatorv1.StorageCluster{ ObjectMeta: metav1.ObjectMeta{ @@ -121,7 +121,7 @@ func PullStorageCluster(apiClient *clients.Settings, name, namespace string) (*S builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Get fetches existing storageCluster from cluster. diff --git a/pkg/storage/pv.go b/pkg/storage/pv.go index 02a3a9310..2c3e8b328 100644 --- a/pkg/storage/pv.go +++ b/pkg/storage/pv.go @@ -38,7 +38,7 @@ func PullPersistentVolume(apiClient *clients.Settings, name string) (*PVBuilder, return nil, fmt.Errorf("persistentVolume 'apiClient' cannot be empty") } - builder := PVBuilder{ + builder := &PVBuilder{ apiClient: apiClient, Definition: &corev1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -59,7 +59,7 @@ func PullPersistentVolume(apiClient *clients.Settings, name string) (*PVBuilder, builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Exists checks whether the given PersistentVolume exists. diff --git a/pkg/storage/pvc.go b/pkg/storage/pvc.go index e5850d8d7..b0860505e 100644 --- a/pkg/storage/pvc.go +++ b/pkg/storage/pvc.go @@ -48,7 +48,7 @@ func NewPVCBuilder(apiClient *clients.Settings, name, nsname string) *PVCBuilder return nil } - builder := PVCBuilder{ + builder := &PVCBuilder{ Definition: &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -64,15 +64,19 @@ func NewPVCBuilder(apiClient *clients.Settings, name, nsname string) *PVCBuilder glog.V(100).Infof("PVC name is empty") builder.errorMsg = "PVC name is empty" + + return builder } if nsname == "" { glog.V(100).Infof("PVC namespace is empty") builder.errorMsg = "PVC namespace is empty" + + return builder } - return &builder + return builder } // WithPVCAccessMode configure access mode for the PV. @@ -118,15 +122,12 @@ func (builder *PVCBuilder) WithPVCCapacity(capacity string) (*PVCBuilder, error) if capacity == "" { glog.V(100).Infof("Capacity of the PersistentVolumeClaim is empty") - builder.errorMsg = "Capacity of the PersistentVolumeClaim is empty" - - return builder, fmt.Errorf(builder.errorMsg) + return builder, fmt.Errorf("capacity of the PersistentVolumeClaim is empty") } defer func() (*PVCBuilder, error) { if r := recover(); r != nil { glog.V(100).Infof("Failed to parse %v", capacity) - builder.errorMsg = fmt.Sprintf("Failed to parse: %v", capacity) return builder, fmt.Errorf("failed to parse: %v", capacity) } @@ -149,10 +150,8 @@ func (builder *PVCBuilder) WithStorageClass(storageClass string) (*PVCBuilder, e if storageClass == "" { glog.V(100).Infof("Empty storageClass requested for the PersistentVolumeClaim", storageClass) - builder.errorMsg = fmt.Sprintf("Empty storageClass requested for the PersistentVolumeClaim %s", + return builder, fmt.Errorf("empty storageClass requested for the PersistentVolumeClaim %s", builder.Definition.Name) - - return builder, fmt.Errorf(builder.errorMsg) } builder.Definition.Spec.StorageClassName = &storageClass @@ -168,19 +167,15 @@ func (builder *PVCBuilder) WithVolumeMode(volumeMode string) (*PVCBuilder, error glog.V(100).Infof(fmt.Sprintf("Empty volumeMode requested for the PersistentVolumeClaim %s in %s namespace", builder.Definition.Name, builder.Definition.Namespace)) - builder.errorMsg = fmt.Sprintf("Empty volumeMode requested for the PersistentVolumeClaim %s in %s namespace", + return builder, fmt.Errorf("empty volumeMode requested for the PersistentVolumeClaim %s in %s namespace", builder.Definition.Name, builder.Definition.Namespace) - - return builder, fmt.Errorf(builder.errorMsg) } if !validateVolumeMode(volumeMode) { glog.V(100).Infof(fmt.Sprintf("Unsupported VolumeMode: %s", volumeMode)) - builder.errorMsg = fmt.Sprintf("Unsupported VolumeMode %q requested for %s PersistentVolumeClaim in %s namespace", + return builder, fmt.Errorf("unsupported VolumeMode %q requested for %s PersistentVolumeClaim in %s namespace", volumeMode, builder.Definition.Name, builder.Definition.Name) - - return builder, fmt.Errorf(builder.errorMsg) } // volumeMode is string while Spec.VolumeMode requires pointer to corev1.PersistentVolumeMode, @@ -295,7 +290,7 @@ func PullPersistentVolumeClaim( return nil, fmt.Errorf("persistentVolumeClaim 'apiClient' cannot be empty") } - builder := PVCBuilder{ + builder := &PVCBuilder{ apiClient: apiClient, Definition: &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ @@ -324,7 +319,7 @@ func PullPersistentVolumeClaim( builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Exists checks whether the given PersistentVolumeClaim exists. diff --git a/pkg/storage/pvc_test.go b/pkg/storage/pvc_test.go index b2730a0e6..2fd41e8ba 100644 --- a/pkg/storage/pvc_test.go +++ b/pkg/storage/pvc_test.go @@ -290,12 +290,12 @@ func TestPersistentVolumeClaimWithVolumeMode(t *testing.T) { }, { testPVC: "Disk", - expectedError: "Unsupported VolumeMode \"Disk\" requested for persistentvolumeclaim-test " + + expectedError: "unsupported VolumeMode \"Disk\" requested for persistentvolumeclaim-test " + "PersistentVolumeClaim in persistentvolumeclaim-test namespace", }, { testPVC: "", - expectedError: "Empty volumeMode requested for the PersistentVolumeClaim persistentvolumeclaim-test " + + expectedError: "empty volumeMode requested for the PersistentVolumeClaim persistentvolumeclaim-test " + "in persistentvolumeclaim-namespace namespace", }, } @@ -326,7 +326,7 @@ func TestPersistentVolumeClaimWithStorageClass(t *testing.T) { }, { testStorageClass: "", - expectedError: "Empty storageClass requested for the PersistentVolumeClaim persistentvolumeclaim-test", + expectedError: "empty storageClass requested for the PersistentVolumeClaim persistentvolumeclaim-test", }, } @@ -356,7 +356,7 @@ func TestPersistentVolumeClaimWithPVCCapacity(t *testing.T) { }, { testCapacity: "", - expectedError: "Capacity of the PersistentVolumeClaim is empty", + expectedError: "capacity of the PersistentVolumeClaim is empty", }, }