Skip to content

Commit

Permalink
Fix firestore error logging and wrapping
Browse files Browse the repository at this point in the history
  • Loading branch information
sunnylovestiramisu committed Dec 27, 2022
1 parent 23b33e4 commit af114e8
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 37 deletions.
10 changes: 5 additions & 5 deletions pkg/cloud_provider/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@ func NewCloud(ctx context.Context, version, configPath, primaryFilestoreServiceE

file, err := file.NewGCFSService(version, client, primaryFilestoreServiceEndpoint, testFilestoreServiceEndpoint)
if err != nil {
return nil, fmt.Errorf("failed to initialize Filestore service: %v", err)
return nil, fmt.Errorf("failed to initialize Filestore service: %w", err)
}

project, zone, err := getProjectAndZone(configFile)
if err != nil {
return nil, fmt.Errorf("failed to initialize project information: %v", err)
return nil, fmt.Errorf("failed to initialize project information: %w", err)
}
return &Cloud{
File: file,
Expand All @@ -89,13 +89,13 @@ func maybeReadConfig(configPath string) (*ConfigFile, error) {

reader, err := os.Open(configPath)
if err != nil {
return nil, fmt.Errorf("couldn't open cloud provider configuration at %s: %v", configPath, err)
return nil, fmt.Errorf("couldn't open cloud provider configuration at %s: %w", configPath, err)
}
defer reader.Close()

cfg := &ConfigFile{}
if err := gcfg.FatalOnly(gcfg.ReadInto(cfg, reader)); err != nil {
return nil, fmt.Errorf("couldn't read cloud provider configuration at %s: %v", configPath, err)
return nil, fmt.Errorf("couldn't read cloud provider configuration at %s: %w", configPath, err)
}
klog.Infof("Config file read %#v", cfg)
return cfg, nil
Expand Down Expand Up @@ -128,7 +128,7 @@ func generateTokenSource(ctx context.Context, configFile *ConfigFile) (oauth2.To
func newOauthClient(ctx context.Context, tokenSource oauth2.TokenSource) (*http.Client, error) {
if err := wait.PollImmediate(5*time.Second, 30*time.Second, func() (bool, error) {
if _, err := tokenSource.Token(); err != nil {
klog.Errorf("error fetching initial token: %v", err)
klog.Errorf("error fetching initial token: %v", err.Error())
return false, nil
}
return true, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/cloud_provider/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
func NewFakeCloud() (*Cloud, error) {
file, err := file.NewFakeService()
if err != nil {
return nil, fmt.Errorf("failed to initialize Filestore service: %v", err)
return nil, fmt.Errorf("failed to initialize Filestore service: %w", err)
}

return &Cloud{
Expand Down
32 changes: 16 additions & 16 deletions pkg/cloud_provider/file/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,18 +396,18 @@ func (manager *gcfsServiceManager) DeleteInstance(ctx context.Context, obj *Serv
klog.V(4).Infof("Starting DeleteInstance cloud operation for instance %s", uri)
op, err := manager.instancesService.Delete(uri).Context(ctx).Do()
if err != nil {
return fmt.Errorf("DeleteInstance operation failed: %v", err)
return fmt.Errorf("DeleteInstance operation failed: %w", err)
}

klog.V(4).Infof("For instance %s, waiting for delete op %v to complete", uri, op.Name)
err = manager.waitForOp(ctx, op)
if err != nil {
return fmt.Errorf("WaitFor DeleteInstance op %s failed: %v", op.Name, err)
return fmt.Errorf("WaitFor DeleteInstance op %s failed: %w", op.Name, err)
}

instance, err := manager.GetInstance(ctx, obj)
if err != nil && !IsNotFoundErr(err) {
return fmt.Errorf("failed to get instance after deletion: %v", err)
return fmt.Errorf("failed to get instance after deletion: %w", err)
}
if instance != nil {
return fmt.Errorf("instance %s still exists after delete operation in state %v", uri, instance.State)
Expand Down Expand Up @@ -483,18 +483,18 @@ func (manager *gcfsServiceManager) ResizeInstance(ctx context.Context, obj *Serv
)
op, err := manager.instancesService.Patch(instanceuri, betaObj).UpdateMask(fileShareUpdateMask).Context(ctx).Do()
if err != nil {
return nil, fmt.Errorf("patch operation failed: %v", err)
return nil, fmt.Errorf("patch operation failed: %w", err)
}

klog.V(4).Infof("For instance %s, waiting for patch op %v to complete", instanceuri, op.Name)
err = manager.waitForOp(ctx, op)
if err != nil {
return nil, fmt.Errorf("WaitFor patch op %s failed: %v", op.Name, err)
return nil, fmt.Errorf("WaitFor patch op %s failed: %w", op.Name, err)
}

instance, err := manager.GetInstance(ctx, obj)
if err != nil {
return nil, fmt.Errorf("failed to get instance after creation: %v", err)
return nil, fmt.Errorf("failed to get instance after creation: %w", err)
}
klog.V(4).Infof("After resize got instance %#v", instance)
return instance, nil
Expand Down Expand Up @@ -533,7 +533,7 @@ func (manager *gcfsServiceManager) CreateBackup(ctx context.Context, obj *Servic
klog.V(4).Infof("For backup uri %s, waiting for backup op %v to complete", backupUri, opbackup.Name)
err = manager.waitForOp(ctx, opbackup)
if err != nil {
return nil, fmt.Errorf("WaitFor CreateBackup op %s for source instance %v, backup uri: %v, operation failed: %v", opbackup.Name, backupSource, backupUri, err)
return nil, fmt.Errorf("WaitFor CreateBackup op %s for source instance %v, backup uri: %v, operation failed: %w", opbackup.Name, backupSource, backupUri, err)
}

backupObj, err := manager.backupService.Get(backupUri).Context(ctx).Do()
Expand All @@ -550,13 +550,13 @@ func (manager *gcfsServiceManager) CreateBackup(ctx context.Context, obj *Servic
func (manager *gcfsServiceManager) DeleteBackup(ctx context.Context, backupId string) error {
opbackup, err := manager.backupService.Delete(backupId).Context(ctx).Do()
if err != nil {
return fmt.Errorf("for backup Id %s, delete backup operation %s failed: %v", backupId, opbackup.Name, err)
return fmt.Errorf("for backup Id %s, delete backup operation %s failed: %w", backupId, opbackup.Name, err)
}

klog.V(4).Infof("For backup Id %s, waiting for backup op %v to complete", backupId, opbackup.Name)
err = manager.waitForOp(ctx, opbackup)
if err != nil {
return fmt.Errorf("delete backup: %v, op %s failed: %v", backupId, opbackup.Name, err)
return fmt.Errorf("delete backup: %v, op %s failed: %w", backupId, opbackup.Name, err)
}

klog.Infof("Backup %v successfully deleted", backupId)
Expand Down Expand Up @@ -680,7 +680,7 @@ func (manager *gcfsServiceManager) HasOperations(ctx context.Context, obj *Servi
for {
resp, err := manager.operationsService.List(locationURI(obj.Project, obj.Location)).PageToken(nextToken).Context(ctx).Do()
if err != nil {
return false, fmt.Errorf("list operations for instance %q, token %q failed: %v", uri, nextToken, err)
return false, fmt.Errorf("list operations for instance %q, token %q failed: %w", uri, nextToken, err)
}

filteredOps, err := ApplyFilter(resp.Operations, uri, operationType, done)
Expand Down Expand Up @@ -777,7 +777,7 @@ func (manager *gcfsServiceManager) StartCreateMultishareInstanceOp(ctx context.C

op, err := manager.multishareInstancesService.Create(locationURI(instance.Project, instance.Location), targetinstance).InstanceId(instance.Name).Context(ctx).Do()
if err != nil {
return nil, fmt.Errorf("CreateInstance operation failed: %v", err)
return nil, fmt.Errorf("CreateInstance operation failed: %w", err)
}
klog.Infof("Started create instance op %s, for instance %q project %q, location %q, tier %q, capacity %v, network %q, ipRange %q, connectMode %q, KmsKeyName %q, labels %v, description %s", op.Name, instance.Name, instance.Project, instance.Location, targetinstance.Tier, targetinstance.CapacityGb, targetinstance.Networks[0].Network, targetinstance.Networks[0].ReservedIpRange, targetinstance.Networks[0].ConnectMode, targetinstance.KmsKeyName, targetinstance.Labels, targetinstance.Description)
return op, nil
Expand All @@ -787,7 +787,7 @@ func (manager *gcfsServiceManager) StartDeleteMultishareInstanceOp(ctx context.C
uri := instanceURI(instance.Project, instance.Location, instance.Name)
op, err := manager.multishareInstancesService.Delete(uri).Context(ctx).Do()
if err != nil {
return nil, fmt.Errorf("DeleteInstance operation failed: %v", err)
return nil, fmt.Errorf("DeleteInstance operation failed: %w", err)
}
klog.Infof("Started Delete Instance op %s for instance uri %s", op.Name, uri)
return op, nil
Expand All @@ -813,7 +813,7 @@ func (manager *gcfsServiceManager) StartResizeMultishareInstanceOp(ctx context.C
}
op, err := manager.multishareInstancesService.Patch(instanceuri, targetinstance).UpdateMask(multishareCapacityUpdateMask).Context(ctx).Do()
if err != nil {
return nil, fmt.Errorf("patch operation failed: %v for instance %+v", err, targetinstance)
return nil, fmt.Errorf("patch operation failed: %w for instance %+v", err, targetinstance)
}

klog.Infof("Started instance update operation %s for instance %+v", op.Name, targetinstance)
Expand All @@ -830,7 +830,7 @@ func (manager *gcfsServiceManager) StartCreateShareOp(ctx context.Context, share

op, err := manager.multishareInstancesSharesService.Create(instanceuri, targetshare).ShareId(share.Name).Context(ctx).Do()
if err != nil {
return nil, fmt.Errorf("CreateShare operation failed: %v", err)
return nil, fmt.Errorf("CreateShare operation failed: %w", err)
}
klog.Infof("Started Create Share op %s for share %q instance uri %q, with capacity(GB) %v, Labels %v", op.Name, share.Name, instanceuri, targetshare.CapacityGb, targetshare.Labels)
return op, nil
Expand All @@ -840,7 +840,7 @@ func (manager *gcfsServiceManager) StartDeleteShareOp(ctx context.Context, share
uri := shareURI(share.Parent.Project, share.Parent.Location, share.Parent.Name, share.Name)
op, err := manager.multishareInstancesSharesService.Delete(uri).Context(ctx).Do()
if err != nil {
return nil, fmt.Errorf("DeleteShare operation failed: %v", err)
return nil, fmt.Errorf("DeleteShare operation failed: %w", err)
}
klog.Infof("Started Delete Share op %s for share uri %q ", op.Name, uri)
return op, nil
Expand All @@ -855,7 +855,7 @@ func (manager *gcfsServiceManager) StartResizeShareOp(ctx context.Context, share
}
op, err := manager.multishareInstancesSharesService.Patch(uri, targetShare).UpdateMask(multishareCapacityUpdateMask).Context(ctx).Do()
if err != nil {
return nil, fmt.Errorf("ResizeShare operation failed: %v", err)
return nil, fmt.Errorf("ResizeShare operation failed: %w", err)
}
klog.Infof("Started Resize Share op %s for share uri %q ", op.Name, uri)
return op, nil
Expand Down
4 changes: 2 additions & 2 deletions pkg/cloud_provider/metadata/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ var _ Service = &metadataServiceManager{}
func NewMetadataService() (Service, error) {
zone, err := metadata.Zone()
if err != nil {
return nil, fmt.Errorf("failed to get current zone: %v", err)
return nil, fmt.Errorf("failed to get current zone: %w", err)
}
projectID, err := metadata.ProjectID()
if err != nil {
return nil, fmt.Errorf("failed to get project: %v", err)
return nil, fmt.Errorf("failed to get project: %w", err)
}

return &metadataServiceManager{
Expand Down
6 changes: 3 additions & 3 deletions pkg/csi_driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func (s *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu
}
_, err = s.config.fileService.GetBackup(ctx, id)
if err != nil {
klog.Errorf("Failed to get volume %v source snapshot %v: %v", name, id, err)
klog.Errorf("Failed to get volume %v source snapshot %v: %v", name, id, err.Error())
if errCode := file.IsUserError(err); errCode != nil {
return nil, status.Error(*errCode, err.Error())
}
Expand Down Expand Up @@ -266,7 +266,7 @@ func (s *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu
filer, createErr = s.config.fileService.CreateInstance(ctx, newFiler)
}
if createErr != nil {
klog.Errorf("Create volume for volume Id %s failed: %v", volumeID, createErr)
klog.Errorf("Create volume for volume Id %s failed: %v", volumeID, createErr.Error())
if errCode := file.IsUserError(err); errCode != nil {
return nil, status.Error(*errCode, err.Error())
}
Expand Down Expand Up @@ -376,7 +376,7 @@ func (s *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVolu

err = s.config.fileService.DeleteInstance(ctx, filer)
if err != nil {
klog.Errorf("Delete volume for volume Id %s failed: %v", volumeID, err)
klog.Errorf("Delete volume for volume Id %s failed: %v", volumeID, err.Error())
return nil, status.Error(codes.Internal, err.Error())
}

Expand Down
2 changes: 1 addition & 1 deletion test/k8s-integration/filter-junit.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func MergeJUnit(testFilter string, sourceDirectories []string, destination strin
for _, dir := range sourceDirectories {
files, err := ioutil.ReadDir(dir)
if err != nil {
klog.Errorf("Failed to read juint directory %s: %v", dir, err)
klog.Errorf("Failed to read juint directory %s: %v", dir, err.Error())
mergeErrors = append(mergeErrors, err.Error())
continue
}
Expand Down
14 changes: 7 additions & 7 deletions test/k8s-integration/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func handle() error {
defer func() {
err = setEnvProject(oldProjectStr)
if err != nil {
klog.Errorf("failed to set project environment to %s: %v", oldProject, err)
klog.Errorf("failed to set project environment to %s: %v", oldProject, err.Error())
}
}()

Expand All @@ -240,7 +240,7 @@ func handle() error {
if *teardownCluster {
err := deleteImage(*stagingImage, testParams.stagingVersion)
if err != nil {
klog.Errorf("failed to delete image: %v", err)
klog.Errorf("failed to delete image: %v", err.Error())
}
}
}()
Expand Down Expand Up @@ -287,12 +287,12 @@ func handle() error {
case "gce":
err := clusterDownGCE(testParams.testDir)
if err != nil {
klog.Errorf("failed to cluster down: %v", err)
klog.Errorf("failed to cluster down: %v", err.Error())
}
case "gke":
err := clusterDownGKE(*gceZone, *gceRegion)
if err != nil {
klog.Errorf("failed to cluster down: %v", err)
klog.Errorf("failed to cluster down: %v", err.Error())
}
default:
klog.Errorf("deployment-strategy must be set to 'gce', but is: %s", *deploymentStrat)
Expand All @@ -305,18 +305,18 @@ func handle() error {
if *teardownDriver {
defer func() {
if teardownErr := deleteDriver(testParams.goPath, testParams.pkgDir, *deployOverlayName); teardownErr != nil {
klog.Errorf("failed to delete driver: %v", teardownErr)
klog.Errorf("failed to delete driver: %v", teardownErr.Error())
}
}()
}
if err != nil {
return fmt.Errorf("failed to install CSI Driver: %v", err)
return fmt.Errorf("failed to install CSI Driver: %w", err)
}
}

cancel, err := dumpDriverLogs()
if err != nil {
return fmt.Errorf("failed to start driver logging: %v", err)
return fmt.Errorf("failed to start driver logging: %w", err)
}
defer func() {
if cancel != nil {
Expand Down
4 changes: 2 additions & 2 deletions test/remote/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (i *InstanceInfo) CreateOrGetInstance(serviceAccount string) error {

instance, err = i.computeService.Instances.Get(i.project, i.zone, i.name).Do()
if err != nil {
klog.Errorf("Failed to get instance %v: %v", i.name, err)
klog.Errorf("Failed to get instance %v: %v", i.name, err.Error())
return false, nil
}

Expand Down Expand Up @@ -188,7 +188,7 @@ func (i *InstanceInfo) DeleteInstance() {
if isGCEError(err, "notFound") {
return
}
klog.Errorf("Error deleting instance %q: %v", i.name, err)
klog.Errorf("Error deleting instance %q: %v", i.name, err.Error())
}
}

Expand Down

0 comments on commit af114e8

Please sign in to comment.