From f01c93ff32b7fbdcf35eeb4bce2b07add3608c60 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Wed, 22 Jan 2025 07:45:28 +0100 Subject: [PATCH] chore: enable early-return and superfluous-else from revive Signed-off-by: Matthieu MOREL --- .github/workflows/pr-codespell.yml | 2 +- .golangci.yaml | 8 +- internal/volume/volumes_information.go | 257 +++++++++--------- pkg/backup/item_backupper.go | 7 +- pkg/cmd/server/server.go | 7 +- pkg/cmd/util/output/backup_describer.go | 5 +- .../output/backup_structured_describer.go | 5 +- pkg/cmd/util/output/restore_describer.go | 5 +- pkg/controller/restore_controller.go | 6 +- .../server_status_request_controller_test.go | 5 +- pkg/restore/actions/service_account_action.go | 3 +- pkg/uploader/kopia/block_restore.go | 10 +- pkg/util/logging/error_location_hook.go | 5 +- test/e2e/basic/namespace-mapping.go | 5 +- test/e2e/basic/resources-check/rbac.go | 5 +- test/e2e/e2e_suite_test.go | 13 +- .../resourcemodifiers/resource_modifiers.go | 5 +- .../e2e/resourcepolicies/resource_policies.go | 5 +- test/util/csi/common.go | 9 +- test/util/velero/install.go | 5 +- 20 files changed, 174 insertions(+), 198 deletions(-) diff --git a/.github/workflows/pr-codespell.yml b/.github/workflows/pr-codespell.yml index 0d3138e403..900371ca2b 100644 --- a/.github/workflows/pr-codespell.yml +++ b/.github/workflows/pr-codespell.yml @@ -13,7 +13,7 @@ jobs: - name: Codespell uses: codespell-project/actions-codespell@master with: - # ignore the config/.../crd.go file as it's generated binary data that is edited elswhere. + # ignore the config/.../crd.go file as it's generated binary data that is edited elsewhere. skip: .git,*.png,*.jpg,*.woff,*.ttf,*.gif,*.ico,./config/crd/v1beta1/crds/crds.go,./config/crd/v1/crds/crds.go,./config/crd/v2alpha1/crds/crds.go,./go.sum,./LICENSE ignore_words_list: iam,aks,ist,bridget,ue,shouldnot,atleast,notin,sme,optin check_filenames: true diff --git a/.golangci.yaml b/.golangci.yaml index 8d2b6c75d2..1939339d7f 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -240,11 +240,9 @@ linters-settings: - name: context-as-argument disabled: true - name: context-keys-type - disabled: false - name: dot-imports disabled: true - name: early-return - disabled: true arguments: - "preserveScope" - name: empty-block @@ -262,23 +260,19 @@ linters-settings: - name: indent-error-flow disabled: true - name: range - disabled: false - name: receiver-naming disabled: true - name: redefines-builtin-id disabled: true - name: superfluous-else - disabled: true arguments: - "preserveScope" - name: time-naming - disabled: false - name: unexported-return disabled: true - name: unnecessary-stmt disabled: true - name: unreachable-code - disabled: false - name: unused-parameter disabled: true - name: use-any @@ -333,7 +327,7 @@ linters-settings: force-case-trailing-whitespace: 0 # Force cuddling of err checks with err var assignment force-err-cuddling: false - # Allow leading comments to be separated with empty liens + # Allow leading comments to be separated with empty lines allow-separated-leading-comment: false linters: diff --git a/internal/volume/volumes_information.go b/internal/volume/volumes_information.go index 9af6d7fe34..a4e1e0f0ae 100644 --- a/internal/volume/volumes_information.go +++ b/internal/volume/volumes_information.go @@ -338,24 +338,24 @@ func (v *BackupVolumesInformation) generateVolumeInfoForSkippedPV() { 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...) @@ -366,32 +366,32 @@ func (v *BackupVolumesInformation) generateVolumeInfoForVeleroNativeSnapshot() { 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...) @@ -461,38 +461,38 @@ func (v *BackupVolumesInformation) generateVolumeInfoForCSIVolumeSnapshot() { 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...) @@ -524,18 +524,18 @@ func (v *BackupVolumesInformation) generateVolumeInfoFromPVB() { 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) } @@ -615,51 +615,50 @@ func (v *BackupVolumesInformation) generateVolumeInfoFromDataUpload() { 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...) @@ -754,17 +753,16 @@ func (t *RestoreVolumeInfoTracker) Populate(ctx context.Context, restoredResourc 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 != "") { 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) + } } if err := t.client.List(ctx, t.datadownloadList, &kbclient.ListOptions{ Namespace: t.restore.Namespace, @@ -791,19 +789,18 @@ func (t *RestoreVolumeInfoTracker) Result() []*RestoreVolumeInfo { 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) } diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go index 9e5caef0ed..156ea215df 100644 --- a/pkg/backup/item_backupper.go +++ b/pkg/backup/item_backupper.go @@ -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]) } } } diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 396970b92a..de7a2ff873 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -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 } diff --git a/pkg/cmd/util/output/backup_describer.go b/pkg/cmd/util/output/backup_describer.go index b81bad002f..a7f7c674a4 100644 --- a/pkg/cmd/util/output/backup_describer.go +++ b/pkg/cmd/util/output/backup_describer.go @@ -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: \n") return } + uploaderType = podVolumeBackups[0].Spec.UploaderType if details { d.Printf("\tPod Volume Backups - %s:\n", uploaderType) diff --git a/pkg/cmd/util/output/backup_structured_describer.go b/pkg/cmd/util/output/backup_structured_describer.go index 63bfbbd5ba..2c91b8c159 100644 --- a/pkg/cmd/util/output/backup_structured_describer.go +++ b/pkg/cmd/util/output/backup_structured_describer.go @@ -482,12 +482,11 @@ func describePodVolumeBackupsInSF(backups []velerov1api.PodVolumeBackup, details // 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(backups) > 0 { - uploaderType = backups[0].Spec.UploaderType - } else { + if len(backups) == 0 { backupVolumes["podVolumeBackups"] = "" return } + uploaderType = backups[0].Spec.UploaderType // type display the type of pod volume backups podVolumeBackupsInfo["uploderType"] = uploaderType diff --git a/pkg/cmd/util/output/restore_describer.go b/pkg/cmd/util/output/restore_describer.go index 55363b4b63..268188cab0 100644 --- a/pkg/cmd/util/output/restore_describer.go +++ b/pkg/cmd/util/output/restore_describer.go @@ -344,11 +344,10 @@ func describePodVolumeRestores(d *Describer, restores []velerov1api.PodVolumeRes // 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(restores) > 0 { - uploaderType = restores[0].Spec.UploaderType - } else { + if len(restores) == 0 { return } + uploaderType = restores[0].Spec.UploaderType if details { d.Printf("%s Restores:\n", uploaderType) diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go index 336f2e1944..203a4ed2ba 100644 --- a/pkg/controller/restore_controller.go +++ b/pkg/controller/restore_controller.go @@ -377,12 +377,12 @@ func (r *restoreReconciler) validateAndComplete(restore *api.Restore) (backupInf restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, "No backups found for schedule") } - if backup := mostRecentCompletedBackup(backupList.Items); backup.Name != "" { - restore.Spec.BackupName = backup.Name - } else { + backup := mostRecentCompletedBackup(backupList.Items) + if backup.Name == "" { restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, "No completed backups found for schedule") return backupInfo{}, nil } + restore.Spec.BackupName = backup.Name } info, err := r.fetchBackupInfo(restore.Spec.BackupName) diff --git a/pkg/controller/server_status_request_controller_test.go b/pkg/controller/server_status_request_controller_test.go index eb95c7e87e..33d52fd297 100644 --- a/pkg/controller/server_status_request_controller_test.go +++ b/pkg/controller/server_status_request_controller_test.go @@ -77,12 +77,11 @@ var _ = Describe("Server Status Request Reconciler", func() { }) Expect(actualResult).To(BeEquivalentTo(test.expectedRequeue)) - if test.expectedErrMsg == "" { - Expect(err).ToNot(HaveOccurred()) - } else { + if test.expectedErrMsg != "" { Expect(err.Error()).To(BeEquivalentTo(test.expectedErrMsg)) return } + Expect(err).ToNot(HaveOccurred()) instance := &velerov1api.ServerStatusRequest{} err = r.client.Get(ctx, kbclient.ObjectKey{Name: test.req.Name, Namespace: test.req.Namespace}, instance) diff --git a/pkg/restore/actions/service_account_action.go b/pkg/restore/actions/service_account_action.go index 85b9bc3dfc..483281a0bc 100644 --- a/pkg/restore/actions/service_account_action.go +++ b/pkg/restore/actions/service_account_action.go @@ -65,9 +65,8 @@ func (a *ServiceAccountAction) Execute(input *velero.RestoreItemActionExecuteInp log.Debug("Match found - excluding this secret") serviceAccount.Secrets = append(serviceAccount.Secrets[:i], serviceAccount.Secrets[i+1:]...) break - } else { - log.Debug("No match found - including this secret") } + log.Debug("No match found - including this secret") } res, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&serviceAccount) diff --git a/pkg/uploader/kopia/block_restore.go b/pkg/uploader/kopia/block_restore.go index b9fba99fc8..58d3fb4f0b 100644 --- a/pkg/uploader/kopia/block_restore.go +++ b/pkg/uploader/kopia/block_restore.go @@ -69,13 +69,13 @@ func (o *BlockOutput) WriteFile(ctx context.Context, relativePath string, remote if bytesToWrite > 0 { offset := 0 for bytesToWrite > 0 { - if bytesWritten, err := targetFile.Write(buffer[offset:bytesToWrite]); err == nil { - progressCb(int64(bytesWritten)) - bytesToWrite -= bytesWritten - offset += bytesWritten - } else { + bytesWritten, err := targetFile.Write(buffer[offset:bytesToWrite]) + if err != nil { return errors.Wrapf(err, "failed to write data to file %s", o.targetFileName) } + progressCb(int64(bytesWritten)) + bytesToWrite -= bytesWritten + offset += bytesWritten } } } diff --git a/pkg/util/logging/error_location_hook.go b/pkg/util/logging/error_location_hook.go index 5246a8318c..4b344f3993 100644 --- a/pkg/util/logging/error_location_hook.go +++ b/pkg/util/logging/error_location_hook.go @@ -140,10 +140,9 @@ func getInnermostTrace(err error) stackTracer { } c, isCauser := err.(causer) - if isCauser { - err = c.Cause() - } else { + if !isCauser { return tracer } + err = c.Cause() } } diff --git a/test/e2e/basic/namespace-mapping.go b/test/e2e/basic/namespace-mapping.go index a2ebc24c8b..79c8c8852f 100644 --- a/test/e2e/basic/namespace-mapping.go +++ b/test/e2e/basic/namespace-mapping.go @@ -116,9 +116,7 @@ func (n *NamespaceMapping) Verify() error { } func (n *NamespaceMapping) Clean() error { - if CurrentSpecReport().Failed() && n.VeleroCfg.FailFast { - fmt.Println("Test case failed and fail fast is enabled. Skip resource clean up.") - } else { + if !(CurrentSpecReport().Failed() && n.VeleroCfg.FailFast) { if err := DeleteStorageClass(context.Background(), n.Client, KibishiiStorageClassName); err != nil { return err } @@ -130,6 +128,7 @@ func (n *NamespaceMapping) Clean() error { return n.GetTestCase().Clean() } + fmt.Println("Test case failed and fail fast is enabled. Skip resource clean up.") return nil } diff --git a/test/e2e/basic/resources-check/rbac.go b/test/e2e/basic/resources-check/rbac.go index b79c3615f4..0304332bd8 100644 --- a/test/e2e/basic/resources-check/rbac.go +++ b/test/e2e/basic/resources-check/rbac.go @@ -177,11 +177,10 @@ func (r *RBACCase) Destroy() error { } func (r *RBACCase) Clean() error { - if CurrentSpecReport().Failed() && r.VeleroCfg.FailFast { - fmt.Println("Test case failed and fail fast is enabled. Skip resource clean up.") - } else { + if !(CurrentSpecReport().Failed() && r.VeleroCfg.FailFast) { return r.Destroy() } + fmt.Println("Test case failed and fail fast is enabled. Skip resource clean up.") return nil } diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 68de2ecc46..dec7501311 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -645,15 +645,14 @@ func GetKubeConfigContext() error { if err != nil { return err } - if test.VeleroCfg.StandbyClusterContext != "" { - tcStandby, err = k8s.NewTestClient(test.VeleroCfg.StandbyClusterContext) - test.VeleroCfg.StandbyClient = &tcStandby - if err != nil { - return err - } - } else { + if test.VeleroCfg.StandbyClusterContext == "" { return errors.New("migration test needs 2 clusters to run") } + tcStandby, err = k8s.NewTestClient(test.VeleroCfg.StandbyClusterContext) + test.VeleroCfg.StandbyClient = &tcStandby + if err != nil { + return err + } } return nil diff --git a/test/e2e/resourcemodifiers/resource_modifiers.go b/test/e2e/resourcemodifiers/resource_modifiers.go index e3bd9ea11d..cde4c1cc8d 100644 --- a/test/e2e/resourcemodifiers/resource_modifiers.go +++ b/test/e2e/resourcemodifiers/resource_modifiers.go @@ -131,15 +131,14 @@ func (r *ResourceModifiersCase) Verify() error { func (r *ResourceModifiersCase) Clean() error { // If created some resources which is not in current test namespace, we NEED to override the base Clean function - if CurrentSpecReport().Failed() && r.VeleroCfg.FailFast { - fmt.Println("Test case failed and fail fast is enabled. Skip resource clean up.") - } else { + if !(CurrentSpecReport().Failed() && r.VeleroCfg.FailFast) { if err := DeleteConfigMap(r.Client.ClientGo, r.VeleroCfg.VeleroNamespace, r.cmName); err != nil { return err } return r.GetTestCase().Clean() // only clean up resources in test namespace } + fmt.Println("Test case failed and fail fast is enabled. Skip resource clean up.") return nil } diff --git a/test/e2e/resourcepolicies/resource_policies.go b/test/e2e/resourcepolicies/resource_policies.go index 94e238eda3..2d127ca362 100644 --- a/test/e2e/resourcepolicies/resource_policies.go +++ b/test/e2e/resourcepolicies/resource_policies.go @@ -173,15 +173,14 @@ func (r *ResourcePoliciesCase) Verify() error { func (r *ResourcePoliciesCase) Clean() error { // If created some resources which is not in current test namespace, we NEED to override the base Clean function - if CurrentSpecReport().Failed() && r.VeleroCfg.FailFast { - fmt.Println("Test case failed and fail fast is enabled. Skip resource clean up.") - } else { + if !(CurrentSpecReport().Failed() && r.VeleroCfg.FailFast) { if err := DeleteConfigMap(r.Client.ClientGo, r.VeleroCfg.VeleroNamespace, r.cmName); err != nil { return err } return r.GetTestCase().Clean() // only clean up resources in test namespace } + fmt.Println("Test case failed and fail fast is enabled. Skip resource clean up.") return nil } diff --git a/test/util/csi/common.go b/test/util/csi/common.go index 5e58ec37df..e5f60e2a0d 100644 --- a/test/util/csi/common.go +++ b/test/util/csi/common.go @@ -149,13 +149,12 @@ func CheckVolumeSnapshotCR(client TestClient, index map[string]string, expectedC return nil, errors.New("Fail to get APIVersion") } - if apiVersion[0] == "v1" { - if snapshotContentNameList, err = GetCsiSnapshotHandle(client, apiVersion[0], index); err != nil { - return nil, errors.Wrap(err, "Fail to get CSI snapshot content") - } - } else { + if apiVersion[0] != "v1" { return nil, errors.New("API version is invalid") } + if snapshotContentNameList, err = GetCsiSnapshotHandle(client, apiVersion[0], index); err != nil { + return nil, errors.Wrap(err, "Fail to get CSI snapshot content") + } if expectedCount >= 0 { if len(snapshotContentNameList) != expectedCount { return nil, errors.New(fmt.Sprintf("Snapshot content count %d is not as expect %d", len(snapshotContentNameList), expectedCount)) diff --git a/test/util/velero/install.go b/test/util/velero/install.go index 027ca43048..0afd571071 100644 --- a/test/util/velero/install.go +++ b/test/util/velero/install.go @@ -192,15 +192,14 @@ func generateVSpherePlugin(veleroCfg *test.VeleroConfig) error { if err := createVCCredentialSecret(cli.ClientGo, veleroCfg.VeleroNamespace); err != nil { // For TKGs/uTKG the VC secret is not supposed to exist. - if apierrors.IsNotFound(err) { - clusterFlavor = "GUEST" - } else { + if !apierrors.IsNotFound(err) { return errors.WithMessagef( err, "Failed to create virtual center credential secret in %s namespace", veleroCfg.VeleroNamespace, ) } + clusterFlavor = "GUEST" } _, err := k8s.CreateConfigMap(