Skip to content

Commit

Permalink
fixup! Allow using remote HTTP(S) Dockerfiles with both deploy and …
Browse files Browse the repository at this point in the history
…`build-images` commands
  • Loading branch information
rm3l committed Jul 29, 2022
1 parent 841ec04 commit 878285b
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 28 deletions.
14 changes: 5 additions & 9 deletions pkg/devfile/image/docker_compatible.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,21 @@ import (
// DockerCompatibleBackend uses a CLI compatible with the docker CLI (at least docker itself and podman)
type DockerCompatibleBackend struct {
name string
fs filesystem.Filesystem
}

var _ Backend = (*DockerCompatibleBackend)(nil)

func NewDockerCompatibleBackend(name string, fs filesystem.Filesystem) *DockerCompatibleBackend {
return &DockerCompatibleBackend{
name: name,
fs: fs,
}
func NewDockerCompatibleBackend(name string) *DockerCompatibleBackend {
return &DockerCompatibleBackend{name: name}
}

// Build an image, as defined in devfile, using a Docker compatible CLI
func (o *DockerCompatibleBackend) Build(image *devfile.ImageComponent, devfilePath string) error {
func (o *DockerCompatibleBackend) Build(fs filesystem.Filesystem, image *devfile.ImageComponent, devfilePath string) error {

dockerfile, isTemp, err := resolveAndDownloadDockerfile(o.fs, image.Dockerfile.Uri)
dockerfile, isTemp, err := resolveAndDownloadDockerfile(fs, image.Dockerfile.Uri)
if isTemp {
defer func(path string) {
if e := o.fs.Remove(path); e != nil {
if e := fs.Remove(path); e != nil {
klog.V(3).Infof("could not remove temporary Dockerfile at path %q: %v", path, err)
}
}(dockerfile)
Expand Down
23 changes: 12 additions & 11 deletions pkg/devfile/image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ import (

// Backend is in interface that must be implemented by container runtimes
type Backend interface {
// Build the image as defined in the devfile
Build(image *devfile.ImageComponent, devfilePath string) error
// Build the image as defined in the devfile.
// The filesystem specified will be used to download and store the Dockerfile it is referenced as a remote URL.
Build(fs filesystem.Filesystem, image *devfile.ImageComponent, devfilePath string) error
// Push the image to its registry as defined in the devfile
Push(image string) error
// Return the name of the backend
Expand All @@ -32,7 +33,7 @@ var getEnvFunc = os.Getenv
// If push is true, also push the images to their registries
func BuildPushImages(fs filesystem.Filesystem, devfileObj parser.DevfileObj, path string, push bool) error {

backend, err := selectBackend(fs)
backend, err := selectBackend()
if err != nil {
return err
}
Expand All @@ -48,7 +49,7 @@ func BuildPushImages(fs filesystem.Filesystem, devfileObj parser.DevfileObj, pat
}

for _, component := range components {
err = buildPushImage(backend, component.Image, path, push)
err = buildPushImage(backend, fs, component.Image, path, push)
if err != nil {
return err
}
Expand All @@ -59,22 +60,22 @@ func BuildPushImages(fs filesystem.Filesystem, devfileObj parser.DevfileObj, pat
// BuildPushSpecificImage build an image defined in the devfile present in devfilePath
// If push is true, also push the image to its registry
func BuildPushSpecificImage(fs filesystem.Filesystem, devfilePath string, component devfile.Component, push bool) error {
backend, err := selectBackend(fs)
backend, err := selectBackend()
if err != nil {
return err
}

return buildPushImage(backend, component.Image, devfilePath, push)
return buildPushImage(backend, fs, component.Image, devfilePath, push)
}

// buildPushImage build an image using the provided backend
// If push is true, also push the image to its registry
func buildPushImage(backend Backend, image *devfile.ImageComponent, devfilePath string, push bool) error {
func buildPushImage(backend Backend, fs filesystem.Filesystem, image *devfile.ImageComponent, devfilePath string, push bool) error {
if image == nil {
return errors.New("image should not be nil")
}
log.Sectionf("Building & Pushing Container: %s", image.ImageName)
err := backend.Build(image, devfilePath)
err := backend.Build(fs, image, devfilePath)
if err != nil {
return err
}
Expand All @@ -90,7 +91,7 @@ func buildPushImage(backend Backend, image *devfile.ImageComponent, devfilePath
// selectBackend selects the container backend to use for building and pushing images
// It will detect podman and docker CLIs (in this order),
// or return an error if none are present locally
func selectBackend(fs filesystem.Filesystem) (Backend, error) {
func selectBackend() (Backend, error) {

podmanCmd := getEnvFunc("PODMAN_CMD")
if podmanCmd == "" {
Expand All @@ -112,15 +113,15 @@ func selectBackend(fs filesystem.Filesystem) (Backend, error) {
log.Warning("WARNING: Building images on Apple Silicon / M1 is not (yet) supported natively on Podman")
log.Warning("There is however a temporary workaround: https://github.com/containers/podman/discussions/12899")
}
return NewDockerCompatibleBackend(podmanCmd, fs), nil
return NewDockerCompatibleBackend(podmanCmd), nil
}

dockerCmd := getEnvFunc("DOCKER_CMD")
if dockerCmd == "" {
dockerCmd = "docker"
}
if _, err := lookPathCmd(dockerCmd); err == nil {
return NewDockerCompatibleBackend(dockerCmd, fs), nil
return NewDockerCompatibleBackend(dockerCmd), nil
}
//revive:disable:error-strings This is a top-level error message displayed as is to the end user
return nil, errors.New("odo requires either Podman or Docker to be installed in your environment. Please install one of them and try again.")
Expand Down
9 changes: 5 additions & 4 deletions pkg/devfile/image/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
)

func TestBuildPushImage(t *testing.T) {
fakeFs := filesystem.NewFakeFs()
tests := []struct {
name string
devfilePath string
Expand Down Expand Up @@ -97,16 +98,16 @@ func TestBuildPushImage(t *testing.T) {
ctrl := gomock.NewController(t)
backend := NewMockBackend(ctrl)
if tt.wantBuildCalled {
backend.EXPECT().Build(tt.image, tt.devfilePath).Return(tt.BuildReturns).Times(1)
backend.EXPECT().Build(fakeFs, tt.image, tt.devfilePath).Return(tt.BuildReturns).Times(1)
} else {
backend.EXPECT().Build(nil, tt.devfilePath).Times(0)
backend.EXPECT().Build(fakeFs, nil, tt.devfilePath).Times(0)
}
if tt.wantPushCalled {
backend.EXPECT().Push(tt.image.ImageName).Return(tt.PushReturns).Times(1)
} else {
backend.EXPECT().Push(nil).Times(0)
}
err := buildPushImage(backend, tt.image, "", tt.push)
err := buildPushImage(backend, fakeFs, tt.image, "", tt.push)

if tt.wantErr != (err != nil) {
t.Errorf("%s: Error result wanted %v, got %v", tt.name, tt.wantErr, err != nil)
Expand Down Expand Up @@ -210,7 +211,7 @@ func TestSelectBackend(t *testing.T) {
defer func() { getEnvFunc = os.Getenv }()
lookPathCmd = tt.lookPathCmd
defer func() { lookPathCmd = exec.LookPath }()
backend, err := selectBackend(filesystem.NewFakeFs())
backend, err := selectBackend()
if tt.wantErr != (err != nil) {
t.Errorf("%s: Error result wanted %v, got %v", tt.name, tt.wantErr, err != nil)
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/devfile/image/mock_Backend.go

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

0 comments on commit 878285b

Please sign in to comment.