Skip to content

Commit

Permalink
Refactor container image management
Browse files Browse the repository at this point in the history
Encapsulate container image management into a separate module that provides
an image manager class.

Closes #799
Part of #794

Signed-off-by: Georgiy Lebedev <[email protected]>
  • Loading branch information
CuriousGeorgiy authored and ustiugov committed Sep 10, 2023
1 parent a4b4eb9 commit 993b486
Show file tree
Hide file tree
Showing 6 changed files with 352 additions and 74 deletions.
12 changes: 10 additions & 2 deletions .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ jobs:
firecracker-containerd-interface-test:
name: "Unit tests: Firecracker-containerd interface"
runs-on: [self-hosted, integ]
strategy:
fail-fast: false
matrix:
module: [ ctriface, ctriface/image ]
steps:

- name: Set up Go 1.19
Expand Down Expand Up @@ -94,9 +98,13 @@ jobs:
run: go build -race -v -a ./...

- name: Run tests in submodules
env:
MODULE: ${{ matrix.module }}
AWS_ACCESS_KEY: ${{ secrets.AWS_ACCESS_KEY }}
AWS_SECRET_KEY: ${{ secrets.AWS_SECRET_KEY }}
run: |
make -C ctriface test
make -C ctriface test-man
make -C $MODULE test
make -C $MODULE test-man
- name: Cleaning
if: ${{ always() }}
Expand Down
73 changes: 1 addition & 72 deletions ctriface/iface.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ package ctriface

import (
"context"
"fmt"
"net"
"net/http"
"net/url"
"os"
"os/exec"
"strings"
Expand All @@ -41,7 +37,6 @@ import (
"github.com/containerd/containerd/cio"
"github.com/containerd/containerd/namespaces"
"github.com/containerd/containerd/oci"
"github.com/containerd/containerd/remotes/docker"

"github.com/firecracker-microvm/firecracker-containerd/proto" // note: from the original repo
"github.com/firecracker-microvm/firecracker-containerd/runtime/firecrackeroci"
Expand Down Expand Up @@ -285,74 +280,8 @@ func (o *Orchestrator) StopSingleVM(ctx context.Context, vmID string) error {
return nil
}

// Checks whether a URL has a .local domain
func isLocalDomain(s string) (bool, error) {
if !strings.Contains(s, "://") {
s = "dummy://" + s
}

u, err := url.Parse(s)
if err != nil {
return false, err
}

host, _, err := net.SplitHostPort(u.Host)
if err != nil {
host = u.Host
}

i := strings.LastIndex(host, ".")
tld := host[i+1:]

return tld == "local", nil
}

// Converts an image name to a url if it is not a URL
func getImageURL(image string) string {
// Pull from dockerhub by default if not specified (default k8s behavior)
if strings.Contains(image, ".") {
return image
}
return "docker.io/" + image

}

func (o *Orchestrator) getImage(ctx context.Context, imageName string) (*containerd.Image, error) {
image, found := o.cachedImages[imageName]
if !found {
var err error
log.Debug(fmt.Sprintf("Pulling image %s", imageName))

imageURL := getImageURL(imageName)
local, _ := isLocalDomain(imageURL)
if local {
// Pull local image using HTTP
resolver := docker.NewResolver(docker.ResolverOptions{
Client: http.DefaultClient,
Hosts: docker.ConfigureDefaultRegistries(
docker.WithPlainHTTP(docker.MatchAllHosts),
),
})
image, err = o.client.Pull(ctx, imageURL,
containerd.WithPullUnpack,
containerd.WithPullSnapshotter(o.snapshotter),
containerd.WithResolver(resolver),
)
} else {
// Pull remote image
image, err = o.client.Pull(ctx, imageURL,
containerd.WithPullUnpack,
containerd.WithPullSnapshotter(o.snapshotter),
)
}

if err != nil {
return &image, err
}
o.cachedImages[imageName] = image
}

return &image, nil
return o.imageManager.GetImage(ctx, imageName)
}

func getK8sDNS() []string {
Expand Down
35 changes: 35 additions & 0 deletions ctriface/image/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# MIT License
#
# Copyright (c) 2023 Georgiy Lebedev, Dmitrii Ustiugov, Plamen Petrov and vHive team
#
# Permission is hereby granted, free of charge, to any person obtaining a copy
# of this software and associated documentation files (the "Software"), to deal
# in the Software without restriction, including without limitation the rights
# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
# copies of the Software, and to permit persons to whom the Software is
# furnished to do so, subject to the following conditions:
#
# The above copyright notice and this permission notice shall be included in all
# copies or substantial portions of the Software.
#
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
# SOFTWARE.

EXTRAGOARGS:=-v -race -cover
CTRDLOGDIR:=/tmp/ctrd-logs

test:
./../../scripts/clean_fcctr.sh
sudo mkdir -m777 -p $(CTRDLOGDIR) && sudo env "PATH=$(PATH)" /usr/local/bin/firecracker-containerd --config /etc/firecracker-containerd/config.toml 1>$(CTRDLOGDIR)/ctriface_log.out 2>$(CTRDLOGDIR)/ctriface_log.err &
sudo env "PATH=$(PATH)" go test ./ $(EXTRAGOARGS)
./../../scripts/clean_fcctr.sh

test-man:
echo "Nothing to test manually"

.PHONY: test test-man
167 changes: 167 additions & 0 deletions ctriface/image/manager.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
// MIT License
//
// Copyright (c) 2023 Georgiy Lebedev, Amory Hoste and vHive team
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in all
// copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.

// Package ctrimages provides an image manager that manages and caches container images.
package image

import (
"context"
"github.com/containerd/containerd"
"github.com/containerd/containerd/remotes/docker"
log "github.com/sirupsen/logrus"
"net"
"net/http"
"net/url"
"strings"
"sync"
)

// ImageState is used for synchronization to avoid pulling the same image multiple times concurrently.
type ImageState struct {
sync.Mutex
isCached bool
}

// NewImageState creates a new ImageState object that can be used to synchronize pulling a single image
func NewImageState() *ImageState {
state := new(ImageState)
state.isCached = false
return state
}

// ImageManager manages the images that have been pulled to the node.
type ImageManager struct {
sync.Mutex
snapshotter string // image snapshotter
cachedImages map[string]containerd.Image // Cached container images
imageStates map[string]*ImageState
client *containerd.Client
}

// NewImageManager creates a new image manager that can be used to fetch container images.
func NewImageManager(client *containerd.Client, snapshotter string) *ImageManager {
log.Info("Creating image manager")
manager := new(ImageManager)
manager.snapshotter = snapshotter
manager.cachedImages = make(map[string]containerd.Image)
manager.imageStates = make(map[string]*ImageState)
manager.client = client
return manager
}

// pullImage fetches an image and adds it to the cached image list
func (mgr *ImageManager) pullImage(ctx context.Context, imageName string) error {
var err error
var image containerd.Image

imageURL := getImageURL(imageName)
local, _ := isLocalDomain(imageURL)
if local {
// Pull local image using HTTP
resolver := docker.NewResolver(docker.ResolverOptions{
Client: http.DefaultClient,
Hosts: docker.ConfigureDefaultRegistries(
docker.WithPlainHTTP(docker.MatchAllHosts),
),
})
image, err = mgr.client.Pull(ctx, imageURL,
containerd.WithPullUnpack,
containerd.WithPullSnapshotter(mgr.snapshotter),
containerd.WithResolver(resolver),
)
} else {
// Pull remote image
image, err = mgr.client.Pull(ctx, imageURL,
containerd.WithPullUnpack,
containerd.WithPullSnapshotter(mgr.snapshotter),
)
}
if err != nil {
return err
}
mgr.Lock()
mgr.cachedImages[imageName] = image
mgr.Unlock()
return nil
}

// GetImage fetches an image that can be used to create a container using containerd. Synchronization is implemented
// on a per image level to keep waiting to a minimum.
func (mgr *ImageManager) GetImage(ctx context.Context, imageName string) (*containerd.Image, error) {
// Get reference to synchronization object for image
mgr.Lock()
imgState, found := mgr.imageStates[imageName]
if !found {
imgState = NewImageState()
mgr.imageStates[imageName] = imgState
}
mgr.Unlock()

// Pull image if necessary. The image will only be pulled by the first thread to take the lock.
imgState.Lock()
if !imgState.isCached {
if err := mgr.pullImage(ctx, imageName); err != nil {
imgState.Unlock()
return nil, err
}
imgState.isCached = true
}
imgState.Unlock()

mgr.Lock()
image := mgr.cachedImages[imageName]
mgr.Unlock()

return &image, nil
}

// Converts an image name to a url if it is not a URL
func getImageURL(image string) string {
// Pull from dockerhub by default if not specified (default k8s behavior)
if strings.Contains(image, ".") {
return image
}
return "docker.io/" + image

}

// Checks whether a URL has a .local domain
func isLocalDomain(s string) (bool, error) {
if ! strings.Contains(s, "://") {
s = "dummy://" + s
}

u, err := url.Parse(s)
if err != nil {
return false, err
}

host, _, err := net.SplitHostPort(u.Host)
if err != nil {
host = u.Host
}

i := strings.LastIndex(host, ".")
tld := host[i+1:]

return tld == "local", nil
}
Loading

0 comments on commit 993b486

Please sign in to comment.