diff --git a/pkg/kubelet/rkt/image.go b/pkg/kubelet/rkt/image.go index 5487a07f6fb..56b8e43d638 100644 --- a/pkg/kubelet/rkt/image.go +++ b/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. diff --git a/pkg/kubelet/rkt/rkt.go b/pkg/kubelet/rkt/rkt.go index 31b2bc6a0b6..130fde20108 100644 --- a/pkg/kubelet/rkt/rkt.go +++ b/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) diff --git a/pkg/kubelet/rkt/rkt_test.go b/pkg/kubelet/rkt/rkt_test.go index 3dd7a773065..239ac47a9f9 100644 --- a/pkg/kubelet/rkt/rkt_test.go +++ b/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} diff --git a/pkg/kubelet/rktshim/cli.go b/pkg/kubelet/rktshim/cli.go new file mode 100644 index 00000000000..1b03bb1f435 --- /dev/null +++ b/pkg/kubelet/rktshim/cli.go @@ -0,0 +1,152 @@ +/* +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} +} diff --git a/pkg/kubelet/rktshim/imagestore.go b/pkg/kubelet/rktshim/imagestore.go index bba1fc4ddb6..428f166a65f 100644 --- a/pkg/kubelet/rktshim/imagestore.go +++ b/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) } diff --git a/pkg/kubelet/rktshim/imagestore_test.go b/pkg/kubelet/rktshim/imagestore_test.go deleted file mode 100644 index 3eb966ec7ec..00000000000 --- a/pkg/kubelet/rktshim/imagestore_test.go +++ /dev/null @@ -1,210 +0,0 @@ -/* -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) - } - } -}