Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(docker): docker build failed when multiple Dockerfile in one dir #980

Merged
merged 1 commit into from
Jun 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/pkg/cli/svc_deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ func (o *deploySvcOpts) getDockerfilePath() (string, error) {
if !ok {
return "", fmt.Errorf("service %s does not have a dockerfile path", o.Name)
}
return strings.TrimSuffix(mf.DockerfilePath(), "/Dockerfile"), nil
return mf.DockerfilePath(), nil
}

// pushAddonsTemplateToS3Bucket generates the addons template for the service and pushes it to S3.
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/cli/svc_deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ image:
wantPath: "",
wantErr: fmt.Errorf("read manifest file %s: %w", "serviceA", mockError),
},
"should trim the manifest DockerfilePath if it contains /Dockerfile": {
"success": {
inputSvc: "serviceA",
setupMocks: func(controller *gomock.Controller) {
mockWorkspace = mocks.NewMockwsSvcReader(controller)
Expand All @@ -229,7 +229,7 @@ image:
mockWorkspace.EXPECT().ReadServiceManifest("serviceA").Times(1).Return(mockManifest, nil),
)
},
wantPath: "serviceA",
wantPath: "serviceA/Dockerfile",
wantErr: nil,
},
}
Expand Down
26 changes: 14 additions & 12 deletions internal/pkg/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,35 @@ package docker

import (
"fmt"
"path/filepath"
"strings"

"github.com/aws/amazon-ecs-cli-v2/internal/pkg/term/command"
"github.com/aws/amazon-ecs-cli-v2/internal/pkg/term/log"
)

// Service wraps a runner.
type Service struct {
runner runner
// Runner represents a command that can be run.
type Runner struct {
runner
}

type runner interface {
Run(name string, args []string, options ...command.Option) error
}

// New returns a Service.
func New() Service {
return Service{
// New returns a Runner.
func New() Runner {
return Runner{
runner: command.New(),
}
}

// Build will run a `docker build` command with the input uri, tag, and Dockerfile image path.
func (s Service) Build(uri, imageTag, path string) error {
func (r Runner) Build(uri, imageTag, path string) error {
imageName := imageName(uri, imageTag)
dfPath, dfName := filepath.Split(path)

err := s.runner.Run("docker", []string{"build", "-t", imageName, path})
err := r.Run("docker", []string{"build", "-t", imageName, dfPath, "-f", dfName})
Copy link
Contributor

Choose a reason for hiding this comment

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

awesome tyty 🙏


if err != nil {
return fmt.Errorf("building image: %w", err)
Expand All @@ -42,8 +44,8 @@ func (s Service) Build(uri, imageTag, path string) error {
}

// Login will run a `docker login` command against the Service repository URI with the input uri and auth data.
func (s Service) Login(uri, username, password string) error {
err := s.runner.Run("docker",
func (r Runner) Login(uri, username, password string) error {
err := r.Run("docker",
[]string{"login", "-u", username, "--password-stdin", uri},
command.Stdin(strings.NewReader(password)))

Expand All @@ -55,10 +57,10 @@ func (s Service) Login(uri, username, password string) error {
}

// Push will run `docker push` command against the Service repository URI with the input uri and image tag.
func (s Service) Push(uri, imageTag string) error {
func (r Runner) Push(uri, imageTag string) error {
path := imageName(uri, imageTag)

err := s.runner.Run("docker", []string{"push", path})
err := r.Run("docker", []string{"push", path})

if err != nil {
// TODO: improve the error handling here.
Expand Down
17 changes: 10 additions & 7 deletions internal/pkg/docker/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,31 @@ func TestBuild(t *testing.T) {

mockURI := "mockURI"
mockImageTag := "mockImageTag"
mockPath := "mockPath"
mockPath := "mockPath/mockDockerfile"

var mockRunner *mocks.Mockrunner

tests := map[string]struct {
path string
setupMocks func(controller *gomock.Controller)

want error
}{
"wrap error returned from Run()": {
path: mockPath,
setupMocks: func(controller *gomock.Controller) {
mockRunner = mocks.NewMockrunner(controller)

mockRunner.EXPECT().Run("docker", []string{"build", "-t", imageName(mockURI, mockImageTag), mockPath}).Return(mockError)
mockRunner.EXPECT().Run("docker", []string{"build", "-t", imageName(mockURI, mockImageTag), "mockPath/", "-f", "mockDockerfile"}).Return(mockError)
},
want: fmt.Errorf("building image: %w", mockError),
},
"happy path": {
path: mockPath,
setupMocks: func(controller *gomock.Controller) {
mockRunner = mocks.NewMockrunner(controller)

mockRunner.EXPECT().Run("docker", []string{"build", "-t", imageName(mockURI, mockImageTag), mockPath}).Return(nil)
mockRunner.EXPECT().Run("docker", []string{"build", "-t", imageName(mockURI, mockImageTag), "mockPath/", "-f", "mockDockerfile"}).Return(nil)
},
},
}
Expand All @@ -47,11 +50,11 @@ func TestBuild(t *testing.T) {
t.Run(name, func(t *testing.T) {
controller := gomock.NewController(t)
test.setupMocks(controller)
s := Service{
s := Runner{
runner: mockRunner,
}

got := s.Build(mockURI, mockImageTag, mockPath)
got := s.Build(mockURI, mockImageTag, test.path)

require.Equal(t, test.want, got)
})
Expand Down Expand Up @@ -94,7 +97,7 @@ func TestLogin(t *testing.T) {
t.Run(name, func(t *testing.T) {
controller := gomock.NewController(t)
test.setupMocks(controller)
s := Service{
s := Runner{
runner: mockRunner,
}

Expand Down Expand Up @@ -140,7 +143,7 @@ func TestPush(t *testing.T) {
t.Run(name, func(t *testing.T) {
controller := gomock.NewController(t)
test.setupMocks(controller)
s := Service{
s := Runner{
runner: mockRunner,
}

Expand Down