-
Notifications
You must be signed in to change notification settings - Fork 321
fix: azd down returns error instead of prompting init steps when no environment exists #6850
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
Changes from 3 commits
51d38b9
75e6317
8996252
06fa2aa
9c092ff
7ec387e
d8c9ec5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ import ( | |
| inf "github.com/azure/azure-dev/cli/azd/pkg/infra" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/input" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/lazy" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/output" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/output/ux" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/project" | ||
|
|
@@ -64,38 +65,38 @@ func newDownCmd() *cobra.Command { | |
| } | ||
|
|
||
| type downAction struct { | ||
| flags *downFlags | ||
| args []string | ||
| provisionManager *provisioning.Manager | ||
| importManager *project.ImportManager | ||
| env *environment.Environment | ||
| envManager environment.Manager | ||
| console input.Console | ||
| projectConfig *project.ProjectConfig | ||
| alphaFeatureManager *alpha.FeatureManager | ||
| flags *downFlags | ||
| args []string | ||
| lazyProvisionManager *lazy.Lazy[*provisioning.Manager] | ||
| importManager *project.ImportManager | ||
| lazyEnv *lazy.Lazy[*environment.Environment] | ||
| envManager environment.Manager | ||
| console input.Console | ||
| projectConfig *project.ProjectConfig | ||
| alphaFeatureManager *alpha.FeatureManager | ||
| } | ||
|
|
||
| func newDownAction( | ||
| args []string, | ||
| flags *downFlags, | ||
| provisionManager *provisioning.Manager, | ||
| env *environment.Environment, | ||
| lazyProvisionManager *lazy.Lazy[*provisioning.Manager], | ||
| lazyEnv *lazy.Lazy[*environment.Environment], | ||
| envManager environment.Manager, | ||
| projectConfig *project.ProjectConfig, | ||
| console input.Console, | ||
| alphaFeatureManager *alpha.FeatureManager, | ||
| importManager *project.ImportManager, | ||
| ) actions.Action { | ||
| return &downAction{ | ||
| flags: flags, | ||
| provisionManager: provisionManager, | ||
| env: env, | ||
| envManager: envManager, | ||
| console: console, | ||
| projectConfig: projectConfig, | ||
| importManager: importManager, | ||
| alphaFeatureManager: alphaFeatureManager, | ||
| args: args, | ||
| flags: flags, | ||
| lazyProvisionManager: lazyProvisionManager, | ||
| lazyEnv: lazyEnv, | ||
| envManager: envManager, | ||
| console: console, | ||
| projectConfig: projectConfig, | ||
| importManager: importManager, | ||
| alphaFeatureManager: alphaFeatureManager, | ||
| args: args, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -108,6 +109,27 @@ func (a *downAction) Run(ctx context.Context) (*actions.ActionResult, error) { | |
|
|
||
| startTime := time.Now() | ||
|
|
||
| // Check if there are any environments before proceeding | ||
|
vhvb1989 marked this conversation as resolved.
Outdated
|
||
| envList, err := a.envManager.List(ctx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("listing environments: %w", err) | ||
| } | ||
| if len(envList) == 0 { | ||
|
vhvb1989 marked this conversation as resolved.
Outdated
|
||
| return nil, errors.New("no environments found. Run \"azd init\" or \"azd env new\" to create one") | ||
| } | ||
|
vhvb1989 marked this conversation as resolved.
Outdated
vhvb1989 marked this conversation as resolved.
Outdated
|
||
|
|
||
| // Get the environment non-interactively (respects -e flag or default environment) | ||
| env, err := a.lazyEnv.GetValue() | ||
| if err != nil { | ||
|
vhvb1989 marked this conversation as resolved.
vhvb1989 marked this conversation as resolved.
|
||
| return nil, err | ||
| } | ||
|
|
||
| // Get the provisioning manager (resolved lazily to avoid premature env prompts) | ||
| provisionManager, err := a.lazyProvisionManager.GetValue() | ||
| if err != nil { | ||
| return nil, fmt.Errorf("getting provisioning manager: %w", err) | ||
| } | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [High]
Per @vhvb1989's earlier review comment,
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot fix this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in d8c9ec5. Changed |
||
| infra, err := a.importManager.ProjectInfrastructure(ctx, a.projectConfig) | ||
| if err != nil { | ||
| return nil, err | ||
|
|
@@ -141,12 +163,12 @@ func (a *downAction) Run(ctx context.Context) (*actions.ActionResult, error) { | |
| } | ||
|
|
||
| layer.Mode = provisioning.ModeDestroy | ||
| if err := a.provisionManager.Initialize(ctx, a.projectConfig.Path, layer); err != nil { | ||
| if err := provisionManager.Initialize(ctx, a.projectConfig.Path, layer); err != nil { | ||
| return nil, fmt.Errorf("initializing provisioning manager: %w", err) | ||
| } | ||
|
|
||
| destroyOptions := provisioning.NewDestroyOptions(a.flags.forceDelete, a.flags.purgeDelete) | ||
| _, err := a.provisionManager.Destroy(ctx, destroyOptions) | ||
| _, err := provisionManager.Destroy(ctx, destroyOptions) | ||
| if errors.Is(err, inf.ErrDeploymentsNotFound) || errors.Is(err, inf.ErrDeploymentResourcesNotFound) { | ||
| a.console.MessageUxItem(ctx, &ux.DoneMessage{Message: "No Azure resources were found."}) | ||
| } else if err != nil { | ||
|
|
@@ -155,7 +177,7 @@ func (a *downAction) Run(ctx context.Context) (*actions.ActionResult, error) { | |
| } | ||
|
|
||
| // Invalidate cache after successful down so azd show will refresh | ||
| if err := a.envManager.InvalidateEnvCache(ctx, a.env.Name()); err != nil { | ||
| if err := a.envManager.InvalidateEnvCache(ctx, env.Name()); err != nil { | ||
| log.Printf("warning: failed to invalidate state cache: %v", err) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/azure/azure-dev/cli/azd/pkg/alpha" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/environment" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/lazy" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/project" | ||
| "github.com/azure/azure-dev/cli/azd/test/mocks" | ||
| "github.com/azure/azure-dev/cli/azd/test/mocks/mockenv" | ||
| "github.com/stretchr/testify/mock" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func Test_DownAction_NoEnvironments_ReturnsError(t *testing.T) { | ||
| mockContext := mocks.NewMockContext(context.Background()) | ||
|
|
||
| envManager := &mockenv.MockEnvManager{} | ||
| envManager.On("List", mock.Anything). | ||
| Return([]*environment.Description{}, nil) | ||
|
|
||
| lazyEnv := lazy.NewLazy(func() (*environment.Environment, error) { | ||
| return environment.New("test-env"), nil | ||
| }) | ||
|
|
||
| lazyProvisionManager := lazy.NewLazy(func() (*provisioning.Manager, error) { | ||
|
vhvb1989 marked this conversation as resolved.
Outdated
|
||
| return nil, nil | ||
| }) | ||
|
|
||
| alphaManager := alpha.NewFeaturesManagerWithConfig(nil) | ||
|
|
||
| action := newDownAction( | ||
| []string{}, | ||
| &downFlags{}, | ||
| lazyProvisionManager, | ||
| lazyEnv, | ||
| envManager, | ||
| &project.ProjectConfig{}, | ||
| mockContext.Console, | ||
| alphaManager, | ||
| project.NewImportManager(nil), | ||
| ) | ||
|
|
||
| _, err := action.Run(*mockContext.Context) | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), "no environments found") | ||
|
|
||
| envManager.AssertExpectations(t) | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.