From a393f3a084388a0cfc85b9937d9ab371bed4e6b9 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Mon, 5 Jun 2017 17:26:16 +0000 Subject: [PATCH 1/4] Add new metadata store. Signed-off-by: Lantao Liu --- pkg/store/container/container.go | 113 ++++++++++++++++++++ pkg/store/container/container_test.go | 138 ++++++++++++++++++++++++ pkg/store/container/metadata.go | 76 ++++++++++++++ pkg/store/container/metadata_test.go | 53 ++++++++++ pkg/store/container/status.go | 146 ++++++++++++++++++++++++++ pkg/store/container/status_test.go | 101 ++++++++++++++++++ pkg/store/errors.go | 27 +++++ pkg/store/image/image.go | 120 +++++++++++++++++++++ pkg/store/image/image_test.go | 106 +++++++++++++++++++ pkg/store/sandbox/metadata.go | 79 ++++++++++++++ pkg/store/sandbox/metadata_test.go | 55 ++++++++++ pkg/store/sandbox/sandbox.go | 88 ++++++++++++++++ pkg/store/sandbox/sandbox_test.go | 115 ++++++++++++++++++++ 13 files changed, 1217 insertions(+) create mode 100644 pkg/store/container/container.go create mode 100644 pkg/store/container/container_test.go create mode 100644 pkg/store/container/metadata.go create mode 100644 pkg/store/container/metadata_test.go create mode 100644 pkg/store/container/status.go create mode 100644 pkg/store/container/status_test.go create mode 100644 pkg/store/errors.go create mode 100644 pkg/store/image/image.go create mode 100644 pkg/store/image/image_test.go create mode 100644 pkg/store/sandbox/metadata.go create mode 100644 pkg/store/sandbox/metadata_test.go create mode 100644 pkg/store/sandbox/sandbox.go create mode 100644 pkg/store/sandbox/sandbox_test.go diff --git a/pkg/store/container/container.go b/pkg/store/container/container.go new file mode 100644 index 000000000..b0dec89e3 --- /dev/null +++ b/pkg/store/container/container.go @@ -0,0 +1,113 @@ +/* +Copyright 2017 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 container + +import ( + "sync" + + "github.com/kubernetes-incubator/cri-containerd/pkg/store" +) + +// Container contains all resources associated with the container. All methods to +// mutate the internal state are thread-safe. +type Container struct { + // Metadata is the metadata of the container, it is **immutable** after created. + Metadata + // Status stores the status of the container. + Status StatusStorage + // TODO(random-liu): Add containerd container client. + // TODO(random-liu): Add stop channel to get rid of stop poll waiting. +} + +// NewContainer creates an internally used container type. +func NewContainer(metadata Metadata, status Status) (Container, error) { + s, err := StoreStatus(metadata.ID, status) + if err != nil { + return Container{}, err + } + return Container{ + Metadata: metadata, + Status: s, + }, nil +} + +// Delete deletes checkpoint for the container. +func (c *Container) Delete() error { + return c.Status.Delete() +} + +// LoadContainer loads the internal used container type. +func LoadContainer() (Container, error) { + return Container{}, nil +} + +// Store stores all Containers. +type Store struct { + lock sync.RWMutex + containers map[string]Container + // TODO(random-liu): Add trunc index. +} + +// LoadStore loads containers from runtime. +// TODO(random-liu): Implement LoadStore. +func LoadStore() *Store { return nil } + +// NewStore creates a container store. +func NewStore() *Store { + return &Store{containers: make(map[string]Container)} +} + +// Add a container into the store. Returns store.ErrAlreadyExist if the +// container already exists. +func (s *Store) Add(c Container) error { + s.lock.Lock() + defer s.lock.Unlock() + if _, ok := s.containers[c.ID]; ok { + return store.ErrAlreadyExist + } + s.containers[c.ID] = c + return nil +} + +// Get returns the container with specified id. Returns store.ErrNotExist +// if the container doesn't exist. +func (s *Store) Get(id string) (Container, error) { + s.lock.RLock() + defer s.lock.RUnlock() + if c, ok := s.containers[id]; ok { + return c, nil + } + return Container{}, store.ErrNotExist +} + +// List lists all containers. +func (s *Store) List() []Container { + s.lock.RLock() + defer s.lock.RUnlock() + var containers []Container + for _, c := range s.containers { + containers = append(containers, c) + } + return containers +} + +// Delete deletes the container from store with specified id. +func (s *Store) Delete(id string) { + s.lock.Lock() + defer s.lock.Unlock() + delete(s.containers, id) +} diff --git a/pkg/store/container/container_test.go b/pkg/store/container/container_test.go new file mode 100644 index 000000000..0e40b556e --- /dev/null +++ b/pkg/store/container/container_test.go @@ -0,0 +1,138 @@ +/* +Copyright 2017 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 container + +import ( + "testing" + "time" + + assertlib "github.com/stretchr/testify/assert" + "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" + + "github.com/kubernetes-incubator/cri-containerd/pkg/store" +) + +func TestContainerStore(t *testing.T) { + ids := []string{"1", "2", "3"} + metadatas := map[string]Metadata{ + "1": { + ID: "1", + Name: "Container-1", + SandboxID: "Sandbox-1", + Config: &runtime.ContainerConfig{ + Metadata: &runtime.ContainerMetadata{ + Name: "TestPod-1", + Attempt: 1, + }, + }, + ImageRef: "TestImage-1", + }, + "2": { + ID: "2", + Name: "Container-2", + SandboxID: "Sandbox-2", + Config: &runtime.ContainerConfig{ + Metadata: &runtime.ContainerMetadata{ + Name: "TestPod-2", + Attempt: 2, + }, + }, + ImageRef: "TestImage-2", + }, + "3": { + ID: "3", + Name: "Container-3", + SandboxID: "Sandbox-3", + Config: &runtime.ContainerConfig{ + Metadata: &runtime.ContainerMetadata{ + Name: "TestPod-3", + Attempt: 3, + }, + }, + ImageRef: "TestImage-3", + }, + } + statuses := map[string]Status{ + "1": { + Pid: 1, + CreatedAt: time.Now().UnixNano(), + StartedAt: time.Now().UnixNano(), + FinishedAt: time.Now().UnixNano(), + ExitCode: 1, + Reason: "TestReason-1", + Message: "TestMessage-1", + }, + "2": { + Pid: 2, + CreatedAt: time.Now().UnixNano(), + StartedAt: time.Now().UnixNano(), + FinishedAt: time.Now().UnixNano(), + ExitCode: 2, + Reason: "TestReason-2", + Message: "TestMessage-2", + }, + "3": { + Pid: 3, + CreatedAt: time.Now().UnixNano(), + StartedAt: time.Now().UnixNano(), + FinishedAt: time.Now().UnixNano(), + ExitCode: 3, + Reason: "TestReason-3", + Message: "TestMessage-3", + Removing: true, + }, + } + assert := assertlib.New(t) + containers := map[string]Container{} + for _, id := range ids { + container, err := NewContainer(metadatas[id], statuses[id]) + assert.NoError(err) + containers[id] = container + } + + s := NewStore() + + t.Logf("should be able to add container") + for _, c := range containers { + assert.NoError(s.Add(c)) + } + + t.Logf("should be able to get container") + for id, c := range containers { + got, err := s.Get(id) + assert.NoError(err) + assert.Equal(c, got) + } + + t.Logf("should be able to list containers") + cs := s.List() + assert.Len(cs, 3) + + testID := "2" + t.Logf("add should return already exists error for duplicated container") + assert.Equal(store.ErrAlreadyExist, s.Add(containers[testID])) + + t.Logf("should be able to delete container") + s.Delete(testID) + cs = s.List() + assert.Len(cs, 2) + + t.Logf("get should return not exist error after deletion") + c, err := s.Get(testID) + assert.Equal(Container{}, c) + assert.Equal(store.ErrNotExist, err) +} diff --git a/pkg/store/container/metadata.go b/pkg/store/container/metadata.go new file mode 100644 index 000000000..44797bbfa --- /dev/null +++ b/pkg/store/container/metadata.go @@ -0,0 +1,76 @@ +/* +Copyright 2017 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 container + +import ( + "encoding/json" + "fmt" + + "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" +) + +// NOTE(random-liu): +// 1) Metadata is immutable after created. +// 2) Metadata is checkpointed as containerd container label. + +// metadataVersion is current version of container metadata. +const metadataVersion = "v1" // nolint + +// versionedMetadata is the internal versioned container metadata. +// nolint +type versionedMetadata struct { + // Version indicates the version of the versioned container metadata. + Version string + Metadata +} + +// Metadata is the unversioned container metadata. +type Metadata struct { + // ID is the container id. + ID string + // Name is the container name. + Name string + // SandboxID is the sandbox id the container belongs to. + SandboxID string + // Config is the CRI container config. + Config *runtime.ContainerConfig + // ImageRef is the reference of image used by the container. + ImageRef string +} + +// Encode encodes Metadata into bytes in json format. +func (c *Metadata) Encode() ([]byte, error) { + return json.Marshal(&versionedMetadata{ + Version: metadataVersion, + Metadata: *c, + }) +} + +// Decode decodes Metadata from bytes. +func (c *Metadata) Decode(data []byte) error { + versioned := &versionedMetadata{} + if err := json.Unmarshal(data, versioned); err != nil { + return err + } + // Handle old version after upgrade. + switch versioned.Version { + case metadataVersion: + *c = versioned.Metadata + return nil + } + return fmt.Errorf("unsupported version") +} diff --git a/pkg/store/container/metadata_test.go b/pkg/store/container/metadata_test.go new file mode 100644 index 000000000..2457d12aa --- /dev/null +++ b/pkg/store/container/metadata_test.go @@ -0,0 +1,53 @@ +/* +Copyright 2017 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 container + +import ( + "encoding/json" + "testing" + + assertlib "github.com/stretchr/testify/assert" + "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" +) + +func TestMetadataEncodeDecode(t *testing.T) { + meta := &Metadata{ + ID: "test-id", + Name: "test-name", + SandboxID: "test-sandbox-id", + Config: &runtime.ContainerConfig{ + Metadata: &runtime.ContainerMetadata{ + Name: "test-name", + Attempt: 1, + }, + }, + ImageRef: "test-image-ref", + } + assert := assertlib.New(t) + data, err := meta.Encode() + assert.NoError(err) + newMeta := &Metadata{} + assert.NoError(newMeta.Decode(data)) + assert.Equal(meta, newMeta) + + unsupported, err := json.Marshal(&versionedMetadata{ + Version: "random-test-version", + Metadata: *meta, + }) + assert.NoError(err) + assert.Error(newMeta.Decode(unsupported)) +} diff --git a/pkg/store/container/status.go b/pkg/store/container/status.go new file mode 100644 index 000000000..39b8ff6a9 --- /dev/null +++ b/pkg/store/container/status.go @@ -0,0 +1,146 @@ +/* +Copyright 2017 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 container + +import ( + "sync" + + "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" +) + +// TODO(random-liu): Handle versioning. +// TODO(random-liu): Add checkpoint support. + +// version is current version of container status. +const version = "v1" // nolint + +// versionedStatus is the internal used versioned container status. +// nolint +type versionedStatus struct { + // Version indicates the version of the versioned container status. + Version string + Status +} + +// Status is the status of a container. +type Status struct { + // Pid is the init process id of the container. + Pid uint32 + // CreatedAt is the created timestamp. + CreatedAt int64 + // StartedAt is the started timestamp. + StartedAt int64 + // FinishedAt is the finished timestamp. + FinishedAt int64 + // ExitCode is the container exit code. + ExitCode int32 + // CamelCase string explaining why container is in its current state. + Reason string + // Human-readable message indicating details about why container is in its + // current state. + Message string + // Removing indicates that the container is in removing state. + // This field doesn't need to be checkpointed. + // TODO(random-liu): Reset this field to false during state recoverry. + Removing bool +} + +// State returns current state of the container based on the container status. +func (c Status) State() runtime.ContainerState { + if c.FinishedAt != 0 { + return runtime.ContainerState_CONTAINER_EXITED + } + if c.StartedAt != 0 { + return runtime.ContainerState_CONTAINER_RUNNING + } + if c.CreatedAt != 0 { + return runtime.ContainerState_CONTAINER_CREATED + } + return runtime.ContainerState_CONTAINER_UNKNOWN +} + +// UpdateFunc is function used to update the container status. If there +// is an error, the update will be rolled back. +type UpdateFunc func(Status) (Status, error) + +// StatusStorage manages the container status with a storage backend. +type StatusStorage interface { + // Get a container status. + Get() Status + // Update the container status. Note that the update MUST be applied + // in one transaction. + // TODO(random-liu): Distinguish `UpdateSync` and `Update`, only + // `UpdateSync` should sync data onto disk, so that disk operation + // for non-critical status change could be avoided. + Update(UpdateFunc) error + // Delete the container status. + // Note: + // * Delete should be idempotent. + // * The status must be deleted in one trasaction. + Delete() error +} + +// TODO(random-liu): Add factory function and configure checkpoint path. + +// StoreStatus creates the storage containing the passed in container status with the +// specified id. +// The status MUST be created in one transaction. +func StoreStatus(id string, status Status) (StatusStorage, error) { + return &statusStorage{status: status}, nil + // TODO(random-liu): Create the data on disk atomically. +} + +// LoadStatus loads container status from checkpoint. +func LoadStatus(id string) (StatusStorage, error) { + // TODO(random-liu): Load container status from disk. + return nil, nil +} + +type statusStorage struct { + sync.RWMutex + status Status +} + +// Get a copy of container status. +func (m *statusStorage) Get() Status { + m.RLock() + defer m.RUnlock() + return m.status +} + +// Update the container status. +func (m *statusStorage) Update(u UpdateFunc) error { + m.Lock() + defer m.Unlock() + newStatus, err := u(m.status) + if err != nil { + return err + } + // TODO(random-liu) *Update* existing status on disk atomically, + // return error if checkpoint failed. + m.status = newStatus + return nil +} + +// Delete deletes the container status from disk atomically. +func (m *statusStorage) Delete() error { + // TODO(random-liu): Rename the data on the disk, returns error + // if fails. No lock is needed because file rename is atomic. + // TODO(random-liu): Cleanup temporary files generated, do not + // return error. + return nil +} diff --git a/pkg/store/container/status_test.go b/pkg/store/container/status_test.go new file mode 100644 index 000000000..8549bf7de --- /dev/null +++ b/pkg/store/container/status_test.go @@ -0,0 +1,101 @@ +/* +Copyright 2017 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 container + +import ( + "errors" + "testing" + "time" + + assertlib "github.com/stretchr/testify/assert" + "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" +) + +func TestContainerState(t *testing.T) { + for c, test := range map[string]struct { + status Status + state runtime.ContainerState + }{ + "unknown state": { + status: Status{}, + state: runtime.ContainerState_CONTAINER_UNKNOWN, + }, + "created state": { + status: Status{ + CreatedAt: time.Now().UnixNano(), + }, + state: runtime.ContainerState_CONTAINER_CREATED, + }, + "running state": { + status: Status{ + CreatedAt: time.Now().UnixNano(), + StartedAt: time.Now().UnixNano(), + }, + state: runtime.ContainerState_CONTAINER_RUNNING, + }, + "exited state": { + status: Status{ + CreatedAt: time.Now().UnixNano(), + FinishedAt: time.Now().UnixNano(), + }, + state: runtime.ContainerState_CONTAINER_EXITED, + }, + } { + t.Logf("TestCase %q", c) + assertlib.Equal(t, test.state, test.status.State()) + } +} + +func TestStatus(t *testing.T) { + testID := "test-id" + testStatus := Status{ + CreatedAt: time.Now().UnixNano(), + } + updateStatus := Status{ + CreatedAt: time.Now().UnixNano(), + StartedAt: time.Now().UnixNano(), + } + updateErr := errors.New("update error") + assert := assertlib.New(t) + + t.Logf("simple store and get") + s, err := StoreStatus(testID, testStatus) + assert.NoError(err) + old := s.Get() + assert.Equal(testStatus, old) + + t.Logf("failed update should not take effect") + err = s.Update(func(o Status) (Status, error) { + o = updateStatus + return o, updateErr + }) + assert.Equal(updateErr, err) + assert.Equal(testStatus, s.Get()) + + t.Logf("successful update should take effect") + err = s.Update(func(o Status) (Status, error) { + o = updateStatus + return o, nil + }) + assert.NoError(err) + assert.Equal(updateStatus, s.Get()) + + t.Logf("successful update should not affect existing snapshot") + assert.Equal(testStatus, old) + + // TODO(random-liu): Test Load and Delete after disc checkpoint is added. +} diff --git a/pkg/store/errors.go b/pkg/store/errors.go new file mode 100644 index 000000000..37652b3e7 --- /dev/null +++ b/pkg/store/errors.go @@ -0,0 +1,27 @@ +/* +Copyright 2017 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 store + +import "errors" + +var ( + // ErrAlreadyExist is the error returned when data added in the store + // already exists. + ErrAlreadyExist = errors.New("already exists") + // ErrNotExist is the error returned when data is not in the store. + ErrNotExist = errors.New("does not exist") +) diff --git a/pkg/store/image/image.go b/pkg/store/image/image.go new file mode 100644 index 000000000..49b9c6624 --- /dev/null +++ b/pkg/store/image/image.go @@ -0,0 +1,120 @@ +/* +Copyright 2017 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 image + +import ( + "sync" + + imagespec "github.com/opencontainers/image-spec/specs-go/v1" + + "github.com/kubernetes-incubator/cri-containerd/pkg/store" +) + +// Image contains all resources associated with the image. All fields +// MUST not be mutated directly after created. +type Image struct { + // Id of the image. Normally the digest of image config. + ID string + // Other names by which this image is known. + RepoTags []string + // Digests by which this image is known. + RepoDigests []string + // ChainID is the chainID of the image. + ChainID string + // Size is the compressed size of the image. + Size int64 + // Config is the oci image config of the image. + Config *imagespec.ImageConfig + // TODO(random-liu): Add containerd image client. +} + +// Store stores all images. +type Store struct { + lock sync.RWMutex + images map[string]Image + // TODO(random-liu): Add trunc index. +} + +// LoadStore loads images from runtime. +// TODO(random-liu): Implement LoadStore. +func LoadStore() *Store { return nil } + +// NewStore creates an image store. +func NewStore() *Store { + return &Store{images: make(map[string]Image)} +} + +// Add an image into the store. +func (s *Store) Add(img Image) { + s.lock.Lock() + defer s.lock.Unlock() + i, ok := s.images[img.ID] + if !ok { + // If the image doesn't exist, add it. + s.images[img.ID] = img + return + } + // Or else, merge the repo tags/digests. + i.RepoTags = mergeStringSlices(i.RepoTags, img.RepoTags) + i.RepoDigests = mergeStringSlices(i.RepoDigests, img.RepoDigests) + s.images[img.ID] = i +} + +// Get returns the image with specified id. Returns store.ErrNotExist if the +// image doesn't exist. +func (s *Store) Get(id string) (Image, error) { + s.lock.RLock() + defer s.lock.RUnlock() + if i, ok := s.images[id]; ok { + return i, nil + } + return Image{}, store.ErrNotExist +} + +// List lists all images. +func (s *Store) List() []Image { + s.lock.RLock() + defer s.lock.RUnlock() + var images []Image + for _, sb := range s.images { + images = append(images, sb) + } + return images +} + +// Delete deletes the image with specified id. +func (s *Store) Delete(id string) { + s.lock.Lock() + defer s.lock.Unlock() + delete(s.images, id) +} + +// mergeStringSlices merges 2 string slices into one and remove duplicated elements. +func mergeStringSlices(a []string, b []string) []string { + set := map[string]struct{}{} + for _, s := range a { + set[s] = struct{}{} + } + for _, s := range b { + set[s] = struct{}{} + } + var ss []string + for s := range set { + ss = append(ss, s) + } + return ss +} diff --git a/pkg/store/image/image_test.go b/pkg/store/image/image_test.go new file mode 100644 index 000000000..3f4197058 --- /dev/null +++ b/pkg/store/image/image_test.go @@ -0,0 +1,106 @@ +/* +Copyright 2017 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 image + +import ( + "testing" + + imagespec "github.com/opencontainers/image-spec/specs-go/v1" + assertlib "github.com/stretchr/testify/assert" + + "github.com/kubernetes-incubator/cri-containerd/pkg/store" +) + +func TestImageStore(t *testing.T) { + images := map[string]Image{ + "1": { + ID: "1", + ChainID: "test-chain-id-1", + RepoTags: []string{"tag-1"}, + RepoDigests: []string{"digest-1"}, + Size: 10, + Config: &imagespec.ImageConfig{}, + }, + "2": { + ID: "2", + ChainID: "test-chain-id-2", + RepoTags: []string{"tag-2"}, + RepoDigests: []string{"digest-2"}, + Size: 20, + Config: &imagespec.ImageConfig{}, + }, + "3": { + ID: "3", + RepoTags: []string{"tag-3"}, + RepoDigests: []string{"digest-3"}, + ChainID: "test-chain-id-3", + Size: 30, + Config: &imagespec.ImageConfig{}, + }, + } + assert := assertlib.New(t) + + s := NewStore() + + t.Logf("should be able to add image") + for _, img := range images { + s.Add(img) + } + + t.Logf("should be able to get image") + for id, img := range images { + got, err := s.Get(id) + assert.NoError(err) + assert.Equal(img, got) + } + + t.Logf("should be able to list images") + imgs := s.List() + assert.Len(imgs, 3) + + testID := "2" + t.Logf("should be able to add new repo tags/digests") + newImg := images[testID] + newImg.RepoTags = []string{"tag-new"} + newImg.RepoDigests = []string{"digest-new"} + s.Add(newImg) + got, err := s.Get(testID) + assert.NoError(err) + assert.Len(got.RepoTags, 2) + assert.Contains(got.RepoTags, "tag-2", "tag-new") + assert.Len(got.RepoDigests, 2) + assert.Contains(got.RepoDigests, "digest-2", "digest-new") + + t.Logf("should not be able to add duplicated repo tags/digests") + s.Add(newImg) + got, err = s.Get(testID) + assert.NoError(err) + assert.Len(got.RepoTags, 2) + assert.Contains(got.RepoTags, "tag-2", "tag-new") + assert.Len(got.RepoDigests, 2) + assert.Contains(got.RepoDigests, "digest-2", "digest-new") + + t.Logf("should be able to delete image") + s.Delete(testID) + imgs = s.List() + assert.Len(imgs, 2) + + t.Logf("get should return nil after deletion") + img, err := s.Get(testID) + assert.Equal(Image{}, img) + assert.Equal(store.ErrNotExist, err) +} diff --git a/pkg/store/sandbox/metadata.go b/pkg/store/sandbox/metadata.go new file mode 100644 index 000000000..e3a739773 --- /dev/null +++ b/pkg/store/sandbox/metadata.go @@ -0,0 +1,79 @@ +/* +Copyright 2017 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 sandbox + +import ( + "encoding/json" + "fmt" + + "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" +) + +// NOTE(random-liu): +// 1) Metadata is immutable after created. +// 2) Metadata is checkpointed as containerd container label. + +// metadataVersion is current version of sandbox metadata. +const metadataVersion = "v1" // nolint + +// versionedMetadata is the internal versioned sandbox metadata. +// nolint +type versionedMetadata struct { + // Version indicates the version of the versioned sandbox metadata. + Version string + Metadata +} + +// Metadata is the unversioned sandbox metadata. +type Metadata struct { + // ID is the sandbox id. + ID string + // Name is the sandbox name. + Name string + // Config is the CRI sandbox config. + Config *runtime.PodSandboxConfig + // CreatedAt is the created timestamp. + // TODO(random-liu): Use containerd container CreatedAt (containerd#933) + CreatedAt int64 + // Pid is the process id of the sandbox. + Pid uint32 + // NetNS is the network namespace used by the sandbox. + NetNS string +} + +// Encode encodes Metadata into bytes in json format. +func (c *Metadata) Encode() ([]byte, error) { + return json.Marshal(&versionedMetadata{ + Version: metadataVersion, + Metadata: *c, + }) +} + +// Decode decodes Metadata from bytes. +func (c *Metadata) Decode(data []byte) error { + versioned := &versionedMetadata{} + if err := json.Unmarshal(data, versioned); err != nil { + return err + } + // Handle old version after upgrade. + switch versioned.Version { + case metadataVersion: + *c = versioned.Metadata + return nil + } + return fmt.Errorf("unsupported version") +} diff --git a/pkg/store/sandbox/metadata_test.go b/pkg/store/sandbox/metadata_test.go new file mode 100644 index 000000000..01aa3477c --- /dev/null +++ b/pkg/store/sandbox/metadata_test.go @@ -0,0 +1,55 @@ +/* +Copyright 2017 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 sandbox + +import ( + "encoding/json" + "testing" + "time" + + assertlib "github.com/stretchr/testify/assert" + "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" +) + +func TestMetadataEncodeDecode(t *testing.T) { + meta := &Metadata{ + ID: "test-id", + Name: "test-name", + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: "test-name", + Uid: "test-uid", + Namespace: "test-namespace", + Attempt: 1, + }, + }, + CreatedAt: time.Now().UnixNano(), + } + assert := assertlib.New(t) + data, err := meta.Encode() + assert.NoError(err) + newMeta := &Metadata{} + assert.NoError(newMeta.Decode(data)) + assert.Equal(meta, newMeta) + + unsupported, err := json.Marshal(&versionedMetadata{ + Version: "random-test-version", + Metadata: *meta, + }) + assert.NoError(err) + assert.Error(newMeta.Decode(unsupported)) +} diff --git a/pkg/store/sandbox/sandbox.go b/pkg/store/sandbox/sandbox.go new file mode 100644 index 000000000..31c908722 --- /dev/null +++ b/pkg/store/sandbox/sandbox.go @@ -0,0 +1,88 @@ +/* +Copyright 2017 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 sandbox + +import ( + "sync" + + "github.com/kubernetes-incubator/cri-containerd/pkg/store" +) + +// Sandbox contains all resources associated with the sandbox. All methods to +// mutate the internal state are thread safe. +type Sandbox struct { + // Metadata is the metadata of the sandbox, it is immutable after created. + Metadata + // TODO(random-liu): Add containerd container client. + // TODO(random-liu): Add cni network namespace client. +} + +// Store stores all sandboxes. +type Store struct { + lock sync.RWMutex + sandboxes map[string]Sandbox + // TODO(random-liu): Add trunc index. +} + +// LoadStore loads sandboxes from runtime. +// TODO(random-liu): Implement LoadStore. +func LoadStore() *Store { return nil } + +// NewStore creates a sandbox store. +func NewStore() *Store { + return &Store{sandboxes: make(map[string]Sandbox)} +} + +// Add a sandbox into the store. +func (s *Store) Add(sb Sandbox) error { + s.lock.Lock() + defer s.lock.Unlock() + if _, ok := s.sandboxes[sb.ID]; ok { + return store.ErrAlreadyExist + } + s.sandboxes[sb.ID] = sb + return nil +} + +// Get returns the sandbox with specified id. Returns nil +// if the sandbox doesn't exist. +func (s *Store) Get(id string) (Sandbox, error) { + s.lock.RLock() + defer s.lock.RUnlock() + if sb, ok := s.sandboxes[id]; ok { + return sb, nil + } + return Sandbox{}, store.ErrNotExist +} + +// List lists all sandboxes. +func (s *Store) List() []Sandbox { + s.lock.RLock() + defer s.lock.RUnlock() + var sandboxes []Sandbox + for _, sb := range s.sandboxes { + sandboxes = append(sandboxes, sb) + } + return sandboxes +} + +// Delete deletes the sandbox with specified id. +func (s *Store) Delete(id string) { + s.lock.Lock() + defer s.lock.Unlock() + delete(s.sandboxes, id) +} diff --git a/pkg/store/sandbox/sandbox_test.go b/pkg/store/sandbox/sandbox_test.go new file mode 100644 index 000000000..6b754c472 --- /dev/null +++ b/pkg/store/sandbox/sandbox_test.go @@ -0,0 +1,115 @@ +/* +Copyright 2017 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 sandbox + +import ( + "testing" + "time" + + assertlib "github.com/stretchr/testify/assert" + "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" + + "github.com/kubernetes-incubator/cri-containerd/pkg/store" +) + +func TestSandboxStore(t *testing.T) { + ids := []string{"1", "2", "3"} + metadatas := map[string]Metadata{ + "1": { + ID: "1", + Name: "Sandbox-1", + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: "TestPod-1", + Uid: "TestUid-1", + Namespace: "TestNamespace-1", + Attempt: 1, + }, + }, + CreatedAt: time.Now().UnixNano(), + Pid: 1001, + NetNS: "TestNetNS-1", + }, + "2": { + ID: "2", + Name: "Sandbox-2", + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: "TestPod-2", + Uid: "TestUid-2", + Namespace: "TestNamespace-2", + Attempt: 2, + }, + }, + CreatedAt: time.Now().UnixNano(), + Pid: 1002, + NetNS: "TestNetNS-2", + }, + "3": { + ID: "3", + Name: "Sandbox-3", + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: "TestPod-3", + Uid: "TestUid-3", + Namespace: "TestNamespace-3", + Attempt: 3, + }, + }, + CreatedAt: time.Now().UnixNano(), + Pid: 1003, + NetNS: "TestNetNS-3", + }, + } + assert := assertlib.New(t) + sandboxes := map[string]Sandbox{} + for _, id := range ids { + sandboxes[id] = Sandbox{metadatas[id]} + } + + s := NewStore() + + t.Logf("should be able to add sandbox") + for _, sb := range sandboxes { + assert.NoError(s.Add(sb)) + } + + t.Logf("should be able to get sandbox") + for id, sb := range sandboxes { + got, err := s.Get(id) + assert.NoError(err) + assert.Equal(sb, got) + } + + t.Logf("should be able to list sandboxes") + sbs := s.List() + assert.Len(sbs, 3) + + testID := "2" + t.Logf("add should return already exists error for duplicated sandbox") + assert.Equal(store.ErrAlreadyExist, s.Add(sandboxes[testID])) + + t.Logf("should be able to delete sandbox") + s.Delete(testID) + sbs = s.List() + assert.Len(sbs, 2) + + t.Logf("get should return not exist error after deletion") + sb, err := s.Get(testID) + assert.Equal(Sandbox{}, sb) + assert.Equal(store.ErrNotExist, err) +} From 4317e6119ae8c1068b8155e6b7e6d0457788c063 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Mon, 5 Jun 2017 20:59:50 +0000 Subject: [PATCH 2/4] Remove sandbox truncindex. Signed-off-by: Lantao Liu --- pkg/server/container_create.go | 2 +- pkg/server/container_start.go | 2 +- pkg/server/helpers.go | 24 ----------------- pkg/server/helpers_test.go | 44 ------------------------------- pkg/server/sandbox_list.go | 12 +-------- pkg/server/sandbox_remove.go | 5 +--- pkg/server/sandbox_remove_test.go | 4 --- pkg/server/sandbox_run.go | 10 ------- pkg/server/sandbox_run_test.go | 4 --- pkg/server/sandbox_status.go | 2 +- pkg/server/sandbox_status_test.go | 1 - pkg/server/sandbox_stop.go | 2 +- pkg/server/sandbox_stop_test.go | 2 -- pkg/server/service.go | 23 +++++----------- pkg/server/service_test.go | 2 -- 15 files changed, 13 insertions(+), 126 deletions(-) diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go index 81f3d636b..84709a44f 100644 --- a/pkg/server/container_create.go +++ b/pkg/server/container_create.go @@ -47,7 +47,7 @@ func (c *criContainerdService) CreateContainer(ctx context.Context, r *runtime.C config := r.GetConfig() sandboxConfig := r.GetSandboxConfig() - sandbox, err := c.getSandbox(r.GetPodSandboxId()) + sandbox, err := c.sandboxStore.Get(r.GetPodSandboxId()) if err != nil { return nil, fmt.Errorf("failed to find sandbox id %q: %v", r.GetPodSandboxId(), err) } diff --git a/pkg/server/container_start.go b/pkg/server/container_start.go index b0203cb55..369d0f2b3 100644 --- a/pkg/server/container_start.go +++ b/pkg/server/container_start.go @@ -90,7 +90,7 @@ func (c *criContainerdService) startContainer(ctx context.Context, id string, me }() // Get sandbox config from sandbox store. - sandboxMeta, err := c.getSandbox(meta.SandboxID) + sandboxMeta, err := c.sandboxStore.Get(meta.SandboxID) if err != nil { return fmt.Errorf("sandbox %q not found: %v", meta.SandboxID, err) } diff --git a/pkg/server/helpers.go b/pkg/server/helpers.go index 546bc638b..d790ce31a 100644 --- a/pkg/server/helpers.go +++ b/pkg/server/helpers.go @@ -28,7 +28,6 @@ import ( containerdmetadata "github.com/containerd/containerd/metadata" "github.com/docker/distribution/reference" "github.com/docker/docker/pkg/stringid" - "github.com/docker/docker/pkg/truncindex" imagedigest "github.com/opencontainers/go-digest" "github.com/opencontainers/image-spec/identity" imagespec "github.com/opencontainers/image-spec/specs-go/v1" @@ -236,29 +235,6 @@ func isRuncProcessAlreadyFinishedError(grpcError error) bool { return strings.Contains(grpc.ErrorDesc(grpcError), "os: process already finished") } -// getSandbox gets the sandbox metadata from the sandbox store. It returns nil without -// error if the sandbox metadata is not found. It also tries to get full sandbox id and -// retry if the sandbox metadata is not found with the initial id. -func (c *criContainerdService) getSandbox(id string) (*metadata.SandboxMetadata, error) { - sandbox, err := c.sandboxStore.Get(id) - if err != nil && !metadata.IsNotExistError(err) { - return nil, fmt.Errorf("sandbox metadata not found: %v", err) - } - if err == nil { - return sandbox, nil - } - // sandbox is not found in metadata store, try to extract full id. - id, indexErr := c.sandboxIDIndex.Get(id) - if indexErr != nil { - if indexErr == truncindex.ErrNotExist { - // Return the original error if sandbox id is not found. - return nil, err - } - return nil, fmt.Errorf("sandbox id not found: %v", err) - } - return c.sandboxStore.Get(id) -} - // criContainerStateToString formats CRI container state to string. func criContainerStateToString(state runtime.ContainerState) string { return runtime.ContainerState_name[int32(state)] diff --git a/pkg/server/helpers_test.go b/pkg/server/helpers_test.go index 868705668..02fd1b7bc 100644 --- a/pkg/server/helpers_test.go +++ b/pkg/server/helpers_test.go @@ -28,7 +28,6 @@ import ( "github.com/stretchr/testify/assert" "golang.org/x/net/context" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" ostesting "github.com/kubernetes-incubator/cri-containerd/pkg/os/testing" ) @@ -119,49 +118,6 @@ func TestPrepareStreamingPipesError(t *testing.T) { } } -func TestGetSandbox(t *testing.T) { - c := newTestCRIContainerdService() - testID := "abcdefg" - testSandbox := metadata.SandboxMetadata{ - ID: testID, - Name: "test-name", - } - assert.NoError(t, c.sandboxStore.Create(testSandbox)) - assert.NoError(t, c.sandboxIDIndex.Add(testID)) - - for desc, test := range map[string]struct { - id string - expected *metadata.SandboxMetadata - expectErr bool - }{ - "full id": { - id: testID, - expected: &testSandbox, - expectErr: false, - }, - "partial id": { - id: testID[:3], - expected: &testSandbox, - expectErr: false, - }, - "non-exist id": { - id: "gfedcba", - expected: nil, - expectErr: true, - }, - } { - t.Logf("TestCase %q", desc) - sb, err := c.getSandbox(test.id) - if test.expectErr { - assert.Error(t, err) - assert.True(t, metadata.IsNotExistError(err)) - } else { - assert.NoError(t, err) - } - assert.Equal(t, test.expected, sb) - } -} - func TestNormalizeImageRef(t *testing.T) { for _, test := range []struct { input string diff --git a/pkg/server/sandbox_list.go b/pkg/server/sandbox_list.go index 1ff9e0058..69583e847 100644 --- a/pkg/server/sandbox_list.go +++ b/pkg/server/sandbox_list.go @@ -93,20 +93,10 @@ func (c *criContainerdService) filterCRISandboxes(sandboxes []*runtime.PodSandbo return sandboxes } - var filterID string - if filter.GetId() != "" { - // Handle truncate id. Use original filter if failed to convert. - var err error - filterID, err = c.sandboxIDIndex.Get(filter.GetId()) - if err != nil { - filterID = filter.GetId() - } - } - filtered := []*runtime.PodSandbox{} for _, s := range sandboxes { // Filter by id - if filterID != "" && filterID != s.Id { + if filter.GetId() != "" && filter.GetId() != s.Id { continue } // Filter by state diff --git a/pkg/server/sandbox_remove.go b/pkg/server/sandbox_remove.go index 6d93b2a28..8493e1c7c 100644 --- a/pkg/server/sandbox_remove.go +++ b/pkg/server/sandbox_remove.go @@ -39,7 +39,7 @@ func (c *criContainerdService) RemovePodSandbox(ctx context.Context, r *runtime. } }() - sandbox, err := c.getSandbox(r.GetPodSandboxId()) + sandbox, err := c.sandboxStore.Get(r.GetPodSandboxId()) if err != nil { if !metadata.IsNotExistError(err) { return nil, fmt.Errorf("an error occurred when try to find sandbox %q: %v", @@ -117,9 +117,6 @@ func (c *criContainerdService) RemovePodSandbox(ctx context.Context, r *runtime. return nil, fmt.Errorf("failed to delete sandbox metadata for %q: %v", id, err) } - // Release the sandbox id from id index. - c.sandboxIDIndex.Delete(id) // nolint: errcheck - // Release the sandbox name reserved for the sandbox. c.sandboxNameIndex.ReleaseByKey(id) diff --git a/pkg/server/sandbox_remove_test.go b/pkg/server/sandbox_remove_test.go index 50cff7c6b..71d37690e 100644 --- a/pkg/server/sandbox_remove_test.go +++ b/pkg/server/sandbox_remove_test.go @@ -116,7 +116,6 @@ func TestRemovePodSandbox(t *testing.T) { fakeExecutionClient.SetFakeTasks(test.sandboxTasks) if test.injectMetadata { c.sandboxNameIndex.Reserve(testName, testID) - c.sandboxIDIndex.Add(testID) c.sandboxStore.Create(testMetadata) } if test.removeSnapshotErr == nil { @@ -158,8 +157,6 @@ func TestRemovePodSandbox(t *testing.T) { assert.NotNil(t, res) assert.NoError(t, c.sandboxNameIndex.Reserve(testName, testID), "sandbox name should be released") - _, err = c.sandboxIDIndex.Get(testID) - assert.Error(t, err, "sandbox id should be removed") meta, err := c.sandboxStore.Get(testID) assert.Error(t, err) assert.True(t, metadata.IsNotExistError(err)) @@ -209,7 +206,6 @@ func TestRemoveContainersInSandbox(t *testing.T) { c := newTestCRIContainerdService() WithFakeSnapshotClient(c) assert.NoError(t, c.sandboxNameIndex.Reserve(testName, testID)) - assert.NoError(t, c.sandboxIDIndex.Add(testID)) assert.NoError(t, c.sandboxStore.Create(testMetadata)) for _, cntr := range testContainersMetadata { assert.NoError(t, c.containerNameIndex.Reserve(cntr.Name, cntr.ID)) diff --git a/pkg/server/sandbox_run.go b/pkg/server/sandbox_run.go index fe91fdaaa..af1759b81 100644 --- a/pkg/server/sandbox_run.go +++ b/pkg/server/sandbox_run.go @@ -64,16 +64,6 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run c.sandboxNameIndex.ReleaseByName(name) } }() - // Register the sandbox id. - if err := c.sandboxIDIndex.Add(id); err != nil { - return nil, fmt.Errorf("failed to insert sandbox id %q: %v", id, err) - } - defer func() { - // Delete the sandbox id if the function returns with an error. - if retErr != nil { - c.sandboxIDIndex.Delete(id) // nolint: errcheck - } - }() // Create initial sandbox metadata. meta := metadata.SandboxMetadata{ diff --git a/pkg/server/sandbox_run_test.go b/pkg/server/sandbox_run_test.go index 24c5c6cd9..8417ff55d 100644 --- a/pkg/server/sandbox_run_test.go +++ b/pkg/server/sandbox_run_test.go @@ -362,10 +362,6 @@ func TestRunPodSandbox(t *testing.T) { pid := info.Task.Pid assert.Equal(t, meta.NetNS, getNetworkNamespace(pid), "metadata network namespace should be correct") - gotID, err := c.sandboxIDIndex.Get(id) - assert.NoError(t, err) - assert.Equal(t, id, gotID, "sandbox id should be indexed") - expectedCNICalls := []string{"SetUpPod"} assert.Equal(t, expectedCNICalls, fakeCNIPlugin.GetCalledNames(), "expect SetUpPod should be called") calls = fakeCNIPlugin.GetCalledDetails() diff --git a/pkg/server/sandbox_status.go b/pkg/server/sandbox_status.go index 4e0c45a0b..74a1ea743 100644 --- a/pkg/server/sandbox_status.go +++ b/pkg/server/sandbox_status.go @@ -39,7 +39,7 @@ func (c *criContainerdService) PodSandboxStatus(ctx context.Context, r *runtime. } }() - sandbox, err := c.getSandbox(r.GetPodSandboxId()) + sandbox, err := c.sandboxStore.Get(r.GetPodSandboxId()) if err != nil { return nil, fmt.Errorf("an error occurred when try to find sandbox %q: %v", r.GetPodSandboxId(), err) diff --git a/pkg/server/sandbox_status_test.go b/pkg/server/sandbox_status_test.go index 2cf22fc0c..724eca828 100644 --- a/pkg/server/sandbox_status_test.go +++ b/pkg/server/sandbox_status_test.go @@ -186,7 +186,6 @@ func TestPodSandboxStatus(t *testing.T) { fakeCNIPlugin := c.netPlugin.(*servertesting.FakeCNIPlugin) fake.SetFakeTasks(test.sandboxTasks) if test.injectMetadata { - assert.NoError(t, c.sandboxIDIndex.Add(metadata.ID)) assert.NoError(t, c.sandboxStore.Create(*metadata)) } if test.injectErr != nil { diff --git a/pkg/server/sandbox_stop.go b/pkg/server/sandbox_stop.go index dc56f0f7c..ca8062f10 100644 --- a/pkg/server/sandbox_stop.go +++ b/pkg/server/sandbox_stop.go @@ -38,7 +38,7 @@ func (c *criContainerdService) StopPodSandbox(ctx context.Context, r *runtime.St } }() - sandbox, err := c.getSandbox(r.GetPodSandboxId()) + sandbox, err := c.sandboxStore.Get(r.GetPodSandboxId()) if err != nil { return nil, fmt.Errorf("an error occurred when try to find sandbox %q: %v", r.GetPodSandboxId(), err) diff --git a/pkg/server/sandbox_stop_test.go b/pkg/server/sandbox_stop_test.go index bc8e8777c..21ebc73ee 100644 --- a/pkg/server/sandbox_stop_test.go +++ b/pkg/server/sandbox_stop_test.go @@ -137,7 +137,6 @@ func TestStopPodSandbox(t *testing.T) { if test.injectSandbox { assert.NoError(t, c.sandboxStore.Create(testSandbox)) - c.sandboxIDIndex.Add(testID) } if test.injectErr != nil { fake.InjectError("delete", test.injectErr) @@ -234,7 +233,6 @@ func TestStopContainersInSandbox(t *testing.T) { c.taskService = fake fake.SetFakeTasks(testContainerdContainers) assert.NoError(t, c.sandboxStore.Create(testSandbox)) - assert.NoError(t, c.sandboxIDIndex.Add(testID)) for _, cntr := range testContainers { assert.NoError(t, c.containerStore.Create(cntr)) } diff --git a/pkg/server/service.go b/pkg/server/service.go index 5f9c1bd3b..5b92ced0e 100644 --- a/pkg/server/service.go +++ b/pkg/server/service.go @@ -27,7 +27,6 @@ import ( "github.com/containerd/containerd/images" diffservice "github.com/containerd/containerd/services/diff" "github.com/containerd/containerd/snapshot" - "github.com/docker/docker/pkg/truncindex" "github.com/kubernetes-incubator/cri-o/pkg/ocicni" healthapi "google.golang.org/grpc/health/grpc_health_v1" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" @@ -65,10 +64,6 @@ type criContainerdService struct { // sandboxNameIndex stores all sandbox names and make sure each name // is unique. sandboxNameIndex *registrar.Registrar - // sandboxIDIndex is trie tree for truncated id indexing, e.g. after an - // id "abcdefg" is added, we could use "abcd" to identify the same thing - // as long as there is no ambiguity. - sandboxIDIndex *truncindex.TruncIndex // containerStore stores all container metadata. containerStore metadata.ContainerStore // containerNameIndex stores all container names and make sure each @@ -109,17 +104,13 @@ func NewCRIContainerdService(containerdEndpoint, rootDir, networkPluginBinDir, n } c := &criContainerdService{ - os: osinterface.RealOS{}, - rootDir: rootDir, - sandboxImage: defaultSandboxImage, - sandboxStore: metadata.NewSandboxStore(store.NewMetadataStore()), - containerStore: metadata.NewContainerStore(store.NewMetadataStore()), - imageMetadataStore: metadata.NewImageMetadataStore(store.NewMetadataStore()), - // TODO(random-liu): Register sandbox/container id/name for recovered sandbox/container. - // TODO(random-liu): Use the same name and id index for both container and sandbox. - sandboxNameIndex: registrar.NewRegistrar(), - sandboxIDIndex: truncindex.NewTruncIndex(nil), - // TODO(random-liu): Add container id index. + os: osinterface.RealOS{}, + rootDir: rootDir, + sandboxImage: defaultSandboxImage, + sandboxStore: metadata.NewSandboxStore(store.NewMetadataStore()), + containerStore: metadata.NewContainerStore(store.NewMetadataStore()), + imageMetadataStore: metadata.NewImageMetadataStore(store.NewMetadataStore()), + sandboxNameIndex: registrar.NewRegistrar(), containerNameIndex: registrar.NewRegistrar(), containerService: client.ContainerService(), taskService: client.TaskService(), diff --git a/pkg/server/service_test.go b/pkg/server/service_test.go index 6a55d327b..4c15bfd24 100644 --- a/pkg/server/service_test.go +++ b/pkg/server/service_test.go @@ -23,7 +23,6 @@ import ( "github.com/containerd/containerd/api/services/execution" snapshotservice "github.com/containerd/containerd/services/snapshot" - "github.com/docker/docker/pkg/truncindex" imagespec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -62,7 +61,6 @@ func newTestCRIContainerdService() *criContainerdService { sandboxStore: metadata.NewSandboxStore(store.NewMetadataStore()), imageMetadataStore: metadata.NewImageMetadataStore(store.NewMetadataStore()), sandboxNameIndex: registrar.NewRegistrar(), - sandboxIDIndex: truncindex.NewTruncIndex(nil), containerStore: metadata.NewContainerStore(store.NewMetadataStore()), containerNameIndex: registrar.NewRegistrar(), taskService: servertesting.NewFakeExecutionClient(), From 7b16a35287b609829bd3a0fdb84dbf9e3d1ca49d Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Fri, 28 Jul 2017 03:51:24 +0000 Subject: [PATCH 3/4] Use new metadata store. Signed-off-by: Lantao Liu --- pkg/server/container_create.go | 57 +++++++----- pkg/server/container_create_test.go | 81 ++++++++--------- pkg/server/container_execsync.go | 11 +-- pkg/server/container_list.go | 38 ++++---- pkg/server/container_list_test.go | 114 +++++++++++++++--------- pkg/server/container_remove.go | 78 +++++++++-------- pkg/server/container_remove_test.go | 80 ++++++++--------- pkg/server/container_start.go | 44 +++++----- pkg/server/container_start_test.go | 113 ++++++++++++------------ pkg/server/container_status.go | 32 +++---- pkg/server/container_status_test.go | 42 +++++---- pkg/server/container_stop.go | 34 ++++---- pkg/server/container_stop_test.go | 73 +++++++++------- pkg/server/events.go | 34 ++++---- pkg/server/events_test.go | 70 ++++++++------- pkg/server/helpers.go | 27 +++--- pkg/server/image_list.go | 16 ++-- pkg/server/image_list_test.go | 6 +- pkg/server/image_pull.go | 87 +++++-------------- pkg/server/image_pull_test.go | 51 ----------- pkg/server/image_remove.go | 21 ++--- pkg/server/image_status.go | 16 ++-- pkg/server/image_status_test.go | 6 +- pkg/server/sandbox_list.go | 13 ++- pkg/server/sandbox_list_test.go | 32 ++++--- pkg/server/sandbox_remove.go | 20 ++--- pkg/server/sandbox_remove_test.go | 130 +++++++++++++++------------- pkg/server/sandbox_run.go | 40 ++++----- pkg/server/sandbox_run_test.go | 24 ++--- pkg/server/sandbox_status.go | 6 +- pkg/server/sandbox_status_test.go | 50 +++++------ pkg/server/sandbox_stop.go | 5 +- pkg/server/sandbox_stop_test.go | 107 +++++++++++++---------- pkg/server/service.go | 27 +++--- pkg/server/service_test.go | 15 ++-- 35 files changed, 791 insertions(+), 809 deletions(-) diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go index 84709a44f..d4ce0aa6d 100644 --- a/pkg/server/container_create.go +++ b/pkg/server/container_create.go @@ -32,7 +32,7 @@ import ( "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" ) // CreateContainer creates a new container in the given PodSandbox. @@ -58,7 +58,7 @@ func (c *criContainerdService) CreateContainer(ctx context.Context, r *runtime.C // the same container. id := generateID() name := makeContainerName(config.GetMetadata(), sandboxConfig.GetMetadata()) - if err := c.containerNameIndex.Reserve(name, id); err != nil { + if err = c.containerNameIndex.Reserve(name, id); err != nil { return nil, fmt.Errorf("failed to reserve container name %q: %v", name, err) } defer func() { @@ -68,8 +68,8 @@ func (c *criContainerdService) CreateContainer(ctx context.Context, r *runtime.C } }() - // Create initial container metadata. - meta := metadata.ContainerMetadata{ + // Create initial internal container metadata. + meta := containerstore.Metadata{ ID: id, Name: name, SandboxID: sandboxID, @@ -78,18 +78,18 @@ func (c *criContainerdService) CreateContainer(ctx context.Context, r *runtime.C // Prepare container image snapshot. For container, the image should have // been pulled before creating the container, so do not ensure the image. - image := config.GetImage().GetImage() - imageMeta, err := c.localResolve(ctx, image) + imageRef := config.GetImage().GetImage() + image, err := c.localResolve(ctx, imageRef) if err != nil { - return nil, fmt.Errorf("failed to resolve image %q: %v", image, err) + return nil, fmt.Errorf("failed to resolve image %q: %v", imageRef, err) } - if imageMeta == nil { - return nil, fmt.Errorf("image %q not found", image) + if image == nil { + return nil, fmt.Errorf("image %q not found", imageRef) } // Generate container runtime spec. mounts := c.generateContainerMounts(getSandboxRootDir(c.rootDir, sandboxID), config) - spec, err := c.generateContainerSpec(id, sandbox.Pid, config, sandboxConfig, imageMeta.Config, mounts) + spec, err := c.generateContainerSpec(id, sandbox.Pid, config, sandboxConfig, image.Config, mounts) if err != nil { return nil, fmt.Errorf("failed to generate container %q spec: %v", id, err) } @@ -101,12 +101,12 @@ func (c *criContainerdService) CreateContainer(ctx context.Context, r *runtime.C // Prepare container rootfs. if config.GetLinux().GetSecurityContext().GetReadonlyRootfs() { - if _, err := c.snapshotService.View(ctx, id, imageMeta.ChainID); err != nil { - return nil, fmt.Errorf("failed to view container rootfs %q: %v", imageMeta.ChainID, err) + if _, err := c.snapshotService.View(ctx, id, image.ChainID); err != nil { + return nil, fmt.Errorf("failed to view container rootfs %q: %v", image.ChainID, err) } } else { - if _, err := c.snapshotService.Prepare(ctx, id, imageMeta.ChainID); err != nil { - return nil, fmt.Errorf("failed to prepare container rootfs %q: %v", imageMeta.ChainID, err) + if _, err := c.snapshotService.Prepare(ctx, id, image.ChainID); err != nil { + return nil, fmt.Errorf("failed to prepare container rootfs %q: %v", image.ChainID, err) } } defer func() { @@ -116,18 +116,18 @@ func (c *criContainerdService) CreateContainer(ctx context.Context, r *runtime.C } } }() - meta.ImageRef = imageMeta.ID + meta.ImageRef = image.ID // Create container root directory. containerRootDir := getContainerRootDir(c.rootDir, id) - if err := c.os.MkdirAll(containerRootDir, 0755); err != nil { + if err = c.os.MkdirAll(containerRootDir, 0755); err != nil { return nil, fmt.Errorf("failed to create container root directory %q: %v", containerRootDir, err) } defer func() { if retErr != nil { // Cleanup the container root directory. - if err := c.os.RemoveAll(containerRootDir); err != nil { + if err = c.os.RemoveAll(containerRootDir); err != nil { glog.Errorf("Failed to remove container root directory %q: %v", containerRootDir, err) } @@ -139,7 +139,7 @@ func (c *criContainerdService) CreateContainer(ctx context.Context, r *runtime.C Container: containers.Container{ ID: id, // TODO(random-liu): Checkpoint metadata into container labels. - Image: imageMeta.ID, + Image: image.ID, Runtime: defaultRuntime, Spec: &prototypes.Any{ TypeUrl: runtimespec.Version, @@ -158,12 +158,23 @@ func (c *criContainerdService) CreateContainer(ctx context.Context, r *runtime.C } }() - // Update container CreatedAt. - meta.CreatedAt = time.Now().UnixNano() + container, err := containerstore.NewContainer(meta, containerstore.Status{CreatedAt: time.Now().UnixNano()}) + if err != nil { + return nil, fmt.Errorf("failed to create internal container object for %q: %v", + id, err) + } + defer func() { + if retErr != nil { + // Cleanup container checkpoint on error. + if err := container.Delete(); err != nil { + glog.Errorf("Failed to cleanup container checkpoint for %q: %v", id, err) + } + } + }() + // Add container into container store. - if err := c.containerStore.Create(meta); err != nil { - return nil, fmt.Errorf("failed to add container metadata %+v into store: %v", - meta, err) + if err := c.containerStore.Add(container); err != nil { + return nil, fmt.Errorf("failed to add container %q into store: %v", id, err) } return &runtime.CreateContainerResponse{ContainerId: id}, nil diff --git a/pkg/server/container_create_test.go b/pkg/server/container_create_test.go index be25b255b..019a8dbb9 100644 --- a/pkg/server/container_create_test.go +++ b/pkg/server/container_create_test.go @@ -32,9 +32,11 @@ import ( "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" ostesting "github.com/kubernetes-incubator/cri-containerd/pkg/os/testing" servertesting "github.com/kubernetes-incubator/cri-containerd/pkg/server/testing" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" + imagestore "github.com/kubernetes-incubator/cri-containerd/pkg/store/image" + sandboxstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/sandbox" ) func checkMount(t *testing.T, mounts []runtimespec.Mount, src, dest, typ string, @@ -443,64 +445,66 @@ func TestCreateContainer(t *testing.T) { testSandboxID := "test-sandbox-id" testSandboxPid := uint32(4321) config, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData() - testSandboxMetadata := &metadata.SandboxMetadata{ - ID: testSandboxID, - Name: "test-sandbox-name", - Config: sandboxConfig, - Pid: testSandboxPid, + testSandbox := &sandboxstore.Sandbox{ + Metadata: sandboxstore.Metadata{ + ID: testSandboxID, + Name: "test-sandbox-name", + Config: sandboxConfig, + Pid: testSandboxPid, + }, } testContainerName := makeContainerName(config.Metadata, sandboxConfig.Metadata) // Use an image id to avoid image name resolution. // TODO(random-liu): Change this to image name after we have complete image // management unit test framework. - testImage := config.GetImage().GetImage() + testImageRef := config.GetImage().GetImage() testChainID := "test-chain-id" - testImageMetadata := metadata.ImageMetadata{ - ID: testImage, + testImage := imagestore.Image{ + ID: testImageRef, ChainID: testChainID, Config: imageConfig, } for desc, test := range map[string]struct { - sandboxMetadata *metadata.SandboxMetadata + sandbox *sandboxstore.Sandbox reserveNameErr bool - imageMetadataErr bool + imageStoreErr bool prepareSnapshotErr error createRootDirErr error expectErr bool - expectMeta *metadata.ContainerMetadata + expectedMeta containerstore.Metadata }{ "should return error if sandbox does not exist": { - sandboxMetadata: nil, - expectErr: true, + sandbox: nil, + expectErr: true, }, "should return error if name is reserved": { - sandboxMetadata: testSandboxMetadata, - reserveNameErr: true, - expectErr: true, + sandbox: testSandbox, + reserveNameErr: true, + expectErr: true, }, "should return error if fail to create root directory": { - sandboxMetadata: testSandboxMetadata, + sandbox: testSandbox, createRootDirErr: errors.New("random error"), expectErr: true, }, "should return error if image is not pulled": { - sandboxMetadata: testSandboxMetadata, - imageMetadataErr: true, - expectErr: true, + sandbox: testSandbox, + imageStoreErr: true, + expectErr: true, }, "should return error if prepare snapshot fails": { - sandboxMetadata: testSandboxMetadata, + sandbox: testSandbox, prepareSnapshotErr: errors.New("random error"), expectErr: true, }, "should be able to create container successfully": { - sandboxMetadata: testSandboxMetadata, - expectErr: false, - expectMeta: &metadata.ContainerMetadata{ + sandbox: testSandbox, + expectErr: false, + expectedMeta: containerstore.Metadata{ Name: testContainerName, SandboxID: testSandboxID, - ImageRef: testImage, + ImageRef: testImageRef, Config: config, }, }, @@ -510,14 +514,14 @@ func TestCreateContainer(t *testing.T) { fake := c.containerService.(*servertesting.FakeContainersClient) fakeSnapshotClient := WithFakeSnapshotClient(c) fakeOS := c.os.(*ostesting.FakeOS) - if test.sandboxMetadata != nil { - assert.NoError(t, c.sandboxStore.Create(*test.sandboxMetadata)) + if test.sandbox != nil { + assert.NoError(t, c.sandboxStore.Add(*test.sandbox)) } if test.reserveNameErr { assert.NoError(t, c.containerNameIndex.Reserve(testContainerName, "random id")) } - if !test.imageMetadataErr { - assert.NoError(t, c.imageMetadataStore.Create(testImageMetadata)) + if !test.imageStoreErr { + c.imageStore.Add(testImage) } if test.prepareSnapshotErr != nil { fakeSnapshotClient.InjectError("prepare", test.prepareSnapshotErr) @@ -554,9 +558,7 @@ func TestCreateContainer(t *testing.T) { listResp, err := fake.List(context.Background(), &containers.ListContainersRequest{}) assert.NoError(t, err) assert.Empty(t, listResp.Containers, "containerd container should be cleaned up") - metas, err := c.containerStore.List() - assert.NoError(t, err) - assert.Empty(t, metas, "container metadata should not be created") + assert.Empty(t, c.containerStore.List(), "container metadata should not be created") continue } assert.NoError(t, err) @@ -571,7 +573,7 @@ func TestCreateContainer(t *testing.T) { createOpts, ok := containersCalls[0].Argument.(*containers.CreateContainerRequest) assert.True(t, ok, "should create containerd container") assert.Equal(t, id, createOpts.Container.ID, "container id should be correct") - assert.Equal(t, testImage, createOpts.Container.Image, "test image should be correct") + assert.Equal(t, testImageRef, createOpts.Container.Image, "test image should be correct") assert.Equal(t, id, createOpts.Container.RootFS, "rootfs should be correct") spec := &runtimespec.Spec{} assert.NoError(t, json.Unmarshal(createOpts.Container.Spec.Value, spec)) @@ -586,12 +588,11 @@ func TestCreateContainer(t *testing.T) { Parent: testChainID, }, prepareOpts, "prepare request should be correct") - meta, err := c.containerStore.Get(id) + container, err := c.containerStore.Get(id) assert.NoError(t, err) - require.NotNil(t, meta) - test.expectMeta.ID = id - // TODO(random-liu): Use fake clock to test CreatedAt. - test.expectMeta.CreatedAt = meta.CreatedAt - assert.Equal(t, test.expectMeta, meta, "container metadata should be created") + test.expectedMeta.ID = id + assert.Equal(t, test.expectedMeta, container.Metadata, "container metadata should be created") + assert.Equal(t, runtime.ContainerState_CONTAINER_CREATED, container.Status.Get().State(), + "container should be in created state") } } diff --git a/pkg/server/container_execsync.go b/pkg/server/container_execsync.go index dcdf9fc8a..c24602f76 100644 --- a/pkg/server/container_execsync.go +++ b/pkg/server/container_execsync.go @@ -45,15 +45,16 @@ func (c *criContainerdService) ExecSync(ctx context.Context, r *runtime.ExecSync } }() - // Get container metadata from our container store. - meta, err := c.containerStore.Get(r.GetContainerId()) + // Get container from our container store. + cntr, err := c.containerStore.Get(r.GetContainerId()) if err != nil { return nil, fmt.Errorf("an error occurred when try to find container %q: %v", r.GetContainerId(), err) } - id := meta.ID + id := cntr.ID - if meta.State() != runtime.ContainerState_CONTAINER_RUNNING { - return nil, fmt.Errorf("container %q is in %s state", id, criContainerStateToString(meta.State())) + state := cntr.Status.Get().State() + if state != runtime.ContainerState_CONTAINER_RUNNING { + return nil, fmt.Errorf("container %q is in %s state", id, criContainerStateToString(state)) } // Get exec process spec. diff --git a/pkg/server/container_list.go b/pkg/server/container_list.go index 573a9ae89..d0f587657 100644 --- a/pkg/server/container_list.go +++ b/pkg/server/container_list.go @@ -17,14 +17,12 @@ limitations under the License. package server import ( - "fmt" - "github.com/golang/glog" "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" ) // ListContainers lists all containers matching the filter. @@ -36,33 +34,31 @@ func (c *criContainerdService) ListContainers(ctx context.Context, r *runtime.Li } }() - // List all container metadata from store. - metas, err := c.containerStore.List() - if err != nil { - return nil, fmt.Errorf("failed to list metadata from container store: %v", err) - } + // List all containers from store. + containersInStore := c.containerStore.List() var containers []*runtime.Container - for _, meta := range metas { - containers = append(containers, toCRIContainer(meta)) + for _, container := range containersInStore { + containers = append(containers, toCRIContainer(container)) } containers = c.filterCRIContainers(containers, r.GetFilter()) return &runtime.ListContainersResponse{Containers: containers}, nil } -// toCRIContainer converts container metadata into CRI container. -func toCRIContainer(meta *metadata.ContainerMetadata) *runtime.Container { +// toCRIContainer converts internal container object into CRI container. +func toCRIContainer(container containerstore.Container) *runtime.Container { + status := container.Status.Get() return &runtime.Container{ - Id: meta.ID, - PodSandboxId: meta.SandboxID, - Metadata: meta.Config.GetMetadata(), - Image: meta.Config.GetImage(), - ImageRef: meta.ImageRef, - State: meta.State(), - CreatedAt: meta.CreatedAt, - Labels: meta.Config.GetLabels(), - Annotations: meta.Config.GetAnnotations(), + Id: container.ID, + PodSandboxId: container.SandboxID, + Metadata: container.Config.GetMetadata(), + Image: container.Config.GetImage(), + ImageRef: container.ImageRef, + State: status.State(), + CreatedAt: status.CreatedAt, + Labels: container.Config.GetLabels(), + Annotations: container.Config.GetAnnotations(), } } diff --git a/pkg/server/container_list_test.go b/pkg/server/container_list_test.go index 4c98c2107..3570afca1 100644 --- a/pkg/server/container_list_test.go +++ b/pkg/server/container_list_test.go @@ -23,10 +23,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/context" - "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" ) func TestToCRIContainer(t *testing.T) { @@ -40,20 +39,25 @@ func TestToCRIContainer(t *testing.T) { Annotations: map[string]string{"c": "d"}, } createdAt := time.Now().UnixNano() - meta := &metadata.ContainerMetadata{ - ID: "test-id", - Name: "test-name", - SandboxID: "test-sandbox-id", - Config: config, - ImageRef: "test-image-ref", - Pid: 1234, - CreatedAt: createdAt, - StartedAt: time.Now().UnixNano(), - FinishedAt: time.Now().UnixNano(), - ExitCode: 1, - Reason: "test-reason", - Message: "test-message", - } + container, err := containerstore.NewContainer( + containerstore.Metadata{ + ID: "test-id", + Name: "test-name", + SandboxID: "test-sandbox-id", + Config: config, + ImageRef: "test-image-ref", + }, + containerstore.Status{ + Pid: 1234, + CreatedAt: createdAt, + StartedAt: time.Now().UnixNano(), + FinishedAt: time.Now().UnixNano(), + ExitCode: 1, + Reason: "test-reason", + Message: "test-message", + }, + ) + assert.NoError(t, err) expect := &runtime.Container{ Id: "test-id", PodSandboxId: "test-sandbox-id", @@ -65,7 +69,7 @@ func TestToCRIContainer(t *testing.T) { Labels: config.GetLabels(), Annotations: config.GetAnnotations(), } - c := toCRIContainer(meta) + c := toCRIContainer(container) assert.Equal(t, expect, c) } @@ -147,43 +151,67 @@ func TestFilterContainers(t *testing.T) { } } +// containerForTest is a helper type for test. +type containerForTest struct { + metadata containerstore.Metadata + status containerstore.Status +} + +func (c containerForTest) toContainer() (containerstore.Container, error) { + return containerstore.NewContainer(c.metadata, c.status) +} + func TestListContainers(t *testing.T) { c := newTestCRIContainerdService() createdAt := time.Now().UnixNano() startedAt := time.Now().UnixNano() finishedAt := time.Now().UnixNano() - containersInStore := []metadata.ContainerMetadata{ + containersInStore := []containerForTest{ { - ID: "1", - Name: "name-1", - SandboxID: "s-1", - Config: &runtime.ContainerConfig{Metadata: &runtime.ContainerMetadata{Name: "name-1"}}, - CreatedAt: createdAt, + metadata: containerstore.Metadata{ + ID: "1", + Name: "name-1", + SandboxID: "s-1", + Config: &runtime.ContainerConfig{Metadata: &runtime.ContainerMetadata{Name: "name-1"}}, + }, + status: containerstore.Status{CreatedAt: createdAt}, }, { - ID: "2", - Name: "name-2", - SandboxID: "s-1", - Config: &runtime.ContainerConfig{Metadata: &runtime.ContainerMetadata{Name: "name-2"}}, - CreatedAt: createdAt, - StartedAt: startedAt, + metadata: containerstore.Metadata{ + ID: "2", + Name: "name-2", + SandboxID: "s-1", + Config: &runtime.ContainerConfig{Metadata: &runtime.ContainerMetadata{Name: "name-2"}}, + }, + status: containerstore.Status{ + CreatedAt: createdAt, + StartedAt: startedAt, + }, }, { - ID: "3", - Name: "name-3", - SandboxID: "s-1", - Config: &runtime.ContainerConfig{Metadata: &runtime.ContainerMetadata{Name: "name-3"}}, - CreatedAt: createdAt, - StartedAt: startedAt, - FinishedAt: finishedAt, + metadata: containerstore.Metadata{ + ID: "3", + Name: "name-3", + SandboxID: "s-1", + Config: &runtime.ContainerConfig{Metadata: &runtime.ContainerMetadata{Name: "name-3"}}, + }, + status: containerstore.Status{ + CreatedAt: createdAt, + StartedAt: startedAt, + FinishedAt: finishedAt, + }, }, { - ID: "4", - Name: "name-4", - SandboxID: "s-2", - Config: &runtime.ContainerConfig{Metadata: &runtime.ContainerMetadata{Name: "name-4"}}, - CreatedAt: createdAt, + metadata: containerstore.Metadata{ + ID: "4", + Name: "name-4", + SandboxID: "s-2", + Config: &runtime.ContainerConfig{Metadata: &runtime.ContainerMetadata{Name: "name-4"}}, + }, + status: containerstore.Status{ + CreatedAt: createdAt, + }, }, } filter := &runtime.ContainerFilter{ @@ -215,7 +243,9 @@ func TestListContainers(t *testing.T) { // Inject test metadata for _, cntr := range containersInStore { - c.containerStore.Create(cntr) + container, err := cntr.toContainer() + assert.NoError(t, err) + assert.NoError(t, c.containerStore.Add(container)) } resp, err := c.ListContainers(context.Background(), &runtime.ListContainersRequest{Filter: filter}) diff --git a/pkg/server/container_remove.go b/pkg/server/container_remove.go index 99c9fc213..5897d7f9b 100644 --- a/pkg/server/container_remove.go +++ b/pkg/server/container_remove.go @@ -25,7 +25,8 @@ import ( "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + "github.com/kubernetes-incubator/cri-containerd/pkg/store" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" ) // RemoveContainer removes the container. @@ -37,31 +38,29 @@ func (c *criContainerdService) RemoveContainer(ctx context.Context, r *runtime.R } }() - id := r.GetContainerId() + container, err := c.containerStore.Get(r.GetContainerId()) + if err != nil { + if err != store.ErrNotExist { + return nil, fmt.Errorf("an error occurred when try to find container %q: %v", r.GetContainerId(), err) + } + // Do not return error if container metadata doesn't exist. + glog.V(5).Infof("RemoveContainer called for container %q that does not exist", r.GetContainerId()) + return &runtime.RemoveContainerResponse{}, nil + } + id := container.ID // Set removing state to prevent other start/remove operations against this container // while it's being removed. - if err := c.setContainerRemoving(id); err != nil { - if !metadata.IsNotExistError(err) { - return nil, fmt.Errorf("failed to set removing state for container %q: %v", - id, err) - } - // Do not return error if container metadata doesn't exist. - glog.V(5).Infof("RemoveContainer called for container %q that does not exist", id) - return &runtime.RemoveContainerResponse{}, nil + if err := setContainerRemoving(container); err != nil { + return nil, fmt.Errorf("failed to set removing state for container %q: %v", id, err) } defer func() { - if retErr == nil { - // Cleanup all index after successfully remove the container. - c.containerNameIndex.ReleaseByKey(id) - return - } - // Reset removing if remove failed. - if err := c.resetContainerRemoving(id); err != nil { - // TODO(random-liu): Deal with update failure. Actually Removing doesn't need to - // be checkpointed, we only need it to have the same lifecycle with container metadata. - glog.Errorf("failed to reset removing state for container %q: %v", - id, err) + if retErr != nil { + // Reset removing if remove failed. + if err := resetContainerRemoving(container); err != nil { + // TODO(random-liu): Do not checkpoint `Removing` state. + glog.Errorf("failed to reset removing state for container %q: %v", id, err) + } } }() @@ -78,13 +77,17 @@ func (c *criContainerdService) RemoveContainer(ctx context.Context, r *runtime.R glog.V(5).Infof("Remove called for snapshot %q that does not exist", id) } - // Cleanup container root directory. containerRootDir := getContainerRootDir(c.rootDir, id) if err := c.os.RemoveAll(containerRootDir); err != nil { return nil, fmt.Errorf("failed to remove container root directory %q: %v", containerRootDir, err) } + // Delete container checkpoint. + if err := container.Delete(); err != nil { + return nil, fmt.Errorf("failed to delete container checkpoint for %q: %v", id, err) + } + // Delete containerd container. if _, err := c.containerService.Delete(ctx, &containers.DeleteContainerRequest{ID: id}); err != nil { if !isContainerdGRPCNotFoundError(err) { @@ -93,35 +96,34 @@ func (c *criContainerdService) RemoveContainer(ctx context.Context, r *runtime.R glog.V(5).Infof("Remove called for containerd container %q that does not exist", id, err) } - // Delete container metadata. - if err := c.containerStore.Delete(id); err != nil { - return nil, fmt.Errorf("failed to delete container metadata for %q: %v", id, err) - } + c.containerStore.Delete(id) + + c.containerNameIndex.ReleaseByKey(id) return &runtime.RemoveContainerResponse{}, nil } // setContainerRemoving sets the container into removing state. In removing state, the // container will not be started or removed again. -func (c *criContainerdService) setContainerRemoving(id string) error { - return c.containerStore.Update(id, func(meta metadata.ContainerMetadata) (metadata.ContainerMetadata, error) { +func setContainerRemoving(container containerstore.Container) error { + return container.Status.Update(func(status containerstore.Status) (containerstore.Status, error) { // Do not remove container if it's still running. - if meta.State() == runtime.ContainerState_CONTAINER_RUNNING { - return meta, fmt.Errorf("container %q is still running", id) + if status.State() == runtime.ContainerState_CONTAINER_RUNNING { + return status, fmt.Errorf("container is still running") } - if meta.Removing { - return meta, fmt.Errorf("container is already in removing state") + if status.Removing { + return status, fmt.Errorf("container is already in removing state") } - meta.Removing = true - return meta, nil + status.Removing = true + return status, nil }) } // resetContainerRemoving resets the container removing state on remove failure. So // that we could remove the container again. -func (c *criContainerdService) resetContainerRemoving(id string) error { - return c.containerStore.Update(id, func(meta metadata.ContainerMetadata) (metadata.ContainerMetadata, error) { - meta.Removing = false - return meta, nil +func resetContainerRemoving(container containerstore.Container) error { + return container.Status.Update(func(status containerstore.Status) (containerstore.Status, error) { + status.Removing = false + return status, nil }) } diff --git a/pkg/server/container_remove_test.go b/pkg/server/container_remove_test.go index d3c93ebeb..2f6caec33 100644 --- a/pkg/server/container_remove_test.go +++ b/pkg/server/container_remove_test.go @@ -25,13 +25,13 @@ import ( snapshotapi "github.com/containerd/containerd/api/services/snapshot" "github.com/containerd/containerd/api/types/mount" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" ostesting "github.com/kubernetes-incubator/cri-containerd/pkg/os/testing" servertesting "github.com/kubernetes-incubator/cri-containerd/pkg/server/testing" + "github.com/kubernetes-incubator/cri-containerd/pkg/store" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" ) // TestSetContainerRemoving tests setContainerRemoving sets removing @@ -39,20 +39,18 @@ import ( func TestSetContainerRemoving(t *testing.T) { testID := "test-id" for desc, test := range map[string]struct { - metadata *metadata.ContainerMetadata + status containerstore.Status expectErr bool }{ "should return error when container is in running state": { - metadata: &metadata.ContainerMetadata{ - ID: testID, + status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), }, expectErr: true, }, "should return error when container is in removing state": { - metadata: &metadata.ContainerMetadata{ - ID: testID, + status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), FinishedAt: time.Now().UnixNano(), @@ -61,8 +59,7 @@ func TestSetContainerRemoving(t *testing.T) { expectErr: true, }, "should not return error when container is not running and removing": { - metadata: &metadata.ContainerMetadata{ - ID: testID, + status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), FinishedAt: time.Now().UnixNano(), @@ -71,19 +68,18 @@ func TestSetContainerRemoving(t *testing.T) { }, } { t.Logf("TestCase %q", desc) - c := newTestCRIContainerdService() - if test.metadata != nil { - assert.NoError(t, c.containerStore.Create(*test.metadata)) - } - err := c.setContainerRemoving(testID) - meta, getErr := c.containerStore.Get(testID) - assert.NoError(t, getErr) + container, err := containerstore.NewContainer( + containerstore.Metadata{ID: testID}, + test.status, + ) + assert.NoError(t, err) + err = setContainerRemoving(container) if test.expectErr { assert.Error(t, err) - assert.Equal(t, test.metadata, meta, "metadata should not be updated") + assert.Equal(t, test.status, container.Status.Get(), "metadata should not be updated") } else { assert.NoError(t, err) - assert.True(t, meta.Removing, "removing should be set") + assert.True(t, container.Status.Get().Removing, "removing should be set") } } } @@ -91,15 +87,14 @@ func TestSetContainerRemoving(t *testing.T) { func TestRemoveContainer(t *testing.T) { testID := "test-id" testName := "test-name" - testContainerMetadata := &metadata.ContainerMetadata{ - ID: testID, + testContainerStatus := &containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), FinishedAt: time.Now().UnixNano(), } for desc, test := range map[string]struct { - metadata *metadata.ContainerMetadata + status *containerstore.Status removeSnapshotErr error deleteContainerErr error removeDirErr error @@ -107,16 +102,14 @@ func TestRemoveContainer(t *testing.T) { expectUnsetRemoving bool }{ "should return error when container is still running": { - metadata: &metadata.ContainerMetadata{ - ID: testID, + status: &containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), }, expectErr: true, }, "should return error when there is ongoing removing": { - metadata: &metadata.ContainerMetadata{ - ID: testID, + status: &containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), FinishedAt: time.Now().UnixNano(), @@ -124,40 +117,40 @@ func TestRemoveContainer(t *testing.T) { }, expectErr: true, }, - "should not return error if container metadata does not exist": { - metadata: nil, + "should not return error if container does not exist": { + status: nil, removeSnapshotErr: servertesting.SnapshotNotExistError, deleteContainerErr: servertesting.ContainerNotExistError, expectErr: false, }, "should not return error if snapshot does not exist": { - metadata: testContainerMetadata, + status: testContainerStatus, removeSnapshotErr: servertesting.SnapshotNotExistError, expectErr: false, }, "should return error if remove snapshot fails": { - metadata: testContainerMetadata, + status: testContainerStatus, removeSnapshotErr: errors.New("random error"), expectErr: true, }, "should not return error if containerd container does not exist": { - metadata: testContainerMetadata, + status: testContainerStatus, deleteContainerErr: servertesting.ContainerNotExistError, expectErr: false, }, "should return error if delete containerd container fails": { - metadata: testContainerMetadata, + status: testContainerStatus, deleteContainerErr: errors.New("random error"), expectErr: true, }, "should return error if remove container root fails": { - metadata: testContainerMetadata, + status: testContainerStatus, removeDirErr: errors.New("random error"), expectErr: true, expectUnsetRemoving: true, }, "should be able to remove container successfully": { - metadata: testContainerMetadata, + status: testContainerStatus, expectErr: false, }, } { @@ -166,9 +159,14 @@ func TestRemoveContainer(t *testing.T) { fake := c.containerService.(*servertesting.FakeContainersClient) fakeSnapshotClient := WithFakeSnapshotClient(c) fakeOS := c.os.(*ostesting.FakeOS) - if test.metadata != nil { + if test.status != nil { assert.NoError(t, c.containerNameIndex.Reserve(testName, testID)) - assert.NoError(t, c.containerStore.Create(*test.metadata)) + container, err := containerstore.NewContainer( + containerstore.Metadata{ID: testID}, + *test.status, + ) + assert.NoError(t, err) + assert.NoError(t, c.containerStore.Add(container)) } fakeOS.RemoveAllFn = func(path string) error { assert.Equal(t, getContainerRootDir(c.rootDir, testID), path) @@ -202,19 +200,16 @@ func TestRemoveContainer(t *testing.T) { if !test.expectUnsetRemoving { continue } - meta, err := c.containerStore.Get(testID) + container, err := c.containerStore.Get(testID) assert.NoError(t, err) - require.NotNil(t, meta) // Also covers resetContainerRemoving. - assert.False(t, meta.Removing, "removing state should be unset") + assert.False(t, container.Status.Get().Removing, "removing state should be unset") continue } assert.NoError(t, err) assert.NotNil(t, resp) - meta, err := c.containerStore.Get(testID) - assert.Error(t, err) - assert.True(t, metadata.IsNotExistError(err)) - assert.Nil(t, meta, "container metadata should be removed") + _, err = c.containerStore.Get(testID) + assert.Equal(t, store.ErrNotExist, err) assert.NoError(t, c.containerNameIndex.Reserve(testName, testID), "container name should be released") mountsResp, err := fakeSnapshotClient.Mounts(context.Background(), &snapshotapi.MountsRequest{Key: testID}) @@ -229,6 +224,5 @@ func TestRemoveContainer(t *testing.T) { }) assert.NoError(t, err) assert.NotNil(t, resp, "remove should be idempotent") - } } diff --git a/pkg/server/container_start.go b/pkg/server/container_start.go index 369d0f2b3..32a2bcbe5 100644 --- a/pkg/server/container_start.go +++ b/pkg/server/container_start.go @@ -30,8 +30,8 @@ import ( "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" "github.com/kubernetes-incubator/cri-containerd/pkg/server/agents" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" ) // StartContainer starts the container. @@ -50,12 +50,12 @@ func (c *criContainerdService) StartContainer(ctx context.Context, r *runtime.St id := container.ID var startErr error - // start container in one transaction to avoid race with event monitor. - if err := c.containerStore.Update(id, func(meta metadata.ContainerMetadata) (metadata.ContainerMetadata, error) { - // Always apply metadata change no matter startContainer fails or not. Because startContainer + // update container status in one transaction to avoid race with event monitor. + if err := container.Status.Update(func(status containerstore.Status) (containerstore.Status, error) { + // Always apply status change no matter startContainer fails or not. Because startContainer // may change container state no matter it fails or succeeds. - startErr = c.startContainer(ctx, id, &meta) - return meta, nil + startErr = c.startContainer(ctx, id, container.Metadata, &status) + return status, nil }); startErr != nil { return nil, startErr } else if err != nil { @@ -65,36 +65,36 @@ func (c *criContainerdService) StartContainer(ctx context.Context, r *runtime.St } // startContainer actually starts the container. The function needs to be run in one transaction. Any updates -// to the metadata passed in will be applied to container store no matter the function returns error or not. -func (c *criContainerdService) startContainer(ctx context.Context, id string, meta *metadata.ContainerMetadata) (retErr error) { +// to the status passed in will be applied no matter the function returns error or not. +func (c *criContainerdService) startContainer(ctx context.Context, id string, meta containerstore.Metadata, status *containerstore.Status) (retErr error) { config := meta.Config // Return error if container is not in created state. - if meta.State() != runtime.ContainerState_CONTAINER_CREATED { - return fmt.Errorf("container %q is in %s state", id, criContainerStateToString(meta.State())) + if status.State() != runtime.ContainerState_CONTAINER_CREATED { + return fmt.Errorf("container %q is in %s state", id, criContainerStateToString(status.State())) } // Do not start the container when there is a removal in progress. - if meta.Removing { + if status.Removing { return fmt.Errorf("container %q is in removing state", id) } defer func() { if retErr != nil { // Set container to exited if fail to start. - meta.Pid = 0 - meta.FinishedAt = time.Now().UnixNano() - meta.ExitCode = errorStartExitCode - meta.Reason = errorStartReason - meta.Message = retErr.Error() + status.Pid = 0 + status.FinishedAt = time.Now().UnixNano() + status.ExitCode = errorStartExitCode + status.Reason = errorStartReason + status.Message = retErr.Error() } }() // Get sandbox config from sandbox store. - sandboxMeta, err := c.sandboxStore.Get(meta.SandboxID) + sandbox, err := c.sandboxStore.Get(meta.SandboxID) if err != nil { return fmt.Errorf("sandbox %q not found: %v", meta.SandboxID, err) } - sandboxConfig := sandboxMeta.Config + sandboxConfig := sandbox.Config sandboxID := meta.SandboxID // Make sure sandbox is running. sandboxInfo, err := c.taskService.Info(ctx, &execution.InfoRequest{ContainerID: sandboxID}) @@ -137,12 +137,12 @@ func (c *criContainerdService) startContainer(ctx context.Context, id string, me if config.GetLogPath() != "" { // Only generate container log when log path is specified. logPath := filepath.Join(sandboxConfig.GetLogDirectory(), config.GetLogPath()) - if err := c.agentFactory.NewContainerLogger(logPath, agents.Stdout, stdoutPipe).Start(); err != nil { + if err = c.agentFactory.NewContainerLogger(logPath, agents.Stdout, stdoutPipe).Start(); err != nil { return fmt.Errorf("failed to start container stdout logger: %v", err) } // Only redirect stderr when there is no tty. if !config.GetTty() { - if err := c.agentFactory.NewContainerLogger(logPath, agents.Stderr, stderrPipe).Start(); err != nil { + if err = c.agentFactory.NewContainerLogger(logPath, agents.Stderr, stderrPipe).Start(); err != nil { return fmt.Errorf("failed to start container stderr logger: %v", err) } } @@ -192,7 +192,7 @@ func (c *criContainerdService) startContainer(ctx context.Context, id string, me } // Update container start timestamp. - meta.Pid = createResp.Pid - meta.StartedAt = time.Now().UnixNano() + status.Pid = createResp.Pid + status.StartedAt = time.Now().UnixNano() return nil } diff --git a/pkg/server/container_start_test.go b/pkg/server/container_start_test.go index c2d31083e..920f31b90 100644 --- a/pkg/server/container_start_test.go +++ b/pkg/server/container_start_test.go @@ -27,27 +27,29 @@ import ( "github.com/containerd/containerd/api/types/mount" "github.com/containerd/containerd/api/types/task" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" ostesting "github.com/kubernetes-incubator/cri-containerd/pkg/os/testing" servertesting "github.com/kubernetes-incubator/cri-containerd/pkg/server/testing" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" + sandboxstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/sandbox" ) func TestStartContainer(t *testing.T) { testID := "test-id" testSandboxID := "test-sandbox-id" - testMetadata := &metadata.ContainerMetadata{ + testMetadata := containerstore.Metadata{ ID: testID, Name: "test-name", SandboxID: testSandboxID, - CreatedAt: time.Now().UnixNano(), } - testSandboxMetadata := &metadata.SandboxMetadata{ - ID: testSandboxID, - Name: "test-sandbox-name", + testStatus := &containerstore.Status{CreatedAt: time.Now().UnixNano()} + testSandbox := &sandboxstore.Sandbox{ + Metadata: sandboxstore.Metadata{ + ID: testSandboxID, + Name: "test-sandbox-name", + }, } testSandboxContainer := &task.Task{ ID: testSandboxID, @@ -56,8 +58,8 @@ func TestStartContainer(t *testing.T) { } testMounts := []*mount.Mount{{Type: "bind", Source: "test-source"}} for desc, test := range map[string]struct { - containerMetadata *metadata.ContainerMetadata - sandboxMetadata *metadata.SandboxMetadata + status *containerstore.Status + sandbox *sandboxstore.Sandbox sandboxContainerdContainer *task.Task snapshotMountsErr bool prepareFIFOErr error @@ -67,50 +69,44 @@ func TestStartContainer(t *testing.T) { expectCalls []string expectErr bool }{ - "should return error when container metadata does not exist": { - containerMetadata: nil, - sandboxMetadata: testSandboxMetadata, + "should return error when container does not exist": { + status: nil, + sandbox: testSandbox, sandboxContainerdContainer: testSandboxContainer, expectCalls: []string{}, expectErr: true, }, "should return error when container is not in created state": { - containerMetadata: &metadata.ContainerMetadata{ - ID: testID, - Name: "test-name", - SandboxID: testSandboxID, + status: &containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), }, - sandboxMetadata: testSandboxMetadata, + sandbox: testSandbox, sandboxContainerdContainer: testSandboxContainer, expectCalls: []string{}, expectErr: true, }, "should return error when container is in removing state": { - containerMetadata: &metadata.ContainerMetadata{ - ID: testID, - Name: "test-name", - SandboxID: testSandboxID, + status: &containerstore.Status{ CreatedAt: time.Now().UnixNano(), Removing: true, }, - sandboxMetadata: testSandboxMetadata, + sandbox: testSandbox, sandboxContainerdContainer: testSandboxContainer, expectCalls: []string{}, expectErr: true, }, "should return error when sandbox does not exist": { - containerMetadata: testMetadata, - sandboxMetadata: nil, + status: testStatus, + sandbox: nil, sandboxContainerdContainer: testSandboxContainer, expectStateChange: true, expectCalls: []string{}, expectErr: true, }, "should return error when sandbox is not running": { - containerMetadata: testMetadata, - sandboxMetadata: testSandboxMetadata, + status: testStatus, + sandbox: testSandbox, sandboxContainerdContainer: &task.Task{ ID: testSandboxID, Pid: uint32(4321), @@ -121,8 +117,8 @@ func TestStartContainer(t *testing.T) { expectErr: true, }, "should return error when snapshot mounts fails": { - containerMetadata: testMetadata, - sandboxMetadata: testSandboxMetadata, + status: testStatus, + sandbox: testSandbox, sandboxContainerdContainer: testSandboxContainer, snapshotMountsErr: true, expectStateChange: true, @@ -130,8 +126,8 @@ func TestStartContainer(t *testing.T) { expectErr: true, }, "should return error when fail to open streaming pipes": { - containerMetadata: testMetadata, - sandboxMetadata: testSandboxMetadata, + status: testStatus, + sandbox: testSandbox, sandboxContainerdContainer: testSandboxContainer, prepareFIFOErr: errors.New("open error"), expectStateChange: true, @@ -139,8 +135,8 @@ func TestStartContainer(t *testing.T) { expectErr: true, }, "should return error when fail to create container": { - containerMetadata: testMetadata, - sandboxMetadata: testSandboxMetadata, + status: testStatus, + sandbox: testSandbox, sandboxContainerdContainer: testSandboxContainer, createContainerErr: errors.New("create error"), expectStateChange: true, @@ -148,8 +144,8 @@ func TestStartContainer(t *testing.T) { expectErr: true, }, "should return error when fail to start container": { - containerMetadata: testMetadata, - sandboxMetadata: testSandboxMetadata, + status: testStatus, + sandbox: testSandbox, sandboxContainerdContainer: testSandboxContainer, startContainerErr: errors.New("start error"), expectStateChange: true, @@ -158,8 +154,8 @@ func TestStartContainer(t *testing.T) { expectErr: true, }, "should be able to start container successfully": { - containerMetadata: testMetadata, - sandboxMetadata: testSandboxMetadata, + status: testStatus, + sandbox: testSandbox, sandboxContainerdContainer: testSandboxContainer, expectStateChange: true, expectCalls: []string{"info", "create", "start"}, @@ -171,11 +167,16 @@ func TestStartContainer(t *testing.T) { fake := c.taskService.(*servertesting.FakeExecutionClient) fakeOS := c.os.(*ostesting.FakeOS) fakeSnapshotClient := WithFakeSnapshotClient(c) - if test.containerMetadata != nil { - assert.NoError(t, c.containerStore.Create(*test.containerMetadata)) + if test.status != nil { + cntr, err := containerstore.NewContainer( + testMetadata, + *test.status, + ) + assert.NoError(t, err) + assert.NoError(t, c.containerStore.Add(cntr)) } - if test.sandboxMetadata != nil { - assert.NoError(t, c.sandboxStore.Create(*test.sandboxMetadata)) + if test.sandbox != nil { + assert.NoError(t, c.sandboxStore.Add(*test.sandbox)) } if test.sandboxContainerdContainer != nil { fake.SetFakeTasks([]task.Task{*test.sandboxContainerdContainer}) @@ -206,35 +207,39 @@ func TestStartContainer(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, resp) } - // Check container state. - meta, err := c.containerStore.Get(testID) - if !test.expectStateChange { - // Do not check the error, because container may not exist - // in the test case. - assert.Equal(t, meta, test.containerMetadata) + // Skip following validation if no container is injected initially. + if test.status == nil { continue } + // Check container state. + cntr, err := c.containerStore.Get(testID) assert.NoError(t, err) - require.NotNil(t, meta) + status := cntr.Status.Get() + if !test.expectStateChange { + assert.Equal(t, testMetadata, cntr.Metadata) + assert.Equal(t, *test.status, status) + continue + } if test.expectErr { t.Logf("container state should be in exited state when fail to start") - assert.Equal(t, runtime.ContainerState_CONTAINER_EXITED, meta.State()) - assert.Zero(t, meta.Pid) - assert.EqualValues(t, errorStartExitCode, meta.ExitCode) - assert.Equal(t, errorStartReason, meta.Reason) - assert.NotEmpty(t, meta.Message) + assert.Equal(t, runtime.ContainerState_CONTAINER_EXITED, status.State()) + assert.Zero(t, status.Pid) + assert.EqualValues(t, errorStartExitCode, status.ExitCode) + assert.Equal(t, errorStartReason, status.Reason) + assert.NotEmpty(t, status.Message) _, err := fake.Info(context.Background(), &execution.InfoRequest{ContainerID: testID}) assert.True(t, isContainerdGRPCNotFoundError(err), "containerd task should be cleaned up when fail to start") continue } t.Logf("container state should be running when start successfully") - assert.Equal(t, runtime.ContainerState_CONTAINER_RUNNING, meta.State()) + assert.Equal(t, runtime.ContainerState_CONTAINER_RUNNING, status.State()) info, err := fake.Info(context.Background(), &execution.InfoRequest{ContainerID: testID}) assert.NoError(t, err) pid := info.Task.Pid - assert.Equal(t, pid, meta.Pid) + assert.Equal(t, pid, status.Pid) assert.Equal(t, task.StatusRunning, info.Task.Status) + // Check runtime spec calls := fake.GetCalledDetails() createOpts, ok := calls[1].Argument.(*execution.CreateRequest) assert.True(t, ok, "2nd call should be create") diff --git a/pkg/server/container_status.go b/pkg/server/container_status.go index 515ff9b03..aa7dca8eb 100644 --- a/pkg/server/container_status.go +++ b/pkg/server/container_status.go @@ -21,10 +21,9 @@ import ( "github.com/golang/glog" "golang.org/x/net/context" - "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" ) // ContainerStatus inspects the container and returns the status. @@ -36,22 +35,23 @@ func (c *criContainerdService) ContainerStatus(ctx context.Context, r *runtime.C } }() - meta, err := c.containerStore.Get(r.GetContainerId()) + container, err := c.containerStore.Get(r.GetContainerId()) if err != nil { return nil, fmt.Errorf("an error occurred when try to find container %q: %v", r.GetContainerId(), err) } return &runtime.ContainerStatusResponse{ - Status: toCRIContainerStatus(meta), + Status: toCRIContainerStatus(container), }, nil } -// toCRIContainerStatus converts container metadata to CRI container status. -func toCRIContainerStatus(meta *metadata.ContainerMetadata) *runtime.ContainerStatus { - state := meta.State() - reason := meta.Reason - if state == runtime.ContainerState_CONTAINER_EXITED && reason == "" { - if meta.ExitCode == 0 { +// toCRIContainerStatus converts internal container object to CRI container status. +func toCRIContainerStatus(container containerstore.Container) *runtime.ContainerStatus { + meta := container.Metadata + status := container.Status.Get() + reason := status.Reason + if status.State() == runtime.ContainerState_CONTAINER_EXITED && reason == "" { + if status.ExitCode == 0 { reason = completeExitReason } else { reason = errorExitReason @@ -60,15 +60,15 @@ func toCRIContainerStatus(meta *metadata.ContainerMetadata) *runtime.ContainerSt return &runtime.ContainerStatus{ Id: meta.ID, Metadata: meta.Config.GetMetadata(), - State: state, - CreatedAt: meta.CreatedAt, - StartedAt: meta.StartedAt, - FinishedAt: meta.FinishedAt, - ExitCode: meta.ExitCode, + State: status.State(), + CreatedAt: status.CreatedAt, + StartedAt: status.StartedAt, + FinishedAt: status.FinishedAt, + ExitCode: status.ExitCode, Image: meta.Config.GetImage(), ImageRef: meta.ImageRef, Reason: reason, - Message: meta.Message, + Message: status.Message, Labels: meta.Config.GetLabels(), Annotations: meta.Config.GetAnnotations(), Mounts: meta.Config.GetMounts(), diff --git a/pkg/server/container_status_test.go b/pkg/server/container_status_test.go index 9e3f58189..8ef334993 100644 --- a/pkg/server/container_status_test.go +++ b/pkg/server/container_status_test.go @@ -22,13 +22,12 @@ import ( "github.com/stretchr/testify/assert" "golang.org/x/net/context" - "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" ) -func getContainerStatusTestData() (*metadata.ContainerMetadata, *runtime.ContainerStatus) { +func getContainerStatusTestData() (*containerstore.Metadata, *containerstore.Status, *runtime.ContainerStatus) { testID := "test-id" config := &runtime.ContainerConfig{ Metadata: &runtime.ContainerMetadata{ @@ -47,17 +46,18 @@ func getContainerStatusTestData() (*metadata.ContainerMetadata, *runtime.Contain createdAt := time.Now().UnixNano() startedAt := time.Now().UnixNano() - metadata := &metadata.ContainerMetadata{ + metadata := &containerstore.Metadata{ ID: testID, Name: "test-long-name", SandboxID: "test-sandbox-id", Config: config, ImageRef: "test-image-ref", + } + status := &containerstore.Status{ Pid: 1234, CreatedAt: createdAt, StartedAt: startedAt, } - expected := &runtime.ContainerStatus{ Id: testID, Metadata: config.GetMetadata(), @@ -72,7 +72,7 @@ func getContainerStatusTestData() (*metadata.ContainerMetadata, *runtime.Contain Mounts: config.GetMounts(), } - return metadata, expected + return metadata, status, expected } func TestToCRIContainerStatus(t *testing.T) { @@ -110,19 +110,21 @@ func TestToCRIContainerStatus(t *testing.T) { expectedReason: errorExitReason, }, } { - meta, expected := getContainerStatusTestData() - // Update metadata with test case. - meta.FinishedAt = test.finishedAt - meta.ExitCode = test.exitCode - meta.Reason = test.reason - meta.Message = test.message + metadata, status, expected := getContainerStatusTestData() + // Update status with test case. + status.FinishedAt = test.finishedAt + status.ExitCode = test.exitCode + status.Reason = test.reason + status.Message = test.message + container, err := containerstore.NewContainer(*metadata, *status) + assert.NoError(t, err) // Set expectation based on test case. expected.State = test.expectedState expected.Reason = test.expectedReason expected.FinishedAt = test.finishedAt expected.ExitCode = test.exitCode expected.Message = test.message - assert.Equal(t, expected, toCRIContainerStatus(meta), desc) + assert.Equal(t, expected, toCRIContainerStatus(container), desc) } } @@ -151,14 +153,16 @@ func TestContainerStatus(t *testing.T) { } { t.Logf("TestCase %q", desc) c := newTestCRIContainerdService() - meta, expected := getContainerStatusTestData() - // Update metadata with test case. - meta.FinishedAt = test.finishedAt - meta.Reason = test.reason + metadata, status, expected := getContainerStatusTestData() + // Update status with test case. + status.FinishedAt = test.finishedAt + status.Reason = test.reason + container, err := containerstore.NewContainer(*metadata, *status) + assert.NoError(t, err) if test.exist { - assert.NoError(t, c.containerStore.Create(*meta)) + assert.NoError(t, c.containerStore.Add(container)) } - resp, err := c.ContainerStatus(context.Background(), &runtime.ContainerStatusRequest{ContainerId: meta.ID}) + resp, err := c.ContainerStatus(context.Background(), &runtime.ContainerStatusRequest{ContainerId: container.ID}) if test.expectErr { assert.Error(t, err) assert.Nil(t, resp) diff --git a/pkg/server/container_stop.go b/pkg/server/container_stop.go index f4e8614c4..299550d4b 100644 --- a/pkg/server/container_stop.go +++ b/pkg/server/container_stop.go @@ -27,7 +27,8 @@ import ( "golang.org/x/sys/unix" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + "github.com/kubernetes-incubator/cri-containerd/pkg/store" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" ) const ( @@ -50,12 +51,12 @@ func (c *criContainerdService) StopContainer(ctx context.Context, r *runtime.Sto }() // Get container config from container store. - meta, err := c.containerStore.Get(r.GetContainerId()) + container, err := c.containerStore.Get(r.GetContainerId()) if err != nil { return nil, fmt.Errorf("an error occurred when try to find container %q: %v", r.GetContainerId(), err) } - if err := c.stopContainer(ctx, meta, time.Duration(r.GetTimeout())*time.Second); err != nil { + if err := c.stopContainer(ctx, container, time.Duration(r.GetTimeout())*time.Second); err != nil { return nil, err } @@ -63,32 +64,33 @@ func (c *criContainerdService) StopContainer(ctx context.Context, r *runtime.Sto } // stopContainer stops a container based on the container metadata. -func (c *criContainerdService) stopContainer(ctx context.Context, meta *metadata.ContainerMetadata, timeout time.Duration) error { - id := meta.ID +func (c *criContainerdService) stopContainer(ctx context.Context, container containerstore.Container, timeout time.Duration) error { + id := container.ID // Return without error if container is not running. This makes sure that // stop only takes real action after the container is started. - if meta.State() != runtime.ContainerState_CONTAINER_RUNNING { + state := container.Status.Get().State() + if state != runtime.ContainerState_CONTAINER_RUNNING { glog.V(2).Infof("Container to stop %q is not running, current state %q", - id, criContainerStateToString(meta.State())) + id, criContainerStateToString(state)) return nil } if timeout > 0 { stopSignal := unix.SIGTERM - imageMeta, err := c.imageMetadataStore.Get(meta.ImageRef) + image, err := c.imageStore.Get(container.ImageRef) if err != nil { // NOTE(random-liu): It's possible that the container is stopped, // deleted and image is garbage collected before this point. However, // the chance is really slim, even it happens, it's still fine to return // an error here. - return fmt.Errorf("failed to get image metadata %q: %v", meta.ImageRef, err) + return fmt.Errorf("failed to get image metadata %q: %v", container.ImageRef, err) } - if imageMeta.Config.StopSignal != "" { - stopSignal, err = signal.ParseSignal(imageMeta.Config.StopSignal) + if image.Config.StopSignal != "" { + stopSignal, err = signal.ParseSignal(image.Config.StopSignal) if err != nil { return fmt.Errorf("failed to parse stop signal %q: %v", - imageMeta.Config.StopSignal, err) + image.Config.StopSignal, err) } } glog.V(2).Infof("Stop container %q with signal %v", id, stopSignal) @@ -140,10 +142,10 @@ func (c *criContainerdService) waitContainerStop(ctx context.Context, id string, defer timeoutTimer.Stop() for { // Poll once before waiting for stopCheckPollInterval. - meta, err := c.containerStore.Get(id) + container, err := c.containerStore.Get(id) if err != nil { - if !metadata.IsNotExistError(err) { - return fmt.Errorf("failed to get container %q metadata: %v", id, err) + if err != store.ErrNotExist { + return fmt.Errorf("failed to get container %q: %v", id, err) } // Do not return error here because container was removed means // it is already stopped. @@ -151,7 +153,7 @@ func (c *criContainerdService) waitContainerStop(ctx context.Context, id string, return nil } // TODO(random-liu): Use channel with event handler instead of polling. - if meta.State() == runtime.ContainerState_CONTAINER_EXITED { + if container.Status.Get().State() == runtime.ContainerState_CONTAINER_EXITED { return nil } select { diff --git a/pkg/server/container_stop_test.go b/pkg/server/container_stop_test.go index 3aa6bdf97..80ee1e549 100644 --- a/pkg/server/container_stop_test.go +++ b/pkg/server/container_stop_test.go @@ -29,21 +29,21 @@ import ( "golang.org/x/sys/unix" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" servertesting "github.com/kubernetes-incubator/cri-containerd/pkg/server/testing" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" + imagestore "github.com/kubernetes-incubator/cri-containerd/pkg/store/image" ) func TestWaitContainerStop(t *testing.T) { id := "test-id" for desc, test := range map[string]struct { - metadata *metadata.ContainerMetadata + status *containerstore.Status cancel bool timeout time.Duration expectErr bool }{ "should return error if timeout exceeds": { - metadata: &metadata.ContainerMetadata{ - ID: id, + status: &containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), }, @@ -51,8 +51,7 @@ func TestWaitContainerStop(t *testing.T) { expectErr: true, }, "should return error if context is cancelled": { - metadata: &metadata.ContainerMetadata{ - ID: id, + status: &containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), }, @@ -61,13 +60,12 @@ func TestWaitContainerStop(t *testing.T) { expectErr: true, }, "should not return error if container is removed before timeout": { - metadata: nil, + status: nil, timeout: time.Hour, expectErr: false, }, "should not return error if container is stopped before timeout": { - metadata: &metadata.ContainerMetadata{ - ID: id, + status: &containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), FinishedAt: time.Now().UnixNano(), @@ -77,8 +75,13 @@ func TestWaitContainerStop(t *testing.T) { }, } { c := newTestCRIContainerdService() - if test.metadata != nil { - assert.NoError(t, c.containerStore.Create(*test.metadata)) + if test.status != nil { + container, err := containerstore.NewContainer( + containerstore.Metadata{ID: id}, + *test.status, + ) + assert.NoError(t, err) + assert.NoError(t, c.containerStore.Add(container)) } ctx := context.Background() if test.cancel { @@ -94,15 +97,14 @@ func TestWaitContainerStop(t *testing.T) { func TestStopContainer(t *testing.T) { testID := "test-id" testPid := uint32(1234) - testMetadata := metadata.ContainerMetadata{ - ID: testID, + testImageID := "test-image-id" + testStatus := containerstore.Status{ Pid: testPid, - ImageRef: "test-image-id", CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), } - testImageMetadata := metadata.ImageMetadata{ - ID: "test-image-id", + testImage := imagestore.Image{ + ID: testImageID, Config: &imagespec.ImageConfig{}, } testContainer := task.Task{ @@ -111,7 +113,7 @@ func TestStopContainer(t *testing.T) { Status: task.StatusRunning, } for desc, test := range map[string]struct { - metadata *metadata.ContainerMetadata + status *containerstore.Status containerdContainer *task.Task stopSignal string stopErr error @@ -120,20 +122,17 @@ func TestStopContainer(t *testing.T) { expectCalls []servertesting.CalledDetail }{ "should return error when container does not exist": { - metadata: nil, + status: nil, expectErr: true, expectCalls: []servertesting.CalledDetail{}, }, "should not return error when container is not running": { - metadata: &metadata.ContainerMetadata{ - ID: testID, - CreatedAt: time.Now().UnixNano(), - }, + status: &containerstore.Status{CreatedAt: time.Now().UnixNano()}, expectErr: false, expectCalls: []servertesting.CalledDetail{}, }, "should not return error if containerd task does not exist": { - metadata: &testMetadata, + status: &testStatus, containerdContainer: &testContainer, // Since it's hard to inject event during `StopContainer` is running, // we only test the case that first stop returns error, but container @@ -166,7 +165,7 @@ func TestStopContainer(t *testing.T) { }, }, "should not return error if containerd task process already finished": { - metadata: &testMetadata, + status: &testStatus, containerdContainer: &testContainer, stopErr: errors.New("os: process already finished"), expectErr: false, @@ -194,7 +193,7 @@ func TestStopContainer(t *testing.T) { }, }, "should return error if graceful stop returns random error": { - metadata: &testMetadata, + status: &testStatus, containerdContainer: &testContainer, stopErr: errors.New("random stop error"), expectErr: true, @@ -210,7 +209,7 @@ func TestStopContainer(t *testing.T) { }, }, "should not return error if containerd task is gracefully stopped": { - metadata: &testMetadata, + status: &testStatus, containerdContainer: &testContainer, expectErr: false, // deleted by the event monitor. @@ -230,7 +229,7 @@ func TestStopContainer(t *testing.T) { }, }, "should use stop signal specified in image config if not empty": { - metadata: &testMetadata, + status: &testStatus, containerdContainer: &testContainer, stopSignal: "SIGHUP", expectErr: false, @@ -251,7 +250,7 @@ func TestStopContainer(t *testing.T) { }, }, "should directly kill container if timeout is 0": { - metadata: &testMetadata, + status: &testStatus, containerdContainer: &testContainer, noTimeout: true, expectErr: false, @@ -279,16 +278,24 @@ func TestStopContainer(t *testing.T) { defer fake.Stop() c.taskService = fake - // Inject metadata. - if test.metadata != nil { - assert.NoError(t, c.containerStore.Create(*test.metadata)) + // Inject the container. + if test.status != nil { + cntr, err := containerstore.NewContainer( + containerstore.Metadata{ + ID: testID, + ImageRef: testImageID, + }, + *test.status, + ) + assert.NoError(t, err) + assert.NoError(t, c.containerStore.Add(cntr)) } // Inject containerd task. if test.containerdContainer != nil { fake.SetFakeTasks([]task.Task{*test.containerdContainer}) } - testImageMetadata.Config.StopSignal = test.stopSignal - assert.NoError(t, c.imageMetadataStore.Create(testImageMetadata)) + testImage.Config.StopSignal = test.stopSignal + c.imageStore.Add(testImage) if test.stopErr != nil { fake.InjectError("kill", test.stopErr) } diff --git a/pkg/server/events.go b/pkg/server/events.go index 1ff70c788..0d9c96914 100644 --- a/pkg/server/events.go +++ b/pkg/server/events.go @@ -25,7 +25,7 @@ import ( "github.com/jpillora/backoff" "golang.org/x/net/context" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" ) const ( @@ -87,12 +87,12 @@ func (c *criContainerdService) handleEvent(e *task.Event) { // fine to leave out that case for now. // TODO(random-liu): [P2] Handle containerd-shim exit. case task.Event_EXIT: - meta, err := c.containerStore.Get(e.ID) + cntr, err := c.containerStore.Get(e.ID) if err != nil { - glog.Errorf("Failed to get container %q metadata: %v", e.ID, err) + glog.Errorf("Failed to get container %q: %v", e.ID, err) return } - if e.Pid != meta.Pid { + if e.Pid != cntr.Status.Get().Pid { // Non-init process died, ignore the event. return } @@ -103,16 +103,16 @@ func (c *criContainerdService) handleEvent(e *task.Event) { glog.Errorf("Failed to delete container %q: %v", e.ID, err) return } - err = c.containerStore.Update(e.ID, func(meta metadata.ContainerMetadata) (metadata.ContainerMetadata, error) { + err = cntr.Status.Update(func(status containerstore.Status) (containerstore.Status, error) { // If FinishedAt has been set (e.g. with start failure), keep as // it is. - if meta.FinishedAt != 0 { - return meta, nil + if status.FinishedAt != 0 { + return status, nil } - meta.Pid = 0 - meta.FinishedAt = e.ExitedAt.UnixNano() - meta.ExitCode = int32(e.ExitStatus) - return meta, nil + status.Pid = 0 + status.FinishedAt = e.ExitedAt.UnixNano() + status.ExitCode = int32(e.ExitStatus) + return status, nil }) if err != nil { glog.Errorf("Failed to update container %q state: %v", e.ID, err) @@ -120,11 +120,15 @@ func (c *criContainerdService) handleEvent(e *task.Event) { return } case task.Event_OOM: - err := c.containerStore.Update(e.ID, func(meta metadata.ContainerMetadata) (metadata.ContainerMetadata, error) { - meta.Reason = oomExitReason - return meta, nil + cntr, err := c.containerStore.Get(e.ID) + if err != nil { + glog.Errorf("Failed to get container %q: %v", e.ID, err) + } + err = cntr.Status.Update(func(status containerstore.Status) (containerstore.Status, error) { + status.Reason = oomExitReason + return status, nil }) - if err != nil && !metadata.IsNotExistError(err) { + if err != nil { glog.Errorf("Failed to update container %q oom: %v", e.ID, err) return } diff --git a/pkg/server/events_test.go b/pkg/server/events_test.go index b3ed3a83a..336d60fc7 100644 --- a/pkg/server/events_test.go +++ b/pkg/server/events_test.go @@ -27,8 +27,8 @@ import ( "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" servertesting "github.com/kubernetes-incubator/cri-containerd/pkg/server/testing" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" ) func TestHandleEvent(t *testing.T) { @@ -36,11 +36,13 @@ func TestHandleEvent(t *testing.T) { testPid := uint32(1234) testCreatedAt := time.Now().UnixNano() testStartedAt := time.Now().UnixNano() - // Container metadata in running state. - testMetadata := metadata.ContainerMetadata{ + testMetadata := containerstore.Metadata{ ID: testID, Name: "test-name", SandboxID: "test-sandbox-id", + } + // Container status in running state. + testStatus := containerstore.Status{ Pid: testPid, CreatedAt: testCreatedAt, StartedAt: testStartedAt, @@ -53,17 +55,14 @@ func TestHandleEvent(t *testing.T) { ExitStatus: 1, ExitedAt: testExitedAt, } - testFinishedMetadata := metadata.ContainerMetadata{ - ID: testID, - Name: "test-name", - SandboxID: "test-sandbox-id", + testFinishedStatus := containerstore.Status{ Pid: 0, CreatedAt: testCreatedAt, StartedAt: testStartedAt, FinishedAt: testExitedAt.UnixNano(), ExitCode: 1, } - assert.Equal(t, runtime.ContainerState_CONTAINER_RUNNING, testMetadata.State()) + assert.Equal(t, runtime.ContainerState_CONTAINER_RUNNING, testStatus.State()) testContainerdContainer := task.Task{ ID: testID, Pid: testPid, @@ -72,12 +71,12 @@ func TestHandleEvent(t *testing.T) { for desc, test := range map[string]struct { event *task.Event - metadata *metadata.ContainerMetadata + status *containerstore.Status containerdContainer *task.Task containerdErr error - expected *metadata.ContainerMetadata + expected *containerstore.Status }{ - "should not update state when no corresponding metadata for event": { + "should not update state when no corresponding container for event": { event: &testExitEvent, expected: nil, }, @@ -89,16 +88,16 @@ func TestHandleEvent(t *testing.T) { ExitStatus: 1, ExitedAt: testExitedAt, }, - metadata: &testMetadata, + status: &testStatus, containerdContainer: &testContainerdContainer, - expected: &testMetadata, + expected: &testStatus, }, "should not update state when fail to delete containerd task": { event: &testExitEvent, - metadata: &testMetadata, + status: &testStatus, containerdContainer: &testContainerdContainer, containerdErr: fmt.Errorf("random error"), - expected: &testMetadata, + expected: &testStatus, }, "should not update state for irrelevant events": { event: &task.Event{ @@ -106,31 +105,28 @@ func TestHandleEvent(t *testing.T) { Type: task.Event_PAUSED, Pid: testPid, }, - metadata: &testMetadata, + status: &testStatus, containerdContainer: &testContainerdContainer, - expected: &testMetadata, + expected: &testStatus, }, "should update state when containerd task is already deleted": { event: &testExitEvent, - metadata: &testMetadata, - expected: &testFinishedMetadata, + status: &testStatus, + expected: &testFinishedStatus, }, "should update state when delete containerd task successfully": { event: &testExitEvent, - metadata: &testMetadata, + status: &testStatus, containerdContainer: &testContainerdContainer, - expected: &testFinishedMetadata, + expected: &testFinishedStatus, }, "should update exit reason when container is oom killed": { event: &task.Event{ ID: testID, Type: task.Event_OOM, }, - metadata: &testMetadata, - expected: &metadata.ContainerMetadata{ - ID: testID, - Name: "test-name", - SandboxID: "test-sandbox-id", + status: &testStatus, + expected: &containerstore.Status{ Pid: testPid, CreatedAt: testCreatedAt, StartedAt: testStartedAt, @@ -148,10 +144,14 @@ func TestHandleEvent(t *testing.T) { if test.event != nil { fakeEvents.Events <- test.event } - // Inject metadata. - if test.metadata != nil { - // Make sure that original data will not be changed. - assert.NoError(t, c.containerStore.Create(*test.metadata)) + // Inject internal container object. + if test.status != nil { + cntr, err := containerstore.NewContainer( // nolint: vetshadow + testMetadata, + *test.status, + ) + assert.NoError(t, err) + assert.NoError(t, c.containerStore.Add(cntr)) } // Inject containerd task. if test.containerdContainer != nil { @@ -161,8 +161,12 @@ func TestHandleEvent(t *testing.T) { if test.containerdErr != nil { fake.InjectError("delete", test.containerdErr) } - c.handleEventStream(e) - got, _ := c.containerStore.Get(testID) - assert.Equal(t, test.expected, got) + assert.NoError(t, c.handleEventStream(e)) + if test.expected == nil { + continue + } + got, err := c.containerStore.Get(testID) + assert.NoError(t, err) + assert.Equal(t, *test.expected, got.Status.Get()) } } diff --git a/pkg/server/helpers.go b/pkg/server/helpers.go index d790ce31a..f95a7b382 100644 --- a/pkg/server/helpers.go +++ b/pkg/server/helpers.go @@ -36,7 +36,8 @@ import ( "google.golang.org/grpc/codes" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + "github.com/kubernetes-incubator/cri-containerd/pkg/store" + imagestore "github.com/kubernetes-incubator/cri-containerd/pkg/store/image" ) const ( @@ -328,7 +329,7 @@ func getRepoDigestAndTag(namedRef reference.Named, digest imagedigest.Digest, sc // localResolve resolves image reference locally and returns corresponding image metadata. It returns // nil without error if the reference doesn't exist. -func (c *criContainerdService) localResolve(ctx context.Context, ref string) (*metadata.ImageMetadata, error) { +func (c *criContainerdService) localResolve(ctx context.Context, ref string) (*imagestore.Image, error) { _, err := imagedigest.Parse(ref) if err != nil { // ref is not image id, try to resolve it locally. @@ -336,7 +337,7 @@ func (c *criContainerdService) localResolve(ctx context.Context, ref string) (*m if err != nil { return nil, fmt.Errorf("invalid image reference %q: %v", ref, err) } - image, err := c.imageStoreService.Get(ctx, normalized.String()) + imageInContainerd, err := c.imageStoreService.Get(ctx, normalized.String()) if err != nil { if containerdmetadata.IsNotFound(err) { return nil, nil @@ -344,21 +345,21 @@ func (c *criContainerdService) localResolve(ctx context.Context, ref string) (*m return nil, fmt.Errorf("an error occurred when getting image %q from containerd image store: %v", normalized.String(), err) } - desc, err := image.Config(ctx, c.contentStoreService) + desc, err := imageInContainerd.Config(ctx, c.contentStoreService) if err != nil { return nil, fmt.Errorf("failed to get image config descriptor: %v", err) } ref = desc.Digest.String() } imageID := ref - meta, err := c.imageMetadataStore.Get(imageID) + image, err := c.imageStore.Get(imageID) if err != nil { - if metadata.IsNotExistError(err) { + if err == store.ErrNotExist { return nil, nil } return nil, fmt.Errorf("failed to get image %q metadata: %v", imageID, err) } - return meta, nil + return &image, nil } // getUserFromImage gets uid or user name of the image user. @@ -382,13 +383,13 @@ func getUserFromImage(user string) (*int64, string) { // ensureImageExists returns corresponding metadata of the image reference, if image is not // pulled yet, the function will pull the image. -func (c *criContainerdService) ensureImageExists(ctx context.Context, ref string) (*metadata.ImageMetadata, error) { - meta, err := c.localResolve(ctx, ref) +func (c *criContainerdService) ensureImageExists(ctx context.Context, ref string) (*imagestore.Image, error) { + image, err := c.localResolve(ctx, ref) if err != nil { return nil, fmt.Errorf("failed to resolve image %q: %v", ref, err) } - if meta != nil { - return meta, nil + if image != nil { + return image, nil } // Pull image to ensure the image exists resp, err := c.PullImage(ctx, &runtime.PullImageRequest{Image: &runtime.ImageSpec{Image: ref}}) @@ -396,10 +397,10 @@ func (c *criContainerdService) ensureImageExists(ctx context.Context, ref string return nil, fmt.Errorf("failed to pull image %q: %v", ref, err) } imageID := resp.GetImageRef() - meta, err = c.imageMetadataStore.Get(imageID) + newImage, err := c.imageStore.Get(imageID) if err != nil { // It's still possible that someone removed the image right after it is pulled. return nil, fmt.Errorf("failed to get image %q metadata after pulling: %v", imageID, err) } - return meta, nil + return &newImage, nil } diff --git a/pkg/server/image_list.go b/pkg/server/image_list.go index d1f8af59b..dc80d98ec 100644 --- a/pkg/server/image_list.go +++ b/pkg/server/image_list.go @@ -17,13 +17,11 @@ limitations under the License. package server import ( - "fmt" - "github.com/golang/glog" "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + imagestore "github.com/kubernetes-incubator/cri-containerd/pkg/store/image" ) // ListImages lists existing images. @@ -37,12 +35,10 @@ func (c *criContainerdService) ListImages(ctx context.Context, r *runtime.ListIm } }() - imageMetadataA, err := c.imageMetadataStore.List() - if err != nil { - return nil, fmt.Errorf("failed to list image metadata from store: %v", err) - } + imagesInStore := c.imageStore.List() + var images []*runtime.Image - for _, image := range imageMetadataA { + for _, image := range imagesInStore { // TODO(random-liu): [P0] Make sure corresponding snapshot exists. What if snapshot // doesn't exist? images = append(images, toCRIImage(image)) @@ -51,8 +47,8 @@ func (c *criContainerdService) ListImages(ctx context.Context, r *runtime.ListIm return &runtime.ListImagesResponse{Images: images}, nil } -// toCRIImage converts image metadata to CRI image type. -func toCRIImage(image *metadata.ImageMetadata) *runtime.Image { +// toCRIImage converts image to CRI image type. +func toCRIImage(image imagestore.Image) *runtime.Image { runtimeImage := &runtime.Image{ Id: image.ID, RepoTags: image.RepoTags, diff --git a/pkg/server/image_list_test.go b/pkg/server/image_list_test.go index 181cdd73b..e8606642c 100644 --- a/pkg/server/image_list_test.go +++ b/pkg/server/image_list_test.go @@ -25,12 +25,12 @@ import ( "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + imagestore "github.com/kubernetes-incubator/cri-containerd/pkg/store/image" ) func TestListImages(t *testing.T) { c := newTestCRIContainerdService() - imagesInStore := []metadata.ImageMetadata{ + imagesInStore := []imagestore.Image{ { ID: "test-id-1", ChainID: "test-chainid-1", @@ -87,7 +87,7 @@ func TestListImages(t *testing.T) { } for _, i := range imagesInStore { - assert.NoError(t, c.imageMetadataStore.Create(i)) + c.imageStore.Add(i) } resp, err := c.ListImages(context.Background(), &runtime.ListImagesRequest{}) diff --git a/pkg/server/image_pull.go b/pkg/server/image_pull.go index 269c09864..6eb4992df 100644 --- a/pkg/server/image_pull.go +++ b/pkg/server/image_pull.go @@ -37,7 +37,7 @@ import ( "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + imagestore "github.com/kubernetes-incubator/cri-containerd/pkg/store/image" ) // For image management: @@ -87,60 +87,41 @@ func (c *criContainerdService) PullImage(ctx context.Context, r *runtime.PullIma r.GetImage().GetImage(), retRes.GetImageRef()) } }() - image := r.GetImage().GetImage() + imageRef := r.GetImage().GetImage() // TODO(mikebrow): add truncIndex for image id - imageID, repoTag, repoDigest, err := c.pullImage(ctx, image, r.GetAuth()) + imageID, repoTag, repoDigest, err := c.pullImage(ctx, imageRef, r.GetAuth()) if err != nil { - return nil, fmt.Errorf("failed to pull image %q: %v", image, err) + return nil, fmt.Errorf("failed to pull image %q: %v", imageRef, err) } - glog.V(4).Infof("Pulled image %q with image id %q, repo tag %q, repo digest %q", image, imageID, + glog.V(4).Infof("Pulled image %q with image id %q, repo tag %q, repo digest %q", imageRef, imageID, repoTag, repoDigest) - _, err = c.imageMetadataStore.Get(imageID) - if err != nil && !metadata.IsNotExistError(err) { - return nil, fmt.Errorf("failed to get image %q metadata: %v", imageID, err) - } - // There is a known race here because the image metadata could be created after `Get`. - // TODO(random-liu): [P1] Do not use metadata store. Use simple in-memory data structure to - // maintain the id -> information index. And use the container image store as backup and - // recover in-memory state during startup. - if err == nil { - // Update existing image metadata. - if err := c.imageMetadataStore.Update(imageID, func(m metadata.ImageMetadata) (metadata.ImageMetadata, error) { - updateImageMetadata(&m, repoTag, repoDigest) - return m, nil - }); err != nil { - return nil, fmt.Errorf("failed to update image %q metadata: %v", imageID, err) - } - return &runtime.PullImageResponse{ImageRef: imageID}, err - } - // Get image information. - chainID, size, config, err := c.getImageInfo(ctx, image) + chainID, size, config, err := c.getImageInfo(ctx, imageRef) if err != nil { - return nil, fmt.Errorf("failed to get image %q information: %v", image, err) + return nil, fmt.Errorf("failed to get image %q information: %v", imageRef, err) } - - // NOTE(random-liu): the actual state in containerd is the source of truth, even we maintain - // in-memory image metadata, it's only for in-memory indexing. The image could be removed - // by someone else anytime, before/during/after we create the metadata. We should always - // check the actual state in containerd before using the image or returning status of the - // image. - - // Create corresponding image metadata. - newMeta := metadata.ImageMetadata{ + image := imagestore.Image{ ID: imageID, ChainID: chainID.String(), Size: size, Config: config, } - // Add the image reference used into repo tags. Note if the image is pulled with - // repo digest, it will also be added in to repo tags, which is fine. - updateImageMetadata(&newMeta, repoTag, repoDigest) - if err := c.imageMetadataStore.Create(newMeta); err != nil { - return nil, fmt.Errorf("failed to create image %q metadata: %v", imageID, err) + + if repoDigest != "" { + image.RepoDigests = []string{repoDigest} } + if repoTag != "" { + image.RepoTags = []string{repoTag} + } + c.imageStore.Add(image) + + // NOTE(random-liu): the actual state in containerd is the source of truth, even we maintain + // in-memory image store, it's only for in-memory indexing. The image could be removed + // by someone else anytime, before/during/after we create the metadata. We should always + // check the actual state in containerd before using the image or returning status of the + // image. return &runtime.PullImageResponse{ImageRef: imageID}, err } @@ -393,29 +374,3 @@ func (c *criContainerdService) waitForResourcesDownloading(ctx context.Context, } } } - -// insertToStringSlice is a helper function to insert a string into the string slice -// if the string is not in the slice yet. -func insertToStringSlice(ss []string, s string) []string { - found := false - for _, str := range ss { - if s == str { - found = true - break - } - } - if !found { - ss = append(ss, s) - } - return ss -} - -// updateImageMetadata updates existing image meta with new repoTag and repoDigest. -func updateImageMetadata(meta *metadata.ImageMetadata, repoTag, repoDigest string) { - if repoTag != "" { - meta.RepoTags = insertToStringSlice(meta.RepoTags, repoTag) - } - if repoDigest != "" { - meta.RepoDigests = insertToStringSlice(meta.RepoDigests, repoDigest) - } -} diff --git a/pkg/server/image_pull_test.go b/pkg/server/image_pull_test.go index 1272bbeb8..42fc6bb66 100644 --- a/pkg/server/image_pull_test.go +++ b/pkg/server/image_pull_test.go @@ -24,59 +24,8 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" ) -func TestUpdateImageMetadata(t *testing.T) { - meta := metadata.ImageMetadata{ - ID: "test-id", - ChainID: "test-chain-id", - Size: 1234, - } - for desc, test := range map[string]struct { - repoTags []string - repoDigests []string - repoTag string - repoDigest string - expectedRepoTags []string - expectedRepoDigests []string - }{ - "Add duplicated repo tag and digest": { - repoTags: []string{"a", "b"}, - repoDigests: []string{"c", "d"}, - repoTag: "a", - repoDigest: "c", - expectedRepoTags: []string{"a", "b"}, - expectedRepoDigests: []string{"c", "d"}, - }, - "Add new repo tag and digest": { - repoTags: []string{"a", "b"}, - repoDigests: []string{"c", "d"}, - repoTag: "e", - repoDigest: "f", - expectedRepoTags: []string{"a", "b", "e"}, - expectedRepoDigests: []string{"c", "d", "f"}, - }, - "Add empty repo tag and digest": { - repoTags: []string{"a", "b"}, - repoDigests: []string{"c", "d"}, - repoTag: "", - repoDigest: "", - expectedRepoTags: []string{"a", "b"}, - expectedRepoDigests: []string{"c", "d"}, - }, - } { - t.Logf("TestCase %q", desc) - m := meta - m.RepoTags = test.repoTags - m.RepoDigests = test.repoDigests - updateImageMetadata(&m, test.repoTag, test.repoDigest) - assert.Equal(t, test.expectedRepoTags, m.RepoTags) - assert.Equal(t, test.expectedRepoDigests, m.RepoDigests) - } -} - func TestResources(t *testing.T) { const threads = 10 var wg sync.WaitGroup diff --git a/pkg/server/image_remove.go b/pkg/server/image_remove.go index 39061377e..6e3e23956 100644 --- a/pkg/server/image_remove.go +++ b/pkg/server/image_remove.go @@ -19,13 +19,10 @@ package server import ( "fmt" + containerdmetadata "github.com/containerd/containerd/metadata" "github.com/golang/glog" "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - - containerdmetadata "github.com/containerd/containerd/metadata" - - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" ) // RemoveImage removes the image. @@ -41,29 +38,25 @@ func (c *criContainerdService) RemoveImage(ctx context.Context, r *runtime.Remov glog.V(2).Infof("RemoveImage %q returns successfully", r.GetImage().GetImage()) } }() - meta, err := c.localResolve(ctx, r.GetImage().GetImage()) + image, err := c.localResolve(ctx, r.GetImage().GetImage()) if err != nil { return nil, fmt.Errorf("can not resolve %q locally: %v", r.GetImage().GetImage(), err) } - if meta == nil { + if image == nil { // return empty without error when image not found. return &runtime.RemoveImageResponse{}, nil } + // Include all image references, including RepoTag, RepoDigest and id. - for _, ref := range append(append(meta.RepoTags, meta.RepoDigests...), meta.ID) { + for _, ref := range append(append(image.RepoTags, image.RepoDigests...), image.ID) { // TODO(random-liu): Containerd should schedule a garbage collection immediately, // and we may want to wait for the garbage collection to be over here. err = c.imageStoreService.Delete(ctx, ref) if err == nil || containerdmetadata.IsNotFound(err) { continue } - return nil, fmt.Errorf("failed to delete image reference %q for image %q: %v", ref, meta.ID, err) - } - if err = c.imageMetadataStore.Delete(meta.ID); err != nil { - if metadata.IsNotExistError(err) { - return &runtime.RemoveImageResponse{}, nil - } - return nil, fmt.Errorf("an error occurred when delete image %q matadata: %v", meta.ID, err) + return nil, fmt.Errorf("failed to delete image reference %q for image %q: %v", ref, image.ID, err) } + c.imageStore.Delete(image.ID) return &runtime.RemoveImageResponse{}, nil } diff --git a/pkg/server/image_status.go b/pkg/server/image_status.go index d4475b04b..f93462291 100644 --- a/pkg/server/image_status.go +++ b/pkg/server/image_status.go @@ -35,28 +35,28 @@ func (c *criContainerdService) ImageStatus(ctx context.Context, r *runtime.Image r.GetImage().GetImage(), retRes.GetImage()) } }() - meta, err := c.localResolve(ctx, r.GetImage().GetImage()) + image, err := c.localResolve(ctx, r.GetImage().GetImage()) if err != nil { return nil, fmt.Errorf("can not resolve %q locally: %v", r.GetImage().GetImage(), err) } - if meta == nil { + if image == nil { // return empty without error when image not found. return &runtime.ImageStatusResponse{}, nil } // TODO(random-liu): [P0] Make sure corresponding snapshot exists. What if snapshot // doesn't exist? runtimeImage := &runtime.Image{ - Id: meta.ID, - RepoTags: meta.RepoTags, - RepoDigests: meta.RepoDigests, - Size_: uint64(meta.Size), + Id: image.ID, + RepoTags: image.RepoTags, + RepoDigests: image.RepoDigests, + Size_: uint64(image.Size), } - uid, username := getUserFromImage(meta.Config.User) + uid, username := getUserFromImage(image.Config.User) if uid != nil { runtimeImage.Uid = &runtime.Int64Value{Value: *uid} } runtimeImage.Username = username - // TODO(mikebrow): write a ImageMetadata to runtim.Image converter + // TODO(mikebrow): write a ImageMetadata to runtime.Image converter return &runtime.ImageStatusResponse{Image: runtimeImage}, nil } diff --git a/pkg/server/image_status_test.go b/pkg/server/image_status_test.go index 059abdc46..41b45ec4b 100644 --- a/pkg/server/image_status_test.go +++ b/pkg/server/image_status_test.go @@ -25,12 +25,12 @@ import ( "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + imagestore "github.com/kubernetes-incubator/cri-containerd/pkg/store/image" ) func TestImageStatus(t *testing.T) { testID := "sha256:d848ce12891bf78792cda4a23c58984033b0c397a55e93a1556202222ecc5ed4" - meta := metadata.ImageMetadata{ + image := imagestore.Image{ ID: testID, ChainID: "test-chain-id", RepoTags: []string{"a", "b"}, @@ -57,7 +57,7 @@ func TestImageStatus(t *testing.T) { require.NotNil(t, resp) assert.Nil(t, resp.GetImage()) - assert.NoError(t, c.imageMetadataStore.Create(meta)) + c.imageStore.Add(image) t.Logf("should return correct image status for exist image") resp, err = c.ImageStatus(context.Background(), &runtime.ImageStatusRequest{ diff --git a/pkg/server/sandbox_list.go b/pkg/server/sandbox_list.go index 69583e847..fea4f8bf0 100644 --- a/pkg/server/sandbox_list.go +++ b/pkg/server/sandbox_list.go @@ -27,7 +27,7 @@ import ( "github.com/containerd/containerd/api/types/task" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + sandboxstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/sandbox" ) // ListPodSandbox returns a list of Sandbox. @@ -39,11 +39,8 @@ func (c *criContainerdService) ListPodSandbox(ctx context.Context, r *runtime.Li } }() - // List all sandbox metadata from store. - sandboxesInStore, err := c.sandboxStore.List() - if err != nil { - return nil, fmt.Errorf("failed to list metadata from sandbox store: %v", err) - } + // List all sandboxes from store. + sandboxesInStore := c.sandboxStore.List() resp, err := c.taskService.List(ctx, &execution.ListRequest{}) if err != nil { @@ -68,7 +65,7 @@ func (c *criContainerdService) ListPodSandbox(ctx context.Context, r *runtime.Li state = runtime.PodSandboxState_SANDBOX_READY } - sandboxes = append(sandboxes, toCRISandbox(sandboxInStore, state)) + sandboxes = append(sandboxes, toCRISandbox(sandboxInStore.Metadata, state)) } sandboxes = c.filterCRISandboxes(sandboxes, r.GetFilter()) @@ -76,7 +73,7 @@ func (c *criContainerdService) ListPodSandbox(ctx context.Context, r *runtime.Li } // toCRISandbox converts sandbox metadata into CRI pod sandbox. -func toCRISandbox(meta *metadata.SandboxMetadata, state runtime.PodSandboxState) *runtime.PodSandbox { +func toCRISandbox(meta sandboxstore.Metadata, state runtime.PodSandboxState) *runtime.PodSandbox { return &runtime.PodSandbox{ Id: meta.ID, Metadata: meta.Config.GetMetadata(), diff --git a/pkg/server/sandbox_list_test.go b/pkg/server/sandbox_list_test.go index e1f8cfa4c..56684f63b 100644 --- a/pkg/server/sandbox_list_test.go +++ b/pkg/server/sandbox_list_test.go @@ -27,8 +27,8 @@ import ( "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" servertesting "github.com/kubernetes-incubator/cri-containerd/pkg/server/testing" + sandboxstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/sandbox" ) func TestToCRISandbox(t *testing.T) { @@ -43,7 +43,7 @@ func TestToCRISandbox(t *testing.T) { Annotations: map[string]string{"c": "d"}, } createdAt := time.Now().UnixNano() - meta := &metadata.SandboxMetadata{ + meta := sandboxstore.Metadata{ ID: "test-id", Name: "test-name", Config: config, @@ -137,21 +137,27 @@ func TestListPodSandbox(t *testing.T) { fake := c.taskService.(*servertesting.FakeExecutionClient) - sandboxesInStore := []metadata.SandboxMetadata{ + sandboxesInStore := []sandboxstore.Sandbox{ { - ID: "1", - Name: "name-1", - Config: &runtime.PodSandboxConfig{Metadata: &runtime.PodSandboxMetadata{Name: "name-1"}}, + Metadata: sandboxstore.Metadata{ + ID: "1", + Name: "name-1", + Config: &runtime.PodSandboxConfig{Metadata: &runtime.PodSandboxMetadata{Name: "name-1"}}, + }, }, { - ID: "2", - Name: "name-2", - Config: &runtime.PodSandboxConfig{Metadata: &runtime.PodSandboxMetadata{Name: "name-2"}}, + Metadata: sandboxstore.Metadata{ + ID: "2", + Name: "name-2", + Config: &runtime.PodSandboxConfig{Metadata: &runtime.PodSandboxMetadata{Name: "name-2"}}, + }, }, { - ID: "3", - Name: "name-3", - Config: &runtime.PodSandboxConfig{Metadata: &runtime.PodSandboxMetadata{Name: "name-3"}}, + Metadata: sandboxstore.Metadata{ + ID: "3", + Name: "name-3", + Config: &runtime.PodSandboxConfig{Metadata: &runtime.PodSandboxMetadata{Name: "name-3"}}, + }, }, } sandboxesInContainerd := []task.Task{ @@ -194,7 +200,7 @@ func TestListPodSandbox(t *testing.T) { // Inject test metadata for _, s := range sandboxesInStore { - c.sandboxStore.Create(s) + assert.NoError(t, c.sandboxStore.Add(s)) } // Inject fake containerd tasks diff --git a/pkg/server/sandbox_remove.go b/pkg/server/sandbox_remove.go index 8493e1c7c..c064ddce8 100644 --- a/pkg/server/sandbox_remove.go +++ b/pkg/server/sandbox_remove.go @@ -26,7 +26,7 @@ import ( "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + "github.com/kubernetes-incubator/cri-containerd/pkg/store" ) // RemovePodSandbox removes the sandbox. If there are running containers in the @@ -41,7 +41,7 @@ func (c *criContainerdService) RemovePodSandbox(ctx context.Context, r *runtime. sandbox, err := c.sandboxStore.Get(r.GetPodSandboxId()) if err != nil { - if !metadata.IsNotExistError(err) { + if err != store.ErrNotExist { return nil, fmt.Errorf("an error occurred when try to find sandbox %q: %v", r.GetPodSandboxId(), err) } @@ -76,10 +76,7 @@ func (c *criContainerdService) RemovePodSandbox(ctx context.Context, r *runtime. // not rely on this behavior. // TODO(random-liu): Introduce an intermediate state to avoid container creation after // this point. - cntrs, err := c.containerStore.List() - if err != nil { - return nil, fmt.Errorf("failed to list all containers: %v", err) - } + cntrs := c.containerStore.List() for _, cntr := range cntrs { if cntr.SandboxID != id { continue @@ -107,15 +104,12 @@ func (c *criContainerdService) RemovePodSandbox(ctx context.Context, r *runtime. glog.V(5).Infof("Remove called for sandbox container %q that does not exist", id, err) } - // Remove sandbox metadata from metadata store. Note that once the sandbox - // metadata is successfully deleted: + // Remove sandbox from sandbox store. Note that once the sandbox is successfully + // deleted: // 1) ListPodSandbox will not include this sandbox. // 2) PodSandboxStatus and StopPodSandbox will return error. - // 3) On-going operations which have held the metadata reference will not be - // affected. - if err := c.sandboxStore.Delete(id); err != nil { - return nil, fmt.Errorf("failed to delete sandbox metadata for %q: %v", id, err) - } + // 3) On-going operations which have held the reference will not be affected. + c.sandboxStore.Delete(id) // Release the sandbox name reserved for the sandbox. c.sandboxNameIndex.ReleaseByKey(id) diff --git a/pkg/server/sandbox_remove_test.go b/pkg/server/sandbox_remove_test.go index 71d37690e..17baef318 100644 --- a/pkg/server/sandbox_remove_test.go +++ b/pkg/server/sandbox_remove_test.go @@ -27,21 +27,25 @@ import ( "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" ostesting "github.com/kubernetes-incubator/cri-containerd/pkg/os/testing" servertesting "github.com/kubernetes-incubator/cri-containerd/pkg/server/testing" + "github.com/kubernetes-incubator/cri-containerd/pkg/store" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" + sandboxstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/sandbox" ) func TestRemovePodSandbox(t *testing.T) { testID := "test-id" testName := "test-name" - testMetadata := metadata.SandboxMetadata{ - ID: testID, - Name: testName, + testSandbox := sandboxstore.Sandbox{ + Metadata: sandboxstore.Metadata{ + ID: testID, + Name: testName, + }, } for desc, test := range map[string]struct { sandboxTasks []task.Task - injectMetadata bool + injectSandbox bool removeSnapshotErr error deleteContainerErr error taskInfoErr error @@ -51,60 +55,60 @@ func TestRemovePodSandbox(t *testing.T) { expectCalls []string }{ "should not return error if sandbox does not exist": { - injectMetadata: false, + injectSandbox: false, removeSnapshotErr: servertesting.SnapshotNotExistError, deleteContainerErr: servertesting.ContainerNotExistError, expectErr: false, expectCalls: []string{}, }, "should not return error if snapshot does not exist": { - injectMetadata: true, + injectSandbox: true, removeSnapshotErr: servertesting.SnapshotNotExistError, expectRemoved: getSandboxRootDir(testRootDir, testID), expectCalls: []string{"info"}, }, "should return error if remove snapshot fails": { - injectMetadata: true, + injectSandbox: true, removeSnapshotErr: fmt.Errorf("arbitrary error"), expectErr: true, expectCalls: []string{"info"}, }, "should return error when sandbox container task is not deleted": { - injectMetadata: true, - sandboxTasks: []task.Task{{ID: testID}}, - expectErr: true, - expectCalls: []string{"info"}, + injectSandbox: true, + sandboxTasks: []task.Task{{ID: testID}}, + expectErr: true, + expectCalls: []string{"info"}, }, "should return error when arbitrary containerd error is injected": { - injectMetadata: true, - taskInfoErr: fmt.Errorf("arbitrary error"), - expectErr: true, - expectCalls: []string{"info"}, + injectSandbox: true, + taskInfoErr: fmt.Errorf("arbitrary error"), + expectErr: true, + expectCalls: []string{"info"}, }, "should return error when error fs error is injected": { - injectMetadata: true, - injectFSErr: fmt.Errorf("fs error"), - expectRemoved: getSandboxRootDir(testRootDir, testID), - expectErr: true, - expectCalls: []string{"info"}, + injectSandbox: true, + injectFSErr: fmt.Errorf("fs error"), + expectRemoved: getSandboxRootDir(testRootDir, testID), + expectErr: true, + expectCalls: []string{"info"}, }, "should not return error if sandbox container does not exist": { - injectMetadata: true, + injectSandbox: true, deleteContainerErr: servertesting.ContainerNotExistError, expectRemoved: getSandboxRootDir(testRootDir, testID), expectCalls: []string{"info"}, }, "should return error if delete sandbox container fails": { - injectMetadata: true, + injectSandbox: true, deleteContainerErr: fmt.Errorf("arbitrary error"), expectRemoved: getSandboxRootDir(testRootDir, testID), expectErr: true, expectCalls: []string{"info"}, }, "should be able to successfully delete": { - injectMetadata: true, - expectRemoved: getSandboxRootDir(testRootDir, testID), - expectCalls: []string{"info"}, + injectSandbox: true, + expectRemoved: getSandboxRootDir(testRootDir, testID), + expectCalls: []string{"info"}, }, } { t.Logf("TestCase %q", desc) @@ -114,9 +118,9 @@ func TestRemovePodSandbox(t *testing.T) { fakeExecutionClient := c.taskService.(*servertesting.FakeExecutionClient) fakeSnapshotClient := WithFakeSnapshotClient(c) fakeExecutionClient.SetFakeTasks(test.sandboxTasks) - if test.injectMetadata { + if test.injectSandbox { c.sandboxNameIndex.Reserve(testName, testID) - c.sandboxStore.Create(testMetadata) + assert.NoError(t, c.sandboxStore.Add(testSandbox)) } if test.removeSnapshotErr == nil { fakeSnapshotClient.SetFakeMounts(testID, []*mount.Mount{ @@ -157,10 +161,8 @@ func TestRemovePodSandbox(t *testing.T) { assert.NotNil(t, res) assert.NoError(t, c.sandboxNameIndex.Reserve(testName, testID), "sandbox name should be released") - meta, err := c.sandboxStore.Get(testID) - assert.Error(t, err) - assert.True(t, metadata.IsNotExistError(err)) - assert.Nil(t, meta, "sandbox metadata should be removed") + _, err = c.sandboxStore.Get(testID) + assert.Equal(t, store.ErrNotExist, err, "sandbox should be removed") mountsResp, err := fakeSnapshotClient.Mounts(context.Background(), &snapshotapi.MountsRequest{Key: testID}) assert.Equal(t, servertesting.SnapshotNotExistError, err, "snapshot should be removed") assert.Nil(t, mountsResp) @@ -178,38 +180,49 @@ func TestRemovePodSandbox(t *testing.T) { func TestRemoveContainersInSandbox(t *testing.T) { testID := "test-id" testName := "test-name" - testMetadata := metadata.SandboxMetadata{ - ID: testID, - Name: testName, + testSandbox := sandboxstore.Sandbox{ + Metadata: sandboxstore.Metadata{ + ID: testID, + Name: testName, + }, } - testContainersMetadata := []*metadata.ContainerMetadata{ + testContainers := []containerForTest{ { - ID: "test-cid-1", - Name: "test-cname-1", - SandboxID: testID, - FinishedAt: time.Now().UnixNano(), + metadata: containerstore.Metadata{ + ID: "test-cid-1", + Name: "test-cname-1", + SandboxID: testID, + }, + status: containerstore.Status{FinishedAt: time.Now().UnixNano()}, }, { - ID: "test-cid-2", - Name: "test-cname-2", - SandboxID: testID, - FinishedAt: time.Now().UnixNano(), + metadata: containerstore.Metadata{ + + ID: "test-cid-2", + Name: "test-cname-2", + SandboxID: testID, + }, + status: containerstore.Status{FinishedAt: time.Now().UnixNano()}, }, { - ID: "test-cid-3", - Name: "test-cname-3", - SandboxID: "other-sandbox-id", - FinishedAt: time.Now().UnixNano(), + metadata: containerstore.Metadata{ + ID: "test-cid-3", + Name: "test-cname-3", + SandboxID: "other-sandbox-id", + }, + status: containerstore.Status{FinishedAt: time.Now().UnixNano()}, }, } c := newTestCRIContainerdService() WithFakeSnapshotClient(c) assert.NoError(t, c.sandboxNameIndex.Reserve(testName, testID)) - assert.NoError(t, c.sandboxStore.Create(testMetadata)) - for _, cntr := range testContainersMetadata { - assert.NoError(t, c.containerNameIndex.Reserve(cntr.Name, cntr.ID)) - assert.NoError(t, c.containerStore.Create(*cntr)) + assert.NoError(t, c.sandboxStore.Add(testSandbox)) + for _, tc := range testContainers { + assert.NoError(t, c.containerNameIndex.Reserve(tc.metadata.Name, tc.metadata.ID)) + cntr, err := tc.toContainer() + assert.NoError(t, err) + assert.NoError(t, c.containerStore.Add(cntr)) } res, err := c.RemovePodSandbox(context.Background(), &runtime.RemovePodSandboxRequest{ @@ -218,12 +231,11 @@ func TestRemoveContainersInSandbox(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, res) - meta, err := c.sandboxStore.Get(testID) - assert.Error(t, err) - assert.True(t, metadata.IsNotExistError(err)) - assert.Nil(t, meta, "sandbox metadata should be removed") + _, err = c.sandboxStore.Get(testID) + assert.Equal(t, store.ErrNotExist, err, "sandbox metadata should be removed") - cntrs, err := c.containerStore.List() - assert.NoError(t, err) - assert.Equal(t, testContainersMetadata[2:], cntrs, "container metadata should be removed") + cntrs := c.containerStore.List() + assert.Len(t, cntrs, 1) + assert.Equal(t, testContainers[2].metadata, cntrs[0].Metadata, "container should be removed") + assert.Equal(t, testContainers[2].status, cntrs[0].Status.Get(), "container should be removed") } diff --git a/pkg/server/sandbox_run.go b/pkg/server/sandbox_run.go index af1759b81..adb3b57e9 100644 --- a/pkg/server/sandbox_run.go +++ b/pkg/server/sandbox_run.go @@ -35,7 +35,7 @@ import ( "golang.org/x/sys/unix" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + sandboxstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/sandbox" ) // RunPodSandbox creates and starts a pod-level sandbox. Runtimes should ensure @@ -65,22 +65,23 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run } }() - // Create initial sandbox metadata. - meta := metadata.SandboxMetadata{ - ID: id, - Name: name, - Config: config, + // Create initial internal sandbox object. + sandbox := sandboxstore.Sandbox{ + Metadata: sandboxstore.Metadata{ + ID: id, + Name: name, + Config: config, + }, } // Ensure sandbox container image snapshot. - imageMeta, err := c.ensureImageExists(ctx, c.sandboxImage) + image, err := c.ensureImageExists(ctx, c.sandboxImage) if err != nil { return nil, fmt.Errorf("failed to get sandbox image %q: %v", defaultSandboxImage, err) } - - rootfsMounts, err := c.snapshotService.View(ctx, id, imageMeta.ChainID) + rootfsMounts, err := c.snapshotService.View(ctx, id, image.ChainID) if err != nil { - return nil, fmt.Errorf("failed to prepare sandbox rootfs %q: %v", imageMeta.ChainID, err) + return nil, fmt.Errorf("failed to prepare sandbox rootfs %q: %v", image.ChainID, err) } defer func() { if retErr != nil { @@ -99,7 +100,7 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run } // Create sandbox container. - spec, err := c.generateSandboxContainerSpec(id, config, imageMeta.Config) + spec, err := c.generateSandboxContainerSpec(id, config, image.Config) if err != nil { return nil, fmt.Errorf("failed to generate sandbox container spec: %v", err) } @@ -112,7 +113,7 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run Container: containers.Container{ ID: id, // TODO(random-liu): Checkpoint metadata into container labels. - Image: imageMeta.ID, + Image: image.ID, Runtime: defaultRuntime, Spec: &prototypes.Any{ TypeUrl: runtimespec.Version, @@ -205,19 +206,19 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run } }() - meta.Pid = createResp.Pid - meta.NetNS = getNetworkNamespace(createResp.Pid) + sandbox.Pid = createResp.Pid + sandbox.NetNS = getNetworkNamespace(createResp.Pid) if !config.GetLinux().GetSecurityContext().GetNamespaceOptions().GetHostNetwork() { // Setup network for sandbox. // TODO(random-liu): [P2] Replace with permanent network namespace. podName := config.GetMetadata().GetName() - if err = c.netPlugin.SetUpPod(meta.NetNS, config.GetMetadata().GetNamespace(), podName, id); err != nil { + if err = c.netPlugin.SetUpPod(sandbox.NetNS, config.GetMetadata().GetNamespace(), podName, id); err != nil { return nil, fmt.Errorf("failed to setup network for sandbox %q: %v", id, err) } defer func() { if retErr != nil { // Teardown network if an error is returned. - if err := c.netPlugin.TearDownPod(meta.NetNS, config.GetMetadata().GetNamespace(), podName, id); err != nil { + if err := c.netPlugin.TearDownPod(sandbox.NetNS, config.GetMetadata().GetNamespace(), podName, id); err != nil { glog.Errorf("failed to destroy network for sandbox %q: %v", id, err) } } @@ -231,10 +232,9 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run } // Add sandbox into sandbox store. - meta.CreatedAt = time.Now().UnixNano() - if err := c.sandboxStore.Create(meta); err != nil { - return nil, fmt.Errorf("failed to add sandbox metadata %+v into store: %v", - meta, err) + sandbox.CreatedAt = time.Now().UnixNano() + if err := c.sandboxStore.Add(sandbox); err != nil { + return nil, fmt.Errorf("failed to add sandbox %+v into store: %v", sandbox, err) } return &runtime.RunPodSandboxResponse{PodSandboxId: id}, nil diff --git a/pkg/server/sandbox_run_test.go b/pkg/server/sandbox_run_test.go index 8417ff55d..675a9d8f0 100644 --- a/pkg/server/sandbox_run_test.go +++ b/pkg/server/sandbox_run_test.go @@ -33,9 +33,9 @@ import ( "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" ostesting "github.com/kubernetes-incubator/cri-containerd/pkg/os/testing" servertesting "github.com/kubernetes-incubator/cri-containerd/pkg/server/testing" + imagestore "github.com/kubernetes-incubator/cri-containerd/pkg/store/image" ) func getRunPodSandboxTestData() (*runtime.PodSandboxConfig, *imagespec.ImageConfig, func(*testing.T, string, *runtimespec.Spec)) { @@ -290,13 +290,13 @@ func TestRunPodSandbox(t *testing.T) { return nopReadWriteCloser{}, nil } testChainID := "test-sandbox-chain-id" - imageMetadata := metadata.ImageMetadata{ + image := imagestore.Image{ ID: testSandboxImage, ChainID: testChainID, Config: imageConfig, } - // Insert sandbox image metadata. - assert.NoError(t, c.imageMetadataStore.Create(imageMetadata)) + // Insert sandbox image. + c.imageStore.Add(image) expectContainersClientCalls := []string{"create"} expectSnapshotClientCalls := []string{"view"} expectExecutionClientCalls := []string{"create", "start"} @@ -349,25 +349,25 @@ func TestRunPodSandbox(t *testing.T) { startID := calls[1].Argument.(*execution.StartRequest).ContainerID assert.Equal(t, id, startID, "start id should be correct") - meta, err := c.sandboxStore.Get(id) + sandbox, err := c.sandboxStore.Get(id) assert.NoError(t, err) - assert.Equal(t, id, meta.ID, "metadata id should be correct") - err = c.sandboxNameIndex.Reserve(meta.Name, "random-id") - assert.Error(t, err, "metadata name should be reserved") - assert.Equal(t, config, meta.Config, "metadata config should be correct") + assert.Equal(t, id, sandbox.ID, "sandbox id should be correct") + err = c.sandboxNameIndex.Reserve(sandbox.Name, "random-id") + assert.Error(t, err, "sandbox name should be reserved") + assert.Equal(t, config, sandbox.Config, "sandbox config should be correct") // TODO(random-liu): [P2] Add clock interface and use fake clock. - assert.NotZero(t, meta.CreatedAt, "metadata CreatedAt should be set") + assert.NotZero(t, sandbox.CreatedAt, "sandbox CreatedAt should be set") info, err := fakeExecutionClient.Info(context.Background(), &execution.InfoRequest{ContainerID: id}) assert.NoError(t, err) pid := info.Task.Pid - assert.Equal(t, meta.NetNS, getNetworkNamespace(pid), "metadata network namespace should be correct") + assert.Equal(t, sandbox.NetNS, getNetworkNamespace(pid), "sandbox network namespace should be correct") expectedCNICalls := []string{"SetUpPod"} assert.Equal(t, expectedCNICalls, fakeCNIPlugin.GetCalledNames(), "expect SetUpPod should be called") calls = fakeCNIPlugin.GetCalledDetails() pluginArgument := calls[0].Argument.(servertesting.CNIPluginArgument) expectedPluginArgument := servertesting.CNIPluginArgument{ - NetnsPath: meta.NetNS, + NetnsPath: sandbox.NetNS, Namespace: config.GetMetadata().GetNamespace(), Name: config.GetMetadata().GetName(), ContainerID: id, diff --git a/pkg/server/sandbox_status.go b/pkg/server/sandbox_status.go index 74a1ea743..b9364629d 100644 --- a/pkg/server/sandbox_status.go +++ b/pkg/server/sandbox_status.go @@ -27,7 +27,7 @@ import ( "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" + sandboxstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/sandbox" ) // PodSandboxStatus returns the status of the PodSandbox. @@ -66,11 +66,11 @@ func (c *criContainerdService) PodSandboxStatus(ctx context.Context, r *runtime. glog.V(4).Infof("GetContainerNetworkStatus returns error: %v", err) } - return &runtime.PodSandboxStatusResponse{Status: toCRISandboxStatus(sandbox, state, ip)}, nil + return &runtime.PodSandboxStatusResponse{Status: toCRISandboxStatus(sandbox.Metadata, state, ip)}, nil } // toCRISandboxStatus converts sandbox metadata into CRI pod sandbox status. -func toCRISandboxStatus(meta *metadata.SandboxMetadata, state runtime.PodSandboxState, ip string) *runtime.PodSandboxStatus { +func toCRISandboxStatus(meta sandboxstore.Metadata, state runtime.PodSandboxState, ip string) *runtime.PodSandboxStatus { nsOpts := meta.Config.GetLinux().GetSecurityContext().GetNamespaceOptions() return &runtime.PodSandboxStatus{ Id: meta.ID, diff --git a/pkg/server/sandbox_status_test.go b/pkg/server/sandbox_status_test.go index 724eca828..de79d89e8 100644 --- a/pkg/server/sandbox_status_test.go +++ b/pkg/server/sandbox_status_test.go @@ -21,16 +21,14 @@ import ( "testing" "time" + "github.com/containerd/containerd/api/types/task" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/context" - - "github.com/containerd/containerd/api/types/task" - "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" servertesting "github.com/kubernetes-incubator/cri-containerd/pkg/server/testing" + sandboxstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/sandbox" ) // Variables used in the following test. @@ -41,7 +39,7 @@ const ( sandboxStatusTestNetNS = "test-netns" ) -func getSandboxStatusTestData() (*metadata.SandboxMetadata, *runtime.PodSandboxStatus) { +func getSandboxStatusTestData() (*sandboxstore.Sandbox, *runtime.PodSandboxStatus) { config := &runtime.PodSandboxConfig{ Metadata: &runtime.PodSandboxMetadata{ Name: "test-name", @@ -64,12 +62,14 @@ func getSandboxStatusTestData() (*metadata.SandboxMetadata, *runtime.PodSandboxS createdAt := time.Now().UnixNano() - metadata := &metadata.SandboxMetadata{ - ID: sandboxStatusTestID, - Name: "test-name", - Config: config, - CreatedAt: createdAt, - NetNS: sandboxStatusTestNetNS, + sandbox := &sandboxstore.Sandbox{ + Metadata: sandboxstore.Metadata{ + ID: sandboxStatusTestID, + Name: "test-name", + Config: config, + CreatedAt: createdAt, + NetNS: sandboxStatusTestNetNS, + }, } expectedStatus := &runtime.PodSandboxStatus{ @@ -90,13 +90,13 @@ func getSandboxStatusTestData() (*metadata.SandboxMetadata, *runtime.PodSandboxS Annotations: config.GetAnnotations(), } - return metadata, expectedStatus + return sandbox, expectedStatus } func TestPodSandboxStatus(t *testing.T) { for desc, test := range map[string]struct { sandboxTasks []task.Task - injectMetadata bool + injectSandbox bool injectErr error injectIP bool injectCNIErr error @@ -106,7 +106,7 @@ func TestPodSandboxStatus(t *testing.T) { expectedCNICalls []string }{ "sandbox status without metadata": { - injectMetadata: false, + injectSandbox: false, expectErr: true, expectCalls: []string{}, expectedCNICalls: []string{}, @@ -117,7 +117,7 @@ func TestPodSandboxStatus(t *testing.T) { Pid: 1, Status: task.StatusRunning, }}, - injectMetadata: true, + injectSandbox: true, expectState: runtime.PodSandboxState_SANDBOX_READY, expectCalls: []string{"info"}, expectedCNICalls: []string{"GetContainerNetworkStatus"}, @@ -128,14 +128,14 @@ func TestPodSandboxStatus(t *testing.T) { Pid: 1, Status: task.StatusStopped, }}, - injectMetadata: true, + injectSandbox: true, expectState: runtime.PodSandboxState_SANDBOX_NOTREADY, expectCalls: []string{"info"}, expectedCNICalls: []string{"GetContainerNetworkStatus"}, }, "sandbox status with non-existing sandbox container": { sandboxTasks: []task.Task{}, - injectMetadata: true, + injectSandbox: true, expectState: runtime.PodSandboxState_SANDBOX_NOTREADY, expectCalls: []string{"info"}, expectedCNICalls: []string{"GetContainerNetworkStatus"}, @@ -146,7 +146,7 @@ func TestPodSandboxStatus(t *testing.T) { Pid: 1, Status: task.StatusRunning, }}, - injectMetadata: true, + injectSandbox: true, expectState: runtime.PodSandboxState_SANDBOX_READY, injectErr: errors.New("arbitrary error"), expectErr: true, @@ -159,7 +159,7 @@ func TestPodSandboxStatus(t *testing.T) { Pid: 1, Status: task.StatusRunning, }}, - injectMetadata: true, + injectSandbox: true, expectState: runtime.PodSandboxState_SANDBOX_READY, expectCalls: []string{"info"}, injectIP: true, @@ -171,7 +171,7 @@ func TestPodSandboxStatus(t *testing.T) { Pid: 1, Status: task.StatusRunning, }}, - injectMetadata: true, + injectSandbox: true, expectState: runtime.PodSandboxState_SANDBOX_READY, expectCalls: []string{"info"}, expectedCNICalls: []string{"GetContainerNetworkStatus"}, @@ -179,14 +179,14 @@ func TestPodSandboxStatus(t *testing.T) { }, } { t.Logf("TestCase %q", desc) - metadata, expect := getSandboxStatusTestData() + sandbox, expect := getSandboxStatusTestData() expect.Network.Ip = "" c := newTestCRIContainerdService() fake := c.taskService.(*servertesting.FakeExecutionClient) fakeCNIPlugin := c.netPlugin.(*servertesting.FakeCNIPlugin) fake.SetFakeTasks(test.sandboxTasks) - if test.injectMetadata { - assert.NoError(t, c.sandboxStore.Create(*metadata)) + if test.injectSandbox { + assert.NoError(t, c.sandboxStore.Add(*sandbox)) } if test.injectErr != nil { fake.InjectError("info", test.injectErr) @@ -195,8 +195,8 @@ func TestPodSandboxStatus(t *testing.T) { fakeCNIPlugin.InjectError("GetContainerNetworkStatus", test.injectCNIErr) } if test.injectIP { - fakeCNIPlugin.SetFakePodNetwork(metadata.NetNS, metadata.Config.GetMetadata().GetNamespace(), - metadata.Config.GetMetadata().GetName(), sandboxStatusTestID, sandboxStatusTestIP) + fakeCNIPlugin.SetFakePodNetwork(sandbox.NetNS, sandbox.Config.GetMetadata().GetNamespace(), + sandbox.Config.GetMetadata().GetName(), sandboxStatusTestID, sandboxStatusTestIP) expect.Network.Ip = sandboxStatusTestIP } res, err := c.PodSandboxStatus(context.Background(), &runtime.PodSandboxStatusRequest{ diff --git a/pkg/server/sandbox_stop.go b/pkg/server/sandbox_stop.go index ca8062f10..4b9157dcd 100644 --- a/pkg/server/sandbox_stop.go +++ b/pkg/server/sandbox_stop.go @@ -50,10 +50,7 @@ func (c *criContainerdService) StopPodSandbox(ctx context.Context, r *runtime.St // and container may still be so production should not rely on this behavior. // TODO(random-liu): Delete the sandbox container before this after permanent network namespace // is introduced, so that no container will be started after that. - containers, err := c.containerStore.List() - if err != nil { - return nil, fmt.Errorf("failed to list all containers: %v", err) - } + containers := c.containerStore.List() for _, container := range containers { if container.SandboxID != id { continue diff --git a/pkg/server/sandbox_stop_test.go b/pkg/server/sandbox_stop_test.go index 21ebc73ee..ac314ee1c 100644 --- a/pkg/server/sandbox_stop_test.go +++ b/pkg/server/sandbox_stop_test.go @@ -30,23 +30,26 @@ import ( "google.golang.org/grpc/codes" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" ostesting "github.com/kubernetes-incubator/cri-containerd/pkg/os/testing" servertesting "github.com/kubernetes-incubator/cri-containerd/pkg/server/testing" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" + sandboxstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/sandbox" ) func TestStopPodSandbox(t *testing.T) { testID := "test-id" - testSandbox := metadata.SandboxMetadata{ - ID: testID, - Name: "test-name", - Config: &runtime.PodSandboxConfig{ - Metadata: &runtime.PodSandboxMetadata{ - Name: "test-name", - Uid: "test-uid", - Namespace: "test-ns", - }}, - NetNS: "test-netns", + testSandbox := sandboxstore.Sandbox{ + Metadata: sandboxstore.Metadata{ + ID: testID, + Name: "test-name", + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: "test-name", + Uid: "test-uid", + Namespace: "test-ns", + }}, + NetNS: "test-netns", + }, } testContainer := task.Task{ ID: testID, @@ -136,7 +139,7 @@ func TestStopPodSandbox(t *testing.T) { fake.SetFakeTasks(test.sandboxTasks) if test.injectSandbox { - assert.NoError(t, c.sandboxStore.Create(testSandbox)) + assert.NoError(t, c.sandboxStore.Add(testSandbox)) } if test.injectErr != nil { fake.InjectError("delete", test.injectErr) @@ -170,38 +173,53 @@ func TestStopPodSandbox(t *testing.T) { func TestStopContainersInSandbox(t *testing.T) { testID := "test-id" - testSandbox := metadata.SandboxMetadata{ - ID: testID, - Name: "test-name", - Config: &runtime.PodSandboxConfig{ - Metadata: &runtime.PodSandboxMetadata{ - Name: "test-name", - Uid: "test-uid", - Namespace: "test-ns", - }}, - NetNS: "test-netns", + testSandbox := sandboxstore.Sandbox{ + Metadata: sandboxstore.Metadata{ + ID: testID, + Name: "test-name", + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: "test-name", + Uid: "test-uid", + Namespace: "test-ns", + }}, + NetNS: "test-netns", + }, } - testContainers := []metadata.ContainerMetadata{ + testContainers := []containerForTest{ { - ID: "test-cid-1", - Name: "test-cname-1", - SandboxID: testID, - Pid: 2, - StartedAt: time.Now().UnixNano(), + metadata: containerstore.Metadata{ + ID: "test-cid-1", + Name: "test-cname-1", + SandboxID: testID, + }, + status: containerstore.Status{ + Pid: 2, + StartedAt: time.Now().UnixNano(), + }, }, { - ID: "test-cid-2", - Name: "test-cname-2", - SandboxID: testID, - Pid: 3, - StartedAt: time.Now().UnixNano(), + + metadata: containerstore.Metadata{ + ID: "test-cid-2", + Name: "test-cname-2", + SandboxID: testID, + }, + status: containerstore.Status{ + Pid: 3, + StartedAt: time.Now().UnixNano(), + }, }, { - ID: "test-cid-3", - Name: "test-cname-3", - SandboxID: "other-sandbox-id", - Pid: 4, - StartedAt: time.Now().UnixNano(), + metadata: containerstore.Metadata{ + ID: "test-cid-3", + Name: "test-cname-3", + SandboxID: "other-sandbox-id", + }, + status: containerstore.Status{ + Pid: 4, + StartedAt: time.Now().UnixNano(), + }, }, } testContainerdContainers := []task.Task{ @@ -232,9 +250,11 @@ func TestStopContainersInSandbox(t *testing.T) { defer fake.Stop() c.taskService = fake fake.SetFakeTasks(testContainerdContainers) - assert.NoError(t, c.sandboxStore.Create(testSandbox)) - for _, cntr := range testContainers { - assert.NoError(t, c.containerStore.Create(cntr)) + c.sandboxStore.Add(testSandbox) + for _, tc := range testContainers { + cntr, err := tc.toContainer() + assert.NoError(t, err) + assert.NoError(t, c.containerStore.Add(cntr)) } fakeCNIPlugin := c.netPlugin.(*servertesting.FakeCNIPlugin) @@ -257,8 +277,7 @@ func TestStopContainersInSandbox(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, res) - cntrs, err := c.containerStore.List() - assert.NoError(t, err) + cntrs := c.containerStore.List() assert.Len(t, cntrs, 3) expectedStates := map[string]runtime.ContainerState{ "test-cid-1": runtime.ContainerState_CONTAINER_EXITED, @@ -268,6 +287,6 @@ func TestStopContainersInSandbox(t *testing.T) { for id, expected := range expectedStates { cntr, err := c.containerStore.Get(id) assert.NoError(t, err) - assert.Equal(t, expected, cntr.State()) + assert.Equal(t, expected, cntr.Status.Get().State()) } } diff --git a/pkg/server/service.go b/pkg/server/service.go index 5b92ced0e..eb189bf3c 100644 --- a/pkg/server/service.go +++ b/pkg/server/service.go @@ -31,11 +31,12 @@ import ( healthapi "google.golang.org/grpc/health/grpc_health_v1" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata/store" osinterface "github.com/kubernetes-incubator/cri-containerd/pkg/os" "github.com/kubernetes-incubator/cri-containerd/pkg/registrar" "github.com/kubernetes-incubator/cri-containerd/pkg/server/agents" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" + imagestore "github.com/kubernetes-incubator/cri-containerd/pkg/store/image" + sandboxstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/sandbox" ) // k8sContainerdNamespace is the namespace we use to connect containerd. @@ -57,19 +58,19 @@ type criContainerdService struct { // sandboxImage is the image to use for sandbox container. // TODO(random-liu): Make this configurable via flag. sandboxImage string - // sandboxStore stores all sandbox metadata. - sandboxStore metadata.SandboxStore - // imageMetadataStore stores all image metadata. - imageMetadataStore metadata.ImageMetadataStore + // sandboxStore stores all resources associated with sandboxes. + sandboxStore *sandboxstore.Store // sandboxNameIndex stores all sandbox names and make sure each name // is unique. sandboxNameIndex *registrar.Registrar - // containerStore stores all container metadata. - containerStore metadata.ContainerStore + // containerStore stores all resources associated with containers. + containerStore *containerstore.Store // containerNameIndex stores all container names and make sure each // name is unique. containerNameIndex *registrar.Registrar - // containerService is containerd tasks client. + // imageStore stores all resources associated with images. + imageStore *imagestore.Store + // containerService is containerd containers client. containerService containers.ContainersClient // taskService is containerd tasks client. taskService execution.TasksClient @@ -96,7 +97,7 @@ type criContainerdService struct { // NewCRIContainerdService returns a new instance of CRIContainerdService func NewCRIContainerdService(containerdEndpoint, rootDir, networkPluginBinDir, networkPluginConfDir string) (CRIContainerdService, error) { - // TODO(random-liu): [P2] Recover from runtime state and metadata store. + // TODO(random-liu): [P2] Recover from runtime state and checkpoint. client, err := containerd.New(containerdEndpoint, containerd.WithDefaultNamespace(k8sContainerdNamespace)) if err != nil { @@ -107,9 +108,9 @@ func NewCRIContainerdService(containerdEndpoint, rootDir, networkPluginBinDir, n os: osinterface.RealOS{}, rootDir: rootDir, sandboxImage: defaultSandboxImage, - sandboxStore: metadata.NewSandboxStore(store.NewMetadataStore()), - containerStore: metadata.NewContainerStore(store.NewMetadataStore()), - imageMetadataStore: metadata.NewImageMetadataStore(store.NewMetadataStore()), + sandboxStore: sandboxstore.NewStore(), + containerStore: containerstore.NewStore(), + imageStore: imagestore.NewStore(), sandboxNameIndex: registrar.NewRegistrar(), containerNameIndex: registrar.NewRegistrar(), containerService: client.ContainerService(), diff --git a/pkg/server/service_test.go b/pkg/server/service_test.go index 4c15bfd24..3a5734aec 100644 --- a/pkg/server/service_test.go +++ b/pkg/server/service_test.go @@ -29,12 +29,13 @@ import ( "golang.org/x/net/context" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata/store" ostesting "github.com/kubernetes-incubator/cri-containerd/pkg/os/testing" "github.com/kubernetes-incubator/cri-containerd/pkg/registrar" agentstesting "github.com/kubernetes-incubator/cri-containerd/pkg/server/agents/testing" servertesting "github.com/kubernetes-incubator/cri-containerd/pkg/server/testing" + containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" + imagestore "github.com/kubernetes-incubator/cri-containerd/pkg/store/image" + sandboxstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/sandbox" ) type nopReadWriteCloser struct{} @@ -58,10 +59,10 @@ func newTestCRIContainerdService() *criContainerdService { os: ostesting.NewFakeOS(), rootDir: testRootDir, sandboxImage: testSandboxImage, - sandboxStore: metadata.NewSandboxStore(store.NewMetadataStore()), - imageMetadataStore: metadata.NewImageMetadataStore(store.NewMetadataStore()), + sandboxStore: sandboxstore.NewStore(), + imageStore: imagestore.NewStore(), sandboxNameIndex: registrar.NewRegistrar(), - containerStore: metadata.NewContainerStore(store.NewMetadataStore()), + containerStore: containerstore.NewStore(), containerNameIndex: registrar.NewRegistrar(), taskService: servertesting.NewFakeExecutionClient(), containerService: servertesting.NewFakeContainersClient(), @@ -88,11 +89,11 @@ func TestSandboxOperations(t *testing.T) { return nopReadWriteCloser{}, nil } // Insert sandbox image metadata. - assert.NoError(t, c.imageMetadataStore.Create(metadata.ImageMetadata{ + c.imageStore.Add(imagestore.Image{ ID: testSandboxImage, ChainID: "test-chain-id", Config: &imagespec.ImageConfig{Entrypoint: []string{"/pause"}}, - })) + }) config := &runtime.PodSandboxConfig{ Metadata: &runtime.PodSandboxMetadata{ From f4df66eaafaeea024eed2f156c2668983513611d Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Tue, 6 Jun 2017 18:02:09 +0000 Subject: [PATCH 4/4] Remove old metadata store. Signed-off-by: Lantao Liu --- pkg/metadata/container.go | 183 ---------------- pkg/metadata/container_test.go | 180 ---------------- pkg/metadata/image_metadata.go | 151 ------------- pkg/metadata/image_metadata_test.go | 100 --------- pkg/metadata/sandbox.go | 154 -------------- pkg/metadata/sandbox_test.go | 123 ----------- pkg/metadata/store/metadata_store.go | 246 ---------------------- pkg/metadata/store/metadata_store_test.go | 209 ------------------ pkg/metadata/util.go | 25 --- pkg/metadata/util_test.go | 35 --- 10 files changed, 1406 deletions(-) delete mode 100644 pkg/metadata/container.go delete mode 100644 pkg/metadata/container_test.go delete mode 100644 pkg/metadata/image_metadata.go delete mode 100644 pkg/metadata/image_metadata_test.go delete mode 100644 pkg/metadata/sandbox.go delete mode 100644 pkg/metadata/sandbox_test.go delete mode 100644 pkg/metadata/store/metadata_store.go delete mode 100644 pkg/metadata/store/metadata_store_test.go delete mode 100644 pkg/metadata/util.go delete mode 100644 pkg/metadata/util_test.go diff --git a/pkg/metadata/container.go b/pkg/metadata/container.go deleted file mode 100644 index 4fba290d7..000000000 --- a/pkg/metadata/container.go +++ /dev/null @@ -1,183 +0,0 @@ -/* -Copyright 2017 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 metadata - -import ( - "encoding/json" - - "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata/store" -) - -// The code is very similar with sandbox.go, but there is no template support -// in golang, we have to have similar files for different types. -// TODO(random-liu): Figure out a way to simplify this. -// TODO(random-liu): Handle versioning with the same mechanism with container.go - -// containerMetadataVersion is current version of container metadata. -const containerMetadataVersion = "v1" // nolint - -// versionedContainerMetadata is the internal versioned container metadata. -// nolint -type versionedContainerMetadata struct { - // Version indicates the version of the versioned container metadata. - Version string - ContainerMetadata -} - -// ContainerMetadata is the unversioned container metadata. -type ContainerMetadata struct { - // ID is the container id. - ID string - // Name is the container name. - Name string - // SandboxID is the sandbox id the container belongs to. - SandboxID string - // Config is the CRI container config. - Config *runtime.ContainerConfig - // ImageRef is the reference of image used by the container. - ImageRef string - // Pid is the init process id of the container. - Pid uint32 - // CreatedAt is the created timestamp. - CreatedAt int64 - // StartedAt is the started timestamp. - StartedAt int64 - // FinishedAt is the finished timestamp. - FinishedAt int64 - // ExitCode is the container exit code. - ExitCode int32 - // CamelCase string explaining why container is in its current state. - Reason string - // Human-readable message indicating details about why container is in its - // current state. - Message string - // Removing indicates that the container is in removing state. - // In fact, this field doesn't need to be checkpointed. - // TODO(random-liu): Skip this during serialization when we put object - // into the store directly. - // TODO(random-liu): Reset this field to false during state recovery. - Removing bool -} - -// State returns current state of the container based on the metadata. -func (c *ContainerMetadata) State() runtime.ContainerState { - if c.FinishedAt != 0 { - return runtime.ContainerState_CONTAINER_EXITED - } - if c.StartedAt != 0 { - return runtime.ContainerState_CONTAINER_RUNNING - } - if c.CreatedAt != 0 { - return runtime.ContainerState_CONTAINER_CREATED - } - return runtime.ContainerState_CONTAINER_UNKNOWN -} - -// ContainerUpdateFunc is the function used to update ContainerMetadata. -type ContainerUpdateFunc func(ContainerMetadata) (ContainerMetadata, error) - -// ContainerToStoreUpdateFunc generates a metadata store UpdateFunc from ContainerUpdateFunc. -func ContainerToStoreUpdateFunc(u ContainerUpdateFunc) store.UpdateFunc { - return func(data []byte) ([]byte, error) { - meta := &ContainerMetadata{} - if err := json.Unmarshal(data, meta); err != nil { - return nil, err - } - newMeta, err := u(*meta) - if err != nil { - return nil, err - } - return json.Marshal(newMeta) - } -} - -// ContainerStore is the store for metadata of all containers. -type ContainerStore interface { - // Create creates a container from ContainerMetadata in the store. - Create(ContainerMetadata) error - // Get gets a specified container. - Get(string) (*ContainerMetadata, error) - // Update updates a specified container. - Update(string, ContainerUpdateFunc) error - // List lists all containers. - List() ([]*ContainerMetadata, error) - // Delete deletes the container from the store. - Delete(string) error -} - -// containerStore is an implmentation of ContainerStore. -type containerStore struct { - store store.MetadataStore -} - -// NewContainerStore creates a ContainerStore from a basic MetadataStore. -func NewContainerStore(store store.MetadataStore) ContainerStore { - return &containerStore{store: store} -} - -// Create creates a container from ContainerMetadata in the store. -func (c *containerStore) Create(metadata ContainerMetadata) error { - data, err := json.Marshal(&metadata) - if err != nil { - return err - } - return c.store.Create(metadata.ID, data) -} - -// Get gets a specified container. -func (c *containerStore) Get(containerID string) (*ContainerMetadata, error) { - data, err := c.store.Get(containerID) - if err != nil { - return nil, err - } - container := &ContainerMetadata{} - if err := json.Unmarshal(data, container); err != nil { - return nil, err - } - return container, nil -} - -// Update updates a specified container. The function is running in a -// transaction. Update will not be applied when the update function -// returns error. -func (c *containerStore) Update(containerID string, u ContainerUpdateFunc) error { - return c.store.Update(containerID, ContainerToStoreUpdateFunc(u)) -} - -// List lists all containers. -func (c *containerStore) List() ([]*ContainerMetadata, error) { - allData, err := c.store.List() - if err != nil { - return nil, err - } - var containers []*ContainerMetadata - for _, data := range allData { - container := &ContainerMetadata{} - if err := json.Unmarshal(data, container); err != nil { - return nil, err - } - containers = append(containers, container) - } - return containers, nil -} - -// Delete deletes the Container from the store. -func (c *containerStore) Delete(containerID string) error { - return c.store.Delete(containerID) -} diff --git a/pkg/metadata/container_test.go b/pkg/metadata/container_test.go deleted file mode 100644 index 3348493d9..000000000 --- a/pkg/metadata/container_test.go +++ /dev/null @@ -1,180 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authorc. - -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 metadata - -import ( - "testing" - "time" - - assertlib "github.com/stretchr/testify/assert" - - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata/store" - - "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" -) - -func TestContainerState(t *testing.T) { - for c, test := range map[string]struct { - metadata *ContainerMetadata - state runtime.ContainerState - }{ - "unknown state": { - metadata: &ContainerMetadata{ - ID: "1", - Name: "Container-1", - }, - state: runtime.ContainerState_CONTAINER_UNKNOWN, - }, - "created state": { - metadata: &ContainerMetadata{ - ID: "2", - Name: "Container-2", - CreatedAt: time.Now().UnixNano(), - }, - state: runtime.ContainerState_CONTAINER_CREATED, - }, - "running state": { - metadata: &ContainerMetadata{ - ID: "3", - Name: "Container-3", - CreatedAt: time.Now().UnixNano(), - StartedAt: time.Now().UnixNano(), - }, - state: runtime.ContainerState_CONTAINER_RUNNING, - }, - "exited state": { - metadata: &ContainerMetadata{ - ID: "3", - Name: "Container-3", - CreatedAt: time.Now().UnixNano(), - FinishedAt: time.Now().UnixNano(), - }, - state: runtime.ContainerState_CONTAINER_EXITED, - }, - } { - t.Logf("TestCase %q", c) - assertlib.Equal(t, test.state, test.metadata.State()) - } -} - -func TestContainerStore(t *testing.T) { - containers := map[string]*ContainerMetadata{ - "1": { - ID: "1", - Name: "Container-1", - SandboxID: "Sandbox-1", - Config: &runtime.ContainerConfig{ - Metadata: &runtime.ContainerMetadata{ - Name: "TestPod-1", - Attempt: 1, - }, - }, - ImageRef: "TestImage-1", - Pid: 1, - CreatedAt: time.Now().UnixNano(), - StartedAt: time.Now().UnixNano(), - FinishedAt: time.Now().UnixNano(), - ExitCode: 1, - Reason: "TestReason-1", - Message: "TestMessage-1", - }, - "2": { - ID: "2", - Name: "Container-2", - SandboxID: "Sandbox-2", - Config: &runtime.ContainerConfig{ - Metadata: &runtime.ContainerMetadata{ - Name: "TestPod-2", - Attempt: 2, - }, - }, - ImageRef: "TestImage-2", - Pid: 2, - CreatedAt: time.Now().UnixNano(), - StartedAt: time.Now().UnixNano(), - FinishedAt: time.Now().UnixNano(), - ExitCode: 2, - Reason: "TestReason-2", - Message: "TestMessage-2", - }, - "3": { - ID: "3", - Name: "Container-3", - SandboxID: "Sandbox-3", - Config: &runtime.ContainerConfig{ - Metadata: &runtime.ContainerMetadata{ - Name: "TestPod-3", - Attempt: 3, - }, - }, - ImageRef: "TestImage-3", - Pid: 3, - CreatedAt: time.Now().UnixNano(), - StartedAt: time.Now().UnixNano(), - FinishedAt: time.Now().UnixNano(), - ExitCode: 3, - Reason: "TestReason-3", - Message: "TestMessage-3", - Removing: true, - }, - } - assert := assertlib.New(t) - - c := NewContainerStore(store.NewMetadataStore()) - - t.Logf("should be able to create container metadata") - for _, meta := range containers { - assert.NoError(c.Create(*meta)) - } - - t.Logf("should be able to get container metadata") - for id, expectMeta := range containers { - meta, err := c.Get(id) - assert.NoError(err) - assert.Equal(expectMeta, meta) - } - - t.Logf("should be able to list container metadata") - cntrs, err := c.List() - assert.NoError(err) - assert.Len(cntrs, 3) - - t.Logf("should be able to update container metadata") - testID := "2" - newCreatedAt := time.Now().UnixNano() - expectMeta := *containers[testID] - expectMeta.CreatedAt = newCreatedAt - err = c.Update(testID, func(o ContainerMetadata) (ContainerMetadata, error) { - o.CreatedAt = newCreatedAt - return o, nil - }) - assert.NoError(err) - newMeta, err := c.Get(testID) - assert.NoError(err) - assert.Equal(&expectMeta, newMeta) - - t.Logf("should be able to delete container metadata") - assert.NoError(c.Delete(testID)) - cntrs, err = c.List() - assert.NoError(err) - assert.Len(cntrs, 2) - - t.Logf("get should return nil without error after deletion") - meta, err := c.Get(testID) - assert.Error(store.ErrNotExist, err) - assert.True(meta == nil) -} diff --git a/pkg/metadata/image_metadata.go b/pkg/metadata/image_metadata.go deleted file mode 100644 index d6a5a9f90..000000000 --- a/pkg/metadata/image_metadata.go +++ /dev/null @@ -1,151 +0,0 @@ -/* -Copyright 2017 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 metadata - -import ( - "encoding/json" - - imagespec "github.com/opencontainers/image-spec/specs-go/v1" - - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata/store" -) - -// The code is very similar to sandbox.go, but there is no template support -// in golang, thus similar files for different types. -// TODO(random-liu): Figure out a way to simplify this. -// TODO(random-liu): Handle versioning - -// imageMetadataVersion is current version of image metadata. -const imageMetadataVersion = "v1" // nolint - -// versionedImageMetadata is the internal struct representing the versioned -// image metadata -// nolint -type versionedImageMetadata struct { - // Version indicates the version of the versioned image metadata. - Version string `json:"version,omitempty"` - ImageMetadata -} - -// ImageMetadata is the unversioned image metadata. -type ImageMetadata struct { - // Id of the image. Normally the digest of image config. - ID string `json:"id,omitempty"` - // ChainID is the chainID of the image. - ChainID string `json:"chain_id,omitempty"` - // Other names by which this image is known. - RepoTags []string `json:"repo_tags,omitempty"` - // Digests by which this image is known. - RepoDigests []string `json:"repo_digests,omitempty"` - // Size is the compressed size of the image. - Size int64 `json:"size,omitempty"` - // Config is the oci image config of the image. - Config *imagespec.ImageConfig `json:"config,omitempty"` -} - -// ImageMetadataUpdateFunc is the function used to update ImageMetadata. -type ImageMetadataUpdateFunc func(ImageMetadata) (ImageMetadata, error) - -// imageMetadataToStoreUpdateFunc generates a metadata store UpdateFunc from ImageMetadataUpdateFunc. -func imageMetadataToStoreUpdateFunc(u ImageMetadataUpdateFunc) store.UpdateFunc { - return func(data []byte) ([]byte, error) { - meta := &ImageMetadata{} - if err := json.Unmarshal(data, meta); err != nil { - return nil, err - } - newMeta, err := u(*meta) - if err != nil { - return nil, err - } - return json.Marshal(newMeta) - } -} - -// ImageMetadataStore is the store for metadata of all images. -type ImageMetadataStore interface { - // Create creates an image's metadata from ImageMetadata in the store. - Create(ImageMetadata) error - // Get gets the specified image metadata. - Get(string) (*ImageMetadata, error) - // Update updates a specified image metatdata. - Update(string, ImageMetadataUpdateFunc) error - // List lists all image metadatas. - List() ([]*ImageMetadata, error) - // Delete deletes the image's metatdata from the store. - Delete(string) error -} - -// imageMetadataStore is an implmentation of ImageMetadataStore. -type imageMetadataStore struct { - store store.MetadataStore -} - -// NewImageMetadataStore creates an ImageMetadataStore from a basic MetadataStore. -func NewImageMetadataStore(store store.MetadataStore) ImageMetadataStore { - return &imageMetadataStore{store: store} -} - -// Create creates a image's metadata from ImageMetadata in the store. -func (s *imageMetadataStore) Create(metadata ImageMetadata) error { - data, err := json.Marshal(&metadata) - if err != nil { - return err - } - return s.store.Create(metadata.ID, data) -} - -// Get gets the specified image metadata. -func (s *imageMetadataStore) Get(digest string) (*ImageMetadata, error) { - data, err := s.store.Get(digest) - if err != nil { - return nil, err - } - imageMetadata := &ImageMetadata{} - if err := json.Unmarshal(data, imageMetadata); err != nil { - return nil, err - } - return imageMetadata, nil -} - -// Update updates a specified image's metadata. The function is running in a -// transaction. Update will not be applied when the update function -// returns error. -func (s *imageMetadataStore) Update(digest string, u ImageMetadataUpdateFunc) error { - return s.store.Update(digest, imageMetadataToStoreUpdateFunc(u)) -} - -// List lists all image metadata. -func (s *imageMetadataStore) List() ([]*ImageMetadata, error) { - allData, err := s.store.List() - if err != nil { - return nil, err - } - var imageMetadataA []*ImageMetadata - for _, data := range allData { - imageMetadata := &ImageMetadata{} - if err := json.Unmarshal(data, imageMetadata); err != nil { - return nil, err - } - imageMetadataA = append(imageMetadataA, imageMetadata) - } - return imageMetadataA, nil -} - -// Delete deletes the image metadata from the store. -func (s *imageMetadataStore) Delete(digest string) error { - return s.store.Delete(digest) -} diff --git a/pkg/metadata/image_metadata_test.go b/pkg/metadata/image_metadata_test.go deleted file mode 100644 index c86334a13..000000000 --- a/pkg/metadata/image_metadata_test.go +++ /dev/null @@ -1,100 +0,0 @@ -/* -Copyright 2017 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 metadata - -import ( - "testing" - - imagespec "github.com/opencontainers/image-spec/specs-go/v1" - assertlib "github.com/stretchr/testify/assert" - - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata/store" -) - -func TestImageMetadataStore(t *testing.T) { - imageMetadataMap := map[string]*ImageMetadata{ - "1": { - ID: "1", - ChainID: "test-chain-id-1", - RepoTags: []string{"tag-1"}, - RepoDigests: []string{"digest-1"}, - Size: 10, - Config: &imagespec.ImageConfig{}, - }, - "2": { - ID: "2", - ChainID: "test-chain-id-2", - RepoTags: []string{"tag-2"}, - RepoDigests: []string{"digest-2"}, - Size: 20, - Config: &imagespec.ImageConfig{}, - }, - "3": { - ID: "3", - RepoTags: []string{"tag-3"}, - RepoDigests: []string{"digest-3"}, - ChainID: "test-chain-id-3", - Size: 30, - Config: &imagespec.ImageConfig{}, - }, - } - assert := assertlib.New(t) - - s := NewImageMetadataStore(store.NewMetadataStore()) - - t.Logf("should be able to create image metadata") - for _, meta := range imageMetadataMap { - assert.NoError(s.Create(*meta)) - } - - t.Logf("should be able to get image metadata") - for id, expectMeta := range imageMetadataMap { - meta, err := s.Get(id) - assert.NoError(err) - assert.Equal(expectMeta, meta) - } - - t.Logf("should be able to list image metadata") - imgs, err := s.List() - assert.NoError(err) - assert.Len(imgs, 3) - - t.Logf("should be able to update image metadata") - testID := "2" - newSize := int64(200) - expectMeta := *imageMetadataMap[testID] - expectMeta.Size = newSize - err = s.Update(testID, func(o ImageMetadata) (ImageMetadata, error) { - o.Size = newSize - return o, nil - }) - assert.NoError(err) - newMeta, err := s.Get(testID) - assert.NoError(err) - assert.Equal(&expectMeta, newMeta) - - t.Logf("should be able to delete image metadata") - assert.NoError(s.Delete(testID)) - imgs, err = s.List() - assert.NoError(err) - assert.Len(imgs, 2) - - t.Logf("get should return nil not exist error after deletion") - meta, err := s.Get(testID) - assert.Error(store.ErrNotExist, err) - assert.Nil(meta) -} diff --git a/pkg/metadata/sandbox.go b/pkg/metadata/sandbox.go deleted file mode 100644 index ebca5bead..000000000 --- a/pkg/metadata/sandbox.go +++ /dev/null @@ -1,154 +0,0 @@ -/* -Copyright 2017 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 metadata - -import ( - "encoding/json" - - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata/store" - - "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" -) - -// TODO(random-liu): Handle versioning around all marshal/unmarshal. -// version and versionedSandboxMetadata is not used now, but should be used -// in the future: -// 1) Only versioned metadata should be written into the store; -// 2) Only unversioned metadata should be returned to the user; -// 3) A conversion function is needed to convert any supported versioned -// metadata into unversioned metadata. - -// sandboxMetadataVersion is current version of sandbox metadata. -const sandboxMetadataVersion = "v1" // nolint - -// versionedSandboxMetadata is the internal struct representing the versioned -// sandbox metadata -// nolint -type versionedSandboxMetadata struct { - // Version indicates the version of the versioned sandbox metadata. - Version string - SandboxMetadata -} - -// SandboxMetadata is the unversioned sandbox metadata. -type SandboxMetadata struct { - // ID is the sandbox id. - ID string - // Name is the sandbox name. - Name string - // Config is the CRI sandbox config. - Config *runtime.PodSandboxConfig - // CreatedAt is the created timestamp. - CreatedAt int64 - // NetNS is the network namespace used by the sandbox. - NetNS string - // Pid is the process id of the sandbox. - Pid uint32 -} - -// SandboxUpdateFunc is the function used to update SandboxMetadata. -type SandboxUpdateFunc func(SandboxMetadata) (SandboxMetadata, error) - -// sandboxToStoreUpdateFunc generates a metadata store UpdateFunc from SandboxUpdateFunc. -func sandboxToStoreUpdateFunc(u SandboxUpdateFunc) store.UpdateFunc { - return func(data []byte) ([]byte, error) { - meta := &SandboxMetadata{} - if err := json.Unmarshal(data, meta); err != nil { - return nil, err - } - newMeta, err := u(*meta) - if err != nil { - return nil, err - } - return json.Marshal(newMeta) - } -} - -// SandboxStore is the store for metadata of all sandboxes. -type SandboxStore interface { - // Create creates a sandbox from SandboxMetadata in the store. - Create(SandboxMetadata) error - // Get gets the specified sandbox. - Get(string) (*SandboxMetadata, error) - // Update updates a specified sandbox. - Update(string, SandboxUpdateFunc) error - // List lists all sandboxes. - List() ([]*SandboxMetadata, error) - // Delete deletes the sandbox from the store. - Delete(string) error -} - -// sandboxStore is an implmentation of SandboxStore. -type sandboxStore struct { - store store.MetadataStore -} - -// NewSandboxStore creates a SandboxStore from a basic MetadataStore. -func NewSandboxStore(store store.MetadataStore) SandboxStore { - return &sandboxStore{store: store} -} - -// Create creates a sandbox from SandboxMetadata in the store. -func (s *sandboxStore) Create(metadata SandboxMetadata) error { - data, err := json.Marshal(&metadata) - if err != nil { - return err - } - return s.store.Create(metadata.ID, data) -} - -// Get gets the specified sandbox. -func (s *sandboxStore) Get(sandboxID string) (*SandboxMetadata, error) { - data, err := s.store.Get(sandboxID) - if err != nil { - return nil, err - } - sandbox := &SandboxMetadata{} - if err := json.Unmarshal(data, sandbox); err != nil { - return nil, err - } - return sandbox, nil -} - -// Update updates a specified sandbox. The function is running in a -// transaction. Update will not be applied when the update function -// returns error. -func (s *sandboxStore) Update(sandboxID string, u SandboxUpdateFunc) error { - return s.store.Update(sandboxID, sandboxToStoreUpdateFunc(u)) -} - -// List lists all sandboxes. -func (s *sandboxStore) List() ([]*SandboxMetadata, error) { - allData, err := s.store.List() - if err != nil { - return nil, err - } - var sandboxes []*SandboxMetadata - for _, data := range allData { - sandbox := &SandboxMetadata{} - if err := json.Unmarshal(data, sandbox); err != nil { - return nil, err - } - sandboxes = append(sandboxes, sandbox) - } - return sandboxes, nil -} - -// Delete deletes the sandbox from the store. -func (s *sandboxStore) Delete(sandboxID string) error { - return s.store.Delete(sandboxID) -} diff --git a/pkg/metadata/sandbox_test.go b/pkg/metadata/sandbox_test.go deleted file mode 100644 index f97509e90..000000000 --- a/pkg/metadata/sandbox_test.go +++ /dev/null @@ -1,123 +0,0 @@ -/* -Copyright 2017 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 metadata - -import ( - "testing" - "time" - - assertlib "github.com/stretchr/testify/assert" - - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata/store" - - "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" -) - -func TestSandboxStore(t *testing.T) { - sandboxes := map[string]*SandboxMetadata{ - "1": { - ID: "1", - Name: "Sandbox-1", - Config: &runtime.PodSandboxConfig{ - Metadata: &runtime.PodSandboxMetadata{ - Name: "TestPod-1", - Uid: "TestUid-1", - Namespace: "TestNamespace-1", - Attempt: 1, - }, - }, - CreatedAt: time.Now().UnixNano(), - NetNS: "TestNetNS-1", - Pid: 1001, - }, - "2": { - ID: "2", - Name: "Sandbox-2", - Config: &runtime.PodSandboxConfig{ - Metadata: &runtime.PodSandboxMetadata{ - Name: "TestPod-2", - Uid: "TestUid-2", - Namespace: "TestNamespace-2", - Attempt: 2, - }, - }, - CreatedAt: time.Now().UnixNano(), - NetNS: "TestNetNS-2", - Pid: 1002, - }, - "3": { - ID: "3", - Name: "Sandbox-3", - Config: &runtime.PodSandboxConfig{ - Metadata: &runtime.PodSandboxMetadata{ - Name: "TestPod-3", - Uid: "TestUid-3", - Namespace: "TestNamespace-3", - Attempt: 3, - }, - }, - CreatedAt: time.Now().UnixNano(), - NetNS: "TestNetNS-3", - Pid: 1003, - }, - } - assert := assertlib.New(t) - - s := NewSandboxStore(store.NewMetadataStore()) - - t.Logf("should be able to create sandbox metadata") - for _, meta := range sandboxes { - assert.NoError(s.Create(*meta)) - } - - t.Logf("should be able to get sandbox metadata") - for id, expectMeta := range sandboxes { - meta, err := s.Get(id) - assert.NoError(err) - assert.Equal(expectMeta, meta) - } - - t.Logf("should be able to list sandbox metadata") - sbs, err := s.List() - assert.NoError(err) - assert.Len(sbs, 3) - - t.Logf("should be able to update sandbox metadata") - testID := "2" - newCreatedAt := time.Now().UnixNano() - expectMeta := *sandboxes[testID] - expectMeta.CreatedAt = newCreatedAt - err = s.Update(testID, func(o SandboxMetadata) (SandboxMetadata, error) { - o.CreatedAt = newCreatedAt - return o, nil - }) - assert.NoError(err) - newMeta, err := s.Get(testID) - assert.NoError(err) - assert.Equal(&expectMeta, newMeta) - - t.Logf("should be able to delete sandbox metadata") - assert.NoError(s.Delete(testID)) - sbs, err = s.List() - assert.NoError(err) - assert.Len(sbs, 2) - - t.Logf("get should return nil with not exist error after deletion") - meta, err := s.Get(testID) - assert.Error(store.ErrNotExist, err) - assert.Nil(meta) -} diff --git a/pkg/metadata/store/metadata_store.go b/pkg/metadata/store/metadata_store.go deleted file mode 100644 index 7449afe5b..000000000 --- a/pkg/metadata/store/metadata_store.go +++ /dev/null @@ -1,246 +0,0 @@ -/* -Copyright 2017 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 store - -import ( - "errors" - "sync" - - "github.com/golang/glog" -) - -var ( - // ErrNotExist is the error returned when specified id does - // not exist. - ErrNotExist = errors.New("does not exist") - // ErrAlreadyExist is the error returned when specified id already - // exists. - ErrAlreadyExist = errors.New("already exists") -) - -// All byte arrays are expected to be read-only. User MUST NOT modify byte -// array element directly!! - -// UpdateFunc is function used to update a specific metadata. The value -// passed in is the old value, it MUST NOT be changed in the function. -// The function should make a copy of the old value and apply update on -// the copy. The updated value should be returned. If there is an error, -// the update will be rolled back. -type UpdateFunc func([]byte) ([]byte, error) - -// MetadataStore is the interface for storing metadata. All methods should -// be thread-safe. -// TODO(random-liu): Initialize the metadata store with a type, and replace -// []byte with interface{}, so as to avoid extra marshal/unmarshal on the -// user side. -type MetadataStore interface { - // Create the metadata containing the passed in data with the - // specified id. - // Note: - // * Create MUST return error if the id already exists. - // * The id and data MUST be added in one transaction to the store. - Create(string, []byte) error - // Get the data by id. - // Note that Get MUST return ErrNotExist if the id doesn't exist. - Get(string) ([]byte, error) - // Update the data by id. - // Note: - // * Update MUST return ErrNotExist is the id doesn't exist. - // * The update MUST be applied in one transaction. - Update(string, UpdateFunc) error - // List returns entire array of data from the store. - List() ([][]byte, error) - // Delete the data by id. - // Note: - // * Delete should be idempotent, it MUST not return error if the id - // doesn't exist or has been removed. - // * The id and data MUST be deleted in one transaction. - Delete(string) error -} - -// TODO(random-liu) Add checkpoint. When checkpoint is enabled, it should cache data -// in memory and checkpoint metadata into files during update. Metadata should serve -// from memory, but any modification should be checkpointed, so that memory could be -// recovered after restart. It should be possible to disable the checkpoint for testing. -// Note that checkpoint update may fail, so the recovery logic should tolerate that. - -// metadata is the internal type for storing data in metadataStore. -type metadata struct { - sync.RWMutex - data []byte -} - -// newMetadata creates a new metadata. -func newMetadata(data []byte) (*metadata, error) { - return &metadata{data: data}, nil - // TODO(random-liu): Create the data on disk atomically. -} - -// get a snapshot of the metadata. -func (m *metadata) get() []byte { - m.RLock() - defer m.RUnlock() - return m.data -} - -// update the value. -func (m *metadata) update(u UpdateFunc) error { - m.Lock() - defer m.Unlock() - newData, err := u(m.data) - if err != nil { - return err - } - // Replace with newData, user holding the old data will not - // be affected. - // TODO(random-liu) *Update* existing data on disk atomically, - // return error if checkpoint failed. - m.data = newData - return nil -} - -// delete deletes the data on disk atomically. -func (m *metadata) delete() error { - // TODO(random-liu): Hold write lock, rename the data on the disk. - return nil -} - -// cleanup cleans up all temporary files left-over. -func (m *metadata) cleanup() error { - // TODO(random-liu): Hold write lock, Cleanup temporary files generated - // in atomic file operations. The write lock makes sure there is no on-going - // update, so any temporary files could be removed. - return nil -} - -// metadataStore is metadataStore is an implementation of MetadataStore. -type metadataStore struct { - sync.RWMutex - metas map[string]*metadata -} - -// NewMetadataStore creates a MetadataStore. -func NewMetadataStore() MetadataStore { - // TODO(random-liu): Recover state from disk checkpoint. - // TODO(random-liu): Cleanup temporary files left over. - return &metadataStore{metas: map[string]*metadata{}} -} - -// createMetadata creates metadata with a read-write lock -func (m *metadataStore) createMetadata(id string, meta *metadata) error { - m.Lock() - defer m.Unlock() - if _, found := m.metas[id]; found { - return ErrAlreadyExist - } - m.metas[id] = meta - return nil -} - -// Create the metadata with a specific id. -func (m *metadataStore) Create(id string, data []byte) (retErr error) { - // newMetadata takes time, we may not want to lock around it. - meta, err := newMetadata(data) - if err != nil { - return err - } - defer func() { - // This should not happen, because if id already exists, - // newMetadata should fail to checkpoint. Add this just - // in case. - if retErr != nil { - meta.delete() // nolint: errcheck - meta.cleanup() // nolint: errcheck - } - }() - return m.createMetadata(id, meta) -} - -// getMetadata gets metadata by id with a read lock. -func (m *metadataStore) getMetadata(id string) (*metadata, bool) { - m.RLock() - defer m.RUnlock() - meta, found := m.metas[id] - return meta, found -} - -// Get data by id. -func (m *metadataStore) Get(id string) ([]byte, error) { - meta, found := m.getMetadata(id) - if !found { - return nil, ErrNotExist - } - return meta.get(), nil -} - -// Update data by id. -func (m *metadataStore) Update(id string, u UpdateFunc) error { - meta, found := m.getMetadata(id) - if !found { - return ErrNotExist - } - return meta.update(u) -} - -// listMetadata lists all metadata with a read lock. -func (m *metadataStore) listMetadata() []*metadata { - m.RLock() - defer m.RUnlock() - var metas []*metadata - for _, meta := range m.metas { - metas = append(metas, meta) - } - return metas -} - -// List all data. -func (m *metadataStore) List() ([][]byte, error) { - metas := m.listMetadata() - var data [][]byte - for _, meta := range metas { - data = append(data, meta.get()) - } - return data, nil -} - -// Delete the data by id. -func (m *metadataStore) Delete(id string) error { - meta, err := func() (*metadata, error) { - m.Lock() - defer m.Unlock() - meta := m.metas[id] - if meta == nil { - return nil, nil - } - if err := meta.delete(); err != nil { - return nil, err - } - delete(m.metas, id) - return meta, nil - }() - if err != nil { - return err - } - // The metadata is removed from the store at this point. - if meta != nil { - // Do not return error for cleanup. - if err := meta.cleanup(); err != nil { - glog.Errorf("Failed to cleanup metadata %q: %v", id, err) - } - } - return nil -} diff --git a/pkg/metadata/store/metadata_store_test.go b/pkg/metadata/store/metadata_store_test.go deleted file mode 100644 index c507d0804..000000000 --- a/pkg/metadata/store/metadata_store_test.go +++ /dev/null @@ -1,209 +0,0 @@ -/* -Copyright 2017 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 store - -import ( - "bytes" - "errors" - "fmt" - "sync" - "testing" - - assertlib "github.com/stretchr/testify/assert" -) - -func TestMetadata(t *testing.T) { - testData := [][]byte{ - []byte("test-data-1"), - []byte("test-data-2"), - } - updateErr := errors.New("update error") - assert := assertlib.New(t) - - t.Logf("simple create and get") - meta, err := newMetadata(testData[0]) - assert.NoError(err) - old := meta.get() - assert.Equal(testData[0], old) - - t.Logf("failed update should not take effect") - err = meta.update(func(in []byte) ([]byte, error) { - return testData[1], updateErr - }) - assert.Equal(updateErr, err) - assert.Equal(testData[0], meta.get()) - - t.Logf("successful update should take effect") - err = meta.update(func(in []byte) ([]byte, error) { - return testData[1], nil - }) - assert.NoError(err) - assert.Equal(testData[1], meta.get()) - - t.Logf("successful update should not affect existing snapshot") - assert.Equal(testData[0], old) - - // TODO(random-liu): Test deleteCheckpoint and cleanupCheckpoint after - // disk based implementation is added. -} - -func TestMetadataStore(t *testing.T) { - testIds := []string{"id-0", "id-1"} - testMeta := map[string][]byte{ - testIds[0]: []byte("metadata-0"), - testIds[1]: []byte("metadata-1"), - } - assert := assertlib.New(t) - - m := NewMetadataStore() - - t.Logf("should be empty initially") - metas, err := m.List() - assert.NoError(err) - assert.Empty(metas) - - t.Logf("should be able to create metadata") - err = m.Create(testIds[0], testMeta[testIds[0]]) - assert.NoError(err) - - t.Logf("should not be able to create metadata with the same id") - err = m.Create(testIds[0], testMeta[testIds[0]]) - assert.Error(err) - - t.Logf("should be able to list metadata") - err = m.Create(testIds[1], testMeta[testIds[1]]) - assert.NoError(err) - metas, err = m.List() - assert.NoError(err) - assert.True(sliceContainsMap(metas, testMeta)) - - t.Logf("should be able to get metadata by id") - meta, err := m.Get(testIds[1]) - assert.NoError(err) - assert.Equal(testMeta[testIds[1]], meta) - - t.Logf("update should take effect") - err = m.Update(testIds[1], func(in []byte) ([]byte, error) { - return []byte("updated-metadata-1"), nil - }) - assert.NoError(err) - newMeta, err := m.Get(testIds[1]) - assert.NoError(err) - assert.Equal([]byte("updated-metadata-1"), newMeta) - - t.Logf("should be able to delete metadata") - assert.NoError(m.Delete(testIds[1])) - metas, err = m.List() - assert.NoError(err) - assert.Len(metas, 1) - assert.Equal(testMeta[testIds[0]], metas[0]) - meta, err = m.Get(testIds[1]) - assert.Equal(ErrNotExist, err) - assert.Nil(meta) - - t.Logf("update should return not exist error after metadata got deleted") - err = m.Update(testIds[1], func(in []byte) ([]byte, error) { - return in, nil - }) - assert.Equal(ErrNotExist, err) - - t.Logf("existing reference should not be affected by delete") - assert.Equal([]byte("updated-metadata-1"), newMeta) - - t.Logf("should be able to reuse the same id after deletion") - err = m.Create(testIds[1], testMeta[testIds[1]]) - assert.NoError(err) -} - -// sliceMatchMap checks the same elements with a map. -func sliceContainsMap(s [][]byte, m map[string][]byte) bool { - if len(m) != len(s) { - return false - } - for _, expect := range m { - found := false - for _, got := range s { - if bytes.Equal(expect, got) { - found = true - break - } - } - if !found { - return false - } - } - return true -} - -func TestMultithreadAccess(t *testing.T) { - m := NewMetadataStore() - assert := assertlib.New(t) - routineNum := 10 - var wg sync.WaitGroup - for i := 0; i < routineNum; i++ { - wg.Add(1) - go func(i int) { - id := fmt.Sprintf("%d", i) - - t.Logf("should be able to create id %q", id) - expect := []byte(id) - err := m.Create(id, expect) - assert.NoError(err) - - got, err := m.Get(id) - assert.NoError(err) - assert.Equal(expect, got) - - gotList, err := m.List() - assert.NoError(err) - assert.Contains(gotList, expect) - - t.Logf("should be able to update id %q", id) - expect = []byte("update-" + id) - err = m.Update(id, func([]byte) ([]byte, error) { - return expect, nil - }) - assert.NoError(err) - - got, err = m.Get(id) - assert.NoError(err) - assert.Equal(expect, got) - - t.Logf("should be able to delete id %q", id) - err = m.Delete(id) - assert.NoError(err) - - got, err = m.Get(id) - assert.Equal(ErrNotExist, err) - assert.Nil(got) - - err = m.Update(id, func(in []byte) ([]byte, error) { - return in, nil - }) - assert.Equal(ErrNotExist, err) - - gotList, err = m.List() - assert.NoError(err) - assert.NotContains(gotList, expect) - - wg.Done() - }(i) - } - wg.Wait() -} - -// TODO(random-liu): Test recover logic once checkpoint recovery is added. diff --git a/pkg/metadata/util.go b/pkg/metadata/util.go deleted file mode 100644 index 00b5be66e..000000000 --- a/pkg/metadata/util.go +++ /dev/null @@ -1,25 +0,0 @@ -/* -Copyright 2017 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 metadata - -import "github.com/kubernetes-incubator/cri-containerd/pkg/metadata/store" - -// IsNotExistError is a helper function to check whether the error returned -// by metadata store is not exist error. -func IsNotExistError(err error) bool { - return err.Error() == store.ErrNotExist.Error() -} diff --git a/pkg/metadata/util_test.go b/pkg/metadata/util_test.go deleted file mode 100644 index 999ddd1b7..000000000 --- a/pkg/metadata/util_test.go +++ /dev/null @@ -1,35 +0,0 @@ -/* -Copyright 2017 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 metadata - -import ( - "errors" - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata/store" -) - -func TestIsNotExistError(t *testing.T) { - err := store.ErrNotExist - assert.True(t, IsNotExistError(err)) - err = errors.New(store.ErrNotExist.Error()) - assert.True(t, IsNotExistError(err)) - err = errors.New("random error") - assert.False(t, IsNotExistError(err)) -}