Skip to content

Commit 22ca2fa

Browse files
committed
chore: enable early-return and superfluous-else from revive
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
1 parent 5b29a87 commit 22ca2fa

72 files changed

Lines changed: 371 additions & 513 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.golangci.yaml

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,12 @@ linters:
206206
- name: context-keys-type
207207
- name: dot-imports
208208
disabled: true
209+
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#early-return
210+
# In Go it is idiomatic to minimize nesting statements, a typical example is to avoid if-then-else constructions.
209211
- name: early-return
210212
disabled: true
211213
arguments:
212-
- "preserveScope"
214+
- preserve-scope # do not suggest refactorings that would increase variable scope
213215
- name: empty-block
214216
disabled: true
215217
- name: error-naming
@@ -220,18 +222,27 @@ linters:
220222
disabled: true
221223
- name: errorf
222224
disabled: true
225+
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#if-return
226+
# Checking if an error is nil to just after return the error or nil is redundant.
227+
- name: if-return
223228
- name: increment-decrement
229+
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#indent-error-flow
230+
# To improve the readability of code, it is recommended to reduce the indentation as much as possible.
231+
# This rule highlights redundant else-blocks that can be eliminated from the code.
224232
- name: indent-error-flow
225-
disabled: true
233+
arguments:
234+
- preserve-scope # do not suggest refactorings that would increase variable scope.
226235
- name: range
227236
- name: receiver-naming
228237
disabled: true
229238
- name: redefines-builtin-id
230239
disabled: true
240+
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#superfluous-else
241+
# To improve the readability of code, it is recommended to reduce the indentation as much as possible.
242+
# This rule highlights redundant else-blocks that can be eliminated from the code.
231243
- name: superfluous-else
232-
disabled: true
233244
arguments:
234-
- "preserveScope"
245+
- preserve-scope # do not suggest refactorings that would increase variable scope.
235246
- name: time-naming
236247
- name: unexported-return
237248
disabled: true

internal/resourcemodifiers/strategic_merge_patch.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,7 @@ func strategicPatchObject(
7474
return apierrors.NewBadRequest(err.Error())
7575
}
7676

77-
if err := applyPatchToObject(originalObjMap, patchMap, objToUpdate, schemaReferenceObj, strictErrs); err != nil {
78-
return err
79-
}
80-
return nil
77+
return applyPatchToObject(originalObjMap, patchMap, objToUpdate, schemaReferenceObj, strictErrs)
8178
}
8279

8380
// applyPatchToObject applies a strategic merge patch of <patchMap> to

internal/volume/volumes_information.go

Lines changed: 127 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -338,24 +338,24 @@ func (v *BackupVolumesInformation) generateVolumeInfoForSkippedPV() {
338338
tmpVolumeInfos := make([]*BackupVolumeInfo, 0)
339339

340340
for pvName, skippedReason := range v.SkippedPVs {
341-
if pvcPVInfo := v.pvMap.retrieve(pvName, "", ""); pvcPVInfo != nil {
342-
volumeInfo := &BackupVolumeInfo{
343-
PVCName: pvcPVInfo.PVCName,
344-
PVCNamespace: pvcPVInfo.PVCNamespace,
345-
PVName: pvName,
346-
SnapshotDataMoved: false,
347-
Skipped: true,
348-
SkippedReason: skippedReason,
349-
PVInfo: &PVInfo{
350-
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
351-
Labels: pvcPVInfo.PV.Labels,
352-
},
353-
}
354-
tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
355-
} else {
341+
pvcPVInfo := v.pvMap.retrieve(pvName, "", "")
342+
if pvcPVInfo == nil {
356343
v.logger.Warnf("Cannot find info for PV %s", pvName)
357344
continue
358345
}
346+
volumeInfo := &BackupVolumeInfo{
347+
PVCName: pvcPVInfo.PVCName,
348+
PVCNamespace: pvcPVInfo.PVCNamespace,
349+
PVName: pvName,
350+
SnapshotDataMoved: false,
351+
Skipped: true,
352+
SkippedReason: skippedReason,
353+
PVInfo: &PVInfo{
354+
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
355+
Labels: pvcPVInfo.PV.Labels,
356+
},
357+
}
358+
tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
359359
}
360360

361361
v.volumeInfos = append(v.volumeInfos, tmpVolumeInfos...)
@@ -366,32 +366,32 @@ func (v *BackupVolumesInformation) generateVolumeInfoForVeleroNativeSnapshot() {
366366
tmpVolumeInfos := make([]*BackupVolumeInfo, 0)
367367

368368
for _, nativeSnapshot := range v.NativeSnapshots {
369-
if pvcPVInfo := v.pvMap.retrieve(nativeSnapshot.Spec.PersistentVolumeName, "", ""); pvcPVInfo != nil {
370-
volumeResult := VolumeResultFailed
371-
if nativeSnapshot.Status.Phase == SnapshotPhaseCompleted {
372-
volumeResult = VolumeResultSucceeded
373-
}
374-
volumeInfo := &BackupVolumeInfo{
375-
BackupMethod: NativeSnapshot,
376-
PVCName: pvcPVInfo.PVCName,
377-
PVCNamespace: pvcPVInfo.PVCNamespace,
378-
PVName: pvcPVInfo.PV.Name,
379-
SnapshotDataMoved: false,
380-
Skipped: false,
381-
// Only set Succeeded to true when the NativeSnapshot's phase is Completed,
382-
// although NativeSnapshot doesn't check whether the snapshot creation result.
383-
Result: volumeResult,
384-
NativeSnapshotInfo: newNativeSnapshotInfo(nativeSnapshot),
385-
PVInfo: &PVInfo{
386-
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
387-
Labels: pvcPVInfo.PV.Labels,
388-
},
389-
}
390-
tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
391-
} else {
369+
pvcPVInfo := v.pvMap.retrieve(nativeSnapshot.Spec.PersistentVolumeName, "", "")
370+
if pvcPVInfo == nil {
392371
v.logger.Warnf("cannot find info for PV %s", nativeSnapshot.Spec.PersistentVolumeName)
393372
continue
394373
}
374+
volumeResult := VolumeResultFailed
375+
if nativeSnapshot.Status.Phase == SnapshotPhaseCompleted {
376+
volumeResult = VolumeResultSucceeded
377+
}
378+
volumeInfo := &BackupVolumeInfo{
379+
BackupMethod: NativeSnapshot,
380+
PVCName: pvcPVInfo.PVCName,
381+
PVCNamespace: pvcPVInfo.PVCNamespace,
382+
PVName: pvcPVInfo.PV.Name,
383+
SnapshotDataMoved: false,
384+
Skipped: false,
385+
// Only set Succeeded to true when the NativeSnapshot's phase is Completed,
386+
// although NativeSnapshot doesn't check whether the snapshot creation result.
387+
Result: volumeResult,
388+
NativeSnapshotInfo: newNativeSnapshotInfo(nativeSnapshot),
389+
PVInfo: &PVInfo{
390+
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
391+
Labels: pvcPVInfo.PV.Labels,
392+
},
393+
}
394+
tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
395395
}
396396

397397
v.volumeInfos = append(v.volumeInfos, tmpVolumeInfos...)
@@ -449,38 +449,38 @@ func (v *BackupVolumesInformation) generateVolumeInfoForCSIVolumeSnapshot() {
449449
if volumeSnapshotContent.Status.SnapshotHandle != nil {
450450
snapshotHandle = *volumeSnapshotContent.Status.SnapshotHandle
451451
}
452-
if pvcPVInfo := v.pvMap.retrieve("", *volumeSnapshot.Spec.Source.PersistentVolumeClaimName, volumeSnapshot.Namespace); pvcPVInfo != nil {
453-
volumeInfo := &BackupVolumeInfo{
454-
BackupMethod: CSISnapshot,
455-
PVCName: pvcPVInfo.PVCName,
456-
PVCNamespace: pvcPVInfo.PVCNamespace,
457-
PVName: pvcPVInfo.PV.Name,
458-
Skipped: false,
459-
SnapshotDataMoved: false,
460-
PreserveLocalSnapshot: true,
461-
CSISnapshotInfo: &CSISnapshotInfo{
462-
VSCName: *volumeSnapshot.Status.BoundVolumeSnapshotContentName,
463-
Size: size,
464-
Driver: volumeSnapshotContent.Spec.Driver,
465-
SnapshotHandle: snapshotHandle,
466-
OperationID: operation.Spec.OperationID,
467-
ReadyToUse: volumeSnapshot.Status.ReadyToUse,
468-
},
469-
PVInfo: &PVInfo{
470-
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
471-
Labels: pvcPVInfo.PV.Labels,
472-
},
473-
}
474-
475-
if volumeSnapshot.Status.CreationTime != nil {
476-
volumeInfo.StartTimestamp = volumeSnapshot.Status.CreationTime
477-
}
478-
479-
tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
480-
} else {
452+
pvcPVInfo := v.pvMap.retrieve("", *volumeSnapshot.Spec.Source.PersistentVolumeClaimName, volumeSnapshot.Namespace)
453+
if pvcPVInfo == nil {
481454
v.logger.Warnf("cannot find info for PVC %s/%s", volumeSnapshot.Namespace, volumeSnapshot.Spec.Source.PersistentVolumeClaimName)
482455
continue
483456
}
457+
volumeInfo := &BackupVolumeInfo{
458+
BackupMethod: CSISnapshot,
459+
PVCName: pvcPVInfo.PVCName,
460+
PVCNamespace: pvcPVInfo.PVCNamespace,
461+
PVName: pvcPVInfo.PV.Name,
462+
Skipped: false,
463+
SnapshotDataMoved: false,
464+
PreserveLocalSnapshot: true,
465+
CSISnapshotInfo: &CSISnapshotInfo{
466+
VSCName: *volumeSnapshot.Status.BoundVolumeSnapshotContentName,
467+
Size: size,
468+
Driver: volumeSnapshotContent.Spec.Driver,
469+
SnapshotHandle: snapshotHandle,
470+
OperationID: operation.Spec.OperationID,
471+
ReadyToUse: volumeSnapshot.Status.ReadyToUse,
472+
},
473+
PVInfo: &PVInfo{
474+
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
475+
Labels: pvcPVInfo.PV.Labels,
476+
},
477+
}
478+
479+
if volumeSnapshot.Status.CreationTime != nil {
480+
volumeInfo.StartTimestamp = volumeSnapshot.Status.CreationTime
481+
}
482+
483+
tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
484484
}
485485

486486
v.volumeInfos = append(v.volumeInfos, tmpVolumeInfos...)
@@ -512,18 +512,18 @@ func (v *BackupVolumesInformation) generateVolumeInfoFromPVB() {
512512
continue
513513
}
514514
if pvcName != "" {
515-
if pvcPVInfo := v.pvMap.retrieve("", pvcName, pvb.Spec.Pod.Namespace); pvcPVInfo != nil {
516-
volumeInfo.PVCName = pvcPVInfo.PVCName
517-
volumeInfo.PVCNamespace = pvcPVInfo.PVCNamespace
518-
volumeInfo.PVName = pvcPVInfo.PV.Name
519-
volumeInfo.PVInfo = &PVInfo{
520-
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
521-
Labels: pvcPVInfo.PV.Labels,
522-
}
523-
} else {
515+
pvcPVInfo := v.pvMap.retrieve("", pvcName, pvb.Spec.Pod.Namespace)
516+
if pvcPVInfo == nil {
524517
v.logger.Warnf("Cannot find info for PVC %s/%s", pvb.Spec.Pod.Namespace, pvcName)
525518
continue
526519
}
520+
volumeInfo.PVCName = pvcPVInfo.PVCName
521+
volumeInfo.PVCNamespace = pvcPVInfo.PVCNamespace
522+
volumeInfo.PVName = pvcPVInfo.PV.Name
523+
volumeInfo.PVInfo = &PVInfo{
524+
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
525+
Labels: pvcPVInfo.PV.Labels,
526+
}
527527
} else {
528528
v.logger.Debug("The PVB %s doesn't have a corresponding PVC", pvb.Name)
529529
}
@@ -589,51 +589,50 @@ func (v *BackupVolumesInformation) generateVolumeInfoFromDataUpload() {
589589
)
590590
continue
591591
}
592-
593-
if pvcPVInfo := v.pvMap.retrieve(
592+
pvcPVInfo := v.pvMap.retrieve(
594593
"",
595594
operation.Spec.ResourceIdentifier.Name,
596595
operation.Spec.ResourceIdentifier.Namespace,
597-
); pvcPVInfo != nil {
598-
dataMover := veleroDatamover
599-
if dataUpload.Spec.DataMover != "" {
600-
dataMover = dataUpload.Spec.DataMover
601-
}
602-
603-
volumeInfo := &BackupVolumeInfo{
604-
BackupMethod: CSISnapshot,
605-
PVCName: pvcPVInfo.PVCName,
606-
PVCNamespace: pvcPVInfo.PVCNamespace,
607-
PVName: pvcPVInfo.PV.Name,
608-
SnapshotDataMoved: true,
609-
Skipped: false,
610-
CSISnapshotInfo: &CSISnapshotInfo{
611-
SnapshotHandle: FieldValueIsUnknown,
612-
VSCName: FieldValueIsUnknown,
613-
OperationID: FieldValueIsUnknown,
614-
Driver: dataUpload.Spec.CSISnapshot.Driver,
615-
},
616-
SnapshotDataMovementInfo: &SnapshotDataMovementInfo{
617-
DataMover: dataMover,
618-
UploaderType: velerov1api.BackupRepositoryTypeKopia,
619-
OperationID: operation.Spec.OperationID,
620-
Phase: dataUpload.Status.Phase,
621-
},
622-
PVInfo: &PVInfo{
623-
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
624-
Labels: pvcPVInfo.PV.Labels,
625-
},
626-
}
627-
628-
if dataUpload.Status.StartTimestamp != nil {
629-
volumeInfo.StartTimestamp = dataUpload.Status.StartTimestamp
630-
}
631-
632-
tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
633-
} else {
596+
)
597+
if pvcPVInfo == nil {
634598
v.logger.Warnf("Cannot find info for PVC %s/%s", operation.Spec.ResourceIdentifier.Namespace, operation.Spec.ResourceIdentifier.Name)
635599
continue
636600
}
601+
dataMover := veleroDatamover
602+
if dataUpload.Spec.DataMover != "" {
603+
dataMover = dataUpload.Spec.DataMover
604+
}
605+
606+
volumeInfo := &BackupVolumeInfo{
607+
BackupMethod: CSISnapshot,
608+
PVCName: pvcPVInfo.PVCName,
609+
PVCNamespace: pvcPVInfo.PVCNamespace,
610+
PVName: pvcPVInfo.PV.Name,
611+
SnapshotDataMoved: true,
612+
Skipped: false,
613+
CSISnapshotInfo: &CSISnapshotInfo{
614+
SnapshotHandle: FieldValueIsUnknown,
615+
VSCName: FieldValueIsUnknown,
616+
OperationID: FieldValueIsUnknown,
617+
Driver: dataUpload.Spec.CSISnapshot.Driver,
618+
},
619+
SnapshotDataMovementInfo: &SnapshotDataMovementInfo{
620+
DataMover: dataMover,
621+
UploaderType: velerov1api.BackupRepositoryTypeKopia,
622+
OperationID: operation.Spec.OperationID,
623+
Phase: dataUpload.Status.Phase,
624+
},
625+
PVInfo: &PVInfo{
626+
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
627+
Labels: pvcPVInfo.PV.Labels,
628+
},
629+
}
630+
631+
if dataUpload.Status.StartTimestamp != nil {
632+
volumeInfo.StartTimestamp = dataUpload.Status.StartTimestamp
633+
}
634+
635+
tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
637636
}
638637

639638
v.volumeInfos = append(v.volumeInfos, tmpVolumeInfos...)
@@ -728,17 +727,16 @@ func (t *RestoreVolumeInfoTracker) Populate(ctx context.Context, restoredResourc
728727
t.pvcCSISnapshotMap[pvc.Namespace+"/"+pvcName] = *vs
729728
}
730729
}
731-
if pvc.Status.Phase == corev1api.ClaimBound && pvc.Spec.VolumeName != "" {
732-
pv := &corev1api.PersistentVolume{}
733-
if err := t.client.Get(ctx, kbclient.ObjectKey{Name: pvc.Spec.VolumeName}, pv); err != nil {
734-
log.WithError(err).Error("Failed to get PV")
735-
} else {
736-
t.pvPvc.insert(*pv, pvcName, pvcNS)
737-
}
738-
} else {
730+
if pvc.Status.Phase != corev1api.ClaimBound || pvc.Spec.VolumeName == "" {
739731
log.Warn("PVC is not bound or has no volume name")
740732
continue
741733
}
734+
pv := &corev1api.PersistentVolume{}
735+
if err := t.client.Get(ctx, kbclient.ObjectKey{Name: pvc.Spec.VolumeName}, pv); err != nil {
736+
log.WithError(err).Error("Failed to get PV")
737+
} else {
738+
t.pvPvc.insert(*pv, pvcName, pvcNS)
739+
}
742740
}
743741
if err := t.client.List(ctx, t.datadownloadList, &kbclient.ListOptions{
744742
Namespace: t.restore.Namespace,
@@ -765,19 +763,18 @@ func (t *RestoreVolumeInfoTracker) Result() []*RestoreVolumeInfo {
765763
t.log.WithError(err).Warn("Fail to get PVC from PodVolumeRestore: ", pvr.Name)
766764
continue
767765
}
768-
if pvcName != "" {
769-
volumeInfo.PVCName = pvcName
770-
volumeInfo.PVCNamespace = pvr.Spec.Pod.Namespace
771-
if pvcPVInfo := t.pvPvc.retrieve("", pvcName, pvr.Spec.Pod.Namespace); pvcPVInfo != nil {
772-
volumeInfo.PVName = pvcPVInfo.PV.Name
773-
}
774-
} else {
766+
if pvcName == "" {
775767
// In this case, the volume is not bound to a PVC and
776768
// the PVR will not be able to populate into the volume, so we'll skip it
777769
t.log.Warnf("unable to get PVC for PodVolumeRestore %s/%s, pod: %s/%s, volume: %s",
778770
pvr.Namespace, pvr.Name, pvr.Spec.Pod.Namespace, pvr.Spec.Pod.Name, pvr.Spec.Volume)
779771
continue
780772
}
773+
volumeInfo.PVCName = pvcName
774+
volumeInfo.PVCNamespace = pvr.Spec.Pod.Namespace
775+
if pvcPVInfo := t.pvPvc.retrieve("", pvcName, pvr.Spec.Pod.Namespace); pvcPVInfo != nil {
776+
volumeInfo.PVName = pvcPVInfo.PV.Name
777+
}
781778
volumeInfos = append(volumeInfos, volumeInfo)
782779
}
783780

0 commit comments

Comments
 (0)