Skip to content
This repository has been archived by the owner on Mar 9, 2022. It is now read-only.

adding info map for verbose image status #470

Merged
merged 2 commits into from
Dec 12, 2017
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
4 changes: 2 additions & 2 deletions pkg/server/container_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,12 @@ func (c *criContainerdService) CreateContainer(ctx context.Context, r *runtime.C

// Create container volumes mounts.
// TODO(random-liu): Add cri-containerd integration test for image volume.
volumeMounts := c.generateVolumeMounts(containerRootDir, config.GetMounts(), image.Config)
volumeMounts := c.generateVolumeMounts(containerRootDir, config.GetMounts(), &image.ImageSpec.Config)

// Generate container runtime spec.
mounts := c.generateContainerMounts(getSandboxRootDir(c.config.RootDir, sandboxID), config)

spec, err := c.generateContainerSpec(id, sandboxPid, config, sandboxConfig, image.Config, append(mounts, volumeMounts...))
spec, err := c.generateContainerSpec(id, sandboxPid, config, sandboxConfig, &image.ImageSpec.Config, append(mounts, volumeMounts...))
if err != nil {
return nil, fmt.Errorf("failed to generate container %q spec: %v", id, err)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/server/container_stop.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ func (c *criContainerdService) stopContainer(ctx context.Context, container cont
// an error here.
return fmt.Errorf("failed to get image metadata %q: %v", container.ImageRef, err)
}
if image.Config.StopSignal != "" {
stopSignal, err = signal.ParseSignal(image.Config.StopSignal)
if image.ImageSpec.Config.StopSignal != "" {
stopSignal, err = signal.ParseSignal(image.ImageSpec.Config.StopSignal)
if err != nil {
return fmt.Errorf("failed to parse stop signal %q: %v",
image.Config.StopSignal, err)
image.ImageSpec.Config.StopSignal, err)
}
}
glog.V(2).Infof("Stop container %q with signal %v", id, stopSignal)
Expand Down
16 changes: 8 additions & 8 deletions pkg/server/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,10 @@ func loadCgroup(cgroupPath string) (cgroups.Cgroup, error) {

// imageInfo is the information about the image got from containerd.
type imageInfo struct {
id string
chainID imagedigest.Digest
size int64
config imagespec.ImageConfig
id string
chainID imagedigest.Digest
size int64
imagespec imagespec.Image
}

// getImageInfo gets image info from containerd.
Expand Down Expand Up @@ -344,10 +344,10 @@ func getImageInfo(ctx context.Context, image containerd.Image, provider content.
}

return &imageInfo{
id: id,
chainID: chainID,
size: size,
config: ociimage.Config,
id: id,
chainID: chainID,
size: size,
imagespec: ociimage,
}, nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/server/image_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func toCRIImage(image imagestore.Image) *runtime.Image {
RepoDigests: image.RepoDigests,
Size_: uint64(image.Size),
}
uid, username := getUserFromImage(image.Config.User)
uid, username := getUserFromImage(image.ImageSpec.Config.User)
if uid != nil {
runtimeImage.Uid = &runtime.Int64Value{Value: *uid}
}
Expand Down
18 changes: 12 additions & 6 deletions pkg/server/image_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ func TestListImages(t *testing.T) {
RepoTags: []string{"tag-a-1", "tag-b-1"},
RepoDigests: []string{"digest-a-1", "digest-b-1"},
Size: 1000,
Config: &imagespec.ImageConfig{
User: "root",
ImageSpec: imagespec.Image{
Config: imagespec.ImageConfig{
User: "root",
},
},
},
{
Expand All @@ -47,8 +49,10 @@ func TestListImages(t *testing.T) {
RepoTags: []string{"tag-a-2", "tag-b-2"},
RepoDigests: []string{"digest-a-2", "digest-b-2"},
Size: 2000,
Config: &imagespec.ImageConfig{
User: "1234:1234",
ImageSpec: imagespec.Image{
Config: imagespec.ImageConfig{
User: "1234:1234",
},
},
},
{
Expand All @@ -57,8 +61,10 @@ func TestListImages(t *testing.T) {
RepoTags: []string{"tag-a-3", "tag-b-3"},
RepoDigests: []string{"digest-a-3", "digest-b-3"},
Size: 3000,
Config: &imagespec.ImageConfig{
User: "nobody",
ImageSpec: imagespec.Image{
Config: imagespec.ImageConfig{
User: "nobody",
},
},
},
}
Expand Down
13 changes: 7 additions & 6 deletions pkg/server/image_load.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,14 @@ func (c *criContainerdService) LoadImage(ctx context.Context, r *api.LoadImageRe
if err := c.createImageReference(ctx, id, image.Target()); err != nil {
return nil, fmt.Errorf("failed to create image reference %q: %v", id, err)
}

img := imagestore.Image{
ID: id,
RepoTags: []string{repoTag},
ChainID: info.chainID.String(),
Size: info.size,
Config: &info.config,
Image: image,
ID: id,
RepoTags: []string{repoTag},
ChainID: info.chainID.String(),
Size: info.size,
ImageSpec: info.imagespec,
Image: image,
}

if err := c.imageStore.Add(img); err != nil {
Expand Down
10 changes: 5 additions & 5 deletions pkg/server/image_pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,11 @@ func (c *criContainerdService) PullImage(ctx context.Context, r *runtime.PullIma
glog.V(4).Infof("Pulled image %q with image id %q, repo tag %q, repo digest %q", imageRef, imageID,
repoTag, repoDigest)
img := imagestore.Image{
ID: imageID,
ChainID: info.chainID.String(),
Size: info.size,
Config: &info.config,
Image: image,
ID: imageID,
ChainID: info.chainID.String(),
Size: info.size,
ImageSpec: info.imagespec,
Image: image,
}
if repoDigest != "" {
img.RepoDigests = []string{repoDigest}
Expand Down
55 changes: 52 additions & 3 deletions pkg/server/image_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,15 @@ limitations under the License.
package server

import (
"encoding/json"
"fmt"

"github.com/golang/glog"
"golang.org/x/net/context"
"k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime"

imagestore "github.com/kubernetes-incubator/cri-containerd/pkg/store/image"
imagespec "github.com/opencontainers/image-spec/specs-go/v1"
)

// ImageStatus returns the status of the image, returns nil if the image isn't present.
Expand All @@ -37,18 +42,62 @@ func (c *criContainerdService) ImageStatus(ctx context.Context, r *runtime.Image
}
// TODO(random-liu): [P0] Make sure corresponding snapshot exists. What if snapshot
// doesn't exist?

runtimeImage := toCRIRuntimeImage(image)
info, err := c.toCRIImageInfo(ctx, image, r.GetVerbose())
if err != nil {
return nil, fmt.Errorf("failed to generate image info: %v", err)
}

return &runtime.ImageStatusResponse{
Image: runtimeImage,
Info: info,
}, nil
}

// toCRIRuntimeImage converts internal image object to CRI runtime.Image.
func toCRIRuntimeImage(image *imagestore.Image) *runtime.Image {
runtimeImage := &runtime.Image{
Id: image.ID,
RepoTags: image.RepoTags,
RepoDigests: image.RepoDigests,
Size_: uint64(image.Size),
}
uid, username := getUserFromImage(image.Config.User)
uid, username := getUserFromImage(image.ImageSpec.Config.User)
if uid != nil {
runtimeImage.Uid = &runtime.Int64Value{Value: *uid}
}
runtimeImage.Username = username

// TODO(mikebrow): write a ImageMetadata to runtime.Image converter
return &runtime.ImageStatusResponse{Image: runtimeImage}, nil
return runtimeImage
}

// TODO (mikebrow): discuss moving this struct and / or constants for info map for some or all of these fields to CRI
type verboseImageInfo struct {
ChainID string `json:"chainID"`
ImageSpec imagespec.Image `json:"imageSpec"`
}

// toCRIImageInfo converts internal image object information to CRI image status response info map.
func (c *criContainerdService) toCRIImageInfo(ctx context.Context, image *imagestore.Image, verbose bool) (map[string]string, error) {
if !verbose {
return nil, nil
}

info := make(map[string]string)

imi := &verboseImageInfo{
ChainID: image.ChainID,
ImageSpec: image.ImageSpec,
}

m, err := json.Marshal(imi)
if err == nil {
info["info"] = string(m)
} else {
glog.Errorf("failed to marshal info %v: %v", imi, err)
info["info"] = err.Error()
}

return info, nil
}
6 changes: 4 additions & 2 deletions pkg/server/image_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ func TestImageStatus(t *testing.T) {
RepoTags: []string{"a", "b"},
RepoDigests: []string{"c", "d"},
Size: 1234,
Config: &imagespec.ImageConfig{
User: "user:group",
ImageSpec: imagespec.Image{
Config: imagespec.ImageConfig{
User: "user:group",
},
},
}
expected := &runtime.Image{
Expand Down
10 changes: 5 additions & 5 deletions pkg/server/restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,11 +366,11 @@ func loadImages(ctx context.Context, cImages []containerd.Image, provider conten
continue
}
image := imagestore.Image{
ID: id,
ChainID: info.chainID.String(),
Size: info.size,
Config: &info.config,
Image: i,
ID: id,
ChainID: info.chainID.String(),
Size: info.size,
ImageSpec: info.imagespec,
Image: i,
}
// Recover repo digests and repo tags.
for _, i := range imgs {
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/sandbox_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run
}

// Create sandbox container.
spec, err := c.generateSandboxContainerSpec(id, config, image.Config, sandbox.NetNSPath)
spec, err := c.generateSandboxContainerSpec(id, config, &image.ImageSpec.Config, sandbox.NetNSPath)
if err != nil {
return nil, fmt.Errorf("failed to generate sandbox container spec: %v", err)
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/store/image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ type Image struct {
ChainID string
// Size is the compressed size of the image.
Size int64
// Config is the oci image config of the image.
Config *imagespec.ImageConfig
// ImageSpec is the oci image structure which describes basic information about the image.
ImageSpec imagespec.Image

Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary empty line. I'll clean this up in my coming cleanup PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok....?

Copy link
Member

Choose a reason for hiding this comment

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

Done.

// Containerd image reference
Image containerd.Image
}
Expand Down
5 changes: 0 additions & 5 deletions pkg/store/image/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"strings"
"testing"

imagespec "github.com/opencontainers/image-spec/specs-go/v1"
assertlib "github.com/stretchr/testify/assert"

"github.com/kubernetes-incubator/cri-containerd/pkg/store"
Expand All @@ -34,31 +33,27 @@ func TestImageStore(t *testing.T) {
RepoTags: []string{"tag-1"},
RepoDigests: []string{"digest-1"},
Size: 10,
Config: &imagespec.ImageConfig{},
},
{
ID: "sha256:2123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
ChainID: "test-chain-id-2abcd",
RepoTags: []string{"tag-2abcd"},
RepoDigests: []string{"digest-2abcd"},
Size: 20,
Config: &imagespec.ImageConfig{},
Copy link
Member

Choose a reason for hiding this comment

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

Why removing this? Will add it back in my cleanup PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

because the config is in the image struct...

Copy link
Member Author

Choose a reason for hiding this comment

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

Cleanup?

Copy link
Member

Choose a reason for hiding this comment

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

Done.

},
{
ID: "sha256:3123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
RepoTags: []string{"tag-4a333"},
RepoDigests: []string{"digest-4a333"},
ChainID: "test-chain-id-4a333",
Size: 30,
Config: &imagespec.ImageConfig{},
},
{
ID: "sha256:4123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
RepoTags: []string{"tag-4abcd"},
RepoDigests: []string{"digest-4abcd"},
ChainID: "test-chain-id-4abcd",
Size: 40,
Config: &imagespec.ImageConfig{},
},
}
assert := assertlib.New(t)
Expand Down