Skip to content

Commit

Permalink
unify error handling accross all commands
Browse files Browse the repository at this point in the history
- change in dpservice-go library: unified the return values of gRPC functions
- this fixes #639; empty struct is now always returned and accessing Status.Code field doesn't crash dpservice-cli
- changes in dpservice-cli: removed checking for Status.Code as it is handled by error value
- removed the check of Status.Code from RenderList(), Status is checked before, errorous Lists are not rendered and error is returned
  • Loading branch information
vlorinc committed Jan 23, 2025
1 parent 34fac0e commit 6a6fefb
Show file tree
Hide file tree
Showing 40 changed files with 113 additions and 89 deletions.
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/capture_start.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func RunCaptureStart(ctx context.Context, dpdkClientFactory DPDKClientFactory, r
},
})

if err != nil && capture.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error initializing packet capturing: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/capture_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func RunCaptureStatus(
}()

capture, err := client.CaptureStatus(ctx)
if err != nil && capture.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error checking initialization status: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/capture_stop.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func RunCaptureStop(ctx context.Context, dpdkClientFactory DPDKClientFactory, re

captureStop, err := dpdkClient.CaptureStop(ctx)

if err != nil && captureStop.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error stopping capturing: %w", err)
}

Expand Down
9 changes: 0 additions & 9 deletions cli/dpservice-cli/cmd/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,22 +158,13 @@ func (o *RendererOptions) RenderObject(operation string, w io.Writer, obj api.Ob
}

func (o *RendererOptions) RenderList(operation string, w io.Writer, list api.List) error {
if list.GetStatus().Code != 0 {
operation = fmt.Sprintf("server error: %d, %s", list.GetStatus().Code, list.GetStatus().Message)
if o.Output == "table" {
o.Output = "name"
}
}
renderer, err := o.NewRenderer(operation, w)
if err != nil {
return fmt.Errorf("error creating renderer: %w", err)
}
if err := renderer.Render(list); err != nil {
return fmt.Errorf("error rendering %s: %w", list.GetItems()[0].GetKind(), err)
}
if list.GetStatus().Code != 0 {
return fmt.Errorf(strconv.Itoa(apierrors.SERVER_ERROR))
}
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/create_firewall_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func RunCreateFirewallRule(ctx context.Context, dpdkClientFactory DPDKClientFact
Filter: protocolFilter.Filter},
},
})
if err != nil && fwrule.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error creating firewall rule: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/create_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func RunCreateInterface(ctx context.Context, dpdkClientFactory DPDKClientFactory
Metering: &api.MeteringParams{TotalRate: opts.TotalMeterRate, PublicRate: opts.PublicMeterRate},
},
})
if err != nil && iface.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error creating interface: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/create_loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func RunCreateLoadBalancer(ctx context.Context, dpdkClientFactory DPDKClientFact
Lbports: ports,
},
})
if err != nil && lb.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error creating loadbalancer: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/create_loadbalancer_prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func RunCreateLoadBalancerPrefix(
Prefix: opts.Prefix,
},
})
if err != nil && lbprefix.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error creating loadbalancer prefix: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/create_loadbalancer_target.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func RunCreateLoadBalancerTarget(
LoadBalancerTargetMeta: api.LoadBalancerTargetMeta{LoadbalancerID: opts.LoadBalancerID},
Spec: api.LoadBalancerTargetSpec{TargetIP: &opts.TargetIP},
})
if err != nil && lbtarget.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error creating loadbalancer target: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/create_nat.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func RunCreateNat(ctx context.Context, dpdkClientFactory DPDKClientFactory, rend
MaxPort: opts.MaxPort,
},
})
if err != nil && nat.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error creating nat: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/create_neighbor_nat.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func RunCreateNeighborNat(ctx context.Context, dpdkClientFactory DPDKClientFacto
}

nnat, err := client.CreateNeighborNat(ctx, neigbhorNat)
if err != nil && nnat.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error creating neighbor nat: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/create_prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func RunCreatePrefix(
Prefix: opts.Prefix,
},
})
if err != nil && prefix.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error creating prefix: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/create_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func RunCreateRoute(
IP: &opts.NextHopIP,
}},
})
if err != nil && route.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error creating route: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/create_virtualip.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func RunCreateVirtualIP(
IP: &opts.Vip,
},
})
if err != nil && virtualIP.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error creating virtual ip: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/delete_firewall_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func RunDeleteFirewallRule(ctx context.Context, dpdkClientFactory DPDKClientFact
defer DpdkClose(cleanup)

fwrule, err := client.DeleteFirewallRule(ctx, opts.InterfaceID, opts.RuleID)
if err != nil && fwrule.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error deleting firewall rule: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/delete_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func RunDeleteInterface(ctx context.Context, dpdkClientFactory DPDKClientFactory
defer DpdkClose(cleanup)

iface, err := client.DeleteInterface(ctx, opts.ID)
if err != nil && iface.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error deleting interface: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/delete_loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func RunDeleteLoadBalancer(ctx context.Context, dpdkClientFactory DPDKClientFact
defer DpdkClose(cleanup)

lb, err := client.DeleteLoadBalancer(ctx, opts.ID)
if err != nil && lb.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error deleting loadbalancer: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/delete_loadbalancer_prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func RunDeleteLoadBalancerPrefix(ctx context.Context, dpdkClientFactory DPDKClie
defer DpdkClose(cleanup)

lbprefix, err := client.DeleteLoadBalancerPrefix(ctx, opts.InterfaceID, &opts.Prefix)
if err != nil && lbprefix.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error deleting loadbalancer prefix: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/delete_loadbalancer_target.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func RunDeleteLoadBalancerTarget(ctx context.Context, dpdkClientFactory DPDKClie
defer DpdkClose(cleanup)

lbtarget, err := client.DeleteLoadBalancerTarget(ctx, opts.LoadBalancerID, &opts.TargetIP)
if err != nil && lbtarget.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error deleting neighbor nat: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/delete_nat.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func RunDeleteNat(ctx context.Context, dpdkClientFactory DPDKClientFactory, rend
defer DpdkClose(cleanup)

nat, err := client.DeleteNat(ctx, opts.InterfaceID)
if err != nil && nat.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error deleting nat: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/delete_neighbor_nat.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func RunDeleteNeighborNat(ctx context.Context, dpdkClientFactory DPDKClientFacto
},
}
nnat, err := client.DeleteNeighborNat(ctx, &neigbhorNat)
if err != nil && nnat.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error deleting neighbor nat: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/delete_prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func RunDeletePrefix(ctx context.Context, dpdkClientFactory DPDKClientFactory, r
defer DpdkClose(cleanup)

prefix, err := client.DeletePrefix(ctx, opts.InterfaceID, &opts.Prefix)
if err != nil && prefix.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error deleting prefix: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/delete_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func RunDeleteRoute(ctx context.Context, dpdkClientFactory DPDKClientFactory, re
defer DpdkClose(cleanup)

route, err := client.DeleteRoute(ctx, opts.VNI, &opts.Prefix)
if err != nil && route.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error deleting route: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/delete_virtualip.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func RunDeleteVirtualIP(ctx context.Context, dpdkClientFactory DPDKClientFactory
defer DpdkClose(cleanup)

virtualIP, err := client.DeleteVirtualIP(ctx, opts.InterfaceID)
if err != nil && virtualIP.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error deleting virtual ip: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/get_firewall_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func RunGetFirewallRule(
)
} else {
fwrule, err := client.GetFirewallRule(ctx, opts.InterfaceID, opts.RuleID)
if err != nil && fwrule.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error getting firewall rule: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/get_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func RunGetInit(
}()

init, err := client.CheckInitialized(ctx)
if err != nil && init.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error checking initialization status: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/get_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func RunGetInterface(
)
} else {
iface, err := client.GetInterface(ctx, opts.ID)
if err != nil && iface.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error getting interface: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/get_loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func RunGetLoadBalancer(
)
} else {
lb, err := client.GetLoadBalancer(ctx, opts.ID)
if err != nil && lb.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error getting loadbalancer: %w", err)
}

Expand Down
6 changes: 3 additions & 3 deletions cli/dpservice-cli/cmd/get_nat.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,15 @@ func RunGetNat(

if opts.InterfaceID == "" {
ifaces, err := client.ListInterfaces(ctx)
if err != nil && ifaces.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error listing interfaces: %w", err)
}
natList := api.NatList{
TypeMeta: api.TypeMeta{Kind: api.NatListKind},
}
for _, iface := range ifaces.Items {
nat, err := client.GetNat(ctx, iface.ID)
if err != nil && nat.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error getting nat: %w", err)
}
natList.Items = append(natList.Items, *nat)
Expand All @@ -87,7 +87,7 @@ func RunGetNat(
}

nat, err := client.GetNat(ctx, opts.InterfaceID)
if err != nil && nat.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error getting nat: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/get_version.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func RunGetVersion(
ClientVersion: util.BuildVersion,
},
})
if err != nil && svcVersion.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error getting version: %w", err)
}
return rendererFactory.RenderObject("", os.Stdout, svcVersion)
Expand Down
22 changes: 7 additions & 15 deletions cli/dpservice-cli/cmd/get_virtualip.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,31 +70,23 @@ func RunGetVirtualIP(

if opts.InterfaceID == "" {
ifaces, err := client.ListInterfaces(ctx)
if err != nil && ifaces.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error listing interfaces: %w", err)
}
if len(ifaces.Items) == 0 {
return fmt.Errorf("error getting virtual ip: [error code %d] NO_VM", errors.NO_VM)
}

virtualIPs := make([]*api.VirtualIP, 0, len(ifaces.Items))
for _, iface := range ifaces.Items {
vip, err := client.GetVirtualIP(ctx, iface.ID, errors.Ignore(errors.SNAT_NO_DATA))
if err != nil && vip.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error getting virtual ip: %w", err)
}
if vip.Status.Code == 0 {
virtualIPs = append(virtualIPs, vip)
}
}
if len(virtualIPs) == 0 {
noVipFound := api.VirtualIP{
TypeMeta: api.TypeMeta{
Kind: api.VirtualIPKind,
},
Status: api.Status{
Code: errors.SNAT_NO_DATA,
Message: "SNAT_NO_DATA",
},
}
return rendererFactory.RenderObject("no interface has virtual ip configured", os.Stdout, &noVipFound)
}
for _, vip := range virtualIPs {
err = rendererFactory.RenderObject("", os.Stdout, vip)
if err != nil {
Expand All @@ -105,7 +97,7 @@ func RunGetVirtualIP(
}

virtualIP, err := client.GetVirtualIP(ctx, opts.InterfaceID)
if err != nil && virtualIP.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error getting virtual ip: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/get_vni.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func RunGetVni(
defer DpdkClose(cleanup)

vni, err := client.GetVni(ctx, opts.VNI, opts.VniType)
if err != nil && vni.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error getting vni: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/dpservice-cli/cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func RunInit(
}
// else initialize and show uuid
init, err := client.Initialize(ctx)
if err != nil && res.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error initializing: %w", err)
}

Expand Down
4 changes: 2 additions & 2 deletions cli/dpservice-cli/cmd/list_firewall_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@ func RunListFirewallRules(
}
if opts.InterfaceID == "" {
ifaces, err := client.ListInterfaces(ctx)
if err != nil && ifaces.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error listing interfaces: %w", err)
}

for _, iface := range ifaces.Items {
fwrule, err := client.ListFirewallRules(ctx, iface.ID)
if err != nil && fwrule.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error getting firewall rules: %w", err)
}
fwruleList.Items = append(fwruleList.Items, fwrule.Items...)
Expand Down
4 changes: 2 additions & 2 deletions cli/dpservice-cli/cmd/list_loadbalancer_prefixes.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,13 @@ func RunListLoadBalancerPrefixes(
}
if opts.InterfaceID == "" {
ifaces, err := client.ListInterfaces(ctx)
if err != nil && ifaces.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error listing interfaces: %w", err)
}

for _, iface := range ifaces.Items {
prefixes, err := client.ListLoadBalancerPrefixes(ctx, iface.ID)
if err != nil && prefixes.Status.Code == 0 {
if err != nil {
return fmt.Errorf("error getting loadbalancer prefixes: %w", err)
}
for id := range prefixes.Items {
Expand Down
Loading

0 comments on commit 6a6fefb

Please sign in to comment.