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

feat(cli): add prompt for custom path if Dockerfile not found #1341

Merged
merged 26 commits into from
Sep 14, 2020

Conversation

huanjani
Copy link
Contributor

@huanjani huanjani commented Aug 28, 2020

Previously, if a service's Dockerfile could not be found in the current directory or subdirectory one level down, svc init would throw an error. With these changes, users are prompted to input a custom path to their Dockerfiles (similar to the --dockerfile flag) at the point that they would otherwise be selecting from a list of found Dockerfiles.

Fixes #1167.

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

@huanjani huanjani requested a review from a team as a code owner August 28, 2020 22:02
@huanjani huanjani requested a review from kohidave August 28, 2020 22:02
@huanjani
Copy link
Contributor Author

Before:

% copilot init
Note: It's best to run this command in the root of your Git repository.
Welcome to the Copilot CLI! We're going to walk you through some questions
to help you get set up with an application on ECS. An application is a collection of
containerized services that operate together.

Your workspace is registered to application video-store.
Service type: Load Balanced Web Service
Service name: lbws
✘ ask svc init: no Dockerfiles found within . or a sub-directory level below
% 

@huanjani
Copy link
Contributor Author

huanjani commented Aug 28, 2020

After [load balanced web service with a relative path]:

% copilot init
Note: It's best to run this command in the root of your Git repository.
Welcome to the Copilot CLI! We're going to walk you through some questions
to help you get set up with an application on ECS. An application is a collection of
containerized services that operate together.

Your workspace is registered to application video-store.
Service type: Load Balanced Web Service
Service name: bananas
Path to Dockerfile: ../../Dockerfile
Ok great, we'll set up a Load Balanced Web Service named bananas in application video-store listening on port 3000.

✔ Created the infrastructure to manage services under application video-store.

✔ Wrote the manifest for service bananas at ../../../copilot/bananas/manifest.yml
Your manifest contains configurations like your container size and port (:3000).

✔ Created ECR repositories for service bananas.

All right, you're all set for local development.
Deploy: Yes

✔ Environment test already exists in application video-store.

✔ Linked account 240594714208 and region us-west-2 to application video-store.

✔ Created environment test in region us-west-2 under application video-store.
Sending build context to Docker daemon   2.99MB
...
Successfully built 372f5c527125
Successfully tagged 240594714208.dkr.ecr.us-west-2.amazonaws.com/video-store/bananas:b9efcb0
Login Succeeded
The push refers to repository [240594714208.dkr.ecr.us-west-2.amazonaws.com/video-store/bananas]
...
b9efcb0: digest: sha256:149626d358c0dc4946bc0eb18cdf14e17b43d29878902bc993d749caf15371c3 size: 2205


✔ Deployed bananas, you can access it at...

How the path is rendered in the manifest:

image:
  # Docker build arguments. You can specify additional overrides here. Supported: dockerfile, context, args
  build: video-store-consumer/Dockerfile
  # Port exposed through your container to route traffic to it.
  port: 3000

After [backend service with an absolute path]:

 %  copilot svc init
Note: It's best to run this command in the root of your workspace.
Service type: Backend Service
Service name: jellybeans
Path to Dockerfile: /Users/jwh/ECS-DevX/sample-apps/video-store/video-store-consumer-api/Dockerfile
✔ Wrote the manifest for service jellybeans at ../../copilot/jellybeans/manifest.yml
Your manifest contains configurations like your container size and port (:3000).

✔ Created ECR repositories for service jellybeans.

Recommended follow-up actions:
- Update your manifest ../../copilot/jellybeans/manifest.yml to change the defaults.
- Run `copilot svc deploy --name jellybeans --env test` to deploy your service to a test environment.

% copilot svc deploy --name jellybeans --env test
Sending build context to Docker daemon  516.6kB
...
Successfully built 3a61cd5b7154
Successfully tagged 240594714208.dkr.ecr.us-west-2.amazonaws.com/video-store/jellybeans:8e8ade7
Login Succeeded
The push refers to repository [240594714208.dkr.ecr.us-west-2.amazonaws.com/video-store/jellybeans]
...
8e8ade7: digest: sha256:fe32962eb4b9d464def5caaf9b437797948dc44839295aa833e81a8a0821d686 size: 3047


✔ Deployed jellybeans, its service discovery endpoint is...

How the path is rendered in the manifest:

image:
  # Docker build arguments. You can specify additional overrides here. Supported: dockerfile, context, args
  build: video-store-consumer-api/Dockerfile
  # Port exposed through your container to route traffic to it.
  port: 3000

@kohidave kohidave requested a review from efekarakus August 28, 2020 22:47
Copy link
Contributor

@bvtujo bvtujo left a comment

Choose a reason for hiding this comment

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

Thank you so much for this! This is going to be super helpful.

@bvtujo
Copy link
Contributor

bvtujo commented Aug 29, 2020

Oh, another comment on the syntax here--

In your example, we have a customer entering ../Dockerfile as the custom path. We should make sure that the symlink evaluates--since when svc deploy is called, Docker is run in the workspace root (one level up from the copilot directory), we should make sure that we're rendering the path in the manifest relative to that directory. Symbolic links like .. MAY work but I'm not certain. Can you paste the build section that's rendered when a custom dockerfile is specified, and the output of svc deploy when it's a relative path like that?

@huanjani
Copy link
Contributor Author

huanjani commented Sep 1, 2020

Oh, another comment on the syntax here--

In your example, we have a customer entering ../Dockerfile as the custom path. We should make sure that the symlink evaluates--since when svc deploy is called, Docker is run in the workspace root (one level up from the copilot directory), we should make sure that we're rendering the path in the manifest relative to that directory. Symbolic links like .. MAY work but I'm not certain. Can you paste the build section that's rendered when a custom dockerfile is specified, and the output of svc deploy when it's a relative path like that?

Ah, you were right!
build: video-store-consumer/Dockerfile
and
unable to prepare context: unable to evaluate symlinks in Dockerfile path: lstat /Users/jwh/ECS-DevX/sample-apps/Dockerfile: no such file or directory

@bvtujo
Copy link
Contributor

bvtujo commented Sep 1, 2020

Aha! So it seems like we need to get the current directory, get the copilot directory, and use that information plus the provided relative Dockerfile path to generate the relative path from the workspace root to the Dockerfile.

So:

Copilot:  /users/austiely/cool-app/copilot
Workdir:  /users/austiely/cool-app/src/pkg1
Input df: ../../build/Dockerfile

Result: build/Dockerfile

Does that make sense?

Also, what are the paths in question in your example? Where in the repo were you running this command, vs the absolute path of the Dockerfile?

@huanjani huanjani marked this pull request as draft September 1, 2020 15:18
@huanjani huanjani marked this pull request as ready for review September 4, 2020 22:14
Copy link
Contributor

@bvtujo bvtujo left a comment

Choose a reason for hiding this comment

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

Awesome!

@mergify mergify bot merged commit 6a8fa3d into aws:mainline Sep 14, 2020
@huanjani huanjani deleted the prompt branch September 15, 2020 22:00
thrau pushed a commit to localstack/copilot-cli-local that referenced this pull request Dec 9, 2022
Previously, if a service's Dockerfile could not be found in the current directory or subdirectory one level down, `svc init` would throw an error. With these changes, users are prompted to input a custom path to their Dockerfiles (similar to the `--dockerfile` flag) at the point that they would otherwise be selecting from a list of found Dockerfiles. 

Fixes aws#1167.

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
None yet
Development

Successfully merging this pull request may close these issues.

bug: prompt users for Dockerfile location if file not found at root
3 participants