From fab2a20242edcce64967b177d5a5d89f2f8cc7b1 Mon Sep 17 00:00:00 2001 From: Pasha Sviderski Date: Tue, 27 Oct 2026 10:05:35 +1000 Subject: [PATCH] refactor(grpc-proxy): use Metadata.Err() method to get upstream error for a broadcasted request --- api/pb/common.go | 8 ++++++++ cmd/uc/image/ls.go | 2 +- cmd/uc/machine/rtt.go | 4 ++-- cmd/uc/ps.go | 4 ++-- internal/machine/docker/client.go | 12 ++++++------ pkg/api/image.go | 9 --------- pkg/client/image.go | 5 +++-- pkg/client/service.go | 8 ++++---- pkg/client/volume.go | 4 ++-- 9 files changed, 28 insertions(+), 28 deletions(-) diff --git a/api/pb/common.go b/api/pb/common.go index 8eea6fad..32ff60c3 100644 --- a/api/pb/common.go +++ b/api/pb/common.go @@ -4,8 +4,16 @@ import ( "bytes" "fmt" "net/netip" + + "google.golang.org/grpc/status" ) +// Err returns the upstream error, preserving its gRPC status code. +// It returns nil for nil metadata or an absent or successful status. +func (m *Metadata) Err() error { + return status.ErrorProto(m.GetStatus()) +} + func NewIP(addr netip.Addr) *IP { // MarshalBinary always returns a nil error. ip, _ := addr.MarshalBinary() diff --git a/cmd/uc/image/ls.go b/cmd/uc/image/ls.go index 59076181..43679e42 100644 --- a/cmd/uc/image/ls.go +++ b/cmd/uc/image/ls.go @@ -103,7 +103,7 @@ func list(ctx context.Context, uncli *cli.CLI, opts listOptions) error { var rows []imageRow for _, machineImages := range clusterImages { - if err := machineImages.Error(); err != nil { + if err := machineImages.Metadata.Err(); err != nil { tui.PrintWarning(fmt.Sprintf("failed to list images on machine '%s': %s", machineImages.Metadata.MachineName, err)) continue diff --git a/cmd/uc/machine/rtt.go b/cmd/uc/machine/rtt.go index ce662409..3a28057d 100644 --- a/cmd/uc/machine/rtt.go +++ b/cmd/uc/machine/rtt.go @@ -54,8 +54,8 @@ func rtt(ctx context.Context, uncli *cli.CLI) error { tui.PrintWarning("metadata is missing in response from unknown server") continue } - if m.Metadata.Error != "" { - tui.PrintWarning(fmt.Sprintf("failed to inspect machine '%s': %s", m.Metadata.MachineName, m.Metadata.Error)) + if err := m.Metadata.Err(); err != nil { + tui.PrintWarning(fmt.Sprintf("failed to inspect machine '%s': %s", m.Metadata.MachineName, err)) continue } if m.Machine == nil { diff --git a/cmd/uc/ps.go b/cmd/uc/ps.go index 249f6e4b..922fa418 100644 --- a/cmd/uc/ps.go +++ b/cmd/uc/ps.go @@ -207,9 +207,9 @@ func collectContainers(ctx context.Context, cli *client.Client) ([]containerInfo machineName := msc.Metadata.MachineName - if msc.Metadata.Error != "" { + if err := msc.Metadata.Err(); err != nil { tui.PrintWarning(fmt.Sprintf("failed to list service containers on machine %s: %s", - machineName, msc.Metadata.Error)) + machineName, err)) continue } diff --git a/internal/machine/docker/client.go b/internal/machine/docker/client.go index 40901bdc..d45a789c 100644 --- a/internal/machine/docker/client.go +++ b/internal/machine/docker/client.go @@ -167,7 +167,7 @@ func (c *Client) ListContainers(ctx context.Context, opts container.ListOptions) machineContainers := make([]MachineContainers, len(resp.Messages)) for i, msg := range resp.Messages { machineContainers[i].Metadata = msg.Metadata - if msg.Metadata != nil && msg.Metadata.Error != "" { + if msg.Metadata.Err() != nil { continue } @@ -258,7 +258,7 @@ func (c *Client) InspectImage(ctx context.Context, id string) ([]api.MachineImag notFoundCount := 0 for _, msg := range resp.Messages { - if msg.Metadata != nil && msg.Metadata.Status != nil && codes.Code(msg.Metadata.Status.Code) == codes.NotFound { + if status.Code(msg.Metadata.Err()) == codes.NotFound { notFoundCount++ } } @@ -269,7 +269,7 @@ func (c *Client) InspectImage(ctx context.Context, id string) ([]api.MachineImag images := make([]api.MachineImage, len(resp.Messages)) for i, msg := range resp.Messages { images[i].Metadata = msg.Metadata - if msg.Metadata != nil && msg.Metadata.Error != "" { + if msg.Metadata.Err() != nil { continue } @@ -312,7 +312,7 @@ func parseRemoteImageMessage(msg *pb.RemoteImage) (api.MachineRemoteImage, error mri := api.MachineRemoteImage{ Metadata: msg.Metadata, } - if msg.Metadata != nil && msg.Metadata.Error != "" { + if msg.Metadata.Err() != nil { return mri, nil } @@ -393,7 +393,7 @@ func (c *Client) ListVolumes(ctx context.Context, opts volume.ListOptions) ([]Ma machineVolumes := make([]MachineVolumes, len(resp.Messages)) for i, msg := range resp.Messages { machineVolumes[i].Metadata = msg.Metadata - if msg.Metadata != nil && msg.Metadata.Error != "" { + if msg.Metadata.Err() != nil { continue } @@ -472,7 +472,7 @@ func (c *Client) ListServiceContainers( machineContainers := make([]MachineServiceContainers, len(resp.Messages)) for i, msg := range resp.Messages { machineContainers[i].Metadata = msg.Metadata - if msg.Metadata != nil && msg.Metadata.Error != "" { + if msg.Metadata.Err() != nil { continue } diff --git a/pkg/api/image.go b/pkg/api/image.go index 1276f79b..0ca05748 100644 --- a/pkg/api/image.go +++ b/pkg/api/image.go @@ -1,7 +1,6 @@ package api import ( - "errors" "slices" "github.com/distribution/reference" @@ -25,14 +24,6 @@ type MachineImages struct { ContainerdStore bool } -// Error returns a non-nil error if listing images on this machine failed. -func (mi MachineImages) Error() error { - if mi.Metadata != nil && mi.Metadata.Error != "" { - return errors.New(mi.Metadata.Error) - } - return nil -} - // ImageFilter defines criteria to filter images in ListImages. type ImageFilter struct { // Machines filters images to those present on the specified machines (names or IDs). diff --git a/pkg/client/image.go b/pkg/client/image.go index 950c6173..e6e83496 100644 --- a/pkg/client/image.go +++ b/pkg/client/image.go @@ -92,7 +92,7 @@ func (cli *Client) ListImages(ctx context.Context, filter api.ImageFilter) ([]ap ContainerdStore: msg.ContainerdStore, } - if msg.Metadata.Error == "" && len(msg.Images) > 0 { + if msg.Metadata.Err() == nil && len(msg.Images) > 0 { if err = json.Unmarshal(msg.Images, &mi.Images); err != nil { return nil, fmt.Errorf("unmarshal images: %w", err) } @@ -104,6 +104,7 @@ func (cli *Client) ListImages(ctx context.Context, filter api.ImageFilter) ([]ap return machineImages, nil } +// TODO: move PushImageOptions to api package and include PushImage in ImageClient interface. type PushImageOptions struct { // AllMachines pushes the image to all machines in the cluster. Takes precedence over Machines field. AllMachines bool @@ -215,7 +216,7 @@ func (cli *Client) pushImageToMachine( Name: "%invalid-name-to-only-check-store-type%", }) if err == nil { - err = images[0].Error() + err = images[0].Metadata.Err() } if err != nil { return fmt.Errorf("check Docker image store type on machine '%s': %w", machine.Name, err) diff --git a/pkg/client/service.go b/pkg/client/service.go index 428c3efb..eabe230f 100644 --- a/pkg/client/service.go +++ b/pkg/client/service.go @@ -125,10 +125,10 @@ func (cli *Client) InspectService(ctx context.Context, nameOrID string) (api.Ser continue } - if mc.Metadata.Error != "" { + if err := mc.Metadata.Err(); err != nil { // TODO: return failed machines in the response. tui.PrintWarning(fmt.Sprintf("failed to list containers on machine '%s': %s", - mc.Metadata.MachineName, mc.Metadata.Error)) + mc.Metadata.MachineName, err)) continue } @@ -333,10 +333,10 @@ func (cli *Client) ListServices(ctx context.Context) ([]api.Service, error) { // Group containers by service ID using the broadcast results. servicesByID := make(map[string]api.Service) for _, mc := range machineContainers { - if mc.Metadata.Error != "" { + if err := mc.Metadata.Err(); err != nil { // TODO: return failed machines in the response. tui.PrintWarning(fmt.Sprintf("failed to list containers on machine '%s': %s", - mc.Metadata.MachineName, mc.Metadata.Error)) + mc.Metadata.MachineName, err)) continue } diff --git a/pkg/client/volume.go b/pkg/client/volume.go index f8b8de3a..1a09629b 100644 --- a/pkg/client/volume.go +++ b/pkg/client/volume.go @@ -70,10 +70,10 @@ func (cli *Client) ListVolumes(ctx context.Context, filter *api.VolumeFilter) ([ continue } - if mv.Metadata.Error != "" { + if err := mv.Metadata.Err(); err != nil { // TODO: return failed machines in the response. tui.PrintWarning(fmt.Sprintf("failed to list volumes on machine '%s': %s", mv.Metadata.MachineName, - mv.Metadata.Error)) + err)) continue }