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

Infra Provision/Deploy provider model #172

Merged
merged 27 commits into from
Aug 11, 2022
Merged

Infra Provision/Deploy provider model #172

merged 27 commits into from
Aug 11, 2022

Conversation

wbreza
Copy link
Contributor

@wbreza wbreza commented Jul 25, 2022 •
edited

The is the start of the new infrastructure provisioning provider model. This is primarily net new code with a couple minor updates to existing files.

The goal of this PR is to abstract much of the infrastructure provisioning out into a provider interface with a new Bicep implementation. This will need to be tweaked as we continue to understand the differences between different infra providers like terraform, bicep, etc.

Contains the following:

  • New Bicep infra provider implementation
  • New async task model to make it easier to consume long running tasks/operations with progress reporting
  • New approach to mocking and setting up mock definitions that call into dependent executil compatible tools
  • New infrastructure provisioning manager that will greatly reduce the amount of code required in the infra create/delete commands

What happens next?
After this PR, will start on refactoring the existing infra create/delete command to replace the inline bicep implementation with the new Bicep infra provider / provisioning manager code.

@wbreza wbreza changed the title Provision provider modle WIP: Provision provider model Jul 25, 2022
@jongio jongio added this to the Next milestone Jul 26, 2022
ellismg
ellismg reviewed Jul 27, 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.

A few small comments about some bigger stuff around the interface since I think it may need some slight tweaks, but overall looks correct directionally, great job.

cli/azd/pkg/async/async_task.go Outdated Show resolved Hide resolved
cli/azd/pkg/infra/provisioning/provisioning_plan.go Outdated Show resolved Hide resolved
cli/azd/pkg/infra/provisioning/provisioning_plan.go Outdated Show resolved Hide resolved
cli/azd/pkg/infra/provisioning/infra_provider.go Outdated Show resolved Hide resolved
This was linked to issues Aug 3, 2022
Infra provider support #227
Closed
Async Task model for long running operations #228
Closed
Mocking model for better test development #229
Closed
Bicep infra provider #240
Closed
@wbreza wbreza changed the title WIP: Provision provider model Infra Provision/Deploy provider model Aug 3, 2022
@wbreza wbreza marked this pull request as ready for review August 3, 2022 23:19
vhvb1989
vhvb1989 reviewed Aug 4, 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.

partial review

cli/azd/pkg/async/task.go Show resolved Hide resolved
cli/azd/pkg/async/task.go Show resolved Hide resolved
cli/azd/pkg/async/task_context.go Outdated Show resolved Hide resolved
cli/azd/pkg/async/task.go Outdated Show resolved Hide resolved
cli/azd/pkg/async/task_context.go Outdated Show resolved Hide resolved
cli/azd/pkg/async/task.go Show resolved Hide resolved
cli/azd/pkg/async/task.go Show resolved Hide resolved
cli/azd/pkg/async/task.go Outdated Show resolved Hide resolved
cli/azd/pkg/async/task_test.go Outdated Show resolved Hide resolved
cli/azd/pkg/async/task_test.go Outdated Show resolved Hide resolved
vhvb1989
vhvb1989 reviewed Aug 4, 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.

Looks like we are duplicating the Asker? in cmd and now in pkg/input ??
Can we keep just the one from pgk and make cmd to use it from there?

cli/azd/pkg/infra/azure_resource_manager.go Show resolved Hide resolved
cli/azd/pkg/infra/azure_resource_manager.go Show resolved Hide resolved
cli/azd/pkg/infra/provisioning/provider.go Show resolved Hide resolved
cli/azd/pkg/input/asker.go Show resolved Hide resolved
karolz-ms
karolz-ms reviewed Aug 4, 2022
View reviewed changes
Copy link
Contributor

@karolz-ms karolz-ms left a comment

Choose a reason for hiding this comment

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

Very nice overall; having the provider interface will help us make the code more modular.

A few questions/suggestions--HTH!

cli/azd/pkg/async/task_context.go Show resolved Hide resolved
cli/azd/pkg/async/task.go Show resolved Hide resolved
cli/azd/pkg/async/task_context.go Show resolved Hide resolved
cli/azd/pkg/async/task_context.go Show resolved Hide resolved
cli/azd/pkg/infra/provisioning/bicep_provider.go Outdated Show resolved Hide resolved
ellismg
ellismg approved these changes Aug 11, 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.

Nice changes - thanks for introducing the interface here - excited to see it evolve as we bring other IaC providers on line!

@wbreza wbreza enabled auto-merge (squash) August 11, 2022 19:12
@azure-sdk
Copy link
Collaborator

Azure Dev CLI Install Instructions

Install scripts

MacOS/Linux

May elevate using sudo on some platforms and configurations

bash:

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

pwsh:

Invoke-RestMethod 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/172/uninstall-azd.ps1' -OutFile uninstall-azd.ps1; ./uninstall-azd.ps1
Invoke-RestMethod 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/172/install-azd.ps1' -OutFile install-azd.ps1; ./install-azd.ps1 -BaseUrl 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/172' -Version '' -Verbose

Windows

powershell -c "Set-ExecutionPolicy Bypass Process; irm 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/172/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/172/install-azd.ps1' > install-azd.ps1; ./install-azd.ps1 -BaseUrl 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/172' -Version '' -Verbose;"

Standalone Binary

Container

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

@wbreza wbreza merged commit ae649ca into Azure:main Aug 11, 2022
@wbreza wbreza deleted the provision-provider branch August 11, 2022 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weikanglim weikanglim weikanglim left review comments

@vhvb1989 vhvb1989 vhvb1989 left review comments

@ellismg ellismg ellismg approved these changes

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

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

@jongio jongio Awaiting requested review from jongio

Assignees

@wbreza wbreza

Labels
None yet
Projects
None yet
Milestone
Release 0.2
7 participants
@wbreza @azure-sdk @weikanglim @ellismg @karolz-ms @vhvb1989 @jongio