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.

Sign up for GitHub

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

Jump to bottom

Better GCP Error handling. #13

Merged
merged 2 commits into from
May 8, 2024
Merged

Better GCP Error handling. #13

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3fe280
Logging the full GCP API Error
BrennenMM7 May 8, 2024
919cdb9
fix linter
BrennenMM7 May 8, 2024
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
33 changes: 11 additions & 22 deletions cloud/gcperrors/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
package gcperrors

import (
"errors"
"net/http"
"strings"

"google.golang.org/api/googleapi"
)
Expand All @@ -45,31 +45,20 @@
return err
}

// IsAlreadyDeleted reports whether err is a Google API error indicating that the resource is already being deleted.
func IsAlreadyDeleted(err error) bool {
func UnwrapGCPError(err error) error {

Check warning on line 48 in cloud/gcperrors/errors.go

View workflow job for this annotation

GitHub Actions / lint 

exported: exported function UnwrapGCPError should have comment or be unexported (revive)
// If the error is nil, return nil.
if err == nil {
return false
return nil
}
ae, _ := err.(*googleapi.Error)

return strings.Contains(ae.Errors[0].Message, "Instance is already being deleted.")
}

// IsMemberNotFound reports whether err is a Google API error indicating that the member is not found.
func IsMemberNotFound(err error) bool {
if err == nil {
return false
// Check if the error is a Google API error.
ae, ok := err.(*googleapi.Error)
if !ok {
return err
}
ae, _ := err.(*googleapi.Error)

return strings.Contains(ae.Errors[0].Message, "is not a member of")
}
// Unwrap the error and add a prefix to it.
unwrappedGCPError := "GCP error: " + ae.Error()

// PrintGCPError returns the error message from a Google API error.
func PrintGCPError(err error) string {
if err == nil {
return ""
}
ae, _ := err.(*googleapi.Error)
return ae.Message + " " + ae.Errors[0].Message + " " + ae.Errors[0].Reason
return errors.New(unwrappedGCPError)
}
8 changes: 4 additions & 4 deletions cloud/services/compute/instancegroupinstances/instancegroupinstances.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,13 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) {
// Fetch the instance.
instance, err := s.GetInstance(ctx, s.scope.Project(), s.scope.Zone(), s.scope.Name())
if err != nil {
return ctrl.Result{}, err
return ctrl.Result{}, gcperrors.UnwrapGCPError(err)
}

// Fetch the instances disk.
disk, err := s.GetDisk(ctx, s.scope.Project(), s.scope.Zone(), s.scope.Name())
if err != nil {
return ctrl.Result{}, err
return ctrl.Result{}, gcperrors.UnwrapGCPError(err)
}

// Update the GCPMachinePoolMachine status.
Expand Down Expand Up @@ -114,7 +114,7 @@ func (s *Service) Delete(ctx context.Context) (ctrl.Result, error) {
Instances: []string{fmt.Sprintf("zones/%s/instances/%s", s.scope.Zone(), s.scope.Name())},
})
if err != nil {
log.Info("Assuming the instance is already deleted", "error", gcperrors.PrintGCPError(err))
log.Info("Assuming the instance is already deleted", "error", err)
return ctrl.Result{}, nil
}

Expand All @@ -129,7 +129,7 @@ func (s *Service) Delete(ctx context.Context) (ctrl.Result, error) {
// List the instance group instances to check if the instance is deleted.
instances, err := s.ListInstanceGroupInstances(ctx, s.scope.Project(), s.scope.Zone(), s.scope.GCPMachinePool.Name)
if err != nil {
return ctrl.Result{}, err
return ctrl.Result{}, gcperrors.UnwrapGCPError(err)
}

for _, instance := range instances.ManagedInstances {
Expand Down
12 changes: 10 additions & 2 deletions cloud/services/compute/instancegroups/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,11 @@ func (c *GCPClient) WaitUntilOperationCompleted(projectID, operationName string)
}
if operation.Status == "DONE" {
if operation.Error != nil {
return fmt.Errorf("operation failed: %v", operation.Error.Errors)
var errorMessages []string
for _, e := range operation.Error.Errors {
errorMessages = append(errorMessages, e.Message)
}
return fmt.Errorf("operation failed: %v", errorMessages)
}
return nil
}
Expand All @@ -147,7 +151,11 @@ func (c *GCPClient) WaitUntilComputeOperationCompleted(project, zone, operationN
}
if operation.Status == "DONE" {
if operation.Error != nil {
return fmt.Errorf("operation failed: %v", operation.Error.Errors)
var errorMessages []string
for _, e := range operation.Error.Errors {
errorMessages = append(errorMessages, e.Message)
}
return fmt.Errorf("operation failed: %v", errorMessages)
}
return nil
}
Expand Down
20 changes: 10 additions & 10 deletions cloud/services/compute/instancegroups/instancegroups.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) {
instanceTemplate := s.scope.InstanceGroupTemplateBuilder(bootStrapToken)
instanceTemplateHash, err := s.scope.GetInstanceTemplateHash(instanceTemplate)
if err != nil {
return ctrl.Result{}, err
return ctrl.Result{}, gcperrors.UnwrapGCPError(err)
}

// Create the instance template name.
Expand All @@ -86,12 +86,12 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) {
switch {
case err != nil && !gcperrors.IsNotFound(err):
log.Error(err, "Error looking for instance template")
return ctrl.Result{}, err
return ctrl.Result{}, gcperrors.UnwrapGCPError(err)
case err != nil && gcperrors.IsNotFound(err):
log.Info("Instance template not found, creating")
err = s.createInstanceTemplate(ctx, instanceTemplateName, instanceTemplate)
if err != nil {
return ctrl.Result{}, err
return ctrl.Result{}, gcperrors.UnwrapGCPError(err)
}
}

Expand All @@ -100,24 +100,24 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) {
switch {
case err != nil && !gcperrors.IsNotFound(err):
log.Error(err, "Error looking for instance group")
return ctrl.Result{}, err
return ctrl.Result{}, gcperrors.UnwrapGCPError(err)
case err != nil && gcperrors.IsNotFound(err):
log.Info("Instance group not found, creating")
err = s.createInstanceGroup(ctx, instanceTemplateName)
if err != nil {
return ctrl.Result{}, err
return ctrl.Result{}, gcperrors.UnwrapGCPError(err)
}
case err == nil:
log.Info("Instance group found", "instance group", instanceGroup.Name)
patched, err = s.patchInstanceGroup(ctx, instanceTemplateName, instanceGroup)
if err != nil {
log.Error(err, "Error updating instance group")
return ctrl.Result{}, err
return ctrl.Result{}, gcperrors.UnwrapGCPError(err)
}
err = s.removeOldInstanceTemplate(ctx, instanceTemplateName)
if err != nil {
log.Error(err, "Error removing old instance templates")
return ctrl.Result{}, err
return ctrl.Result{}, gcperrors.UnwrapGCPError(err)
}
}

Expand All @@ -127,14 +127,14 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) {
instanceGroup, err = s.Client.GetInstanceGroup(ctx, s.scope.Project(), s.scope.GCPMachinePool.Spec.Zone, s.scope.GCPMachinePool.Name)
if err != nil {
log.Error(err, "Error getting instance group")
return ctrl.Result{}, err
return ctrl.Result{}, gcperrors.UnwrapGCPError(err)
}
}
// List the instance group instances. This is needed to get the provider IDs.
instanceGroupInstances, err := s.Client.ListInstanceGroupInstances(ctx, s.scope.Project(), s.scope.GCPMachinePool.Spec.Zone, s.scope.GCPMachinePool.Name)
if err != nil {
log.Error(err, "Error listing instance group instances")
return ctrl.Result{}, err
return ctrl.Result{}, gcperrors.UnwrapGCPError(err)
}

// Set the MIG state and instances. This is needed to set the status.
Expand All @@ -143,7 +143,7 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) {
s.scope.SetMIGInstances(instanceGroupInstances.ManagedInstances)
} else {
err = fmt.Errorf("instance group or instance group list is nil")
return ctrl.Result{}, err
return ctrl.Result{}, gcperrors.UnwrapGCPError(err)
}
return ctrl.Result{}, nil
}
Expand Down
Loading