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.

Sign up for GitHub

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

Jump to bottom

Adds support for custom docker options #100

Merged
merged 10 commits into from
Jul 15, 2022
Merged

Adds support for custom docker options #100

merged 10 commits into from
Jul 15, 2022

Conversation

wbreza
Copy link
Contributor

@wbreza wbreza commented Jul 13, 2022 •
edited

Adds support for custom docker options.

Example

name: todo-nodejs-mongo-aca
metadata:
  template: todo-nodejs-mongo-aca@0.0.1-beta
services:
  api:
    project: src/api
    language: js
    host: containerapp
    docker:
      path: ./Dockerfile
      context: ../
  web:
    project: src/web
    language: js
    host: containerapp

Resolves Azure/azure-dev-pr#1298

@wbreza wbreza self-assigned this Jul 13, 2022
@wbreza wbreza added the cli label Jul 13, 2022
@wbreza wbreza added this to the Next milestone Jul 13, 2022
@ellismg
Copy link
Member

ellismg commented Jul 13, 2022

As part of this also update schemas/v1.0/azure.yaml.json with this new optional property.

ellismg
ellismg reviewed Jul 13, 2022
View reviewed changes
Copy link
Member

@ellismg ellismg left a comment

Choose a reason for hiding this comment

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

Small comment to save a round trip through the json marhsaller.

cli/azd/pkg/project/service_config.go Outdated Show resolved Hide resolved
jongio
jongio approved these changes Jul 14, 2022
View reviewed changes
Copy link
Member

@jongio jongio left a comment

Choose a reason for hiding this comment

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

Approving from a functional standpoint. Confirmed it addressed my ask.

@nishanil
Copy link
Member

nishanil commented Jul 14, 2022 •
edited

+1 on Docker options and the ability to setup the Context in azure.yaml. With .NET projects, I was hitting this issue when multiple .csproj files are referenced within Dockerfile that are not within the same folder. We are trying to setup an end-to-end sample with azd and hit this issue. As a best practice, we copy multiple .csproj files and restore the packages so subsequent builds are faster. With the options to setup the context similar to the docker-compose files, we could easily achieve this. See this simple app where we do it in compose https://github.com/erjain/azdSampleMicroservice/tree/shared-poc and works but fails with azd deploy.

@nishanil
Copy link
Member

Tested my sample with the exe generated in this PR and it worked ❤️

vhvb1989
vhvb1989 approved these changes Jul 14, 2022
View reviewed changes
Copy link
Member

@vhvb1989 vhvb1989 left a comment

Choose a reason for hiding this comment

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

Your call, but I would really like to avoid having the options field if it is only bringing up an inner section of the config field

cli/azd/pkg/project/framework_service_docker.go Outdated Show resolved Hide resolved
cli/azd/pkg/project/framework_service_docker.go Outdated Show resolved Hide resolved
vhvb1989
vhvb1989 approved these changes Jul 14, 2022
View reviewed changes
Copy link
Member

@vhvb1989 vhvb1989 left a comment

Choose a reason for hiding this comment

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

❤️

@wbreza wbreza marked this pull request as ready for review July 15, 2022 16:34
karolz-ms
karolz-ms approved these changes Jul 15, 2022
View reviewed changes
cli/azd/pkg/tools/docker.go Outdated Show resolved Hide resolved
schemas/v1.0/azure.yaml.json Show resolved Hide resolved
ellismg
ellismg approved these changes Jul 15, 2022
View reviewed changes
Copy link
Member

@ellismg ellismg left a comment

Choose a reason for hiding this comment

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

Small question about getDockerOptionsWithDefaults but LGTM otherwise.

cli/azd/pkg/project/framework_service_docker.go Outdated Show resolved Hide resolved
@wbreza wbreza enabled auto-merge (squash) July 15, 2022 18:44
@azure-sdk
Copy link
Collaborator

Azure Dev CLI Install Instructions

Install scripts

MacOS/Linux

May elevate using sudo on some platforms and configurations

curl -fsSL https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/100/uninstall-azd.sh | bash;
curl -fsSL https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/100/install-azd.sh | bash -s -- --base-url https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/100 --version '' --verbose

Windows

powershell -c "Set-ExecutionPolicy Bypass Process; irm 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/100/uninstall-azd.ps1' > uninstall-azd.ps1; ./uninstall-azd.ps1;"
powershell -c "Set-ExecutionPolicy Bypass Process; irm 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/100/install-azd.ps1' > install-azd.ps1; ./install-azd.ps1 -BaseUrl 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/100' -Version '' -Verbose;"

Standalone Binary

Container

docker run -it azdevcliextacr.azurecr.io/azure-dev:pr-100

@wbreza wbreza merged commit 40b8a3e into main Jul 15, 2022
@wbreza wbreza deleted the wbreza/docker-options branch July 15, 2022 19:52
ellismg added a commit to ellismg/azure-dev that referenced this pull request Jul 19, 2022
@ellismg
Update CHANGELOG.md for Azure#100
f6133dc
ellismg added a commit that referenced this pull request Jul 22, 2022
@ellismg
Update CHANGELOG.md for #100 (#139)
ef408a0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jongio jongio jongio approved these changes

@ellismg ellismg ellismg approved these changes

@karolz-ms karolz-ms karolz-ms approved these changes

@vhvb1989 vhvb1989 vhvb1989 approved these changes

@hemarina hemarina Awaiting requested review from hemarina hemarina is a code owner

@weikanglim weikanglim Awaiting requested review from weikanglim weikanglim is a code owner

Assignees

@wbreza wbreza

Labels
cli
Projects
None yet
Milestone
Release 0.2
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@wbreza @ellismg @nishanil @azure-sdk @jongio @karolz-ms @vhvb1989