github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-23 19:56:01 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #32410 from euank/revert-rktshim-image-service

Browse Source 
Automatic merge from submit-queue

Revert "Merge pull request #30513 from tmrts/kubelet-rkt-cri/use-imag…

This reverts commit aff7dfcaab, reversing
changes made to 7a4d81ea43.

See https://github.com/kubernetes/kubernetes/pull/30513#issuecomment-245949664, this breaks tests for the rkt runtime.

The original PR also changed the source of image truth from the api-service to cli, which could have further implications which @yifan-gu could speak better to, so I think it's safer to just revert for now and discuss further in the rktlet repo / pr. The reverted code effectively already exists (with the bug in question already fixed) here https://github.com/kubernetes-incubator/rktlet/pull/5 .. once that's merged, we can vendor and call it over here in place of #30513.

@yifan-gu / @tmrts  if you think continuing with the changes and rolling forwards (just adding the `--full` flag to list to fix the immediate bug) is better, feel free to close this and open a PR with that change, but I think this approach is overall better for the reasons in the previous paragraph.

cc @tmrts @yifan-gu @kubernetes/sig-rktnetes @pskrzyns
This commit is contained in:
Kubernetes Submit Queue 2016-09-12 22:26:34 -07:00 committed by GitHub
commit ff1a92fa03
6 changed files with 248 additions and 251 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
pkg/kubelet/rkt/image.go
View File

@ -87,19 +87,40 @@ func (r *Runtime) PullImage(image kubecontainer.ImageSpec, pullSecrets []api.Sec
}
func (r *Runtime) IsImagePresent(image kubecontainer.ImageSpec) (bool, error) {
_, err := r.imageService.Status(image)
return err != nil, err
images, err := r.listImages(image.Image, false)
return len(images) > 0, err
}
// ListImages lists all the available appc images on the machine by invoking 'rkt image list'.
func (r *Runtime) ListImages() ([]kubecontainer.Image, error) {
return r.imageService.List()
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
}
// RemoveImage removes an on-disk image using 'rkt image rm'.
func (r *Runtime) RemoveImage(image kubecontainer.ImageSpec) error {
return r.imageService.Remove(image)
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
}
// buildImageName constructs the image name for kubecontainer.Image.

20
pkg/kubelet/rkt/rkt.go
View File

@ -52,7 +52,6 @@ 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"
@ -161,8 +160,6 @@ type Runtime struct {
execer utilexec.Interface
os kubecontainer.OSInterface
imageService *rktshim.ImageStore
// Network plugin.
networkPlugin network.NetworkPlugin
@ -270,28 +267,11 @@ 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)

1
pkg/kubelet/rkt/rkt_test.go
View File

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

152
pkg/kubelet/rktshim/cli.go
View File

@ -1,152 +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 (
"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}
}

85
pkg/kubelet/rktshim/imagestore.go
View File

@ -18,14 +18,8 @@ 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.
@ -36,87 +30,32 @@ var (
// var _ kubeletApi.ImageManagerService = (*ImageStore)(nil)
// ImageStore supports CRUD operations for images.
type ImageStore struct {
rkt CLI
requestTimeout time.Duration
}
type ImageStore struct{}
// TODO(tmrts): fill the image store configuration fields.
type ImageStoreConfig struct {
CLI CLI
RequestTimeout time.Duration
}
type ImageStoreConfig struct{}
// NewImageStore creates an image storage that allows CRUD operations for images.
func NewImageStore(cfg ImageStoreConfig) (*ImageStore, error) {
return &ImageStore{rkt: cfg.CLI, requestTimeout: cfg.RequestTimeout}, nil
func NewImageStore(ImageStoreConfig) (*ImageStore, error) {
return &ImageStore{}, nil
}
// List lists the images residing in the image store.
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
func (*ImageStore) List() ([]runtimeApi.Image, error) {
panic("not implemented")
}
// Pull pulls an image into the image store and uses the given authentication method.
func (s *ImageStore) Pull(container.ImageSpec, runtimeApi.AuthConfig, *runtimeApi.PodSandboxConfig) error {
panic("not implemented yet!")
func (*ImageStore) Pull(runtimeApi.ImageSpec, runtimeApi.AuthConfig, *runtimeApi.PodSandboxConfig) error {
panic("not implemented")
}
// Remove removes the image from the image store.
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
func (*ImageStore) Remove(runtimeApi.ImageSpec) error {
panic("not implemented")
}
// Status returns the status of the image.
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)
func (*ImageStore) Status(runtimeApi.ImageSpec) (runtimeApi.Image, error) {
panic("not implemented")
}

210
pkg/kubelet/rktshim/imagestore_test.go Normal file
View File

@ -0,0 +1,210 @@
/*
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)
}
}
}