diff --git a/cmd/rad/cmd/root.go b/cmd/rad/cmd/root.go index 89c15f849e4..5234dfe86d6 100644 --- a/cmd/rad/cmd/root.go +++ b/cmd/rad/cmd/root.go @@ -381,8 +381,8 @@ func initSubCommands() { envDeleteCmd, _ := env_delete.NewCommand(framework) previewDeleteCmd, _ := env_delete_preview.NewCommand(framework) - wirePreviewSubcommand(envDeleteCmd, previewDeleteCmd) - envCmd.AddCommand(envDeleteCmd) + wirePreviewSubcommandPreviewBase(previewDeleteCmd, envDeleteCmd.RunE, "Use the Radius.Core preview implementation for environment delete", "force") + envCmd.AddCommand(previewDeleteCmd) envListCmd, _ := env_list.NewCommand(framework) previewListCmd, _ := env_list_preview.NewCommand(framework) diff --git a/cmd/rad/cmd/root_test.go b/cmd/rad/cmd/root_test.go index 8231fd9d0b1..f6e2b079292 100644 --- a/cmd/rad/cmd/root_test.go +++ b/cmd/rad/cmd/root_test.go @@ -437,6 +437,36 @@ func Test_ResourceList_ExposesPreviewFlag(t *testing.T) { require.NotNil(t, listCmd.Flags().Lookup("preview"), "rad resource list must expose --preview") } +// Test_EnvDelete_ExposesPreviewAndForceFlags asserts against the fully-assembled command tree that +// `rad env delete` exposes both flags. The preview runner reads --force during Validate, so if the +// wired command does not declare it, every `rad env delete --preview` invocation fails with +// "flag accessed but not defined: force". +func Test_EnvDelete_ExposesPreviewAndForceFlags(t *testing.T) { + deleteCmd, _, err := RootCmd.Find([]string{"env", "delete"}) + require.NoError(t, err) + require.Equal(t, "delete", deleteCmd.Name()) + require.NotNil(t, deleteCmd.Flags().Lookup("preview"), "rad env delete must expose --preview") + require.NotNil(t, deleteCmd.Flags().Lookup("force"), "rad env delete must expose --force") +} + +// Test_EnvDelete_ExposesFlagsLegacyRunnerReads guards the coupling created by registering the +// preview command as the base for `rad env delete`: the legacy runner now validates against the +// preview command's flag set. If any of these flags is dropped from the preview command, plain +// `rad env delete` (without --preview) breaks at runtime with "flag accessed but not defined". +// Test_EnvPreviewOnlyFlagsRejectedWithoutPreview cannot catch this, because it returns in the +// --force guard before it ever reaches the legacy runner. +func Test_EnvDelete_ExposesFlagsLegacyRunnerReads(t *testing.T) { + deleteCmd, _, err := RootCmd.Find([]string{"env", "delete"}) + require.NoError(t, err) + + // Read by the legacy runner's Validate, directly or via cli.RequireWorkspace, + // cli.RequireEnvironmentNameArgs and cli.RequireOutput. + for _, flag := range []string{"workspace", "group", "environment", "yes", "output"} { + require.NotNilf(t, deleteCmd.Flags().Lookup(flag), + "rad env delete must expose --%s: the legacy runner reads it during Validate", flag) + } +} + // Test_EnvPreviewOnlyFlagsRejectedWithoutPreview drives the real, fully-assembled command tree // and asserts that each preview-only flag is rejected when preview mode is off. This guards // against a preview-only flag being added to a command without also being registered in the @@ -452,6 +482,7 @@ func Test_EnvPreviewOnlyFlagsRejectedWithoutPreview(t *testing.T) { {command: "create", flag: "recipe-packs", value: "p1"}, {command: "update", flag: "recipe-packs", value: "p1"}, {command: "update", flag: "clear-kubernetes", value: "true"}, + {command: "delete", flag: "force", value: "true"}, } for _, tc := range testcases { diff --git a/pkg/cli/clients/clients.go b/pkg/cli/clients/clients.go index c857ad9d63d..678bd1e8282 100644 --- a/pkg/cli/clients/clients.go +++ b/pkg/cli/clients/clients.go @@ -176,6 +176,14 @@ type ApplicationsManagementClient interface { // Names target Applications.Core; use a full resource ID for other environment types. ListResourcesInEnvironment(ctx context.Context, environmentNameOrID string) ([]generated.GenericResource, error) + // ListResourcesInEnvironmentOrApplications lists the resources that belong to the given + // environment or to any of the given applications, without duplicates. + // + // This is equivalent to merging ListResourcesInEnvironment with ListResourcesInApplication for + // each application, but enumerates each resource type once instead of once per application. + // Names target Applications.Core; use full resource IDs for other environment and application types. + ListResourcesInEnvironmentOrApplications(ctx context.Context, environmentNameOrID string, applicationNameOrIDs []string) ([]generated.GenericResource, error) + // GetResource retrieves a resource by its type and name (or id). GetResource(ctx context.Context, resourceType string, resourceNameOrID string) (generated.GenericResource, error) diff --git a/pkg/cli/clients/management.go b/pkg/cli/clients/management.go index 0558c35c7ee..5058cf1bc6f 100644 --- a/pkg/cli/clients/management.go +++ b/pkg/cli/clients/management.go @@ -1162,6 +1162,68 @@ func (amc *UCPApplicationsManagementClient) ListResourcesInEnvironment(ctx conte return results, nil } +// ListResourcesInEnvironmentOrApplications lists the resources that belong to the given environment +// or to any of the given applications, without duplicates. +// +// This produces the same set as merging ListResourcesInEnvironment with ListResourcesInApplication +// for each application, but enumerates each resource type once rather than once per application. +// The per-type listing returns every resource of that type in the scope and both membership checks +// run client side, so repeating the listing per application re-fetches identical data. +func (amc *UCPApplicationsManagementClient) ListResourcesInEnvironmentOrApplications(ctx context.Context, environmentNameOrID string, applicationNameOrIDs []string) ([]generated.GenericResource, error) { + environmentID, err := amc.fullyQualifyID(environmentNameOrID, "Applications.Core/environments") + if err != nil { + return nil, err + } + + applicationIDs := make([]string, 0, len(applicationNameOrIDs)) + for _, applicationNameOrID := range applicationNameOrIDs { + applicationID, err := amc.fullyQualifyID(applicationNameOrID, "Applications.Core/applications") + if err != nil { + return nil, err + } + + applicationIDs = append(applicationIDs, applicationID) + } + + resourceTypesList, err := amc.ListAllResourceTypesNames(ctx, "local") + if err != nil { + return nil, err + } + + results := []generated.GenericResource{} + for _, resourceType := range resourceTypesList { + resources, err := amc.ListResourcesOfType(ctx, resourceType) + if err != nil { + return nil, err + } + + for _, resource := range resources { + if isResourceInEnvironmentOrApplications(resource, environmentID, applicationIDs) { + results = append(results, resource) + } + } + } + + return results, nil +} + +// isResourceInEnvironmentOrApplications reports whether the resource belongs to the given +// environment or to any of the given applications. A resource is appended at most once by the +// caller, so the two directions cannot produce duplicates. +func isResourceInEnvironmentOrApplications(resource generated.GenericResource, environmentID string, applicationIDs []string) bool { + if isResourceInEnvironment(resource, environmentID) { + return true + } + + for _, applicationID := range applicationIDs { + if isResourceInApplication(resource, applicationID) { + return true + } + } + + return false +} + // CreateOrUpdateResourceType creates or updates a resource type in the configured plane. func (amc *UCPApplicationsManagementClient) CreateOrUpdateResourceType(ctx context.Context, planeName string, resourceProviderName string, resourceTypeName string, resource *ucpv20231001.ResourceTypeResource) (ucpv20231001.ResourceTypeResource, error) { client, err := amc.createResourceTypeClient() diff --git a/pkg/cli/clients/management_test.go b/pkg/cli/clients/management_test.go index d60a94406c0..b9a9e563657 100644 --- a/pkg/cli/clients/management_test.go +++ b/pkg/cli/clients/management_test.go @@ -591,6 +591,80 @@ func Test_Resource(t *testing.T) { require.Equal(t, expectedResourceList, resources) }) + // ListResourcesInEnvironmentOrApplications replaces one ListResourcesInEnvironment call plus one + // ListResourcesInApplication call per application with a single pass over the resource types. + // These cases pin the two properties the cascade delete depends on: the result is the union of + // both membership directions, and a resource matching both directions is returned only once. + newListEnvironmentOrApplicationsClient := func(t *testing.T) *UCPApplicationsManagementClient { + mockResourceClient := NewMockgenericResourceClient(gomock.NewController(t)) + mockResourceProviderClient := NewMockresourceProviderClient(gomock.NewController(t)) + + client := createResourceAndResourceProviderClient(mockResourceClient, mockResourceProviderClient) + + mockResourceProviderClient.EXPECT().NewListProviderSummariesPager("local", gomock.Any()).Return(pager(resourceProviderSummaryPages)) + mockResourceClient.EXPECT(). + NewListByRootScopePager(gomock.Any()). + Return(pager(listPages)).AnyTimes() + mockResourceProviderClient.EXPECT(). + GetProviderSummary(gomock.Any(), "local", gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx context.Context, plane string, providerName string, opts *ucp.ResourceProvidersClientGetProviderSummaryOptions) (ucp.ResourceProvidersClientGetProviderSummaryResponse, error) { + summary := findProviderSummary(providerName) + if summary != nil { + return ucp.ResourceProvidersClientGetProviderSummaryResponse{ + ResourceProviderSummary: *summary, + }, nil + } + + // Fallback for providers not in test data + return ucp.ResourceProvidersClientGetProviderSummaryResponse{ + ResourceProviderSummary: ucp.ResourceProviderSummary{ + Name: &providerName, + ResourceTypes: map[string]*ucp.ResourceProviderSummaryResourceType{ + "resourceType" + string(providerName[len(providerName)-1]): { + APIVersions: map[string]*ucp.ResourceTypeSummaryResultAPIVersion{ + version: {}, + }, + }, + }, + }, + }, nil + }).AnyTimes() + + return client + } + + t.Run("ListResourcesInEnvironmentOrApplications", func(t *testing.T) { + client := newListEnvironmentOrApplicationsClient(t) + + // test1 belongs to both the environment and the application, test2 only to the environment. + // test1 must appear exactly once even though both membership checks match it. + expectedResourceList := []generated.GenericResource{*listPages[0].Value[0], *listPages[0].Value[1]} + + resources, err := client.ListResourcesInEnvironmentOrApplications(t.Context(), "test-environment", []string{"test-application"}) + require.NoError(t, err) + require.Equal(t, expectedResourceList, resources) + }) + + t.Run("ListResourcesInEnvironmentOrApplications with no applications", func(t *testing.T) { + client := newListEnvironmentOrApplicationsClient(t) + + // An environment with no applications must still return its own resources. + expectedResourceList := []generated.GenericResource{*listPages[0].Value[0], *listPages[0].Value[1]} + + resources, err := client.ListResourcesInEnvironmentOrApplications(t.Context(), "test-environment", []string{}) + require.NoError(t, err) + require.Equal(t, expectedResourceList, resources) + }) + + t.Run("ListResourcesInEnvironmentOrApplications ignores other environments and applications", func(t *testing.T) { + client := newListEnvironmentOrApplicationsClient(t) + + // test3 and test4 live in a different scope, so neither membership check matches them. + resources, err := client.ListResourcesInEnvironmentOrApplications(t.Context(), "other-environment", []string{"other-application"}) + require.NoError(t, err) + require.Empty(t, resources) + }) + t.Run("GetResource", func(t *testing.T) { ctrl := gomock.NewController(t) mock := NewMockgenericResourceClient(ctrl) @@ -2689,6 +2763,172 @@ func setCapture(ctx context.Context, response *http.Response) { } } +// Test_isResourceInApplication covers the ownership matching that rad app delete and +// rad env delete rely on to find the resources owned by an application. The match must be +// case-insensitive, because resource IDs are not case-normalized on the wire. +func Test_isResourceInApplication(t *testing.T) { + applicationID := "/planes/radius/local/resourceGroups/test-group/providers/Radius.Core/applications/test-app" + + testcases := []struct { + name string + properties map[string]any + expected bool + }{ + { + name: "exact match", + properties: map[string]any{"application": applicationID}, + expected: true, + }, + { + name: "case-insensitive match", + properties: map[string]any{"application": strings.ToUpper(applicationID)}, + expected: true, + }, + { + name: "different application", + properties: map[string]any{"application": applicationID + "-other"}, + expected: false, + }, + { + name: "no application property", + properties: map[string]any{}, + expected: false, + }, + { + name: "empty application property", + properties: map[string]any{"application": ""}, + expected: false, + }, + { + name: "non-string application property", + properties: map[string]any{"application": 42}, + expected: false, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + resource := generated.GenericResource{Properties: tc.properties} + require.Equal(t, tc.expected, isResourceInApplication(resource, applicationID)) + }) + } +} + +// Test_isResourceInEnvironment covers the environment matching that rad env delete relies on +// to find the resources deployed into an environment. +func Test_isResourceInEnvironment(t *testing.T) { + environmentID := "/planes/radius/local/resourceGroups/test-group/providers/Radius.Core/environments/test-env" + + testcases := []struct { + name string + properties map[string]any + expected bool + }{ + { + name: "exact match", + properties: map[string]any{"environment": environmentID}, + expected: true, + }, + { + name: "case-insensitive match", + properties: map[string]any{"environment": strings.ToUpper(environmentID)}, + expected: true, + }, + { + name: "different environment", + properties: map[string]any{"environment": environmentID + "-other"}, + expected: false, + }, + { + name: "no environment property", + properties: map[string]any{}, + expected: false, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + resource := generated.GenericResource{Properties: tc.properties} + require.Equal(t, tc.expected, isResourceInEnvironment(resource, environmentID)) + }) + } +} + +// Test_isResourceInEnvironmentOrApplications covers the combined membership check used by the +// single-pass listing behind rad env delete's cascade. A resource is in scope when it belongs to +// the environment or to any of the applications being deleted. +func Test_isResourceInEnvironmentOrApplications(t *testing.T) { + environmentID := "/planes/radius/local/resourceGroups/test-group/providers/Radius.Core/environments/test-env" + applicationID := "/planes/radius/local/resourceGroups/test-group/providers/Radius.Core/applications/test-app" + otherApplicationID := "/planes/radius/local/resourceGroups/test-group/providers/Radius.Core/applications/other-app" + + testcases := []struct { + name string + properties map[string]any + applicationIDs []string + expected bool + }{ + { + name: "environment match only", + properties: map[string]any{"environment": environmentID}, + applicationIDs: []string{applicationID}, + expected: true, + }, + { + name: "application match only", + properties: map[string]any{"application": applicationID}, + applicationIDs: []string{applicationID}, + expected: true, + }, + { + name: "matches a later application in the list", + properties: map[string]any{"application": applicationID}, + applicationIDs: []string{otherApplicationID, applicationID}, + expected: true, + }, + { + name: "both directions match", + properties: map[string]any{"application": applicationID, "environment": environmentID}, + applicationIDs: []string{applicationID}, + expected: true, + }, + { + name: "case-insensitive application match", + properties: map[string]any{"application": strings.ToUpper(applicationID)}, + applicationIDs: []string{applicationID}, + expected: true, + }, + { + name: "environment match with no applications", + properties: map[string]any{"environment": environmentID}, + applicationIDs: []string{}, + expected: true, + }, + { + name: "application match ignored when the application is not being deleted", + properties: map[string]any{"application": otherApplicationID}, + applicationIDs: []string{applicationID}, + expected: false, + }, + { + name: "neither direction matches", + properties: map[string]any{"environment": environmentID + "-other"}, + applicationIDs: []string{applicationID}, + expected: false, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + resource := generated.GenericResource{Properties: tc.properties} + require.Equal(t, tc.expected, isResourceInEnvironmentOrApplications(resource, environmentID, tc.applicationIDs)) + }) + } +} + func testCapture(ctx context.Context, capture **http.Response) context.Context { return context.WithValue(ctx, holder{}, &holder{capture}) } diff --git a/pkg/cli/clients/mock_applicationsclient.go b/pkg/cli/clients/mock_applicationsclient.go index fae258f1029..80e721cde1e 100644 --- a/pkg/cli/clients/mock_applicationsclient.go +++ b/pkg/cli/clients/mock_applicationsclient.go @@ -1483,6 +1483,45 @@ func (c *MockApplicationsManagementClientListResourcesInEnvironmentCall) DoAndRe return c } +// ListResourcesInEnvironmentOrApplications mocks base method. +func (m *MockApplicationsManagementClient) ListResourcesInEnvironmentOrApplications(ctx context.Context, environmentNameOrID string, applicationNameOrIDs []string) ([]generated.GenericResource, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListResourcesInEnvironmentOrApplications", ctx, environmentNameOrID, applicationNameOrIDs) + ret0, _ := ret[0].([]generated.GenericResource) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListResourcesInEnvironmentOrApplications indicates an expected call of ListResourcesInEnvironmentOrApplications. +func (mr *MockApplicationsManagementClientMockRecorder) ListResourcesInEnvironmentOrApplications(ctx, environmentNameOrID, applicationNameOrIDs any) *MockApplicationsManagementClientListResourcesInEnvironmentOrApplicationsCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListResourcesInEnvironmentOrApplications", reflect.TypeOf((*MockApplicationsManagementClient)(nil).ListResourcesInEnvironmentOrApplications), ctx, environmentNameOrID, applicationNameOrIDs) + return &MockApplicationsManagementClientListResourcesInEnvironmentOrApplicationsCall{Call: call} +} + +// MockApplicationsManagementClientListResourcesInEnvironmentOrApplicationsCall wrap *gomock.Call +type MockApplicationsManagementClientListResourcesInEnvironmentOrApplicationsCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockApplicationsManagementClientListResourcesInEnvironmentOrApplicationsCall) Return(arg0 []generated.GenericResource, arg1 error) *MockApplicationsManagementClientListResourcesInEnvironmentOrApplicationsCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockApplicationsManagementClientListResourcesInEnvironmentOrApplicationsCall) Do(f func(context.Context, string, []string) ([]generated.GenericResource, error)) *MockApplicationsManagementClientListResourcesInEnvironmentOrApplicationsCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockApplicationsManagementClientListResourcesInEnvironmentOrApplicationsCall) DoAndReturn(f func(context.Context, string, []string) ([]generated.GenericResource, error)) *MockApplicationsManagementClientListResourcesInEnvironmentOrApplicationsCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // ListResourcesInResourceGroup mocks base method. func (m *MockApplicationsManagementClient) ListResourcesInResourceGroup(ctx context.Context, planeName, resourceGroupName string) ([]generated.GenericResource, error) { m.ctrl.T.Helper() diff --git a/pkg/cli/cmd/app/delete/preview/delete.go b/pkg/cli/cmd/app/delete/preview/delete.go index 9c410bc03a7..58efafd17a7 100644 --- a/pkg/cli/cmd/app/delete/preview/delete.go +++ b/pkg/cli/cmd/app/delete/preview/delete.go @@ -22,11 +22,9 @@ import ( "strings" "github.com/spf13/cobra" - "golang.org/x/sync/errgroup" "github.com/radius-project/radius/pkg/cli" "github.com/radius-project/radius/pkg/cli/clients" - generated "github.com/radius-project/radius/pkg/cli/clients_new/generated" "github.com/radius-project/radius/pkg/cli/clierrors" "github.com/radius-project/radius/pkg/cli/cmd" "github.com/radius-project/radius/pkg/cli/cmd/commonflags" @@ -36,7 +34,6 @@ import ( "github.com/radius-project/radius/pkg/cli/prompt" "github.com/radius-project/radius/pkg/cli/workspaces" corerpv20250801 "github.com/radius-project/radius/pkg/corerp/api/v20250801preview" - "github.com/radius-project/radius/pkg/corerp/datamodel" ) const ( @@ -162,6 +159,10 @@ func (r *Runner) Run(ctx context.Context) error { return err } + if r.Force { + r.Output.LogInfo("WARNING: Force deleting an application. Resources in non-terminal states may leave orphaned external resources that require manual cleanup.") + } + if !r.Confirm { promptMsg := fmt.Sprintf("Are you sure you want to delete application '%s'?", r.ApplicationName) confirmed, err := prompt.YesOrNoPrompt(promptMsg, prompt.ConfirmNo, r.InputPrompter) @@ -174,10 +175,6 @@ func (r *Runner) Run(ctx context.Context) error { } } - if r.Force { - r.Output.LogInfo("WARNING: Force deleting an application. Resources in non-terminal states may leave orphaned external resources that require manual cleanup.") - } - // Use the management client to discover and delete owned resources. // This uses ownership-based filtering (properties.application matches our app ID) // rather than GetGraph which returns a connectivity graph that may include shared resources. @@ -187,9 +184,9 @@ func (r *Runner) Run(ctx context.Context) error { } // Build the fully qualified Radius.Core application ID for ownership matching - applicationID := r.Workspace.Scope + "/providers/" + datamodel.ApplicationResourceType_v20250801preview + "/" + r.ApplicationName + applicationID := cmd.PreviewApplicationID(r.Workspace.Scope, r.ApplicationName) - resourcesList, err := listResourcesOwnedByApplication(ctx, managementClient, applicationID) + resourcesList, err := managementClient.ListResourcesInApplication(ctx, applicationID) if err != nil && !clients.Is404Error(err) { return err } @@ -198,26 +195,7 @@ func (r *Runner) Run(ctx context.Context) error { if len(resourcesList) > 0 { r.Output.LogInfo(msgDeletingResources, len(resourcesList), r.ApplicationName) - g, groupCtx := errgroup.WithContext(ctx) - for _, resource := range resourcesList { - if resource.ID != nil && resource.Type != nil { - // Log before launching the goroutine; output.Interface implementations - // (including the MockOutput used in tests) are not guaranteed to be - // thread-safe, and ordering the log here keeps output deterministic. - r.Output.LogInfo(" Deleting %s...", *resource.ID) - resourceType := *resource.Type - resourceID := *resource.ID - g.Go(func() error { - _, err := managementClient.DeleteResource(groupCtx, resourceType, resourceID, r.Force) - if err != nil && !clients.Is404Error(err) { - return err - } - return nil - }) - } - } - - if err := g.Wait(); err != nil { + if err := cmd.DeleteResourcesInParallel(ctx, managementClient, r.Output, resourcesList, r.Force); err != nil { return clierrors.Message("Failed to delete resources for application '%s': %v", r.ApplicationName, err) } } @@ -237,46 +215,3 @@ func (r *Runner) Run(ctx context.Context) error { r.Output.LogInfo(msgApplicationDeletedPreview) return nil } - -// listResourcesOwnedByApplication lists resources whose properties.application field -// matches the given application ID. This is an ownership-based query that only returns -// resources explicitly owned by the application, unlike GetGraph which returns a -// connectivity graph that may include shared/environment resources. -func listResourcesOwnedByApplication(ctx context.Context, client clients.ApplicationsManagementClient, applicationID string) ([]generated.GenericResource, error) { - resourceTypesList, err := client.ListAllResourceTypesNames(ctx, "local") - if err != nil { - return nil, err - } - - var results []generated.GenericResource - for _, resourceType := range resourceTypesList { - resources, err := client.ListResourcesOfType(ctx, resourceType) - if err != nil { - return nil, err - } - - for _, resource := range resources { - if isResourceOwnedByApplication(resource, applicationID) { - results = append(results, resource) - } - } - } - - return results, nil -} - -// isResourceOwnedByApplication checks if a resource's properties.application field -// matches the given application ID (case-insensitive). -func isResourceOwnedByApplication(resource generated.GenericResource, applicationID string) bool { - obj, found := resource.Properties["application"] - if !found { - return false - } - - associatedAppID, ok := obj.(string) - if !ok || associatedAppID == "" { - return false - } - - return strings.EqualFold(associatedAppID, applicationID) -} diff --git a/pkg/cli/cmd/app/delete/preview/delete_test.go b/pkg/cli/cmd/app/delete/preview/delete_test.go index 9f04c0cd5e1..ffbb1643f74 100644 --- a/pkg/cli/cmd/app/delete/preview/delete_test.go +++ b/pkg/cli/cmd/app/delete/preview/delete_test.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "net/http" - "strings" "testing" azfake "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake" @@ -97,8 +96,8 @@ func Test_Validate(t *testing.T) { func mockManagementClientNoResources(ctrl *gomock.Controller) clients.ApplicationsManagementClient { mock := clients.NewMockApplicationsManagementClient(ctrl) mock.EXPECT(). - ListAllResourceTypesNames(gomock.Any(), "local"). - Return([]string{}, nil). + ListResourcesInApplication(gomock.Any(), gomock.Any()). + Return([]generated.GenericResource{}, nil). AnyTimes() return mock } @@ -112,18 +111,11 @@ func mockManagementClientWithResources(ctrl *gomock.Controller, appID string, fo resourceType := "Applications.Datastores/redisCaches" mock.EXPECT(). - ListAllResourceTypesNames(gomock.Any(), "local"). - Return([]string{resourceType}, nil). - Times(1) - mock.EXPECT(). - ListResourcesOfType(gomock.Any(), resourceType). + ListResourcesInApplication(gomock.Any(), appID). Return([]generated.GenericResource{ { ID: &resourceID, Type: &resourceType, - Properties: map[string]any{ - "application": appID, - }, }, }, nil). Times(1) @@ -310,6 +302,50 @@ func Test_Run(t *testing.T) { require.Contains(t, firstLog.Format, "Force deleting an application") }) + t.Run("Success: the force warning is shown before the confirmation prompt", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + factory, err := test_client_factory.NewRadiusCoreTestClientFactory(workspace.Scope, nil, nil, test_client_factory.WithApplicationsServerNoError) + require.NoError(t, err) + + appID := workspace.Scope + "/providers/Radius.Core/applications/test-app" + mockMgmt := mockManagementClientWithResources(ctrl, appID, true) + + outputSink := &output.MockOutput{} + var logsAtPromptTime []any + + // The warning explains that force can leave orphaned external resources, so it is only + // useful if the user sees it before consenting to the delete. + promptMock := prompt.NewMockInterface(ctrl) + promptMock.EXPECT(). + GetListInput(gomock.Any(), gomock.Any()). + DoAndReturn(func(choices []string, promptMsg string) (string, error) { + logsAtPromptTime = append([]any{}, outputSink.Writes...) + return prompt.ConfirmYes, nil + }). + Times(1) + + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: workspace, + Output: outputSink, + InputPrompter: promptMock, + ApplicationName: "test-app", + Confirm: false, + Force: true, + } + + err = runner.Run(t.Context()) + require.NoError(t, err) + + require.NotEmpty(t, logsAtPromptTime, "the force warning must be logged before the user is asked to confirm") + firstLog, ok := logsAtPromptTime[0].(output.LogOutput) + require.True(t, ok) + require.Contains(t, firstLog.Format, "Force deleting an application") + }) + t.Run("Failure: child resource delete failure surfaces error", func(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -323,15 +359,10 @@ func Test_Run(t *testing.T) { mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) mockMgmt.EXPECT(). - ListAllResourceTypesNames(gomock.Any(), "local"). - Return([]string{resourceType}, nil). - Times(1) - mockMgmt.EXPECT(). - ListResourcesOfType(gomock.Any(), resourceType). + ListResourcesInApplication(gomock.Any(), appID). Return([]generated.GenericResource{{ - ID: &resourceID, - Type: &resourceType, - Properties: map[string]any{"application": appID}, + ID: &resourceID, + Type: &resourceType, }}, nil). Times(1) mockMgmt.EXPECT(). @@ -353,7 +384,7 @@ func Test_Run(t *testing.T) { require.Contains(t, err.Error(), "Failed to delete resources for application 'test-app'") }) - t.Run("Failure: ListAllResourceTypesNames failure surfaces error", func(t *testing.T) { + t.Run("Failure: resource enumeration failure surfaces error", func(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -362,7 +393,7 @@ func Test_Run(t *testing.T) { mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) mockMgmt.EXPECT(). - ListAllResourceTypesNames(gomock.Any(), "local"). + ListResourcesInApplication(gomock.Any(), gomock.Any()). Return(nil, fmt.Errorf("simulated list error")). Times(1) @@ -380,40 +411,21 @@ func Test_Run(t *testing.T) { require.Contains(t, err.Error(), "simulated list error") }) - t.Run("Success: resources owned by other applications are filtered out", func(t *testing.T) { + t.Run("Success: ownership query uses the fully qualified Radius.Core application ID", func(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() factory, err := test_client_factory.NewRadiusCoreTestClientFactory(workspace.Scope, nil, nil, test_client_factory.WithApplicationsServerNoError) require.NoError(t, err) - appID := workspace.Scope + "/providers/Radius.Core/applications/test-app" - otherAppID := workspace.Scope + "/providers/Radius.Core/applications/other-app" - ownedResourceID := "/planes/radius/local/resourceGroups/test-group/providers/Applications.Datastores/redisCaches/owned" - unrelatedResourceID := "/planes/radius/local/resourceGroups/test-group/providers/Applications.Datastores/redisCaches/unrelated" - orphanResourceID := "/planes/radius/local/resourceGroups/test-group/providers/Applications.Datastores/redisCaches/orphan" - resourceType := "Applications.Datastores/redisCaches" + // The management client filters on properties.application, so the runner must hand it a + // Radius.Core ID. A bare name would be qualified as Applications.Core and match nothing. + expectedAppID := workspace.Scope + "/providers/Radius.Core/applications/test-app" mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) mockMgmt.EXPECT(). - ListAllResourceTypesNames(gomock.Any(), "local"). - Return([]string{resourceType}, nil). - Times(1) - mockMgmt.EXPECT(). - ListResourcesOfType(gomock.Any(), resourceType). - Return([]generated.GenericResource{ - // Owned by our app — should be deleted. - {ID: &ownedResourceID, Type: &resourceType, Properties: map[string]any{"application": appID}}, - // Owned by another app — must NOT be deleted. - {ID: &unrelatedResourceID, Type: &resourceType, Properties: map[string]any{"application": otherAppID}}, - // No application property — must NOT be deleted. - {ID: &orphanResourceID, Type: &resourceType, Properties: map[string]any{}}, - }, nil). - Times(1) - // Only the owned resource is deleted. - mockMgmt.EXPECT(). - DeleteResource(gomock.Any(), resourceType, ownedResourceID, false). - Return(true, nil). + ListResourcesInApplication(gomock.Any(), expectedAppID). + Return([]generated.GenericResource{}, nil). Times(1) runner := &Runner{ @@ -429,31 +441,27 @@ func Test_Run(t *testing.T) { require.NoError(t, err) }) - t.Run("Success: case-insensitive ownership match", func(t *testing.T) { + t.Run("Success: resources without an ID or type are skipped", func(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() factory, err := test_client_factory.NewRadiusCoreTestClientFactory(workspace.Scope, nil, nil, test_client_factory.WithApplicationsServerNoError) require.NoError(t, err) - // Resource records the application ID in a different case from the constructed ID. - ownedAppID := strings.ToUpper(workspace.Scope) + "/providers/Radius.Core/applications/TEST-APP" + appID := workspace.Scope + "/providers/Radius.Core/applications/test-app" resourceID := "/planes/radius/local/resourceGroups/test-group/providers/Applications.Datastores/redisCaches/my-redis" resourceType := "Applications.Datastores/redisCaches" mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) mockMgmt.EXPECT(). - ListAllResourceTypesNames(gomock.Any(), "local"). - Return([]string{resourceType}, nil). - Times(1) - mockMgmt.EXPECT(). - ListResourcesOfType(gomock.Any(), resourceType). - Return([]generated.GenericResource{{ - ID: &resourceID, - Type: &resourceType, - Properties: map[string]any{"application": ownedAppID}, - }}, nil). + ListResourcesInApplication(gomock.Any(), appID). + Return([]generated.GenericResource{ + {ID: &resourceID, Type: &resourceType}, + {ID: nil, Type: &resourceType}, + {ID: &resourceID, Type: nil}, + }, nil). Times(1) + // Only the well-formed resource is deleted. mockMgmt.EXPECT(). DeleteResource(gomock.Any(), resourceType, resourceID, false). Return(true, nil). diff --git a/pkg/cli/cmd/env/delete/preview/cascade_test.go b/pkg/cli/cmd/env/delete/preview/cascade_test.go new file mode 100644 index 00000000000..b38da5ba115 --- /dev/null +++ b/pkg/cli/cmd/env/delete/preview/cascade_test.go @@ -0,0 +1,711 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package preview + +import ( + "context" + "fmt" + "net/http" + "sync" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/radius-project/radius/pkg/cli/clients" + generated "github.com/radius-project/radius/pkg/cli/clients_new/generated" + "github.com/radius-project/radius/pkg/cli/cmd" + "github.com/radius-project/radius/pkg/cli/connections" + "github.com/radius-project/radius/pkg/cli/output" + "github.com/radius-project/radius/pkg/cli/prompt" + "github.com/radius-project/radius/pkg/cli/test_client_factory" + "github.com/radius-project/radius/pkg/cli/workspaces" + corerpv20250801 "github.com/radius-project/radius/pkg/corerp/api/v20250801preview" + "github.com/radius-project/radius/pkg/corerp/api/v20250801preview/fake" + + azfake "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake" +) + +const ( + testScope = "/planes/radius/local/resourceGroups/test-group" + testEnvironmentID = testScope + "/providers/Radius.Core/environments/test-env" + testResourceType = "Applications.Datastores/redisCaches" +) + +func testWorkspace() *workspaces.Workspace { + return &workspaces.Workspace{ + Name: "test-workspace", + Scope: testScope, + } +} + +// deletedApplications records the applications deleted through the fake Applications server. +type deletedApplications struct { + mu sync.Mutex + names []string +} + +func (d *deletedApplications) add(name string) { + d.mu.Lock() + defer d.mu.Unlock() + d.names = append(d.names, name) +} + +func (d *deletedApplications) list() []string { + d.mu.Lock() + defer d.mu.Unlock() + return append([]string{}, d.names...) +} + +// applicationsServerWithEnvironment builds a fake Applications server whose list pager returns the +// given applications, and which records deletes into the supplied recorder. +func applicationsServerWithEnvironment(applications []*corerpv20250801.ApplicationResource, deleted *deletedApplications, deleteErr bool) func() fake.ApplicationsServer { + return func() fake.ApplicationsServer { + return fake.ApplicationsServer{ + NewListByScopePager: func(rootScope string, options *corerpv20250801.ApplicationsClientListByScopeOptions) (resp azfake.PagerResponder[corerpv20250801.ApplicationsClientListByScopeResponse]) { + resp.AddPage( + http.StatusOK, + corerpv20250801.ApplicationsClientListByScopeResponse{ + ApplicationResourceListResult: corerpv20250801.ApplicationResourceListResult{ + Value: applications, + }, + }, + nil, + ) + return + }, + Delete: func( + ctx context.Context, + rootScope string, + applicationName string, + options *corerpv20250801.ApplicationsClientDeleteOptions, + ) (resp azfake.Responder[corerpv20250801.ApplicationsClientDeleteResponse], errResp azfake.ErrorResponder) { + if deleteErr { + errResp.SetResponseError(http.StatusInternalServerError, "InternalServerError") + return + } + + if deleted != nil { + deleted.add(applicationName) + } + resp.SetResponse(http.StatusNoContent, corerpv20250801.ApplicationsClientDeleteResponse{}, nil) + return + }, + } + } +} + +// application builds a Radius.Core application resource in the test scope pointing at environmentID. +func application(name string, environmentID string) *corerpv20250801.ApplicationResource { + return &corerpv20250801.ApplicationResource{ + Name: to.Ptr(name), + ID: to.Ptr(testScope + "/providers/Radius.Core/applications/" + name), + Properties: &corerpv20250801.ApplicationProperties{ + Environment: to.Ptr(environmentID), + }, + } +} + +// resource builds a generic resource with the given name. +func resource(name string) generated.GenericResource { + return generated.GenericResource{ + ID: to.Ptr(testScope + "/providers/" + testResourceType + "/" + name), + Type: to.Ptr(testResourceType), + } +} + +func resourceIDFor(name string) string { + return testScope + "/providers/" + testResourceType + "/" + name +} + +// logFormats extracts the format strings of the logs written to the sink. +func logFormats(sink *output.MockOutput) []string { + formats := []string{} + for _, write := range sink.Writes { + if log, ok := write.(output.LogOutput); ok { + formats = append(formats, log.Format) + } + } + return formats +} + +func Test_Run_Cascade(t *testing.T) { + t.Run("Success: environment not found is a no-op", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + factory, err := test_client_factory.NewRadiusCoreTestClientFactory(testScope, test_client_factory.WithEnvironmentServer404OnGet, nil) + require.NoError(t, err) + + // A management client that fails on any call proves nothing is enumerated after the 404. + mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) + + outputSink := &output.MockOutput{} + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: testWorkspace(), + Output: outputSink, + EnvironmentName: "test-env", + Confirm: true, + } + + err = runner.Run(t.Context()) + require.NoError(t, err) + require.Equal(t, []any{ + output.LogOutput{ + Format: msgEnvironmentNotFoundPreview, + Params: []any{"test-env"}, + }, + }, outputSink.Writes) + }) + + t.Run("Success: empty environment is deleted without cascade logs", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + factory, err := test_client_factory.NewRadiusCoreTestClientFactory( + testScope, + test_client_factory.WithEnvironmentServerNoError, + nil, + applicationsServerWithEnvironment(nil, nil, false), + ) + require.NoError(t, err) + + mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) + // An environment with no applications must still be enumerated, with an empty application list. + mockMgmt.EXPECT(). + ListResourcesInEnvironmentOrApplications(gomock.Any(), testEnvironmentID, []string{}). + Return([]generated.GenericResource{}, nil). + Times(1) + + outputSink := &output.MockOutput{} + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: testWorkspace(), + Output: outputSink, + EnvironmentName: "test-env", + Confirm: true, + } + + err = runner.Run(t.Context()) + require.NoError(t, err) + require.Equal(t, []any{ + output.LogOutput{ + Format: msgEnvironmentDeletedPreview, + Params: []any{"test-env"}, + }, + }, outputSink.Writes) + }) + + t.Run("Success: resources and applications are cascade deleted before the environment", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + apps := []*corerpv20250801.ApplicationResource{ + application("app-a", testEnvironmentID), + // Belongs to a different environment and must be left alone. + application("app-b", testScope+"/providers/Radius.Core/environments/other-env"), + } + + deleted := &deletedApplications{} + factory, err := test_client_factory.NewRadiusCoreTestClientFactory( + testScope, + test_client_factory.WithEnvironmentServerNoError, + nil, + applicationsServerWithEnvironment(apps, deleted, false), + ) + require.NoError(t, err) + + mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) + // The application IDs are asserted exactly: the cascade must ask for the applications it + // found in this environment, and no others. + mockMgmt.EXPECT(). + ListResourcesInEnvironmentOrApplications( + gomock.Any(), + testEnvironmentID, + []string{testScope + "/providers/Radius.Core/applications/app-a"}, + ). + Return([]generated.GenericResource{resource("env-scoped"), resource("app-owned")}, nil). + Times(1) + mockMgmt.EXPECT(). + DeleteResource(gomock.Any(), testResourceType, resourceIDFor("env-scoped"), false). + Return(true, nil). + Times(1) + mockMgmt.EXPECT(). + DeleteResource(gomock.Any(), testResourceType, resourceIDFor("app-owned"), false). + Return(true, nil). + Times(1) + + outputSink := &output.MockOutput{} + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: testWorkspace(), + Output: outputSink, + EnvironmentName: "test-env", + Confirm: true, + } + + err = runner.Run(t.Context()) + require.NoError(t, err) + + // Only the application in this environment is deleted. + require.Equal(t, []string{"app-a"}, deleted.list()) + + // Resources are deleted, then applications, then the environment. + formats := logFormats(outputSink) + require.Equal(t, []string{ + msgDeletingResources, + cmd.MsgDeletingResource, + cmd.MsgDeletingResource, + msgDeletingApplications, + msgDeletingApplication, + msgEnvironmentDeletedPreview, + }, formats) + }) + + t.Run("Success: prompt reports the resource count and deletion proceeds on yes", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + promptMock := prompt.NewMockInterface(ctrl) + promptMock.EXPECT(). + GetListInput( + []string{prompt.ConfirmNo, prompt.ConfirmYes}, + "The environment test-env contains 1 deployed resource(s). Are you sure you want to delete the environment and its resources?", + ). + Return(prompt.ConfirmYes, nil). + Times(1) + + factory, err := test_client_factory.NewRadiusCoreTestClientFactory( + testScope, + test_client_factory.WithEnvironmentServerNoError, + nil, + applicationsServerWithEnvironment(nil, nil, false), + ) + require.NoError(t, err) + + mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) + mockMgmt.EXPECT(). + ListResourcesInEnvironmentOrApplications(gomock.Any(), testEnvironmentID, gomock.Any()). + Return([]generated.GenericResource{resource("env-scoped")}, nil). + Times(1) + mockMgmt.EXPECT(). + DeleteResource(gomock.Any(), testResourceType, resourceIDFor("env-scoped"), false). + Return(true, nil). + Times(1) + + outputSink := &output.MockOutput{} + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: testWorkspace(), + Output: outputSink, + InputPrompter: promptMock, + EnvironmentName: "test-env", + Confirm: false, + } + + err = runner.Run(t.Context()) + require.NoError(t, err) + require.Contains(t, logFormats(outputSink), msgEnvironmentDeletedPreview) + }) + + t.Run("Success: prompt reports applications when the environment has no enumerable resources", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // An environment holding an application but no enumerable resources must not be described + // as empty, or the user would consent to deleting the application without being told. + promptMock := prompt.NewMockInterface(ctrl) + promptMock.EXPECT(). + GetListInput( + []string{prompt.ConfirmNo, prompt.ConfirmYes}, + "The environment test-env contains 1 application(s) and 0 deployed resource(s). Are you sure you want to delete the environment, its applications and its resources?", + ). + Return(prompt.ConfirmNo, nil). + Times(1) + + deleted := &deletedApplications{} + apps := []*corerpv20250801.ApplicationResource{application("app-a", testEnvironmentID)} + + factory, err := test_client_factory.NewRadiusCoreTestClientFactory( + testScope, + test_client_factory.WithEnvironmentServerNoError, + nil, + applicationsServerWithEnvironment(apps, deleted, false), + ) + require.NoError(t, err) + + mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) + mockMgmt.EXPECT(). + ListResourcesInEnvironmentOrApplications(gomock.Any(), testEnvironmentID, gomock.Any()). + Return([]generated.GenericResource{}, nil). + Times(1) + + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: testWorkspace(), + Output: &output.MockOutput{}, + InputPrompter: promptMock, + EnvironmentName: "test-env", + Confirm: false, + } + + err = runner.Run(t.Context()) + require.NoError(t, err) + require.Empty(t, deleted.list()) + }) + + t.Run("Success: prompt reports an empty environment", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + promptMock := prompt.NewMockInterface(ctrl) + promptMock.EXPECT(). + GetListInput( + []string{prompt.ConfirmNo, prompt.ConfirmYes}, + "The environment test-env is empty. Are you sure you want to delete the environment?", + ). + Return(prompt.ConfirmNo, nil). + Times(1) + + factory, err := test_client_factory.NewRadiusCoreTestClientFactory( + testScope, + test_client_factory.WithEnvironmentServerNoError, + nil, + applicationsServerWithEnvironment(nil, nil, false), + ) + require.NoError(t, err) + + mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) + mockMgmt.EXPECT(). + ListResourcesInEnvironmentOrApplications(gomock.Any(), testEnvironmentID, gomock.Any()). + Return([]generated.GenericResource{}, nil). + Times(1) + + outputSink := &output.MockOutput{} + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: testWorkspace(), + Output: outputSink, + InputPrompter: promptMock, + EnvironmentName: "test-env", + Confirm: false, + } + + err = runner.Run(t.Context()) + require.NoError(t, err) + require.Empty(t, outputSink.Writes) + }) + + t.Run("Success: declining the prompt deletes nothing", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + promptMock := prompt.NewMockInterface(ctrl) + promptMock.EXPECT(). + GetListInput(gomock.Any(), gomock.Any()). + Return(prompt.ConfirmNo, nil). + Times(1) + + deleted := &deletedApplications{} + apps := []*corerpv20250801.ApplicationResource{application("app-a", testEnvironmentID)} + + factory, err := test_client_factory.NewRadiusCoreTestClientFactory( + testScope, + test_client_factory.WithEnvironmentServerNoError, + nil, + applicationsServerWithEnvironment(apps, deleted, false), + ) + require.NoError(t, err) + + mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) + mockMgmt.EXPECT(). + ListResourcesInEnvironmentOrApplications(gomock.Any(), testEnvironmentID, gomock.Any()). + Return([]generated.GenericResource{resource("env-scoped")}, nil). + Times(1) + // No DeleteResource call is expected. + + outputSink := &output.MockOutput{} + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: testWorkspace(), + Output: outputSink, + InputPrompter: promptMock, + EnvironmentName: "test-env", + Confirm: false, + } + + err = runner.Run(t.Context()) + require.NoError(t, err) + require.Empty(t, deleted.list()) + require.Empty(t, outputSink.Writes) + }) + + t.Run("Success: --force is passed through to resource deletes", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + factory, err := test_client_factory.NewRadiusCoreTestClientFactory( + testScope, + test_client_factory.WithEnvironmentServerNoError, + nil, + applicationsServerWithEnvironment(nil, nil, false), + ) + require.NoError(t, err) + + mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) + mockMgmt.EXPECT(). + ListResourcesInEnvironmentOrApplications(gomock.Any(), testEnvironmentID, gomock.Any()). + Return([]generated.GenericResource{resource("env-scoped")}, nil). + Times(1) + mockMgmt.EXPECT(). + DeleteResource(gomock.Any(), testResourceType, resourceIDFor("env-scoped"), true). + Return(true, nil). + Times(1) + + outputSink := &output.MockOutput{} + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: testWorkspace(), + Output: outputSink, + EnvironmentName: "test-env", + Confirm: true, + Force: true, + } + + err = runner.Run(t.Context()) + require.NoError(t, err) + require.Equal(t, msgForceWarning, logFormats(outputSink)[0]) + }) + + t.Run("Success: the force warning is shown before the confirmation prompt", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + outputSink := &output.MockOutput{} + var logsAtPromptTime []string + + // The warning explains that force can leave orphaned external resources, so it is only + // useful if the user sees it before consenting to the cascade. + promptMock := prompt.NewMockInterface(ctrl) + promptMock.EXPECT(). + GetListInput(gomock.Any(), gomock.Any()). + DoAndReturn(func(choices []string, promptMsg string) (string, error) { + logsAtPromptTime = logFormats(outputSink) + return prompt.ConfirmYes, nil + }). + Times(1) + + factory, err := test_client_factory.NewRadiusCoreTestClientFactory( + testScope, + test_client_factory.WithEnvironmentServerNoError, + nil, + applicationsServerWithEnvironment(nil, nil, false), + ) + require.NoError(t, err) + + mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) + mockMgmt.EXPECT(). + ListResourcesInEnvironmentOrApplications(gomock.Any(), testEnvironmentID, gomock.Any()). + Return([]generated.GenericResource{resource("env-scoped")}, nil). + Times(1) + mockMgmt.EXPECT(). + DeleteResource(gomock.Any(), testResourceType, resourceIDFor("env-scoped"), true). + Return(true, nil). + Times(1) + + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: testWorkspace(), + Output: outputSink, + InputPrompter: promptMock, + EnvironmentName: "test-env", + Confirm: false, + Force: true, + } + + err = runner.Run(t.Context()) + require.NoError(t, err) + require.Contains(t, logsAtPromptTime, msgForceWarning, + "the force warning must be logged before the user is asked to confirm") + }) + + t.Run("Success: an application without a name is reported instead of skipped silently", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // An application that reports no name cannot be addressed, so the cascade cannot delete + // it. It must say so rather than deleting the environment and orphaning it in silence. + unnamed := &corerpv20250801.ApplicationResource{ + ID: to.Ptr(testScope + "/providers/Radius.Core/applications/unnamed"), + Properties: &corerpv20250801.ApplicationProperties{ + Environment: to.Ptr(testEnvironmentID), + }, + } + + deleted := &deletedApplications{} + factory, err := test_client_factory.NewRadiusCoreTestClientFactory( + testScope, + test_client_factory.WithEnvironmentServerNoError, + nil, + applicationsServerWithEnvironment( + []*corerpv20250801.ApplicationResource{application("named", testEnvironmentID), unnamed}, + deleted, + false, + ), + ) + require.NoError(t, err) + + mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) + mockMgmt.EXPECT(). + ListResourcesInEnvironmentOrApplications(gomock.Any(), testEnvironmentID, gomock.Any()). + Return([]generated.GenericResource{}, nil). + Times(1) + + outputSink := &output.MockOutput{} + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: testWorkspace(), + Output: outputSink, + EnvironmentName: "test-env", + Confirm: true, + } + + err = runner.Run(t.Context()) + require.NoError(t, err) + + require.Equal(t, []string{"named"}, deleted.list()) + require.Contains(t, logFormats(outputSink), msgSkippingApplication) + }) + + t.Run("Failure: resource delete failure stops before the environment is deleted", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + factory, err := test_client_factory.NewRadiusCoreTestClientFactory( + testScope, + test_client_factory.WithEnvironmentServerNoError, + nil, + applicationsServerWithEnvironment(nil, nil, false), + ) + require.NoError(t, err) + + mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) + mockMgmt.EXPECT(). + ListResourcesInEnvironmentOrApplications(gomock.Any(), testEnvironmentID, gomock.Any()). + Return([]generated.GenericResource{resource("env-scoped")}, nil). + Times(1) + mockMgmt.EXPECT(). + DeleteResource(gomock.Any(), testResourceType, resourceIDFor("env-scoped"), false). + Return(false, fmt.Errorf("simulated delete failure")). + Times(1) + + outputSink := &output.MockOutput{} + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: testWorkspace(), + Output: outputSink, + EnvironmentName: "test-env", + Confirm: true, + } + + err = runner.Run(t.Context()) + require.Error(t, err) + require.Contains(t, err.Error(), "Failed to delete resources in environment 'test-env'") + require.NotContains(t, logFormats(outputSink), msgEnvironmentDeletedPreview) + }) + + t.Run("Failure: application delete failure stops before the environment is deleted", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + apps := []*corerpv20250801.ApplicationResource{application("app-a", testEnvironmentID)} + + factory, err := test_client_factory.NewRadiusCoreTestClientFactory( + testScope, + test_client_factory.WithEnvironmentServerNoError, + nil, + applicationsServerWithEnvironment(apps, nil, true), + ) + require.NoError(t, err) + + mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) + mockMgmt.EXPECT(). + ListResourcesInEnvironmentOrApplications(gomock.Any(), testEnvironmentID, gomock.Any()). + Return([]generated.GenericResource{}, nil). + Times(1) + + outputSink := &output.MockOutput{} + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: testWorkspace(), + Output: outputSink, + EnvironmentName: "test-env", + Confirm: true, + } + + err = runner.Run(t.Context()) + require.Error(t, err) + require.Contains(t, err.Error(), "Failed to delete application 'app-a' in environment 'test-env'") + require.NotContains(t, logFormats(outputSink), msgEnvironmentDeletedPreview) + }) + + t.Run("Failure: environment resource enumeration failure surfaces error", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + factory, err := test_client_factory.NewRadiusCoreTestClientFactory( + testScope, + test_client_factory.WithEnvironmentServerNoError, + nil, + applicationsServerWithEnvironment(nil, nil, false), + ) + require.NoError(t, err) + + mockMgmt := clients.NewMockApplicationsManagementClient(ctrl) + mockMgmt.EXPECT(). + ListResourcesInEnvironmentOrApplications(gomock.Any(), testEnvironmentID, gomock.Any()). + Return(nil, fmt.Errorf("simulated list error")). + Times(1) + + runner := &Runner{ + RadiusCoreClientFactory: factory, + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: mockMgmt}, + Workspace: testWorkspace(), + Output: &output.MockOutput{}, + EnvironmentName: "test-env", + Confirm: true, + } + + err = runner.Run(t.Context()) + require.Error(t, err) + require.Contains(t, err.Error(), "simulated list error") + }) +} diff --git a/pkg/cli/cmd/env/delete/preview/delete.go b/pkg/cli/cmd/env/delete/preview/delete.go index d1ba2ffd8df..3d765b25828 100644 --- a/pkg/cli/cmd/env/delete/preview/delete.go +++ b/pkg/cli/cmd/env/delete/preview/delete.go @@ -23,8 +23,11 @@ import ( "github.com/spf13/cobra" "github.com/radius-project/radius/pkg/cli" + "github.com/radius-project/radius/pkg/cli/clients" + "github.com/radius-project/radius/pkg/cli/clierrors" "github.com/radius-project/radius/pkg/cli/cmd" "github.com/radius-project/radius/pkg/cli/cmd/commonflags" + "github.com/radius-project/radius/pkg/cli/connections" "github.com/radius-project/radius/pkg/cli/framework" "github.com/radius-project/radius/pkg/cli/output" "github.com/radius-project/radius/pkg/cli/prompt" @@ -35,6 +38,11 @@ import ( const ( msgEnvironmentDeletedPreview = "Radius.Core/environments/%s deleted" msgEnvironmentNotFoundPreview = "Radius.Core/environments/%s not found" + msgDeletingResources = "Deleting %d resource(s) in environment %s...\n" + msgDeletingApplications = "Deleting %d application(s) in environment %s...\n" + msgDeletingApplication = " Deleting application %s..." + msgSkippingApplication = " Warning: skipping an application that reports no name. It must be deleted manually." + msgForceWarning = "WARNING: Force deleting an environment. Resources in non-terminal states may leave orphaned external resources that require manual cleanup." ) // NewCommand creates an instance of the command and runner for the `rad env delete --preview` command. @@ -42,18 +50,40 @@ func NewCommand(factory framework.Factory) (*cobra.Command, framework.Runner) { runner := NewRunner(factory) cmd := &cobra.Command{ - Use: "delete", - Short: "Delete environment (preview)", - Long: `Delete environment using the Radius.Core preview API surface.`, - Args: cobra.MaximumNArgs(1), - RunE: framework.RunCommand(runner), - Example: `rad env delete myenv`, + Use: "delete", + Short: "Delete environment", + Long: `Delete environment. Deletes the user's default environment by default. + +In preview mode, deleting an environment also deletes the applications in that environment +and the resources deployed into it.`, + Args: cobra.MaximumNArgs(1), + RunE: framework.RunCommand(runner), + Example: ` +# Delete current environment +rad env delete + +# Delete current environment and bypass confirmation prompt +rad env delete --yes + +# Delete specified environment +rad env delete my-env + +# Delete specified environment in a specified resource group +rad env delete my-env --group my-env + +# Delete a Radius.Core environment and everything deployed into it +rad env delete my-env --preview + +# Delete a Radius.Core environment, forcing deletion of resources in a non-terminal state +rad env delete my-env --force --preview +`, } commonflags.AddWorkspaceFlag(cmd) commonflags.AddResourceGroupFlag(cmd) commonflags.AddEnvironmentNameFlag(cmd) commonflags.AddConfirmationFlag(cmd) + commonflags.AddForceFlag(cmd) commonflags.AddOutputFlag(cmd) return cmd, runner @@ -64,19 +94,22 @@ type Runner struct { ConfigHolder *framework.ConfigHolder Output output.Interface InputPrompter prompt.Interface + ConnectionFactory connections.Factory Workspace *workspaces.Workspace RadiusCoreClientFactory *corerpv20250801.ClientFactory Confirm bool + Force bool EnvironmentName string } // NewRunner creates a new instance of the preview delete runner. func NewRunner(factory framework.Factory) *Runner { return &Runner{ - ConfigHolder: factory.GetConfigHolder(), - Output: factory.GetOutput(), - InputPrompter: factory.GetPrompter(), + ConfigHolder: factory.GetConfigHolder(), + Output: factory.GetOutput(), + InputPrompter: factory.GetPrompter(), + ConnectionFactory: factory.GetConnectionFactory(), } } @@ -105,6 +138,11 @@ func (r *Runner) Validate(cmd *cobra.Command, args []string) error { return err } + r.Force, err = cmd.Flags().GetBool("force") + if err != nil { + return err + } + _, err = cli.RequireOutput(cmd) // we ignore format for preview delete if err != nil { return err @@ -114,6 +152,11 @@ func (r *Runner) Validate(cmd *cobra.Command, args []string) error { } // Run executes the preview delete command logic. +// +// Deleting an environment cascades to everything deployed into it: the resources that reference +// the environment, the applications in the environment, and finally the environment itself. This +// matches the behavior of the legacy Applications.Core/environments delete path, so that deleting +// an environment does not leave orphaned resources behind. func (r *Runner) Run(ctx context.Context) error { if r.RadiusCoreClientFactory == nil { factory, err := cmd.InitializeRadiusCoreClientFactory(ctx, r.Workspace) @@ -123,13 +166,67 @@ func (r *Runner) Run(ctx context.Context) error { r.RadiusCoreClientFactory = factory } - // For better feedback, list resources in the environment using the generic client facade. - // The ApplicationsManagementClient abstraction isn't used here; instead we use the Radius.Core - // client factory directly and count generic resources by environment ID. + envClient := r.RadiusCoreClientFactory.NewEnvironmentsClient() + + // Check that the environment exists before enumerating its contents, so that deleting an + // already-deleted environment is a no-op rather than an error. + _, err := envClient.Get(ctx, r.Workspace.Scope, r.EnvironmentName, &corerpv20250801.EnvironmentsClientGetOptions{}) + if clients.Is404Error(err) { + r.Output.LogInfo(msgEnvironmentNotFoundPreview, r.EnvironmentName) + return nil + } else if err != nil { + return err + } + + managementClient, err := r.ConnectionFactory.CreateApplicationsManagementClient(ctx, *r.Workspace) + if err != nil { + return err + } + + environmentID := cmd.PreviewEnvironmentID(r.Workspace.Scope, r.EnvironmentName) + + applications, err := cmd.ListPreviewApplicationsInEnvironment(ctx, r.RadiusCoreClientFactory.NewApplicationsClient(), r.Workspace, environmentID) + if err != nil && !clients.Is404Error(err) { + return err + } + + // Resources are collected from two directions: those that reference the environment directly, + // and those owned by an application in the environment. A resource usually carries both + // properties, but neither query is guaranteed to be a superset of the other, so both are + // applied. This is a single enumeration pass; listing per application would re-fetch the same + // data once per application. + applicationIDs := make([]string, 0, len(applications)) + for _, application := range applications { + if application.ID == nil { + continue + } + + applicationIDs = append(applicationIDs, *application.ID) + } + + resourcesToDelete, err := managementClient.ListResourcesInEnvironmentOrApplications(ctx, environmentID, applicationIDs) + if err != nil && !clients.Is404Error(err) { + return err + } + + if r.Force { + r.Output.LogInfo(msgForceWarning) + } // Prompt user to confirm deletion if !r.Confirm { - promptMsg := fmt.Sprintf("The environment %s is empty. Are you sure you want to delete the environment?", r.EnvironmentName) + var promptMsg string + switch { + case len(applications) > 0: + promptMsg = fmt.Sprintf("The environment %s contains %d application(s) and %d deployed resource(s). Are you sure you want to delete the environment, its applications and its resources?", + r.EnvironmentName, len(applications), len(resourcesToDelete)) + case len(resourcesToDelete) > 0: + promptMsg = fmt.Sprintf("The environment %s contains %d deployed resource(s). Are you sure you want to delete the environment and its resources?", + r.EnvironmentName, len(resourcesToDelete)) + default: + promptMsg = fmt.Sprintf("The environment %s is empty. Are you sure you want to delete the environment?", + r.EnvironmentName) + } confirmed, err := prompt.YesOrNoPrompt(promptMsg, prompt.ConfirmNo, r.InputPrompter) if err != nil { @@ -140,11 +237,43 @@ func (r *Runner) Run(ctx context.Context) error { } } - client := r.RadiusCoreClientFactory.NewEnvironmentsClient() - _, err := client.Delete(ctx, r.Workspace.Scope, r.EnvironmentName, &corerpv20250801.EnvironmentsClientDeleteOptions{}) - if err != nil { - // If this is a 404, treat as successful but with a different message - // We don't have the Is404Error helper wired to Radius.Core yet, so always surface the error. + if len(resourcesToDelete) > 0 { + r.Output.LogInfo(msgDeletingResources, len(resourcesToDelete), r.EnvironmentName) + + if err := cmd.DeleteResourcesInParallel(ctx, managementClient, r.Output, resourcesToDelete, r.Force); err != nil { + return clierrors.Message("Failed to delete resources in environment '%s': %v", r.EnvironmentName, err) + } + } + + // Applications are deleted after their resources, so that an interrupted delete leaves the + // application behind as a recovery point rather than orphaning its resources. + if len(applications) > 0 { + r.Output.LogInfo(msgDeletingApplications, len(applications), r.EnvironmentName) + + appClient := r.RadiusCoreClientFactory.NewApplicationsClient() + for _, application := range applications { + if application.Name == nil { + // An application that cannot be addressed cannot be deleted. Warn rather than + // skipping silently, so the cascade does not report success for an application it + // leaves behind when the environment goes away. + r.Output.LogInfo(msgSkippingApplication) + continue + } + + r.Output.LogInfo(msgDeletingApplication, *application.Name) + + _, err := appClient.Delete(ctx, r.Workspace.Scope, *application.Name, &corerpv20250801.ApplicationsClientDeleteOptions{}) + if err != nil && !clients.Is404Error(err) { + return clierrors.Message("Failed to delete application '%s' in environment '%s': %v", *application.Name, r.EnvironmentName, err) + } + } + } + + _, err = envClient.Delete(ctx, r.Workspace.Scope, r.EnvironmentName, &corerpv20250801.EnvironmentsClientDeleteOptions{}) + if clients.Is404Error(err) { + r.Output.LogInfo(msgEnvironmentNotFoundPreview, r.EnvironmentName) + return nil + } else if err != nil { return err } diff --git a/pkg/cli/cmd/env/delete/preview/delete_test.go b/pkg/cli/cmd/env/delete/preview/delete_test.go index 70a740a6aa0..6d60105d58f 100644 --- a/pkg/cli/cmd/env/delete/preview/delete_test.go +++ b/pkg/cli/cmd/env/delete/preview/delete_test.go @@ -19,13 +19,7 @@ package preview import ( "testing" - "github.com/stretchr/testify/require" - "github.com/radius-project/radius/pkg/cli/framework" - "github.com/radius-project/radius/pkg/cli/output" - "github.com/radius-project/radius/pkg/cli/test_client_factory" - "github.com/radius-project/radius/pkg/cli/workspaces" - "github.com/radius-project/radius/pkg/corerp/api/v20250801preview/fake" "github.com/radius-project/radius/test/radcli" ) @@ -81,48 +75,3 @@ func Test_Validate(t *testing.T) { radcli.SharedValidateValidation(t, NewCommand, testcases) } - -func Test_Run(t *testing.T) { - workspace := &workspaces.Workspace{ - Name: "test-workspace", - Scope: "/planes/radius/local/resourceGroups/test-group", - } - - testcases := []struct { - name string - serverFactory func() fake.EnvironmentsServer - expectedLogs []any - }{ - { - name: "Success: environment deleted", - serverFactory: test_client_factory.WithEnvironmentServerNoError, - expectedLogs: []any{ - output.LogOutput{ - Format: msgEnvironmentDeletedPreview, - Params: []any{"test-env"}, - }, - }, - }, - } - - for _, tc := range testcases { - ct := tc - t.Run(ct.name, func(t *testing.T) { - factory, err := test_client_factory.NewRadiusCoreTestClientFactory(workspace.Scope, ct.serverFactory, nil) - require.NoError(t, err) - - outputSink := &output.MockOutput{} - runner := &Runner{ - RadiusCoreClientFactory: factory, - Workspace: workspace, - Output: outputSink, - EnvironmentName: "test-env", - Confirm: true, - } - - err = runner.Run(t.Context()) - require.NoError(t, err) - require.Equal(t, ct.expectedLogs, outputSink.Writes) - }) - } -} diff --git a/pkg/cli/cmd/previewdelete.go b/pkg/cli/cmd/previewdelete.go new file mode 100644 index 00000000000..546ef0e11e7 --- /dev/null +++ b/pkg/cli/cmd/previewdelete.go @@ -0,0 +1,139 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cmd + +import ( + "context" + "strings" + + "golang.org/x/sync/errgroup" + + "github.com/radius-project/radius/pkg/cli/clients" + generated "github.com/radius-project/radius/pkg/cli/clients_new/generated" + "github.com/radius-project/radius/pkg/cli/output" + "github.com/radius-project/radius/pkg/cli/workspaces" + corerpv20250801 "github.com/radius-project/radius/pkg/corerp/api/v20250801preview" + "github.com/radius-project/radius/pkg/corerp/datamodel" +) + +// MsgDeletingResource is logged for each resource before its deletion is started. +const MsgDeletingResource = " Deleting %s..." + +// MsgSkippingResource is logged for each resource the cascade cannot delete, so the count shown in +// the confirmation prompt cannot quietly disagree with what was actually deleted. +const MsgSkippingResource = " Warning: skipping %s because its resource ID or type is missing. It must be deleted manually." + +// maxParallelDeletes bounds the number of deletions in flight. Each delete holds a long-running +// operation poller open against the RP, and an environment cascade can span every resource in +// every application, so the fan-out is capped to avoid overwhelming the server. +const maxParallelDeletes = 10 + +// PreviewResourceID builds a fully qualified Radius.Core resource ID from a workspace +// scope, resource type and resource name. +func PreviewResourceID(scope string, resourceType string, name string) string { + return scope + "/providers/" + resourceType + "/" + name +} + +// PreviewApplicationID builds a fully qualified Radius.Core application ID. +func PreviewApplicationID(scope string, applicationName string) string { + return PreviewResourceID(scope, datamodel.ApplicationResourceType_v20250801preview, applicationName) +} + +// PreviewEnvironmentID builds a fully qualified Radius.Core environment ID. +func PreviewEnvironmentID(scope string, environmentName string) string { + return PreviewResourceID(scope, datamodel.EnvironmentResourceType_v20250801preview, environmentName) +} + +// DeleteResourcesInParallel deletes the given resources concurrently, tolerating resources that +// have already been deleted. The ID of each resource is logged before its deletion is started, +// because output.Interface implementations are not guaranteed to be thread-safe and logging up +// front keeps the output deterministic. +// +// A resource missing an ID or type cannot be addressed and is skipped with a warning rather than +// silently dropped, so the caller's reported count cannot disagree with what was deleted. +// +// Deletions are limited to maxParallelDeletes at a time. On the first failure errgroup cancels the +// shared context, which abandons every other delete. Those deletes are left in mixed states: some +// were already accepted by the server and are still running there, some were canceled before the +// request was sent, and some queued behind the concurrency limit may never have started. The +// command reports a single error, so the outcome of the rest is unknown. Re-running the command is +// the way to converge, which is safe because deleting an already-deleted resource is treated as +// success. +func DeleteResourcesInParallel(ctx context.Context, client clients.ApplicationsManagementClient, out output.Interface, resources []generated.GenericResource, force bool) error { + g, groupCtx := errgroup.WithContext(ctx) + g.SetLimit(maxParallelDeletes) + + for _, resource := range resources { + if resource.ID == nil || resource.Type == nil { + out.LogInfo(MsgSkippingResource, describeResource(resource)) + continue + } + + out.LogInfo(MsgDeletingResource, *resource.ID) + + resourceType := *resource.Type + resourceID := *resource.ID + g.Go(func() error { + _, err := client.DeleteResource(groupCtx, resourceType, resourceID, force) + if err != nil && !clients.Is404Error(err) { + return err + } + return nil + }) + } + + return g.Wait() +} + +// describeResource returns the most identifying label available for a resource, for use in +// messages about resources that cannot be deleted. +func describeResource(resource generated.GenericResource) string { + switch { + case resource.ID != nil: + return *resource.ID + case resource.Name != nil: + return *resource.Name + default: + return "an unnamed resource" + } +} + +// ListPreviewApplicationsInEnvironment lists the Radius.Core applications in the workspace scope +// whose properties.environment references the given environment ID. +func ListPreviewApplicationsInEnvironment(ctx context.Context, client *corerpv20250801.ApplicationsClient, workspace *workspaces.Workspace, environmentID string) ([]corerpv20250801.ApplicationResource, error) { + results := []corerpv20250801.ApplicationResource{} + + pager := client.NewListByScopePager(workspace.Scope, &corerpv20250801.ApplicationsClientListByScopeOptions{}) + for pager.More() { + page, err := pager.NextPage(ctx) + if err != nil { + return nil, err + } + + for _, application := range page.Value { + if application == nil || application.Properties == nil || application.Properties.Environment == nil { + continue + } + + if strings.EqualFold(*application.Properties.Environment, environmentID) { + results = append(results, *application) + } + } + } + + return results, nil +} diff --git a/pkg/cli/cmd/previewdelete_test.go b/pkg/cli/cmd/previewdelete_test.go new file mode 100644 index 00000000000..67faeade883 --- /dev/null +++ b/pkg/cli/cmd/previewdelete_test.go @@ -0,0 +1,154 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cmd + +import ( + "fmt" + "net/http" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/radius-project/radius/pkg/cli/clients" + generated "github.com/radius-project/radius/pkg/cli/clients_new/generated" + "github.com/radius-project/radius/pkg/cli/output" +) + +const ( + testScope = "/planes/radius/local/resourceGroups/test-group" + testResourceType = "Applications.Datastores/redisCaches" +) + +func testResource(name string) generated.GenericResource { + return generated.GenericResource{ + ID: to.Ptr(testScope + "/providers/" + testResourceType + "/" + name), + Type: to.Ptr(testResourceType), + } +} + +func Test_PreviewResourceIDs(t *testing.T) { + require.Equal(t, testScope+"/providers/Radius.Core/applications/my-app", PreviewApplicationID(testScope, "my-app")) + require.Equal(t, testScope+"/providers/Radius.Core/environments/my-env", PreviewEnvironmentID(testScope, "my-env")) +} + +func Test_DeleteResourcesInParallel(t *testing.T) { + t.Run("deletes every resource and logs each one", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + first := testResource("a") + second := testResource("b") + + mock := clients.NewMockApplicationsManagementClient(ctrl) + mock.EXPECT().DeleteResource(gomock.Any(), testResourceType, *first.ID, false).Return(true, nil).Times(1) + mock.EXPECT().DeleteResource(gomock.Any(), testResourceType, *second.ID, false).Return(true, nil).Times(1) + + sink := &output.MockOutput{} + err := DeleteResourcesInParallel(t.Context(), mock, sink, []generated.GenericResource{first, second}, false) + require.NoError(t, err) + require.Len(t, sink.Writes, 2) + }) + + t.Run("warns about resources without an ID or type instead of skipping them silently", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + valid := testResource("a") + noID := generated.GenericResource{Type: to.Ptr(testResourceType)} + noType := generated.GenericResource{ID: to.Ptr(testScope + "/providers/" + testResourceType + "/c")} + + mock := clients.NewMockApplicationsManagementClient(ctrl) + mock.EXPECT().DeleteResource(gomock.Any(), testResourceType, *valid.ID, false).Return(true, nil).Times(1) + + sink := &output.MockOutput{} + err := DeleteResourcesInParallel(t.Context(), mock, sink, []generated.GenericResource{valid, noID, noType}, false) + require.NoError(t, err) + + // The caller reports a count to the user before calling this function, so every resource + // that is not deleted must produce a message rather than disappearing. + require.Equal(t, []any{ + output.LogOutput{Format: MsgDeletingResource, Params: []any{*valid.ID}}, + output.LogOutput{Format: MsgSkippingResource, Params: []any{"an unnamed resource"}}, + output.LogOutput{Format: MsgSkippingResource, Params: []any{*noType.ID}}, + }, sink.Writes) + }) + + t.Run("identifies a skipped resource by name when it has no ID", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mock := clients.NewMockApplicationsManagementClient(ctrl) + named := generated.GenericResource{Name: to.Ptr("my-resource")} + + sink := &output.MockOutput{} + err := DeleteResourcesInParallel(t.Context(), mock, sink, []generated.GenericResource{named}, false) + require.NoError(t, err) + + require.Equal(t, []any{ + output.LogOutput{Format: MsgSkippingResource, Params: []any{"my-resource"}}, + }, sink.Writes) + }) + + t.Run("tolerates resources that are already deleted", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + resource := testResource("a") + + mock := clients.NewMockApplicationsManagementClient(ctrl) + mock.EXPECT(). + DeleteResource(gomock.Any(), testResourceType, *resource.ID, false). + Return(false, &azcore.ResponseError{StatusCode: http.StatusNotFound}). + Times(1) + + err := DeleteResourcesInParallel(t.Context(), mock, &output.MockOutput{}, []generated.GenericResource{resource}, false) + require.NoError(t, err) + }) + + t.Run("surfaces deletion failures", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + resource := testResource("a") + + mock := clients.NewMockApplicationsManagementClient(ctrl) + mock.EXPECT(). + DeleteResource(gomock.Any(), testResourceType, *resource.ID, false). + Return(false, fmt.Errorf("simulated failure")). + Times(1) + + err := DeleteResourcesInParallel(t.Context(), mock, &output.MockOutput{}, []generated.GenericResource{resource}, false) + require.Error(t, err) + require.Contains(t, err.Error(), "simulated failure") + }) + + t.Run("passes force through to the client", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + resource := testResource("a") + + mock := clients.NewMockApplicationsManagementClient(ctrl) + mock.EXPECT().DeleteResource(gomock.Any(), testResourceType, *resource.ID, true).Return(true, nil).Times(1) + + err := DeleteResourcesInParallel(t.Context(), mock, &output.MockOutput{}, []generated.GenericResource{resource}, true) + require.NoError(t, err) + }) +} diff --git a/test/functional-portable/corerp/noncloud/resources/environment_test.go b/test/functional-portable/corerp/noncloud/resources/environment_test.go index eab5901af3f..d73aa1e4e85 100644 --- a/test/functional-portable/corerp/noncloud/resources/environment_test.go +++ b/test/functional-portable/corerp/noncloud/resources/environment_test.go @@ -17,10 +17,18 @@ limitations under the License. package resource_test import ( + "context" + "fmt" "testing" + "time" + "github.com/stretchr/testify/require" + + "github.com/radius-project/radius/pkg/cli/clients" + "github.com/radius-project/radius/test/radcli" "github.com/radius-project/radius/test/rp" "github.com/radius-project/radius/test/step" + "github.com/radius-project/radius/test/testutil" "github.com/radius-project/radius/test/validation" ) @@ -46,3 +54,107 @@ func Test_Environment(t *testing.T) { test.Test(t) } + +// Test_Environment_CascadeDelete verifies that deleting a Radius.Core environment also deletes the +// applications in that environment and the resources deployed into it, matching the behavior of +// Applications.Core environments. +// +// The test deploys an application and a container into a preview environment, then deletes only the +// environment and asserts that all three resources are gone. Framework-driven teardown is disabled +// for the step so that the cascade is the only thing that removes the application and container -- +// otherwise the framework would delete them first and the cascade would have nothing left to do. +func Test_Environment_CascadeDelete(t *testing.T) { + template := "testdata/corerp-resources-env-cascade.bicep" + name := "corerp-resources-env-cascade" + containerName := "env-cascade-ctnr" + + test := rp.NewRPTest(t, name, []rp.TestStep{ + { + RPResources: &validation.RPResourceSet{ + Resources: []validation.RPResource{ + { + Name: name, + Type: validation.CoreApplicationsResource, + }, + { + Name: containerName, + Type: validation.ComputeContainersResource, + App: name, + }, + }, + }, + K8sObjects: &validation.K8sObjectSet{ + Namespaces: map[string][]validation.K8sObject{ + name: { + validation.NewK8sPodForResource(name, containerName), + }, + }, + }, + // The cascade triggered by PostStepVerify is what deletes these resources. + SkipResourceDeletion: true, + }, + }) + + preSetup, previewEnvID := rp.NewPreviewEnvPreSetup(name, test.Options.Workspace.Scope, name) + envName := name + "-env" + + test.PreSetup = preSetup + test.Steps[0].Executor = step.NewDeployExecutor(template, testutil.GetMagpieImage(), fmt.Sprintf("environment=%s", previewEnvID)) + test.Steps[0].PostStepVerify = func(ctx context.Context, t *testing.T, ct rp.RPTest) { + scope := ct.Options.Workspace.Scope + applicationID := fmt.Sprintf("%s/providers/Radius.Core/applications/%s", scope, name) + containerID := fmt.Sprintf("%s/providers/Radius.Compute/containers/%s", scope, containerName) + + cli := radcli.NewCLI(t, ct.Options.ConfigFilePath) + + t.Logf("deleting environment %s, expecting the delete to cascade", envName) + _, err := cli.EnvironmentDeletePreview(ctx, envName, "") + require.NoError(t, err, "failed to delete preview environment") + + requireResourceDeleted(ctx, t, ct, validation.ComputeContainersResource, containerID) + requireResourceDeleted(ctx, t, ct, validation.CoreApplicationsResource, applicationID) + requireResourceDeleted(ctx, t, ct, validation.CoreEnvironmentsResource, previewEnvID) + + validation.ValidateNoPodsInApplication(ctx, t, ct.Options.K8sClient, name, name) + + // Deleting an environment that no longer exists is a no-op rather than an error, so + // teardown and repeated invocations stay safe. + _, err = cli.EnvironmentDeletePreview(ctx, envName, "") + require.NoError(t, err, "deleting an already-deleted environment should succeed") + } + + test.Test(t) +} + +// requireResourceDeleted asserts that the given resource is reported as not found. Deletes are +// awaited by the CLI, but the assertion is retried to absorb any propagation delay. +func requireResourceDeleted(ctx context.Context, t *testing.T, ct rp.RPTest, resourceType string, resourceID string) { + t.Helper() + + var lastErr error + deadline := time.Now().Add(60 * time.Second) + for { + _, lastErr = ct.Options.ManagementClient.GetResource(ctx, resourceType, resourceID) + if lastErr != nil && clients.Is404Error(lastErr) { + t.Logf("verified %s was deleted by the environment cascade", resourceID) + return + } + + // Stop retrying once the test context is done, so a canceled or timed-out run fails + // immediately instead of burning the full deadline. + if err := ctx.Err(); err != nil { + require.Failf(t, "context ended before the resource was deleted", + "gave up waiting for %s to be deleted: %v (last error: %v)", resourceID, err, lastErr) + return + } + + if time.Now().After(deadline) { + break + } + + time.Sleep(2 * time.Second) + } + + require.Failf(t, "resource was not deleted", + "expected %s to be deleted by the environment cascade, but it is still present (last error: %v)", resourceID, lastErr) +} diff --git a/test/functional-portable/corerp/noncloud/resources/testdata/corerp-resources-env-cascade.bicep b/test/functional-portable/corerp/noncloud/resources/testdata/corerp-resources-env-cascade.bicep new file mode 100644 index 00000000000..6995e191fdc --- /dev/null +++ b/test/functional-portable/corerp/noncloud/resources/testdata/corerp-resources-env-cascade.bicep @@ -0,0 +1,41 @@ +extension radius + +@description('Specifies the location for resources.') +param location string = 'global' + +@description('Specifies the image of the container resource.') +param magpieimage string + +@description('Specifies the port of the container resource.') +param port int = 3000 + +@description('Specifies the environment for resources.') +param environment string + +resource app 'Radius.Core/applications@2025-08-01-preview' = { + name: 'corerp-resources-env-cascade' + location: location + properties: { + environment: environment + } +} + +resource container 'Radius.Compute/containers@2025-08-01-preview' = { + name: 'env-cascade-ctnr' + location: location + properties: { + application: app.id + environment: environment + containers: { + envcascadectnr: { + image: magpieimage + ports: { + web: { + containerPort: port + } + } + } + } + connections: {} + } +}