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

Conversation

iamhopaul123
Copy link
Contributor

Fix #979

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Contributor

@kohidave kohidave left a comment

Choose a reason for hiding this comment

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

Thanks for your super quick turn around!

internal/pkg/docker/docker.go Outdated Show resolved Hide resolved
@iamhopaul123 iamhopaul123 force-pushed the docker/bug-fix branch 2 times, most recently from fb299fb to 77439cb Compare May 30, 2020 19:33
@efekarakus efekarakus added the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Jun 1, 2020
internal/pkg/docker/docker.go Outdated Show resolved Hide resolved

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 🙏

@iamhopaul123 iamhopaul123 removed the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Jun 1, 2020
@mergify mergify bot merged commit ce5d2d9 into aws:master Jun 1, 2020
Sprint 🏃‍♀️ automation moved this from In review to Pending release Jun 1, 2020
mergify bot pushed a commit that referenced this pull request Jun 6, 2020
<!-- Provide summary of changes -->
With #980, we fixed an issue which occurred when there were multiple dockerfiles present in the build directory. 

However, this may have broken our end to end tests, because the call to Docker was formatted as the following:

```sh
docker build -t <imageTag> path/to -f Dockerfile
```

This should be fine, but the end to end tests began to fail on Monday with the following output:
```bash
addons flow when deploying svc
  svc deploy should succeed
  /github.com/aws/amazon-ecs-cli-v2/e2e/addons/addons_test.go:145
unable to prepare context: unable to evaluate symlinks in Dockerfile path: lstat /github.com/aws/amazon-ecs-cli-v2/e2e/addons/Dockerfile: no such file or directory
Error: build Dockerfile at ./hello/Dockerfile with tag gallopinggurdey: building image: exit status 1
```
This indicates that although the Dockerfile path was set to `hello/Dockerfile`, Docker was looking for the file somewhere in the root directory of that path (.) and subsequently failing. 

This fix always sets the dockerfile path to be relative to the *root* of the docker build context, not the *leaf* of the build context. 

(previously we were parsing paths like `path/to/dockerfile` into `path/to` and `dockerfile`, assuming that the dockerfile would be searched for in `path/to`, not `path`. This was incorrect.)

The new behavior is to call:
```sh
docker build -t <imageTag> path/to -f path/to/dockerfile
```



<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Sprint 🏃‍♀️
  
Pending release
Development

Successfully merging this pull request may close these issues.

Only accepts dockerfile named Dockerfile
3 participants