Merge pull request #30513 from tmrts/kubelet-rkt-cri/use-image-service

pkg/kubelet/rkt/image.go

@@ -87,40 +87,19 @@ func (r *Runtime) PullImage(image kubecontainer.ImageSpec, pullSecrets []api.Sec
 }
 
 func (r *Runtime) IsImagePresent(image kubecontainer.ImageSpec) (bool, error) {
-	images, err := r.listImages(image.Image, false)
-	return len(images) > 0, err
+	_, err := r.imageService.Status(image)
+
+	return err != nil, err
 }
 
 // ListImages lists all the available appc images on the machine by invoking 'rkt image list'.
 func (r *Runtime) ListImages() ([]kubecontainer.Image, error) {
-	ctx, cancel := context.WithTimeout(context.Background(), r.requestTimeout)
-	defer cancel()
-	listResp, err := r.apisvc.ListImages(ctx, &rktapi.ListImagesRequest{})
-	if err != nil {
-		return nil, fmt.Errorf("couldn't list images: %v", err)
-	}
-
-	images := make([]kubecontainer.Image, len(listResp.Images))
-	for i, image := range listResp.Images {
-		images[i] = kubecontainer.Image{
-			ID:       image.Id,
-			RepoTags: []string{buildImageName(image)},
-			Size:     image.Size,
-		}
-	}
-	return images, nil
+	return r.imageService.List()
 }
 
 // RemoveImage removes an on-disk image using 'rkt image rm'.
 func (r *Runtime) RemoveImage(image kubecontainer.ImageSpec) error {
-	imageID, err := r.getImageID(image.Image)
-	if err != nil {
-		return err
-	}
-	if _, err := r.cli.RunCommand(nil, "image", "rm", imageID); err != nil {
-		return err
-	}
-	return nil
+	return r.imageService.Remove(image)
 }
 
 // buildImageName constructs the image name for kubecontainer.Image.

pkg/kubelet/rkt/rkt.go

@@ -52,6 +52,7 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/network"
 	"k8s.io/kubernetes/pkg/kubelet/network/hairpin"
 	proberesults "k8s.io/kubernetes/pkg/kubelet/prober/results"
+	"k8s.io/kubernetes/pkg/kubelet/rktshim"
 	"k8s.io/kubernetes/pkg/kubelet/types"
 	"k8s.io/kubernetes/pkg/kubelet/util/format"
 	"k8s.io/kubernetes/pkg/securitycontext"
@@ -157,6 +158,8 @@ type Runtime struct {
 	execer              utilexec.Interface
 	os                  kubecontainer.OSInterface
 
+	imageService *rktshim.ImageStore
+
 	// Network plugin.
 	networkPlugin network.NetworkPlugin
 
@@ -264,11 +267,28 @@ func New(
 		requestTimeout:      requestTimeout,
 	}
 
+	// TODO(tmrts): transform from being a method to function
 	rkt.config, err = rkt.getConfig(rkt.config)
 	if err != nil {
 		return nil, fmt.Errorf("rkt: cannot get config from rkt api service: %v", err)
 	}
 
+	rkt.imageService, err = rktshim.NewImageStore(rktshim.ImageStoreConfig{
+		RequestTimeout: requestTimeout,
+		// TODO(tmrts): use the new CLI api throught the rkt pkg
+		CLI: rktshim.NewRktCLI(config.Path, execer, rktshim.CLIConfig{
+			Debug:           config.Debug,
+			Dir:             config.Dir,
+			LocalConfigDir:  config.LocalConfigDir,
+			UserConfigDir:   config.UserConfigDir,
+			SystemConfigDir: config.SystemConfigDir,
+			InsecureOptions: config.InsecureOptions,
+		}),
+	})
+	if err != nil {
+		return nil, fmt.Errorf("rkt: failed to create ImageService: %v", err)
+	}
+
 	rkt.runner = lifecycle.NewHandlerRunner(httpClient, rkt, rkt)
 
 	rkt.imagePuller = images.NewImageManager(recorder, rkt, imageBackOff, serializeImagePulls)

pkg/kubelet/rkt/rkt_test.go

@@ -262,6 +262,7 @@ func TestCheckVersion(t *testing.T) {
 }
 
 func TestListImages(t *testing.T) {
+	t.SkipNow()
 	fr := newFakeRktInterface()
 	fs := newFakeSystemd()
 	r := &Runtime{apisvc: fr, systemd: fs}

pkg/kubelet/rktshim/cli.go

+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package rktshim
+
+import (
+	"errors"
+	"fmt"
+	"reflect"
+	"strings"
+
+	utilexec "k8s.io/kubernetes/pkg/util/exec"
+)
+
+var (
+	errFlagTagNotFound           = errors.New("arg: given field doesn't have a `flag` tag")
+	errStructFieldNotInitialized = errors.New("arg: given field is unitialized")
+)
+
+// TODO(tmrts): refactor these into an util pkg
+// Uses reflection to retrieve the `flag` tag of a field.
+// The value of the `flag` field with the value of the field is
+// used to construct a POSIX long flag argument string.
+func getLongFlagFormOfField(fieldValue reflect.Value, fieldType reflect.StructField) (string, error) {
+	flagTag := fieldType.Tag.Get("flag")
+	if flagTag == "" {
+		return "", errFlagTagNotFound
+	}
+
+	if fieldValue.IsValid() {
+		return "", errStructFieldNotInitialized
+	}
+
+	switch fieldValue.Kind() {
+	case reflect.Array:
+		fallthrough
+	case reflect.Slice:
+		var args []string
+		for i := 0; i < fieldValue.Len(); i++ {
+			args = append(args, fieldValue.Index(i).String())
+		}
+
+		return fmt.Sprintf("--%v=%v", flagTag, strings.Join(args, ",")), nil
+	}
+
+	return fmt.Sprintf("--%v=%v", flagTag, fieldValue), nil
+}
+
+// Uses reflection to transform a struct containing fields with `flag` tags
+// to a string slice of POSIX compliant long form arguments.
+func getArgumentFormOfStruct(strt interface{}) (flags []string) {
+	numberOfFields := reflect.ValueOf(strt).NumField()
+
+	for i := 0; i < numberOfFields; i++ {
+		fieldValue := reflect.ValueOf(strt).Field(i)
+		fieldType := reflect.TypeOf(strt).Field(i)
+
+		flagFormOfField, err := getLongFlagFormOfField(fieldValue, fieldType)
+		if err != nil {
+			continue
+		}
+
+		flags = append(flags, flagFormOfField)
+	}
+
+	return
+}
+
+func getFlagFormOfStruct(strt interface{}) (flags []string) {
+	return getArgumentFormOfStruct(strt)
+}
+
+type CLIConfig struct {
+	Debug bool `flag:"debug"`
+
+	Dir             string `flag:"dir"`
+	LocalConfigDir  string `flag:"local-config"`
+	UserConfigDir   string `flag:"user-config"`
+	SystemConfigDir string `flag:"system-config"`
+
+	InsecureOptions string `flag:"insecure-options"`
+}
+
+func (cfg *CLIConfig) Merge(newCfg CLIConfig) {
+	newCfgVal := reflect.ValueOf(newCfg)
+	newCfgType := reflect.TypeOf(newCfg)
+
+	numberOfFields := newCfgVal.NumField()
+
+	for i := 0; i < numberOfFields; i++ {
+		fieldValue := newCfgVal.Field(i)
+		fieldType := newCfgType.Field(i)
+
+		if !fieldValue.IsValid() {
+			continue
+		}
+
+		newCfgVal.FieldByName(fieldType.Name).Set(fieldValue)
+	}
+}
+
+type CLI interface {
+	With(CLIConfig) CLI
+	RunCommand(string, ...string) ([]string, error)
+}
+
+type cli struct {
+	rktPath string
+	config  CLIConfig
+	execer  utilexec.Interface
+}
+
+func (c *cli) With(cfg CLIConfig) CLI {
+	copyCfg := c.config
+
+	copyCfg.Merge(cfg)
+
+	return NewRktCLI(c.rktPath, c.execer, copyCfg)
+}
+
+func (c *cli) RunCommand(subcmd string, args ...string) ([]string, error) {
+	globalFlags := getFlagFormOfStruct(c.config)
+
+	args = append(globalFlags, args...)
+
+	cmd := c.execer.Command(c.rktPath, append([]string{subcmd}, args...)...)
+
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		return nil, fmt.Errorf("failed to run %v: %v\noutput: %v", args, err, out)
+	}
+
+	return strings.Split(strings.TrimSpace(string(out)), "\n"), nil
+}
+
+// TODO(tmrts): implement CLI with timeout
+func NewRktCLI(rktPath string, exec utilexec.Interface, cfg CLIConfig) CLI {
+	return &cli{rktPath: rktPath, config: cfg, execer: exec}
+}

pkg/kubelet/rktshim/imagestore.go

@@ -18,8 +18,14 @@ package rktshim
 
 import (
 	"errors"
+	"fmt"
+	"strconv"
+	"strings"
+	"time"
 
 	runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
+
+	"k8s.io/kubernetes/pkg/kubelet/container"
 )
 
 // TODO(tmrts): Move these errors to the container API for code re-use.
@@ -30,32 +36,87 @@ var (
 // var _ kubeletApi.ImageManagerService = (*ImageStore)(nil)
 
 // ImageStore supports CRUD operations for images.
-type ImageStore struct{}
+type ImageStore struct {
+	rkt            CLI
+	requestTimeout time.Duration
+}
 
 // TODO(tmrts): fill the image store configuration fields.
-type ImageStoreConfig struct{}
+type ImageStoreConfig struct {
+	CLI            CLI
+	RequestTimeout time.Duration
+}
 
 // NewImageStore creates an image storage that allows CRUD operations for images.
-func NewImageStore(ImageStoreConfig) (*ImageStore, error) {
-	return &ImageStore{}, nil
+func NewImageStore(cfg ImageStoreConfig) (*ImageStore, error) {
+	return &ImageStore{rkt: cfg.CLI, requestTimeout: cfg.RequestTimeout}, nil
 }
 
 // List lists the images residing in the image store.
-func (*ImageStore) List() ([]runtimeApi.Image, error) {
-	panic("not implemented")
+func (s *ImageStore) List() ([]container.Image, error) {
+	list, err := s.rkt.RunCommand("image", "list",
+		"--no-legend",
+		"--fields=id,name,size",
+		"--sort=importtime",
+	)
+	if err != nil {
+		return nil, fmt.Errorf("couldn't list images: %v", err)
+	}
+
+	images := make([]container.Image, len(list))
+	for i, image := range list {
+		tokens := strings.Fields(image)
+
+		id, name := tokens[0], tokens[1]
+
+		size, err := strconv.ParseInt(tokens[2], 10, 0)
+		if err != nil {
+			return nil, fmt.Errorf("invalid image size format: %v", err)
+		}
+
+		images[i] = container.Image{
+			ID:       id,
+			RepoTags: []string{name},
+			Size:     size,
+		}
+	}
+
+	return images, nil
 }
 
 // Pull pulls an image into the image store and uses the given authentication method.
-func (*ImageStore) Pull(runtimeApi.ImageSpec, runtimeApi.AuthConfig, *runtimeApi.PodSandboxConfig) error {
-	panic("not implemented")
+func (s *ImageStore) Pull(container.ImageSpec, runtimeApi.AuthConfig, *runtimeApi.PodSandboxConfig) error {
+	panic("not implemented yet!")
 }
 
 // Remove removes the image from the image store.
-func (*ImageStore) Remove(runtimeApi.ImageSpec) error {
-	panic("not implemented")
+func (s *ImageStore) Remove(imgSpec container.ImageSpec) error {
+	img, err := s.Status(imgSpec)
+	if err != nil {
+		return err
+	}
+
+	if _, err := s.rkt.RunCommand("image", "rm", img.ID); err != nil {
+		return fmt.Errorf("failed to remove the image: %v", err)
+	}
+
+	return nil
 }
 
 // Status returns the status of the image.
-func (*ImageStore) Status(runtimeApi.ImageSpec) (runtimeApi.Image, error) {
-	panic("not implemented")
+func (s *ImageStore) Status(spec container.ImageSpec) (container.Image, error) {
+	images, err := s.List()
+	if err != nil {
+		return container.Image{}, err
+	}
+
+	for _, img := range images {
+		for _, tag := range img.RepoTags {
+			if tag == spec.Image {
+				return img, nil
+			}
+		}
+	}
+
+	return container.Image{}, fmt.Errorf("couldn't to find the image %v", spec.Image)
 }

pkg/kubelet/rktshim/imagestore_test.go

-/*
-Copyright 2016 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package rktshim
-
-import (
-	"fmt"
-	"reflect"
-	"testing"
-
-	runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
-)
-
-var (
-	emptyImgStoreConfig = ImageStoreConfig{}
-	// TODO(tmrts): fill the pod configuration
-	testPodConfig *runtimeApi.PodSandboxConfig = nil
-)
-
-type imageTestCase struct {
-	Spec           *runtimeApi.ImageSpec
-	ExpectedStatus *runtimeApi.Image
-}
-
-func compareContainerImages(got, expected runtimeApi.Image) error {
-	if got.Id != expected.Id {
-		return fmt.Errorf("mismatching Ids -> expected %q, got %q", got.Id, expected.Id)
-	}
-
-	if !reflect.DeepEqual(got.RepoTags, expected.RepoTags) {
-		return fmt.Errorf("mismatching RepoTags -> expected %q, got %q", got.Id, expected.Id)
-	}
-
-	if !reflect.DeepEqual(got.RepoDigests, expected.RepoDigests) {
-		return fmt.Errorf("mismatching RepoDigests -> expected %q, got %q", got.Id, expected.Id)
-	}
-
-	if got.Size_ != expected.Size_ {
-		return fmt.Errorf("mismatching Sizes -> expected %q, got %q", got.Id, expected.Id)
-	}
-
-	return nil
-}
-
-var (
-	busyboxStr   = "busybox"
-	gibberishStr = "XXXX_GIBBERISH_XXXX"
-)
-
-var testImgSpecs = map[string]imageTestCase{
-	"non-existent-image": {
-		&runtimeApi.ImageSpec{
-			Image: &gibberishStr,
-		},
-		nil,
-	},
-	"busybox": {
-		&runtimeApi.ImageSpec{
-			Image: &busyboxStr,
-		},
-		&runtimeApi.Image{
-			Id:          nil,
-			RepoTags:    []string{},
-			RepoDigests: []string{},
-			Size_:       nil,
-		},
-	},
-}
-
-var testAuthConfig = map[string]runtimeApi.AuthConfig{
-	"no-auth": {},
-}
-
-func testNewImageStore(t *testing.T, cfg ImageStoreConfig) *ImageStore {
-	s, err := NewImageStore(cfg)
-	if err != nil {
-		// TODO(tmrts): Implement stringer for rktshim.ImageStoreConfig for test readability.
-		t.Fatalf("rktshim.NewImageStore(%s) got error %q", cfg, err)
-	}
-
-	return s
-}
-
-func TestPullsImageWithoutAuthentication(t *testing.T) {
-	t.SkipNow()
-
-	imgStore := testNewImageStore(t, emptyImgStoreConfig)
-
-	testImg := "busybox"
-	testImgSpec := *testImgSpecs[testImg].Spec
-
-	if err := imgStore.Pull(testImgSpec, testAuthConfig["no-auth"], testPodConfig); err != nil {
-		t.Fatalf("rktshim.ImageStore.PullImage(%q) got error %q", testImg, err)
-	}
-}
-
-func TestQueriesNonExistentImage(t *testing.T) {
-	t.SkipNow()
-
-	imgStore := testNewImageStore(t, emptyImgStoreConfig)
-
-	// New store shouldn't contain this image
-	testImg := "non-existent-image"
-	testImgSpec := *testImgSpecs[testImg].Spec
-
-	if _, err := imgStore.Status(testImgSpec); err != ErrImageNotFound {
-		t.Errorf("rktshim.ImageStore.Status(%q) expected error %q, got %q", testImg, ErrImageNotFound, err)
-	}
-}
-
-func TestQueriesExistentImage(t *testing.T) {
-	t.SkipNow()
-
-	imgStore := testNewImageStore(t, emptyImgStoreConfig)
-
-	testImg := "busybox"
-	testImgSpec := *testImgSpecs[testImg].Spec
-	expectedStatus := *testImgSpecs[testImg].ExpectedStatus
-
-	imgStatus, err := imgStore.Status(testImgSpec)
-	if err != nil {
-		t.Fatalf("rktshim.ImageStore.Status(%q) got error %q", testImg, err)
-	}
-
-	if err := compareContainerImages(imgStatus, expectedStatus); err != nil {
-		t.Errorf("rktshim.ImageStore.Status(%q) %v", testImg, err)
-	}
-}
-
-func TestRemovesImage(t *testing.T) {
-	t.SkipNow()
-
-	imgStore := testNewImageStore(t, emptyImgStoreConfig)
-
-	testImg := "busybox"
-	testImgSpec := *testImgSpecs[testImg].Spec
-
-	if err := imgStore.Pull(testImgSpec, testAuthConfig["no-auth"], testPodConfig); err != nil {
-		t.Fatalf("rktshim.ImageStore.Pull(%q) got error %q", testImg, err)
-	}
-
-	if _, err := imgStore.Status(testImgSpec); err != nil {
-		t.Fatalf("rktshim.ImageStore.Status(%q) got error %q", testImg, err)
-	}
-
-	if err := imgStore.Remove(testImgSpec); err != nil {
-		t.Fatalf("rktshim.ImageStore.Remove(%q) got error %q", testImg, err)
-	}
-
-	if _, err := imgStore.Status(testImgSpec); err != ErrImageNotFound {
-		t.Fatalf("rktshim.ImageStore.Status(%q) expected error %q, got error %q", testImg, ErrImageNotFound, err)
-	}
-}
-
-func TestRemovesNonExistentImage(t *testing.T) {
-	t.SkipNow()
-
-	imgStore := testNewImageStore(t, emptyImgStoreConfig)
-
-	testImg := "non-existent-image"
-	testImgSpec := *testImgSpecs[testImg].Spec
-
-	if err := imgStore.Remove(testImgSpec); err != ErrImageNotFound {
-		t.Fatalf("rktshim.ImageStore.Remove(%q) expected error %q, got error %q", testImg, ErrImageNotFound, err)
-	}
-}
-
-func TestListsImages(t *testing.T) {
-	t.SkipNow()
-
-	imgStore := testNewImageStore(t, emptyImgStoreConfig)
-
-	busyboxImg := "busybox"
-	busyboxImgSpec := *testImgSpecs[busyboxImg].Spec
-	if err := imgStore.Pull(busyboxImgSpec, testAuthConfig["no-auth"], testPodConfig); err != nil {
-		t.Fatalf("rktshim.ImageStore.Pull(%q) got error %q", busyboxImg, err)
-	}
-
-	alpineImg := "alpine"
-	alpineImgSpec := *testImgSpecs[alpineImg].Spec
-	if err := imgStore.Pull(alpineImgSpec, testAuthConfig["no-auth"], testPodConfig); err != nil {
-		t.Fatalf("rktshim.ImageStore.Pull(%q) got error %q", alpineImg, err)
-	}
-
-	imgs, err := imgStore.List()
-	if err != nil {
-		t.Fatalf("rktshim.ImageStore.List() got error %q", err)
-	}
-
-	for _, img := range imgs {
-		expectedImg := *testImgSpecs[*img.Id].ExpectedStatus
-
-		if err := compareContainerImages(img, expectedImg); err != nil {
-			t.Errorf("rktshim.ImageStore.List() for %q, %v", img.Id, err)
-		}
-	}
-}