Improved gRPC error codes for IRI methods

internal/server/common.go

@@ -0,0 +1,48 @@
+// SPDX-FileCopyrightText: 2023 SAP SE or an SAP affiliate company and IronCore contributors
+// SPDX-License-Identifier: Apache-2.0
+package server
+
+import (
+	"errors"
+
+	"github.com/ironcore-dev/provider-utils/storeutils/store"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+var (
+	ErrMachineNotFound            = errors.New("machine not found")
+	ErrNicNotFound                = errors.New("nic not found")
+	ErrVolumeNotFound             = errors.New("volume not found")
+	ErrMachineIsntManaged         = errors.New("machine isn't managed")
+	ErrMachineUnavailable         = errors.New("machine isn't available")
+	ErrPCIControllerMaxedOut      = errors.New("pci controllers count already maxed out")
+	ErrActiveConsoleSessionExists = errors.New("active console session exists for this domain")
+	ErrInvalidRequest             = errors.New("invalid request")
+)
+
+func convertInternalErrorToGRPC(err error) error {
+	_, ok := status.FromError(err)
+	if ok {
+		return err
+	}
+
+	code := codes.Internal
+
+	switch {
+	case errors.Is(err, store.ErrNotFound), errors.Is(err, ErrMachineNotFound), errors.Is(err, ErrVolumeNotFound), errors.Is(err, ErrNicNotFound):
+		code = codes.NotFound
+	case errors.Is(err, ErrMachineIsntManaged):
+		code = codes.InvalidArgument
+	case errors.Is(err, ErrPCIControllerMaxedOut):
+		code = codes.ResourceExhausted
+	case errors.Is(err, ErrActiveConsoleSessionExists):
+		code = codes.FailedPrecondition
+	case errors.Is(err, ErrMachineUnavailable):
+		code = codes.Unavailable
+	case errors.Is(err, ErrInvalidRequest):
+		code = codes.InvalidArgument
+	}
+
+	return status.Error(code, err.Error())
+}

internal/server/exec.go

@@ -21,8 +21,6 @@ import (
 	libvirtutils "github.com/ironcore-dev/libvirt-provider/internal/libvirt/utils"
 	"github.com/ironcore-dev/provider-utils/storeutils/store"
 	"github.com/moby/term"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 	"k8s.io/client-go/tools/remotecommand"
 	"libvirt.org/go/libvirtxml"
 )
@@ -43,10 +41,7 @@ func (s *Server) Exec(ctx context.Context, req *iri.ExecRequest) (*iri.ExecRespo
 	log := s.loggerFrom(ctx, "MachineID", req.MachineId)
 	log.V(1).Info("Verifying machine in the store")
 	if _, err := s.machineStore.Get(ctx, req.MachineId); err != nil {
-		if !errors.Is(err, store.ErrNotFound) {
-			return nil, fmt.Errorf("error getting machine: %w", err)
-		}
-		return nil, status.Errorf(codes.NotFound, "machine %s not found", req.MachineId)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("error getting machine: %w", err))
 	}
 
 	log.V(1).Info("Inserting request into cache")
@@ -109,43 +104,43 @@ func (e executorExec) Exec(ctx context.Context, in io.Reader, out io.WriteCloser
 	// Check if a console is already active for this machine
 	_, loaded := e.activeConsoles.LoadOrStore(machineID, true)
 	if loaded {
-		return errors.New("operation failed: Active console session exists for this domain")
+		return convertInternalErrorToGRPC(fmt.Errorf("operation failed: %w", ErrActiveConsoleSessionExists))
 	}
 
 	defer e.activeConsoles.Delete(machineID)
 
 	// Check if the apiMachine doesn't exist, to avoid making the libvirt-lookup call.
 	if e.Machine == nil {
-		return fmt.Errorf("apiMachine %w in the store", store.ErrNotFound)
+		return convertInternalErrorToGRPC(fmt.Errorf("apiMachine %w in the store", ErrMachineNotFound))
 	}
 
 	domain, err := e.Libvirt.DomainLookupByUUID(libvirtutils.UUIDStringToBytes(machineID))
 	if err != nil {
 		if !libvirtutils.IsErrorCode(err, libvirt.ErrNoDomain) {
-			return fmt.Errorf("error looking up domain: %w", err)
+			return convertInternalErrorToGRPC(fmt.Errorf("error looking up domain: %w", err))
 		}
 
-		return fmt.Errorf("machine %s has not yet been synced", machineID)
+		return convertInternalErrorToGRPC(fmt.Errorf("machine %s has not yet been synced: %w %w", machineID, ErrMachineUnavailable, err))
 	}
 
 	domainXMLData, err := e.Libvirt.DomainGetXMLDesc(domain, 0)
 	if err != nil {
-		return fmt.Errorf("failed to lookup domain: %w", err)
+		return convertInternalErrorToGRPC(fmt.Errorf("failed to lookup domain: %w", err))
 	}
 
 	domainXML := &libvirtxml.Domain{}
 	if err := domainXML.Unmarshal(domainXMLData); err != nil {
-		return fmt.Errorf("failed to unmarshal domain: %w", err)
+		return convertInternalErrorToGRPC(fmt.Errorf("failed to unmarshal domain: %w", err))
 	}
 
 	if domainXML.Devices == nil || len(domainXML.Devices.Consoles) == 0 {
-		return errors.New("device console not set in machine domainXML")
+		return convertInternalErrorToGRPC(errors.New("device console not set in machine domainXML"))
 	}
 	ttyPath := domainXML.Devices.Consoles[0].TTY
 
 	f, err := os.OpenFile(ttyPath, os.O_RDWR, 0)
 	if err != nil {
-		return fmt.Errorf("error opening PTY: %w", err)
+		return convertInternalErrorToGRPC(fmt.Errorf("error opening PTY: %w", err))
 	}
 
 	// Wrap the input stream with an escape proxy. Escape Sequence Ctrl + ] = 29

internal/server/machine_annotations_update.go

@@ -5,15 +5,11 @@ package server
 
 import (
 	"context"
-	"errors"
 	"fmt"
 
 	iri "github.com/ironcore-dev/ironcore/iri/apis/machine/v1alpha1"
 	"github.com/ironcore-dev/libvirt-provider/api"
 	apiutils "github.com/ironcore-dev/provider-utils/apiutils/api"
-	"github.com/ironcore-dev/provider-utils/storeutils/store"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 func (s *Server) updateAnnotations(ctx context.Context, machine *api.Machine, annotations map[string]string) error {
@@ -34,14 +30,11 @@ func (s *Server) UpdateMachineAnnotations(ctx context.Context, req *iri.UpdateMa
 	log.V(1).Info("Getting machine")
 	machine, err := s.machineStore.Get(ctx, req.MachineId)
 	if err != nil {
-		if !errors.Is(err, store.ErrNotFound) {
-			return nil, fmt.Errorf("error getting machine: %w", err)
-		}
-		return nil, status.Errorf(codes.NotFound, "machine %s not found", req.MachineId)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("failed to get machine '%s': %w", req.MachineId, err))
 	}
 
 	if err := s.updateAnnotations(ctx, machine, req.Annotations); err != nil {
-		return nil, fmt.Errorf("failed to update machine annotations: %w", err)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("failed to update machine annotations: %w", err))
 	}
 
 	return &iri.UpdateMachineAnnotationsResponse{}, nil

internal/server/machine_create.go

@@ -103,13 +103,13 @@ func (s *Server) CreateMachine(ctx context.Context, req *iri.CreateMachineReques
 	log.V(1).Info("Creating machine from iri machine")
 	machine, err := s.createMachineFromIRIMachine(ctx, log, req.Machine)
 	if err != nil {
-		return nil, fmt.Errorf("unable to get libvirt machine config: %w", err)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("unable to get libvirt machine config: %w", err))
 	}
 
 	log.V(1).Info("Converting machine to iri machine")
 	iriMachine, err := s.convertMachineToIRIMachine(ctx, log, machine)
 	if err != nil {
-		return nil, fmt.Errorf("unable to convert machine: %w", err)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("unable to convert machine: %w", err))
 	}
 
 	return &iri.CreateMachineResponse{

internal/server/machine_delete.go

@@ -5,24 +5,17 @@ package server
 
 import (
 	"context"
-	"errors"
 	"fmt"
 
 	iri "github.com/ironcore-dev/ironcore/iri/apis/machine/v1alpha1"
-	"github.com/ironcore-dev/provider-utils/storeutils/store"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 func (s *Server) DeleteMachine(ctx context.Context, req *iri.DeleteMachineRequest) (*iri.DeleteMachineResponse, error) {
 	log := s.loggerFrom(ctx)
 
 	log.V(1).Info("Deleting machine")
 	if err := s.machineStore.Delete(ctx, req.MachineId); err != nil {
-		if !errors.Is(err, store.ErrNotFound) {
-			return nil, fmt.Errorf("error deleting machine: %w", err)
-		}
-		return nil, status.Errorf(codes.NotFound, "machine %s not found", req.MachineId)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("error deleting machine: '%s': %w", req.MachineId, err))
 	}
 
 	return &iri.DeleteMachineResponse{}, nil

internal/server/machine_list.go

@@ -12,22 +12,20 @@ import (
 	iri "github.com/ironcore-dev/ironcore/iri/apis/machine/v1alpha1"
 	"github.com/ironcore-dev/libvirt-provider/api"
 	"github.com/ironcore-dev/provider-utils/storeutils/store"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 	"k8s.io/apimachinery/pkg/labels"
 )
 
 func (s *Server) getLibvirtMachine(ctx context.Context, id string) (*api.Machine, error) {
 	machine, err := s.machineStore.Get(ctx, id)
 	if err != nil {
 		if errors.Is(err, store.ErrNotFound) {
-			return nil, status.Errorf(codes.NotFound, "machine %s not found", id)
+			return nil, fmt.Errorf("failed to get machine %s: %w", id, ErrMachineNotFound)
 		}
 		return nil, fmt.Errorf("failed to get machine: %w", err)
 	}
 
 	if !api.IsManagedBy(machine, api.MachineManager) {
-		return nil, status.Errorf(codes.NotFound, "machine %s not found", id)
+		return nil, fmt.Errorf("missing manage label for %s: %w", api.MachineManager, ErrMachineIsntManaged)
 	}
 
 	return machine, nil
@@ -89,8 +87,8 @@ func (s *Server) ListMachines(ctx context.Context, req *iri.ListMachinesRequest)
 	if filter := req.Filter; filter != nil && filter.Id != "" {
 		machine, err := s.getMachine(ctx, log, filter.Id)
 		if err != nil {
-			if status.Code(err) != codes.NotFound {
-				return nil, err
+			if !errors.Is(err, ErrMachineNotFound) && !errors.Is(err, ErrMachineIsntManaged) {
+				return nil, convertInternalErrorToGRPC(err)
 			}
 			return &iri.ListMachinesResponse{
 				Machines: []*iri.Machine{},
@@ -104,7 +102,7 @@ func (s *Server) ListMachines(ctx context.Context, req *iri.ListMachinesRequest)
 
 	machines, err := s.listMachines(ctx, log)
 	if err != nil {
-		return nil, err
+		return nil, convertInternalErrorToGRPC(err)
 	}
 
 	machines = s.filterMachines(machines, req.Filter)

internal/server/machine_networkinterface_attach.go

@@ -15,17 +15,17 @@ func (s *Server) AttachNetworkInterface(ctx context.Context, req *iri.AttachNetw
 	log.V(1).Info("Attaching NIC to machine")
 
 	if req == nil {
-		return nil, fmt.Errorf("AttachNetworkInterfaceRequest is nil")
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("AttachNetworkInterfaceRequest is nil: %w", ErrInvalidRequest))
 	}
 
 	apiMachine, err := s.machineStore.Get(ctx, req.MachineId)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get machine: %w", err)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("failed to get machine '%s': %w", req.MachineId, err))
 	}
 
 	nicSpec, err := s.getNICFromIRINIC(req.NetworkInterface)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get nic from iri nic: %w", err)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("failed to get nic from iri nic: %w", err))
 	}
 
 	apiMachine.Spec.NetworkInterfaces = append(apiMachine.Spec.NetworkInterfaces, nicSpec)

internal/server/machine_networkinterface_detach.go

@@ -19,12 +19,12 @@ func (s *Server) DetachNetworkInterface(
 	log.V(1).Info("Detaching nic from machine")
 
 	if req == nil {
-		return nil, fmt.Errorf("DetachNetworkInterface is nil")
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("DetachNetworkInterface is nil: %w", ErrInvalidRequest))
 	}
 
 	apiMachine, err := s.machineStore.Get(ctx, req.MachineId)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get machine: %w", err)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("failed to get machine '%s': %w", req.MachineId, err))
 	}
 
 	var updatedNICS []*api.NetworkInterfaceSpec
@@ -38,13 +38,13 @@ func (s *Server) DetachNetworkInterface(
 	}
 
 	if !found {
-		return nil, fmt.Errorf("nic '%s' not found in machine '%s'", req.Name, req.MachineId)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("nic '%s' not found in machine '%s': %w", req.Name, req.MachineId, ErrNicNotFound))
 	}
 
 	apiMachine.Spec.NetworkInterfaces = updatedNICS
 
 	if _, err := s.machineStore.Update(ctx, apiMachine); err != nil {
-		return nil, fmt.Errorf("failed to update machine: %w", err)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("failed to update machine: %w", err))
 	}
 
 	return &iri.DetachNetworkInterfaceResponse{}, nil

internal/server/machine_powerstate_update.go

@@ -5,14 +5,10 @@ package server
 
 import (
 	"context"
-	"errors"
 	"fmt"
 
 	iri "github.com/ironcore-dev/ironcore/iri/apis/machine/v1alpha1"
 	"github.com/ironcore-dev/libvirt-provider/api"
-	"github.com/ironcore-dev/provider-utils/storeutils/store"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 func (s *Server) updatePowerState(ctx context.Context, machine *api.Machine, iriPower iri.Power) error {
@@ -36,14 +32,11 @@ func (s *Server) UpdateMachinePower(ctx context.Context, req *iri.UpdateMachineP
 	log.V(1).Info("Getting machine")
 	machine, err := s.machineStore.Get(ctx, req.MachineId)
 	if err != nil {
-		if !errors.Is(err, store.ErrNotFound) {
-			return nil, fmt.Errorf("error getting machine: %w", err)
-		}
-		return nil, status.Errorf(codes.NotFound, "machine %s not found", req.MachineId)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("failed to get machine '%s': %w", req.MachineId, err))
 	}
 
 	if err := s.updatePowerState(ctx, machine, req.Power); err != nil {
-		return nil, fmt.Errorf("failed to update power state: %w", err)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("failed to update power state: %w", err))
 	}
 
 	return &iri.UpdateMachinePowerResponse{}, nil

internal/server/machine_volume_attach.go

@@ -15,23 +15,23 @@ func (s *Server) AttachVolume(ctx context.Context, req *iri.AttachVolumeRequest)
 	log.V(1).Info("Attaching volume to machine")
 
 	if req == nil || req.MachineId == "" || req.Volume == nil {
-		return nil, fmt.Errorf("invalid request")
+		return nil, convertInternalErrorToGRPC(ErrInvalidRequest)
 	}
 
 	apiMachine, err := s.machineStore.Get(ctx, req.MachineId)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get machine: %w", err)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("failed to get machine '%s': %w", req.MachineId, err))
 	}
 
 	volumeSpec, err := s.getVolumeFromIRIVolume(req.Volume)
 	if err != nil {
-		return nil, fmt.Errorf("error converting volume: %w", err)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("error converting volume: %w", err))
 	}
 
 	apiMachine.Spec.Volumes = append(apiMachine.Spec.Volumes, volumeSpec)
 
 	if _, err := s.machineStore.Update(ctx, apiMachine); err != nil {
-		return nil, fmt.Errorf("failed to update machine with new volume: %w", err)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("failed to update machine with new volume: %w", err))
 	}
 
 	return &iri.AttachVolumeResponse{}, nil

internal/server/machine_volume_detach.go

@@ -16,12 +16,12 @@ func (s *Server) DetachVolume(ctx context.Context, req *iri.DetachVolumeRequest)
 	log.V(1).Info("Detaching volume from machine")
 
 	if req == nil || req.MachineId == "" || req.Name == "" {
-		return nil, fmt.Errorf("invalid request")
+		return nil, convertInternalErrorToGRPC(ErrInvalidRequest)
 	}
 
 	apiMachine, err := s.machineStore.Get(ctx, req.MachineId)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get machine: %w", err)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("failed to get machine '%s': %w", req.MachineId, err))
 	}
 
 	var updatedVolumes []*api.VolumeSpec
@@ -35,13 +35,13 @@ func (s *Server) DetachVolume(ctx context.Context, req *iri.DetachVolumeRequest)
 	}
 
 	if !found {
-		return nil, fmt.Errorf("volume '%s' not found in machine '%s'", req.Name, req.MachineId)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("volume '%s' not found in machine '%s': %w", req.Name, req.MachineId, ErrVolumeNotFound))
 	}
 
 	apiMachine.Spec.Volumes = updatedVolumes
 
 	if _, err := s.machineStore.Update(ctx, apiMachine); err != nil {
-		return nil, fmt.Errorf("failed to update machine after detaching volume: %w", err)
+		return nil, convertInternalErrorToGRPC(fmt.Errorf("failed to update machine after detaching volume: %w", err))
 	}
 
 	return &iri.DetachVolumeResponse{}, nil