-
Notifications
You must be signed in to change notification settings - Fork 318
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 5 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,8 @@ 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/ioc" | ||
| "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" | ||
|
|
@@ -66,9 +68,9 @@ func newDownCmd() *cobra.Command { | |
| type downAction struct { | ||
| flags *downFlags | ||
| args []string | ||
| provisionManager *provisioning.Manager | ||
| serviceLocator ioc.ServiceLocator | ||
| importManager *project.ImportManager | ||
| env *environment.Environment | ||
| lazyEnv *lazy.Lazy[*environment.Environment] | ||
| envManager environment.Manager | ||
| console input.Console | ||
| projectConfig *project.ProjectConfig | ||
|
|
@@ -78,8 +80,8 @@ type downAction struct { | |
| func newDownAction( | ||
| args []string, | ||
| flags *downFlags, | ||
| provisionManager *provisioning.Manager, | ||
| env *environment.Environment, | ||
| serviceLocator ioc.ServiceLocator, | ||
| lazyEnv *lazy.Lazy[*environment.Environment], | ||
| envManager environment.Manager, | ||
| projectConfig *project.ProjectConfig, | ||
| console input.Console, | ||
|
|
@@ -88,8 +90,8 @@ func newDownAction( | |
| ) actions.Action { | ||
| return &downAction{ | ||
| flags: flags, | ||
| provisionManager: provisionManager, | ||
| env: env, | ||
| serviceLocator: serviceLocator, | ||
| lazyEnv: lazyEnv, | ||
| envManager: envManager, | ||
| console: console, | ||
| projectConfig: projectConfig, | ||
|
|
@@ -108,6 +110,39 @@ func (a *downAction) Run(ctx context.Context) (*actions.ActionResult, error) { | |
|
|
||
| startTime := time.Now() | ||
|
|
||
| // Check if there are any environments before proceeding | ||
| 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.
|
||
| if errors.Is(err, environment.ErrNotFound) { | ||
| return nil, fmt.Errorf( | ||
| "environment not found. Run \"azd env list\" to see available environments, " + | ||
| "\"azd env new\" to create a new one, or specify a valid environment name with -e", | ||
| ) | ||
| } | ||
| if errors.Is(err, environment.ErrNameNotSpecified) { | ||
| return nil, errors.New( | ||
| "no environment selected. Use \"azd env select\" to set a default environment, " + | ||
| "or run \"azd down -e <name>\" to target a specific environment", | ||
| ) | ||
| } | ||
| return nil, err | ||
| } | ||
|
|
||
| // Resolve provisioning manager after env check to avoid premature interactive prompts | ||
| var provisionManager *provisioning.Manager | ||
| if err := a.serviceLocator.Resolve(&provisionManager); 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 +176,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 +190,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,134 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "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/ioc" | ||
| "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" | ||
| ) | ||
|
|
||
| // failServiceLocator is a test helper that fails the test if any IoC resolution is attempted. | ||
| type failServiceLocator struct { | ||
| t *testing.T | ||
| } | ||
|
|
||
| func (f *failServiceLocator) Resolve(instance any) error { | ||
| f.t.Fatal("serviceLocator.Resolve should not be called") | ||
| return nil | ||
| } | ||
|
|
||
| func (f *failServiceLocator) ResolveNamed(name string, instance any) error { | ||
| f.t.Fatal("serviceLocator.ResolveNamed should not be called") | ||
| return nil | ||
| } | ||
|
|
||
| func (f *failServiceLocator) Invoke(resolver any) error { | ||
| f.t.Fatal("serviceLocator.Invoke should not be called") | ||
| return nil | ||
| } | ||
|
|
||
| var _ ioc.ServiceLocator = (*failServiceLocator)(nil) | ||
|
|
||
| func newTestDownAction( | ||
| t *testing.T, | ||
| mockContext *mocks.MockContext, | ||
| envManager *mockenv.MockEnvManager, | ||
| lazyEnv *lazy.Lazy[*environment.Environment], | ||
| serviceLocator ioc.ServiceLocator, | ||
| ) *downAction { | ||
| t.Helper() | ||
| if serviceLocator == nil { | ||
| serviceLocator = &failServiceLocator{t: t} | ||
| } | ||
| alphaManager := alpha.NewFeaturesManagerWithConfig(nil) | ||
| action := newDownAction( | ||
| []string{}, | ||
| &downFlags{}, | ||
| serviceLocator, | ||
| lazyEnv, | ||
| envManager, | ||
| &project.ProjectConfig{}, | ||
| mockContext.Console, | ||
| alphaManager, | ||
| project.NewImportManager(nil), | ||
| ) | ||
| return action.(*downAction) | ||
| } | ||
|
|
||
| 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 and serviceLocator must NOT be called when no environments exist | ||
| lazyEnv := lazy.NewLazy(func() (*environment.Environment, error) { | ||
| t.Fatal("lazyEnv should not be evaluated when no environments exist") | ||
| return nil, nil | ||
| }) | ||
|
|
||
| action := newTestDownAction(t, mockContext, envManager, lazyEnv, nil) | ||
|
|
||
| _, err := action.Run(*mockContext.Context) | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), "no environments found") | ||
|
|
||
| envManager.AssertExpectations(t) | ||
| } | ||
|
|
||
| func Test_DownAction_EnvironmentNotFound_ReturnsError(t *testing.T) { | ||
| mockContext := mocks.NewMockContext(context.Background()) | ||
|
|
||
| envManager := &mockenv.MockEnvManager{} | ||
| envManager.On("List", mock.Anything). | ||
| Return([]*environment.Description{{Name: "some-env"}}, nil) | ||
|
|
||
| // Simulate -e flag pointing to a missing environment | ||
| lazyEnv := lazy.NewLazy(func() (*environment.Environment, error) { | ||
| return nil, fmt.Errorf("'missing-env': %w", environment.ErrNotFound) | ||
| }) | ||
|
|
||
| action := newTestDownAction(t, mockContext, envManager, lazyEnv, nil) | ||
|
|
||
| _, err := action.Run(*mockContext.Context) | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), "environment not found") | ||
| require.Contains(t, err.Error(), "azd env list") | ||
|
|
||
| envManager.AssertExpectations(t) | ||
| } | ||
|
|
||
| func Test_DownAction_NoDefaultEnvironment_ReturnsError(t *testing.T) { | ||
| mockContext := mocks.NewMockContext(context.Background()) | ||
|
|
||
| envManager := &mockenv.MockEnvManager{} | ||
| envManager.On("List", mock.Anything). | ||
| Return([]*environment.Description{{Name: "env1"}, {Name: "env2"}}, nil) | ||
|
|
||
| // No -e flag and no default environment set | ||
| lazyEnv := lazy.NewLazy(func() (*environment.Environment, error) { | ||
| return nil, environment.ErrNameNotSpecified | ||
| }) | ||
|
|
||
| action := newTestDownAction(t, mockContext, envManager, lazyEnv, nil) | ||
|
|
||
| _, err := action.Run(*mockContext.Context) | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), "no environment selected") | ||
| require.Contains(t, err.Error(), "azd env select") | ||
|
|
||
| envManager.AssertExpectations(t) | ||
| } | ||
|
vhvb1989 marked this conversation as resolved.
|
||
Uh oh!
There was an error while loading. Please reload this page.