Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: enable early-return and superfluous-else from revive #8640

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,6 @@ linters-settings:
- name: dot-imports
disabled: true
- name: early-return
disabled: true
arguments:
- "preserveScope"
- name: empty-block
Expand All @@ -266,7 +265,6 @@ linters-settings:
- name: redefines-builtin-id
disabled: true
- name: superfluous-else
disabled: true
arguments:
- "preserveScope"
- name: time-naming
Expand Down
257 changes: 127 additions & 130 deletions internal/volume/volumes_information.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,24 +338,24 @@
tmpVolumeInfos := make([]*BackupVolumeInfo, 0)

for pvName, skippedReason := range v.SkippedPVs {
if pvcPVInfo := v.pvMap.retrieve(pvName, "", ""); pvcPVInfo != nil {
volumeInfo := &BackupVolumeInfo{
PVCName: pvcPVInfo.PVCName,
PVCNamespace: pvcPVInfo.PVCNamespace,
PVName: pvName,
SnapshotDataMoved: false,
Skipped: true,
SkippedReason: skippedReason,
PVInfo: &PVInfo{
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
Labels: pvcPVInfo.PV.Labels,
},
}
tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
} else {
pvcPVInfo := v.pvMap.retrieve(pvName, "", "")
if pvcPVInfo == nil {
v.logger.Warnf("Cannot find info for PV %s", pvName)
continue
}
volumeInfo := &BackupVolumeInfo{
PVCName: pvcPVInfo.PVCName,
PVCNamespace: pvcPVInfo.PVCNamespace,
PVName: pvName,
SnapshotDataMoved: false,
Skipped: true,
SkippedReason: skippedReason,
PVInfo: &PVInfo{
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
Labels: pvcPVInfo.PV.Labels,
},
}
tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
}

v.volumeInfos = append(v.volumeInfos, tmpVolumeInfos...)
Expand All @@ -366,32 +366,32 @@
tmpVolumeInfos := make([]*BackupVolumeInfo, 0)

for _, nativeSnapshot := range v.NativeSnapshots {
if pvcPVInfo := v.pvMap.retrieve(nativeSnapshot.Spec.PersistentVolumeName, "", ""); pvcPVInfo != nil {
volumeResult := VolumeResultFailed
if nativeSnapshot.Status.Phase == SnapshotPhaseCompleted {
volumeResult = VolumeResultSucceeded
}
volumeInfo := &BackupVolumeInfo{
BackupMethod: NativeSnapshot,
PVCName: pvcPVInfo.PVCName,
PVCNamespace: pvcPVInfo.PVCNamespace,
PVName: pvcPVInfo.PV.Name,
SnapshotDataMoved: false,
Skipped: false,
// Only set Succeeded to true when the NativeSnapshot's phase is Completed,
// although NativeSnapshot doesn't check whether the snapshot creation result.
Result: volumeResult,
NativeSnapshotInfo: newNativeSnapshotInfo(nativeSnapshot),
PVInfo: &PVInfo{
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
Labels: pvcPVInfo.PV.Labels,
},
}
tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
} else {
pvcPVInfo := v.pvMap.retrieve(nativeSnapshot.Spec.PersistentVolumeName, "", "")
if pvcPVInfo == nil {
v.logger.Warnf("cannot find info for PV %s", nativeSnapshot.Spec.PersistentVolumeName)
continue
}
volumeResult := VolumeResultFailed
if nativeSnapshot.Status.Phase == SnapshotPhaseCompleted {
volumeResult = VolumeResultSucceeded
}
volumeInfo := &BackupVolumeInfo{
BackupMethod: NativeSnapshot,
PVCName: pvcPVInfo.PVCName,
PVCNamespace: pvcPVInfo.PVCNamespace,
PVName: pvcPVInfo.PV.Name,
SnapshotDataMoved: false,
Skipped: false,
// Only set Succeeded to true when the NativeSnapshot's phase is Completed,
// although NativeSnapshot doesn't check whether the snapshot creation result.
Result: volumeResult,
NativeSnapshotInfo: newNativeSnapshotInfo(nativeSnapshot),
PVInfo: &PVInfo{
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
Labels: pvcPVInfo.PV.Labels,
},
}
tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
}

v.volumeInfos = append(v.volumeInfos, tmpVolumeInfos...)
Expand Down Expand Up @@ -461,38 +461,38 @@
if volumeSnapshotContent.Status.SnapshotHandle != nil {
snapshotHandle = *volumeSnapshotContent.Status.SnapshotHandle
}
if pvcPVInfo := v.pvMap.retrieve("", *volumeSnapshot.Spec.Source.PersistentVolumeClaimName, volumeSnapshot.Namespace); pvcPVInfo != nil {
volumeInfo := &BackupVolumeInfo{
BackupMethod: CSISnapshot,
PVCName: pvcPVInfo.PVCName,
PVCNamespace: pvcPVInfo.PVCNamespace,
PVName: pvcPVInfo.PV.Name,
Skipped: false,
SnapshotDataMoved: false,
PreserveLocalSnapshot: true,
CSISnapshotInfo: &CSISnapshotInfo{
VSCName: *volumeSnapshot.Status.BoundVolumeSnapshotContentName,
Size: size,
Driver: volumeSnapshotClass.Driver,
SnapshotHandle: snapshotHandle,
OperationID: operation.Spec.OperationID,
ReadyToUse: volumeSnapshot.Status.ReadyToUse,
},
PVInfo: &PVInfo{
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
Labels: pvcPVInfo.PV.Labels,
},
}

if volumeSnapshot.Status.CreationTime != nil {
volumeInfo.StartTimestamp = volumeSnapshot.Status.CreationTime
}

tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
} else {
pvcPVInfo := v.pvMap.retrieve("", *volumeSnapshot.Spec.Source.PersistentVolumeClaimName, volumeSnapshot.Namespace)
if pvcPVInfo == nil {
v.logger.Warnf("cannot find info for PVC %s/%s", volumeSnapshot.Namespace, volumeSnapshot.Spec.Source.PersistentVolumeClaimName)
continue
}
volumeInfo := &BackupVolumeInfo{
BackupMethod: CSISnapshot,
PVCName: pvcPVInfo.PVCName,
PVCNamespace: pvcPVInfo.PVCNamespace,
PVName: pvcPVInfo.PV.Name,
Skipped: false,
SnapshotDataMoved: false,
PreserveLocalSnapshot: true,
CSISnapshotInfo: &CSISnapshotInfo{
VSCName: *volumeSnapshot.Status.BoundVolumeSnapshotContentName,
Size: size,
Driver: volumeSnapshotClass.Driver,
SnapshotHandle: snapshotHandle,
OperationID: operation.Spec.OperationID,
ReadyToUse: volumeSnapshot.Status.ReadyToUse,
},
PVInfo: &PVInfo{
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
Labels: pvcPVInfo.PV.Labels,
},
}

if volumeSnapshot.Status.CreationTime != nil {
volumeInfo.StartTimestamp = volumeSnapshot.Status.CreationTime
}

tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
}

v.volumeInfos = append(v.volumeInfos, tmpVolumeInfos...)
Expand Down Expand Up @@ -524,18 +524,18 @@
continue
}
if pvcName != "" {
if pvcPVInfo := v.pvMap.retrieve("", pvcName, pvb.Spec.Pod.Namespace); pvcPVInfo != nil {
volumeInfo.PVCName = pvcPVInfo.PVCName
volumeInfo.PVCNamespace = pvcPVInfo.PVCNamespace
volumeInfo.PVName = pvcPVInfo.PV.Name
volumeInfo.PVInfo = &PVInfo{
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
Labels: pvcPVInfo.PV.Labels,
}
} else {
pvcPVInfo := v.pvMap.retrieve("", pvcName, pvb.Spec.Pod.Namespace)
if pvcPVInfo == nil {
v.logger.Warnf("Cannot find info for PVC %s/%s", pvb.Spec.Pod.Namespace, pvcName)
continue
}
volumeInfo.PVCName = pvcPVInfo.PVCName
volumeInfo.PVCNamespace = pvcPVInfo.PVCNamespace
volumeInfo.PVName = pvcPVInfo.PV.Name
volumeInfo.PVInfo = &PVInfo{
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
Labels: pvcPVInfo.PV.Labels,
}
} else {
v.logger.Debug("The PVB %s doesn't have a corresponding PVC", pvb.Name)
}
Expand Down Expand Up @@ -615,51 +615,50 @@
driverUsedByVSClass = vsClassList[index].Driver
}
}

if pvcPVInfo := v.pvMap.retrieve(
pvcPVInfo := v.pvMap.retrieve(
"",
operation.Spec.ResourceIdentifier.Name,
operation.Spec.ResourceIdentifier.Namespace,
); pvcPVInfo != nil {
dataMover := veleroDatamover
if dataUpload.Spec.DataMover != "" {
dataMover = dataUpload.Spec.DataMover
}

volumeInfo := &BackupVolumeInfo{
BackupMethod: CSISnapshot,
PVCName: pvcPVInfo.PVCName,
PVCNamespace: pvcPVInfo.PVCNamespace,
PVName: pvcPVInfo.PV.Name,
SnapshotDataMoved: true,
Skipped: false,
CSISnapshotInfo: &CSISnapshotInfo{
SnapshotHandle: FieldValueIsUnknown,
VSCName: FieldValueIsUnknown,
OperationID: FieldValueIsUnknown,
Driver: driverUsedByVSClass,
},
SnapshotDataMovementInfo: &SnapshotDataMovementInfo{
DataMover: dataMover,
UploaderType: velerov1api.BackupRepositoryTypeKopia,
OperationID: operation.Spec.OperationID,
Phase: dataUpload.Status.Phase,
},
PVInfo: &PVInfo{
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
Labels: pvcPVInfo.PV.Labels,
},
}

if dataUpload.Status.StartTimestamp != nil {
volumeInfo.StartTimestamp = dataUpload.Status.StartTimestamp
}

tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
} else {
)
if pvcPVInfo == nil {
v.logger.Warnf("Cannot find info for PVC %s/%s", operation.Spec.ResourceIdentifier.Namespace, operation.Spec.ResourceIdentifier.Name)
continue
}
dataMover := veleroDatamover
if dataUpload.Spec.DataMover != "" {
dataMover = dataUpload.Spec.DataMover
}

volumeInfo := &BackupVolumeInfo{
BackupMethod: CSISnapshot,
PVCName: pvcPVInfo.PVCName,
PVCNamespace: pvcPVInfo.PVCNamespace,
PVName: pvcPVInfo.PV.Name,
SnapshotDataMoved: true,
Skipped: false,
CSISnapshotInfo: &CSISnapshotInfo{
SnapshotHandle: FieldValueIsUnknown,
VSCName: FieldValueIsUnknown,
OperationID: FieldValueIsUnknown,
Driver: driverUsedByVSClass,
},
SnapshotDataMovementInfo: &SnapshotDataMovementInfo{
DataMover: dataMover,
UploaderType: velerov1api.BackupRepositoryTypeKopia,
OperationID: operation.Spec.OperationID,
Phase: dataUpload.Status.Phase,
},
PVInfo: &PVInfo{
ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
Labels: pvcPVInfo.PV.Labels,
},
}

if dataUpload.Status.StartTimestamp != nil {
volumeInfo.StartTimestamp = dataUpload.Status.StartTimestamp
}

tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
}

v.volumeInfos = append(v.volumeInfos, tmpVolumeInfos...)
Expand Down Expand Up @@ -754,17 +753,16 @@
t.pvcCSISnapshotMap[pvc.Namespace+"/"+pvcName] = *vs
}
}
if pvc.Status.Phase == corev1api.ClaimBound && pvc.Spec.VolumeName != "" {
pv := &corev1api.PersistentVolume{}
if err := t.client.Get(ctx, kbclient.ObjectKey{Name: pvc.Spec.VolumeName}, pv); err != nil {
log.WithError(err).Error("Failed to get PV")
} else {
t.pvPvc.insert(*pv, pvcName, pvcNS)
}
} else {
if !(pvc.Status.Phase == corev1api.ClaimBound && pvc.Spec.VolumeName != "") {

Check warning on line 756 in internal/volume/volumes_information.go

View check run for this annotation

Codecov / codecov/patch

internal/volume/volumes_information.go#L756

Added line #L756 was not covered by tests
log.Warn("PVC is not bound or has no volume name")
continue
}
pv := &corev1api.PersistentVolume{}
if err := t.client.Get(ctx, kbclient.ObjectKey{Name: pvc.Spec.VolumeName}, pv); err != nil {
log.WithError(err).Error("Failed to get PV")
} else {
t.pvPvc.insert(*pv, pvcName, pvcNS)
}

Check warning on line 765 in internal/volume/volumes_information.go

View check run for this annotation

Codecov / codecov/patch

internal/volume/volumes_information.go#L760-L765

Added lines #L760 - L765 were not covered by tests
}
if err := t.client.List(ctx, t.datadownloadList, &kbclient.ListOptions{
Namespace: t.restore.Namespace,
Expand All @@ -791,19 +789,18 @@
t.log.WithError(err).Warn("Fail to get PVC from PodVolumeRestore: ", pvr.Name)
continue
}
if pvcName != "" {
volumeInfo.PVCName = pvcName
volumeInfo.PVCNamespace = pvr.Spec.Pod.Namespace
if pvcPVInfo := t.pvPvc.retrieve("", pvcName, pvr.Spec.Pod.Namespace); pvcPVInfo != nil {
volumeInfo.PVName = pvcPVInfo.PV.Name
}
} else {
if pvcName == "" {
// In this case, the volume is not bound to a PVC and
// the PVR will not be able to populate into the volume, so we'll skip it
t.log.Warnf("unable to get PVC for PodVolumeRestore %s/%s, pod: %s/%s, volume: %s",
pvr.Namespace, pvr.Name, pvr.Spec.Pod.Namespace, pvr.Spec.Pod.Name, pvr.Spec.Volume)
continue
}
volumeInfo.PVCName = pvcName
volumeInfo.PVCNamespace = pvr.Spec.Pod.Namespace
if pvcPVInfo := t.pvPvc.retrieve("", pvcName, pvr.Spec.Pod.Namespace); pvcPVInfo != nil {
volumeInfo.PVName = pvcPVInfo.PV.Name
}
volumeInfos = append(volumeInfos, volumeInfo)
}

Expand Down
7 changes: 3 additions & 4 deletions pkg/backup/item_backupper.go
Original file line number Diff line number Diff line change
Expand Up @@ -823,12 +823,11 @@ func zoneFromPVNodeAffinity(res *corev1api.PersistentVolume, topologyKeys ...str
}
for _, exp := range term.MatchExpressions {
if keySet.Has(exp.Key) && exp.Operator == "In" && len(exp.Values) > 0 {
if exp.Key == gkeCsiZoneKey {
providerGke = true
zones = append(zones, exp.Values[0])
} else {
if exp.Key != gkeCsiZoneKey {
return exp.Key, exp.Values[0]
}
providerGke = true
zones = append(zones, exp.Values[0])
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions pkg/cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -899,13 +899,12 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
// wasn't found and it returns an error.
func removeControllers(disabledControllers []string, enabledRuntimeControllers map[string]struct{}, logger logrus.FieldLogger) error {
for _, controllerName := range disabledControllers {
if _, ok := enabledRuntimeControllers[controllerName]; ok {
logger.Infof("Disabling controller: %s", controllerName)
delete(enabledRuntimeControllers, controllerName)
} else {
if _, ok := enabledRuntimeControllers[controllerName]; !ok {
msg := fmt.Sprintf("Invalid value for --disable-controllers flag provided: %s. Valid values are: %s", controllerName, strings.Join(config.DisableableControllers, ","))
return errors.New(msg)
}
logger.Infof("Disabling controller: %s", controllerName)
delete(enabledRuntimeControllers, controllerName)
}
return nil
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/cmd/util/output/backup_describer.go
Original file line number Diff line number Diff line change
Expand Up @@ -766,12 +766,11 @@ func describePodVolumeBackups(d *Describer, details bool, podVolumeBackups []vel
// Get the type of pod volume uploader. Since the uploader only comes from a single source, we can
// take the uploader type from the first element of the array.
var uploaderType string
if len(podVolumeBackups) > 0 {
uploaderType = podVolumeBackups[0].Spec.UploaderType
} else {
if len(podVolumeBackups) == 0 {
d.Printf("\tPod Volume Backups: <none included>\n")
return
}
uploaderType = podVolumeBackups[0].Spec.UploaderType

if details {
d.Printf("\tPod Volume Backups - %s:\n", uploaderType)
Expand Down
Loading
Loading