Skip to content

Commit

Permalink
Unit test bug (#22)
Browse files Browse the repository at this point in the history
Ported changes from internal 'Add image load API' commit and added unit tests for load handler and service methods

Signed-off-by: Cezar Rata <[email protected]>
  • Loading branch information
cezar-r authored Sep 5, 2024
1 parent e0e472a commit df31cb6
Show file tree
Hide file tree
Showing 9 changed files with 293 additions and 1 deletion.
2 changes: 2 additions & 0 deletions api/handlers/image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@ type Service interface {
Remove(ctx context.Context, name string, force bool) (deleted, untagged []string, err error)
Tag(ctx context.Context, srcImg string, repo, tag string) error
Inspect(ctx context.Context, name string) (*dockercompat.Image, error)
Load(ctx context.Context, inStream io.Reader, outStream io.Writer, quiet bool) error
}

func RegisterHandlers(r types.VersionedRouter, service Service, conf *config.Config, logger flog.Logger) {
h := newHandler(service, conf, logger)

r.SetPrefix("/images")
r.HandleFunc("/create", h.pull, http.MethodPost)
r.HandleFunc("/load", h.load, http.MethodPost)
r.HandleFunc("/json", h.list, http.MethodGet)
r.HandleFunc("/{name:.*}", h.remove, http.MethodDelete)
r.HandleFunc("/{name:.*}/push", h.push, http.MethodPost)
Expand Down
26 changes: 26 additions & 0 deletions api/handlers/image/load.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package image

import (
"net/http"
"strconv"

"github.com/containerd/containerd/namespaces"
"github.com/runfinch/finch-daemon/api/response"
)

func (h *handler) load(w http.ResponseWriter, r *http.Request) {
ctx := namespaces.WithNamespace(r.Context(), h.Config.Namespace)
quiet, err := strconv.ParseBool(r.URL.Query().Get("quiet"))
if err != nil {
quiet = false
}
out := response.NewStreamWriter(w)
err = h.service.Load(ctx, r.Body, out, quiet)
if err != nil {
out.WriteError(http.StatusInternalServerError, err)
return
}
}
70 changes: 70 additions & 0 deletions api/handlers/image/load_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package image

import (
"fmt"
"net/http"
"net/http/httptest"

"github.com/containerd/nerdctl/pkg/config"
"github.com/golang/mock/gomock"
"github.com/gorilla/mux"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"github.com/runfinch/finch-daemon/mocks/mocks_image"
"github.com/runfinch/finch-daemon/mocks/mocks_logger"
)

var _ = Describe("Image Load API", func() {
var (
mockCtrl *gomock.Controller
logger *mocks_logger.Logger
service *mocks_image.MockService
h *handler
rr *httptest.ResponseRecorder
name string
req *http.Request
)
BeforeEach(func() {
mockCtrl = gomock.NewController(GinkgoT())
defer mockCtrl.Finish()
logger = mocks_logger.NewLogger(mockCtrl)
service = mocks_image.NewMockService(mockCtrl)
c := config.Config{}
h = newHandler(service, &c, logger)
rr = httptest.NewRecorder()
name = "test-image"
var err error
req, err = http.NewRequest(http.MethodPost, "/images/load", nil)
Expect(err).Should(BeNil())
req = mux.SetURLVars(req, map[string]string{"name": name})

logger.EXPECT().Debugf(gomock.Any(), gomock.Any()).AnyTimes()
})
Context("handler", func() {
It("should return 200 status code upon success", func() {
service.EXPECT().Load(
gomock.Any(),
gomock.Any(),
gomock.Any(),
gomock.Any(),
).Return(nil)

// handler should return 200 status code
h.load(rr, req)
Expect(rr).Should(HaveHTTPStatus(http.StatusOK))
})
It("should return 500 status code if service returns an error message", func() {
service.EXPECT().Load(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("error"))
logger.EXPECT().Debugf(gomock.Any(), gomock.Any())

// handler should return error message
h.load(rr, req)
Expect(rr.Body).Should(MatchJSON(`{"message": "error"}`))
Expect(rr).Should(HaveHTTPStatus(http.StatusInternalServerError))
})
})
})
16 changes: 16 additions & 0 deletions internal/backend/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/containerd/containerd/remotes"
"github.com/containerd/containerd/remotes/docker"
dockerconfig "github.com/containerd/containerd/remotes/docker/config"
"github.com/containerd/nerdctl/pkg/api/types"
"github.com/containerd/nerdctl/pkg/cmd/image"
"github.com/containerd/nerdctl/pkg/idutil/imagewalker"
"github.com/containerd/nerdctl/pkg/imageinspector"
"github.com/containerd/nerdctl/pkg/imgutil"
Expand All @@ -28,6 +30,8 @@ type NerdctlImageSvc interface {
PullImage(ctx context.Context, stdout, stderr io.Writer, resolver remotes.Resolver, ref string, platforms []ocispec.Platform) (*imgutil.EnsuredImage, error)
PushImage(ctx context.Context, resolver remotes.Resolver, tracker docker.StatusTracker, stdout io.Writer, pushRef, ref string, platMC platforms.MatchComparer) error
SearchImage(ctx context.Context, name string) (int, int, []*images.Image, error)
LoadImage(ctx context.Context, img string, stdout io.Writer, quiet bool) error
GetDataStore() (string, error)
}

func (w *NerdctlWrapper) InspectImage(ctx context.Context, image images.Image) (*dockercompat.Image, error) {
Expand Down Expand Up @@ -104,3 +108,15 @@ func (w *NerdctlWrapper) SearchImage(ctx context.Context, name string) (int, int
n, err := walker.Walk(ctx, name)
return n, uniqueCount, imgs, err
}

func (w *NerdctlWrapper) LoadImage(ctx context.Context, img string, stdout io.Writer, _ /*quiet*/ bool) error {
// TODO currently the "quiet" flag in nerdctl is hardcoded as "false".
// Ideally this flag should be part of the ImageLoadOptions, we can
// contribute this enhancement at upstream
return image.Load(ctx, w.clientWrapper.client, types.ImageLoadOptions{
Stdout: stdout,
GOptions: *w.globalOptions,
Input: img,
AllPlatforms: true,
})
}
56 changes: 56 additions & 0 deletions internal/service/image/load.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package image

import (
"context"
"crypto/rand"
"encoding/base64"
"fmt"
"io"
"os"
"path/filepath"
"syscall"
"time"

"github.com/containerd/fifo"
)

func (s *service) Load(ctx context.Context, inStream io.Reader, outStream io.Writer, quiet bool) error {
if inStream == nil {
return fmt.Errorf("import stream should not be nil")
}
root, err := s.nctlImageSvc.GetDataStore()
if err != nil {
s.logger.Errorf("failed to get data store dir: %s", err)
return err
}
d := filepath.Join(root, "fifo")
if err = os.Mkdir(d, 0700); err != nil && !os.IsExist(err) {
fmt.Println(err)
s.logger.Errorf("failed to create fifo dir %s: %s", d, err)
return err
}
t := time.Now()
var b [3]byte
rand.Read(b[:])
img := filepath.Join(d, fmt.Sprintf("%d%s", t.Nanosecond(), base64.URLEncoding.EncodeToString(b[:])))
rw, err := fifo.OpenFifo(ctx, img, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700)
if err != nil {
s.logger.Errorf("failed to open fifo %s: %s", img, err)
return err
}
defer func() {
rw.Close()
os.Remove(img)
}()
go func() {
io.Copy(rw, inStream)
}()
if err = s.nctlImageSvc.LoadImage(ctx, img, outStream, quiet); err != nil {
s.logger.Errorf("failed to load image %s: %s", img, err)
return err
}
return nil
}
80 changes: 80 additions & 0 deletions internal/service/image/load_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package image

import (
"context"
"errors"
"io"
"strings"

"github.com/golang/mock/gomock"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"github.com/runfinch/finch-daemon/api/handlers/image"
"github.com/runfinch/finch-daemon/mocks/mocks_backend"
"github.com/runfinch/finch-daemon/mocks/mocks_logger"
)

// Unit tests related to image load API
var _ = Describe("Image Load API", func() {
var (
ctx context.Context
mockCtrl *gomock.Controller
logger *mocks_logger.Logger
cdClient *mocks_backend.MockContainerdClient
ncClient *mocks_backend.MockNerdctlImageSvc
name string
inStream io.Reader
service image.Service
)
BeforeEach(func() {
ctx = context.Background()
mockCtrl = gomock.NewController(GinkgoT())
logger = mocks_logger.NewLogger(mockCtrl)
cdClient = mocks_backend.NewMockContainerdClient(mockCtrl)
ncClient = mocks_backend.NewMockNerdctlImageSvc(mockCtrl)
name = "/tmp"
inStream = strings.NewReader("")
service = NewService(cdClient, ncClient, logger)
})
Context("service", func() {
It("should return no errors upon success", func() {
ncClient.EXPECT().GetDataStore().
Return(name, nil)
ncClient.EXPECT().LoadImage(gomock.Any(), gomock.Any(), nil, gomock.Any()).
Return(nil)

// service should return no error
err := service.Load(ctx, inStream, nil, false)
Expect(err).Should(BeNil())
})
It("should return an error if load image method returns an error", func() {
logger.EXPECT().Errorf(gomock.Any(), gomock.Any())
ncClient.EXPECT().GetDataStore().
Return(name, nil)
ncClient.EXPECT().LoadImage(gomock.Any(), gomock.Any(), nil, gomock.Any()).
Return(errors.New("error message"))

// service should return an error
err := service.Load(ctx, inStream, nil, false)
Expect(err).ShouldNot(BeNil())
})
It("should return an error if get datastore method returns an error", func() {
logger.EXPECT().Errorf(gomock.Any(), gomock.Any())
ncClient.EXPECT().GetDataStore().
Return(name, errors.New("error message"))

// service should return an error
err := service.Load(ctx, inStream, nil, false)
Expect(err).ShouldNot(BeNil())
})
It("should return an error if import stream is nil", func() {
// service should return an error
err := service.Load(ctx, nil, nil, false)
Expect(err).ShouldNot(BeNil())
})
})
})
29 changes: 29 additions & 0 deletions mocks/mocks_backend/nerdctlimagesvc.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions mocks/mocks_image/imagesvc.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion setup-test-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,3 @@ sudo containerd &
sudo buildkitd &

sleep 2

0 comments on commit df31cb6

Please sign in to comment.