Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rkt: rewrote ListImages to use rkt's API service #17968

Merged
merged 1 commit into from
Dec 5, 2015
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion pkg/kubelet/rkt/fake_rkt_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
type fakeRktInterface struct {
sync.Mutex
info rktapi.Info
images []*rktapi.Image
called []string
err error
}
Expand Down Expand Up @@ -62,7 +63,11 @@ func (f *fakeRktInterface) InspectPod(ctx context.Context, in *rktapi.InspectPod
}

func (f *fakeRktInterface) ListImages(ctx context.Context, in *rktapi.ListImagesRequest, opts ...grpc.CallOption) (*rktapi.ListImagesResponse, error) {
return nil, fmt.Errorf("Not implemented")
f.Lock()
defer f.Unlock()

f.called = append(f.called, "ListImages")
return &rktapi.ListImagesResponse{f.images}, f.err
}

func (f *fakeRktInterface) InspectImage(ctx context.Context, in *rktapi.InspectImageRequest, opts ...grpc.CallOption) (*rktapi.InspectImageResponse, error) {
Expand Down
44 changes: 9 additions & 35 deletions pkg/kubelet/rkt/rkt.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/docker/docker/pkg/parsers"
docker "github.com/fsouza/go-dockerclient"
"github.com/golang/glog"
"golang.org/x/net/context"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/client/record"
"k8s.io/kubernetes/pkg/credentialprovider"
Expand Down Expand Up @@ -1327,49 +1328,22 @@ func (r *Runtime) getImageByName(imageName string) (*kubecontainer.Image, error)

// ListImages lists all the available appc images on the machine by invoking 'rkt image list'.
func (r *Runtime) ListImages() ([]kubecontainer.Image, error) {
// Example output of 'rkt image list --fields=id,name --full':
//
// ID NAME
// sha512-374770396f23dd153937cd66694fe705cf375bcec7da00cf87e1d9f72c192da7 nginx:latest
// sha512-bead9e0df8b1b4904d0c57ade2230e6d236e8473f62614a8bc6dcf11fc924123 coreos.com/rkt/stage1:0.8.1
//
// With '--no-legend=true' the fist line (KEY NAME) will be omitted.
output, err := r.runCommand("image", "list", "--no-legend=true", "--fields=id,name", "--full")
listResp, err := r.apisvc.ListImages(context.Background(), &rktapi.ListImagesRequest{})
if err != nil {
return nil, err
}
if len(output) == 0 {
return nil, nil
return nil, fmt.Errorf("couldn't list images: %v", err)
}

var images []kubecontainer.Image
for _, line := range output {
img, err := parseImageInfo(line)
if err != nil {
glog.Warningf("rkt: Cannot parse image info from %q: %v", line, err)
continue
images := make([]kubecontainer.Image, len(listResp.Images))
for i, image := range listResp.Images {
images[i] = kubecontainer.Image{
ID: image.Id,
Tags: []string{image.Name},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgonyeo Lets add a todo here for the image size? rkt/rkt#1814

//TODO: fill in the size of the image
}
images = append(images, *img)
}
return images, nil
}

// parseImageInfo creates the kubecontainer.Image struct by parsing the string in the result of 'rkt image list',
// the input looks like:
//
// sha512-91e98d7f1679a097c878203c9659f2a26ae394656b3147963324c61fa3832f15 coreos.com/etcd:v2.0.9
//
func parseImageInfo(input string) (*kubecontainer.Image, error) {
idName := strings.Split(strings.TrimSpace(input), "\t")
if len(idName) != 2 {
return nil, fmt.Errorf("invalid image information from 'rkt image list': %q", input)
}
return &kubecontainer.Image{
ID: idName[0],
Tags: []string{idName[1]},
}, nil
}

// RemoveImage removes an on-disk image using 'rkt image rm'.
func (r *Runtime) RemoveImage(image kubecontainer.ImageSpec) error {
img, err := r.getImageByName(image.Image)
Expand Down
53 changes: 53 additions & 0 deletions pkg/kubelet/rkt/rkt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,56 @@ func TestCheckVersion(t *testing.T) {
fs.CleanCalls()
}
}

func TestListImages(t *testing.T) {
fr := newFakeRktInterface()
fs := newFakeSystemd()
r := &Runtime{apisvc: fr, systemd: fs}

tests := []struct {
images []*rktapi.Image
}{
{},
{
[]*rktapi.Image{
{
Id: "sha512-a2fb8f390702",
Name: "quay.io/coreos/alpine-sh",
Version: "latest",
},
},
},
{
[]*rktapi.Image{
{
Id: "sha512-a2fb8f390702",
Name: "quay.io/coreos/alpine-sh",
Version: "latest",
},
{
Id: "sha512-c6b597f42816",
Name: "coreos.com/rkt/stage1-coreos",
Version: "0.10.0",
},
},
},
}

for i, tt := range tests {
fr.images = tt.images

images, err := r.ListImages()
if err != nil {
t.Errorf("%v", err)
}
assert.Equal(t, len(images), len(tt.images), fmt.Sprintf("test case %d: mismatched number of images", i))
for i, image := range images {
assert.Equal(t, image.ID, tt.images[i].Id, fmt.Sprintf("test case %d: mismatched image IDs", i))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit (optional): instead of checking every field, you could also add the expected output to tests and simply compare it to the actual output. Since the code here is simple enough, I am okay either way.

assert.Equal(t, []string{tt.images[i].Name}, image.Tags, fmt.Sprintf("test case %d: mismatched image tags", i))
}

assert.Equal(t, fr.called, []string{"ListImages"}, fmt.Sprintf("test case %d: unexpected called list", i))

fr.CleanCalls()
}
}