Skip to content

Commit

Permalink
Fix azd up --template so it can initialize a new project (#1394)
Browse files Browse the repository at this point in the history
In #1296 we changed our logic such that we would inject `AzdContext`
as a dependency of our actions, and expect our IoC container to wire
things up.

This had the side effect of breaking `azd up --template` to initialize
(and then provision and deploy) a new project.

The break comes from the fact that the IoC container will call
`NewAzdContext` as part of building the `deploy` and `infraCreate`
actions, which need to be created because they are dependencies of the
`up` composite action.  However, `NewAzdContext` should not be called
before the project has actually been created (which will happen when
the `up` composite action calls the `init` action), because it looks
for an existing project and if it doesn't find one it fails.

To work around this issue - I've made the infra create and deploy
actions explicitly call `NewAzdContext` so the calls can happen at the
right time.

A regression test has been added (it's a little hacky because we don't
actually care about running the `infra create` or `deploy` parts of
`up` in this test, we just want to ensure that we correctly
initialized via a template.

Fixes #1329
  • Loading branch information
ellismg committed Jan 14, 2023
1 parent 7f992b0 commit 1c23678
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 24 deletions.
8 changes: 2 additions & 6 deletions cli/azd/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
# Release History

## 0.5.0-beta.3 (Unreleased)

### Features Added

### Breaking Changes
## 0.5.0-beta.3 (2023-01-13)

### Bugs Fixed

### Other Changes
- [[#1394]](https://github.com/Azure/azure-dev/pull/1394) Bug when running azd up with a template.

## 0.5.0-beta.2 (2023-01-12)

Expand Down
18 changes: 12 additions & 6 deletions cli/azd/cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ After the deployment is complete, the endpoint is printed. To start the service,
type deployAction struct {
flags *deployFlags
azCli azcli.AzCli
azdCtx *azdcontext.AzdContext
formatter output.Formatter
writer io.Writer
console input.Console
Expand All @@ -96,15 +95,13 @@ func newDeployAction(
flags *deployFlags,
azCli azcli.AzCli,
commandRunner exec.CommandRunner,
azdCtx *azdcontext.AzdContext,
console input.Console,
formatter output.Formatter,
writer io.Writer,
) actions.Action {
return &deployAction{
flags: flags,
azCli: azCli,
azdCtx: azdCtx,
formatter: formatter,
writer: writer,
console: console,
Expand All @@ -118,12 +115,21 @@ type DeploymentResult struct {
}

func (d *deployAction) Run(ctx context.Context) (*actions.ActionResult, error) {
env, err := loadOrInitEnvironment(ctx, &d.flags.environmentName, d.azdCtx, d.console, d.azCli)
// We call `NewAzdContext` here instead of having the value injected because we want to delay the
// walk for the context until this command has started to execute (for example, in the case of `up`,
// the context is not created until the init action actually runs, which is after the infraCreateAction
// object is created.
azdCtx, err := azdcontext.NewAzdContext()
if err != nil {
return nil, err
}

env, err := loadOrInitEnvironment(ctx, &d.flags.environmentName, azdCtx, d.console, d.azCli)
if err != nil {
return nil, fmt.Errorf("loading environment: %w", err)
}

projConfig, err := project.LoadProjectConfig(d.azdCtx.ProjectPath())
projConfig, err := project.LoadProjectConfig(azdCtx.ProjectPath())
if err != nil {
return nil, fmt.Errorf("loading project: %w", err)
}
Expand Down Expand Up @@ -169,7 +175,7 @@ func (d *deployAction) Run(ctx context.Context) (*actions.ActionResult, error) {

stepMessage := fmt.Sprintf("Deploying service %s", svc.Config.Name)
d.console.ShowSpinner(ctx, stepMessage, input.Step)
result, progress := svc.Deploy(ctx, d.azdCtx)
result, progress := svc.Deploy(ctx, azdCtx)

// Report any progress to logs only. Changes for the console are managed by the console object.
// This routine is required to drain all the string messages sent by the `progress`.
Expand Down
16 changes: 11 additions & 5 deletions cli/azd/cmd/infra_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ func newInfraCreateCmd() *cobra.Command {
type infraCreateAction struct {
flags *infraCreateFlags
azCli azcli.AzCli
azdCtx *azdcontext.AzdContext
formatter output.Formatter
writer io.Writer
console input.Console
Expand All @@ -75,7 +74,6 @@ type infraCreateAction struct {
func newInfraCreateAction(
flags *infraCreateFlags,
azCli azcli.AzCli,
azdCtx *azdcontext.AzdContext,
console input.Console,
formatter output.Formatter,
writer io.Writer,
Expand All @@ -84,7 +82,6 @@ func newInfraCreateAction(
return &infraCreateAction{
flags: flags,
azCli: azCli,
azdCtx: azdCtx,
formatter: formatter,
writer: writer,
console: console,
Expand All @@ -93,18 +90,27 @@ func newInfraCreateAction(
}

func (i *infraCreateAction) Run(ctx context.Context) (*actions.ActionResult, error) {
// We call `NewAzdContext` here instead of having the value injected because we want to delay the
// walk for the context until this command has started to execute (for example, in the case of `up`,
// the context is not created until the init action actually runs, which is after the infraCreateAction
// object is created.
azdCtx, err := azdcontext.NewAzdContext()
if err != nil {
return nil, err
}

// Command title
i.console.MessageUxItem(ctx, &ux.MessageTitle{
Title: "Provisioning Azure resources (azd provision)",
TitleNote: "Provisioning Azure resources can take some time"},
)

env, err := loadOrInitEnvironment(ctx, &i.flags.environmentName, i.azdCtx, i.console, i.azCli)
env, err := loadOrInitEnvironment(ctx, &i.flags.environmentName, azdCtx, i.console, i.azCli)
if err != nil {
return nil, fmt.Errorf("loading environment: %w", err)
}

prj, err := project.LoadProjectConfig(i.azdCtx.ProjectPath())
prj, err := project.LoadProjectConfig(azdCtx.ProjectPath())
if err != nil {
return nil, fmt.Errorf("loading project: %w", err)
}
Expand Down
56 changes: 49 additions & 7 deletions cli/azd/test/functional/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,42 @@ func Test_CLI_Init_CanUseTemplate(t *testing.T) {
require.FileExists(t, filepath.Join(dir, "README.md"))
}

// Test_CLI_Up_CanUseTemplateWithoutExistingProject ensures that you can run `azd up --template <some-template>` in an
// empty directory and the project will be initialize as expected.
func Test_CLI_Up_CanUseTemplateWithoutExistingProject(t *testing.T) {
// running this test in parallel is ok as it uses a t.TempDir()
t.Parallel()
ctx, cancel := newTestContext(t)
defer cancel()

dir := tempDirWithDiagnostics(t)

cli := azdcli.NewCLI(t)
cli.WorkingDirectory = dir
cli.Env = append(os.Environ(), "AZURE_LOCATION=eastus2")

// Since we provide a bogus Azure Subscription ID, we expect that this overall command will fail (the provision step of
// up will fail). That's fine - we only care about validating that we were allowed to run `azd up --template` in an
// empty directory and that it brings down the template as expected.
res, _ := cli.RunCommandWithStdIn(
ctx,
"TESTENV\n\nOther (enter manually)\nMY_SUB_ID\n",
"up",
"--template",
"cosmos-dotnet-core-todo-app",
)

require.Contains(t, res.Stdout, "Initializing a new project")

// While `init` uses git behind the scenes to pull a template, we don't want to bring the history over or initialize a
// git
// repository.
require.NoDirExists(t, filepath.Join(dir, ".git"))

// Ensure the project was initialized from the template by checking that a file from the template is present.
require.FileExists(t, filepath.Join(dir, "README.md"))
}

func Test_CLI_InfraCreateAndDelete(t *testing.T) {
// running this test in parallel is ok as it uses a t.TempDir()
t.Parallel()
Expand Down Expand Up @@ -287,26 +323,28 @@ func Test_CLI_ProjectIsNeeded(t *testing.T) {
cli.WorkingDirectory = dir

tests := []struct {
command string
args []string
command string
args []string
errorToStdOut bool
}{
{command: "deploy"},
{command: "deploy", errorToStdOut: true},
{command: "down"},
{command: "env get-values"},
{command: "env list"},
{command: "env new", args: []string{"testEnvironmentName"}},
{command: "env refresh"},
{command: "env select", args: []string{"testEnvironmentName"}},
{command: "env set", args: []string{"testKey", "testValue"}},
{command: "infra create"},
{command: "infra create", errorToStdOut: true},
{command: "infra delete"},
{command: "monitor"},
{command: "pipeline config"},
{command: "provision"},
{command: "provision", errorToStdOut: true},
{command: "restore"},
}

for _, test := range tests {
for _, tt := range tests {
test := tt
args := []string{"--cwd", dir}
args = append(args, strings.Split(test.command, " ")...)
if len(test.args) > 0 {
Expand All @@ -316,7 +354,11 @@ func Test_CLI_ProjectIsNeeded(t *testing.T) {
t.Run(test.command, func(t *testing.T) {
result, err := cli.RunCommand(ctx, args...)
assert.Error(t, err)
assert.Contains(t, result.Stderr, azdcontext.ErrNoProject.Error())
if test.errorToStdOut {
assert.Contains(t, result.Stdout, azdcontext.ErrNoProject.Error())
} else {
assert.Contains(t, result.Stderr, azdcontext.ErrNoProject.Error())
}
})
}
}
Expand Down

0 comments on commit 1c23678

Please sign in to comment.