diff --git a/client/remove_image.go b/client/remove_image.go index 6631e3f..daedcae 100644 --- a/client/remove_image.go +++ b/client/remove_image.go @@ -16,8 +16,6 @@ package client import ( "context" - "errors" - cpb "github.com/openconfig/gnoi/containerz" ) @@ -25,23 +23,12 @@ import ( // returns an error indicating whether the image was not found or is associated to running // container. func (c *Client) RemoveImage(ctx context.Context, image string, tag string, force bool) error { - resp, err := c.cli.RemoveImage(ctx, &cpb.RemoveImageRequest{ + if _, err := c.cli.RemoveImage(ctx, &cpb.RemoveImageRequest{ Name: image, Tag: tag, Force: force, - }) - if err != nil { + }); err != nil { return err } - - switch resp.GetCode() { - case cpb.RemoveImageResponse_SUCCESS: - return nil - case cpb.RemoveImageResponse_NOT_FOUND: - return ErrNotFound - case cpb.RemoveImageResponse_RUNNING: - return ErrRunning - default: - return errors.New("unknown error occurred") - } + return nil } diff --git a/client/remove_image_test.go b/client/remove_image_test.go index 5d5ba0b..b984858 100644 --- a/client/remove_image_test.go +++ b/client/remove_image_test.go @@ -30,11 +30,12 @@ type fakeImageRemovingContainerzServer struct { receivedMsg *cpb.RemoveImageRequest sendMsg *cpb.RemoveImageResponse + sendErr error } func (f *fakeImageRemovingContainerzServer) RemoveImage(_ context.Context, req *cpb.RemoveImageRequest) (*cpb.RemoveImageResponse, error) { f.receivedMsg = req - return f.sendMsg, nil + return f.sendMsg, f.sendErr } func TestImageRemove(t *testing.T) { @@ -49,10 +50,8 @@ func TestImageRemove(t *testing.T) { wantErr error }{ { - name: "success", - inMsg: &cpb.RemoveImageResponse{ - Code: cpb.RemoveImageResponse_SUCCESS, - }, + name: "success", + inMsg: &cpb.RemoveImageResponse{}, inImage: "some-image", inTag: "some-tag", wantMsg: &cpb.RemoveImageRequest{ @@ -62,10 +61,8 @@ func TestImageRemove(t *testing.T) { }, }, { - name: "success-with-force", - inMsg: &cpb.RemoveImageResponse{ - Code: cpb.RemoveImageResponse_SUCCESS, - }, + name: "success-with-force", + inMsg: &cpb.RemoveImageResponse{}, inImage: "some-image", inTag: "some-tag", inForce: true, @@ -76,10 +73,7 @@ func TestImageRemove(t *testing.T) { }, }, { - name: "not-found", - inMsg: &cpb.RemoveImageResponse{ - Code: cpb.RemoveImageResponse_NOT_FOUND, - }, + name: "not-found", inImage: "some-image", inTag: "some-tag", wantMsg: &cpb.RemoveImageRequest{ @@ -90,10 +84,7 @@ func TestImageRemove(t *testing.T) { wantErr: ErrNotFound, }, { - name: "running", - inMsg: &cpb.RemoveImageResponse{ - Code: cpb.RemoveImageResponse_RUNNING, - }, + name: "running", inImage: "some-image", inTag: "some-tag", wantMsg: &cpb.RemoveImageRequest{ @@ -110,6 +101,8 @@ func TestImageRemove(t *testing.T) { t.Run(tc.name, func(t *testing.T) { fcm := &fakeImageRemovingContainerzServer{ sendMsg: tc.inMsg, + // passthrough error from test. + sendErr: tc.wantErr, } addr, stop := newServer(t, fcm) defer stop() diff --git a/client/start_container.go b/client/start_container.go index 2e9a3fa..46f409b 100644 --- a/client/start_container.go +++ b/client/start_container.go @@ -43,7 +43,8 @@ func (c *Client) StartContainer(ctx context.Context, image string, tag string, c case *cpb.StartContainerResponse_StartOk: return resp.GetStartOk().GetInstanceName(), nil case *cpb.StartContainerResponse_StartError: - return "", status.Errorf(codes.Internal, "failed to start container: %s", resp.GetStartError().GetDetails()) + return "", status.Errorf(codes.Unknown, + "got StartError returned - please update containerz server") default: return "", status.Error(codes.Unknown, "unknown container state") } diff --git a/client/start_container_test.go b/client/start_container_test.go index 66be8ce..e47817b 100644 --- a/client/start_container_test.go +++ b/client/start_container_test.go @@ -30,11 +30,12 @@ type fakeStartingContainerzServer struct { receivedMsg *cpb.StartContainerRequest sendMsg *cpb.StartContainerResponse + sendErr error } func (f *fakeStartingContainerzServer) StartContainer(ctx context.Context, req *cpb.StartContainerRequest) (*cpb.StartContainerResponse, error) { f.receivedMsg = req - return f.sendMsg, nil + return f.sendMsg, f.sendErr } func TestStart(t *testing.T) { @@ -89,20 +90,13 @@ func TestStart(t *testing.T) { inTag: "some-tag", inInstance: "some-instance", inCmd: "some-cmd", - inMsg: &cpb.StartContainerResponse{ - Response: &cpb.StartContainerResponse_StartError{ - StartError: &cpb.StartError{ - Details: "oh no!", - }, - }, - }, wantMsg: &cpb.StartContainerRequest{ ImageName: "some-image", Tag: "some-tag", Cmd: "some-cmd", InstanceName: "some-instance", }, - wantErr: status.Error(codes.Internal, "failed to start container: oh no!"), + wantErr: status.Error(codes.Unknown, "arbitrary error, passthrough from test"), }, { name: "simple-with-ports", @@ -442,6 +436,7 @@ func TestStart(t *testing.T) { t.Run(tc.name, func(t *testing.T) { fcm := &fakeStartingContainerzServer{ sendMsg: tc.inMsg, + sendErr: tc.wantErr, } addr, stop := newServer(t, fcm) defer stop() diff --git a/client/update_container.go b/client/update_container.go index 037e9d2..bd3c2d9 100644 --- a/client/update_container.go +++ b/client/update_container.go @@ -51,16 +51,8 @@ func (c *Client) UpdateContainer(ctx context.Context, image string, tag string, case *cpb.UpdateContainerResponse_UpdateOk: return resp.GetUpdateOk().GetInstanceName(), nil case *cpb.UpdateContainerResponse_UpdateError: - switch resp.GetUpdateError().GetErrorCode() { - case cpb.UpdateError_NOT_FOUND: - return "", ErrNotFound - case cpb.UpdateError_NOT_RUNNING: - return "", status.Errorf(codes.FailedPrecondition, "failed to update container as container is not running: %s", resp.GetUpdateError().GetDetails()) - case cpb.UpdateError_PORT_USED: - return "", status.Errorf(codes.AlreadyExists, "failed to update container as port already exists: %s", resp.GetUpdateError().GetDetails()) - default: - return "", status.Errorf(codes.Internal, "failed to update container: %s", resp.GetUpdateError().GetDetails()) - } + return "", status.Errorf(codes.Unknown, + "got UpdateError returned - please update containerz server") default: return "", status.Error(codes.Unknown, "unknown container state") } diff --git a/client/update_container_test.go b/client/update_container_test.go index d30aa1a..e6ab9d0 100644 --- a/client/update_container_test.go +++ b/client/update_container_test.go @@ -30,11 +30,12 @@ type fakeUpdatingContainerzServer struct { receivedMsg *cpb.UpdateContainerRequest sendMsg *cpb.UpdateContainerResponse + sendErr error } func (f *fakeUpdatingContainerzServer) UpdateContainer(ctx context.Context, req *cpb.UpdateContainerRequest) (*cpb.UpdateContainerResponse, error) { f.receivedMsg = req - return f.sendMsg, nil + return f.sendMsg, f.sendErr } func wrapInUpdateRequest(req *cpb.StartContainerRequest, async bool) *cpb.UpdateContainerRequest { @@ -121,13 +122,6 @@ func TestUpdateContainer(t *testing.T) { inTag: "some-tag", inInstance: "some-instance", inCmd: "some-cmd", - inMsg: &cpb.UpdateContainerResponse{ - Response: &cpb.UpdateContainerResponse_UpdateError{ - UpdateError: &cpb.UpdateError{ - Details: "oh no!", - }, - }, - }, wantMsg: wrapInUpdateRequest( &cpb.StartContainerRequest{ ImageName: "some-image", @@ -137,7 +131,7 @@ func TestUpdateContainer(t *testing.T) { }, false, ), - wantErr: status.Error(codes.Internal, "failed to update container: oh no!"), + wantErr: status.Error(codes.Unknown, "arbitrary error, passthrough from test"), }, { name: "simple-with-ports", @@ -289,6 +283,7 @@ func TestUpdateContainer(t *testing.T) { t.Run(tc.name, func(t *testing.T) { fcm := &fakeUpdatingContainerzServer{ sendMsg: tc.inMsg, + sendErr: tc.wantErr, } addr, stop := newServer(t, fcm) defer stop() diff --git a/server/remove_image.go b/server/remove_image.go index cfc2e35..305535a 100644 --- a/server/remove_image.go +++ b/server/remove_image.go @@ -16,12 +16,11 @@ package server import ( "context" - "fmt" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" "github.com/openconfig/containerz/containers" cpb "github.com/openconfig/gnoi/containerz" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) // RemoveImage deletes containers that match the spec defined in the request. If @@ -34,30 +33,12 @@ func (s *Server) RemoveImage(ctx context.Context, request *cpb.RemoveImageReques } if err := s.mgr.ImageRemove(ctx, request.GetName(), request.GetTag(), opts...); err != nil { - stErr, ok := status.FromError(err) - if !ok { - return &cpb.RemoveImageResponse{ - Code: cpb.RemoveImageResponse_RUNNING, - Detail: fmt.Sprintf("unknown containerz state: %v", err), - }, nil - } - - switch stErr.Code() { - case codes.NotFound: - return &cpb.RemoveImageResponse{ - Code: cpb.RemoveImageResponse_NOT_FOUND, - Detail: stErr.Message(), - }, nil - case codes.Unavailable: - return &cpb.RemoveImageResponse{ - Code: cpb.RemoveImageResponse_RUNNING, - Detail: stErr.Message(), - }, nil + if _, ok := status.FromError(err); !ok { + return nil, status.Errorf(codes.Internal, + "unknown containerz state: %v", err) } return nil, err } - return &cpb.RemoveImageResponse{ - Code: cpb.RemoveImageResponse_SUCCESS, - }, nil + return &cpb.RemoveImageResponse{}, nil } diff --git a/server/remove_image_test.go b/server/remove_image_test.go index 8e09418..ea82144 100644 --- a/server/remove_image_test.go +++ b/server/remove_image_test.go @@ -16,6 +16,7 @@ package server import ( "context" + "errors" "testing" "github.com/google/go-cmp/cmp" @@ -35,29 +36,19 @@ func TestImageRemove(t *testing.T) { wantResp *cpb.RemoveImageResponse }{ { - name: "success", - inReq: &cpb.RemoveImageRequest{}, - wantResp: &cpb.RemoveImageResponse{ - Code: cpb.RemoveImageResponse_SUCCESS, - }, + name: "success", + inReq: &cpb.RemoveImageRequest{}, + wantResp: &cpb.RemoveImageResponse{}, }, { name: "not-found", inReq: &cpb.RemoveImageRequest{}, inErr: status.Error(codes.NotFound, "image not found"), - wantResp: &cpb.RemoveImageResponse{ - Code: cpb.RemoveImageResponse_NOT_FOUND, - Detail: "image not found", - }, }, { name: "running-container", inReq: &cpb.RemoveImageRequest{}, inErr: status.Error(codes.Unavailable, "container running"), - wantResp: &cpb.RemoveImageResponse{ - Code: cpb.RemoveImageResponse_RUNNING, - Detail: "container running", - }, }, } @@ -71,7 +62,7 @@ func TestImageRemove(t *testing.T) { defer s.Halt(ctx) resp, err := cli.RemoveImage(ctx, tc.inReq) - if err != nil { + if !errors.Is(err, tc.inErr) { t.Errorf("Remove(%+v) returned error: %v", tc.inReq, err) }