Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 3 additions & 16 deletions client/remove_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,32 +16,19 @@ package client

import (
"context"
"errors"

cpb "github.com/openconfig/gnoi/containerz"
)

// RemoveImage removes an image from the target system. It returns nil upon success. Otherwise it
// 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
}
27 changes: 10 additions & 17 deletions client/remove_image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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{
Expand All @@ -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,
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion client/start_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
13 changes: 4 additions & 9 deletions client/start_container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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()
Expand Down
12 changes: 2 additions & 10 deletions client/update_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
13 changes: 4 additions & 9 deletions client/update_container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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()
Expand Down
31 changes: 6 additions & 25 deletions server/remove_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
19 changes: 5 additions & 14 deletions server/remove_image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package server

import (
"context"
"errors"
"testing"

"github.com/google/go-cmp/cmp"
Expand All @@ -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",
},
},
}

Expand All @@ -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)
}

Expand Down