Of course this can be improved and simplified.
Handling 12123123 err checks is a problem of avoiding short circuit failures. That's fine, but you have multiple return values, so there's no reason not to leverage that properly...or even consistently. The whole file is littered with waiting assignment until a comparison, except in some cases or where if and elseif statements use the return values inline but not other places. It's just random.
//checkVolumeSatisfyClaim checks if the volume requested by the claim satisfies the requirements of the claim
func checkVolumeSatisfyClaim(volume *v1.PersistentVolume, claim *v1.PersistentVolumeClaim) error {
// check if PV's DeletionTimeStamp is set, if so, return error.
if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) {
if volume.ObjectMeta.DeletionTimestamp != nil {
return fmt.Errorf("the volume is marked for deletion")
}
}
volumeSize := volume.Spec.Capacity[v1.ResourceStorage].Value()
requestedSize := claim.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)].Value()
if volumeSize < requestedSize {
return fmt.Errorf("requested PV is too small")
}elseif v1helper.GetPersistentVolumeClass(volume) != v1helper.GetPersistentVolumeClaimClass(claim) {
return fmt.Errorf("storageClassName does not match")
}
// function here, obviously
err = volumeAccessModeChecks(volume, claim)
return err
}
// Here's the function
func volumeAccessModeChecks(volume *v1.PersistentVolume, claim *v1.PersistentVolumeClaim) error {
// even the naming sucks. DISTINCT case error name
isMismatch, volumeModeErr := checkVolumeModeMismatches(&claim.Spec, &volume.Spec)
if err != nil {
return fmt.Errorf("error checking volumeMode: %v", volumeModeErr)
}elseif isMismatch {
return fmt.Errorf("incompatible volumeMode")
}elseif !checkAccessModes(claim, volume) {
return fmt.Errorf("incompatible accessMode")
}
return nil
}
This whole function is already a separated function from the middle of the file, no reason to stop there. No need for separate files when the function calls are going to be distinct and inline beneath their usage.
