Logs and some test changes to help debug test failure in Test_CLI_Del…

…ete (radius-project#6881)

# Description

Changes to debug test failure issue:
radius-project#6872

Used a different app name and namespace for the failing test instead of
reusing from a previous one in the file
Added more logs
## Type of change

<!--

Please select **one** of the following options that describes your
change and delete the others. Clearly identifying the type of change you
are making will help us review your PR faster, and is used in authoring
release notes.

If you are making a bug fix or functionality change to Radius and do not
have an associated issue link please create one now.

-->

- This pull request is a minor refactor, code cleanup, test improvement,
or other maintenance task and doesn't change the functionality of Radius
(issue link optional).

<!--

Please update the following to link the associated issue. This is
required for some kinds of changes (see above).

-->

Fixes: #issue_number

## Auto-generated summary

<!--
GitHub Copilot for docs will auto-generate a summary of the PR
-->

<!--
copilot:all
-->
### <samp>🤖[[deprecated]](https://githubnext.com/copilot-for-prs-sunset)
Generated by Copilot at fb725ff</samp>

### Summary
🗑️🐳🧪

<!--
1. 🗑️ - This emoji represents the deletion of an application and its
associated resources, as well as the improved output message for this
action.
2. 🐳 - This emoji represents the addition of a new bicep template file
that defines an application with two containers, which are a common
resource type for Kubernetes applications.
3. 🧪 - This emoji represents the extension of the test function to cover
the new scenario of deleting an application with unassociated resources,
as well as the added logging to the validation functions.
-->
This pull request adds a test case and a test data file for deleting an
application with unassociated resources using the CLI. It also improves
the output message for deleting an application and adds logging to the
validation functions.

> _Sing, O Muse, of the valiant code reviewers who scrutinized the pull
request_
> _And of the skillful coder who improved the CLI with his diligent
quest_
> _To make the output message clearer when an application is deleted by
name_
> _And to test the deletion of unassociated resources, those containers
of fame_

### Walkthrough
* Change the output message for deleting an application to include the
application name
([link](https://github.com/radius-project/radius/pull/6881/files?diff=unified&w=0#diff-87907dd86f66c5e0992fef2ea8d8bc09cf79e4cf4e6d9108cb137a536da03c10L166-R166))
* Add a new bicep template file
`corerp-kubernetes-cli-with-unassociated-resources.bicep` that defines
an application with two unassociated resources
([link](https://github.com/radius-project/radius/pull/6881/files?diff=unified&w=0#diff-9cf9c8faf8c86483d2b71a394c55ab338f6a1fa2d9cc536f8af502718436cd36R1-R40))
- Declare new variables for the application name and the template file
path of the application with unassociated resources
([link](https://github.com/radius-project/radius/pull/6881/files?diff=unified&w=0#diff-9f50fd192c3de42201f2c910d3f4ba78ec369dd7c50ea61ba500fa7518f4e81dR529),
[link](https://github.com/radius-project/radius/pull/6881/files?diff=unified&w=0#diff-9f50fd192c3de42201f2c910d3f4ba78ec369dd7c50ea61ba500fa7518f4e81dR538-R540))
- Deploy the application with unassociated resources and validate that
the pods are running
([link](https://github.com/radius-project/radius/pull/6881/files?diff=unified&w=0#diff-9f50fd192c3de42201f2c910d3f4ba78ec369dd7c50ea61ba500fa7518f4e81dL579-R590))
- Delete one of the unassociated resources using the management client
([link](https://github.com/radius-project/radius/pull/6881/files?diff=unified&w=0#diff-9f50fd192c3de42201f2c910d3f4ba78ec369dd7c50ea61ba500fa7518f4e81dL592-R602))
- Delete the application without deleting the remaining resources using
the `DeleteAppWithoutDeletingResources` helper function
([link](https://github.com/radius-project/radius/pull/6881/files?diff=unified&w=0#diff-9f50fd192c3de42201f2c910d3f4ba78ec369dd7c50ea61ba500fa7518f4e81dL592-R602))
- Redeploy the application using an empty template file
([link](https://github.com/radius-project/radius/pull/6881/files?diff=unified&w=0#diff-9f50fd192c3de42201f2c910d3f4ba78ec369dd7c50ea61ba500fa7518f4e81dL592-R602))
- Delete the other unassociated resource using the management client
([link](https://github.com/radius-project/radius/pull/6881/files?diff=unified&w=0#diff-9f50fd192c3de42201f2c910d3f4ba78ec369dd7c50ea61ba500fa7518f4e81dL605-R610))
* Modify the validation functions `ValidateObjectsRunning` and
`matchesActualLabels` to log the number and labels of the deployed
resources for debugging purposes
([link](https://github.com/radius-project/radius/pull/6881/files?diff=unified&w=0#diff-a52153bfeb452a3dd497767a98db0dc1a0621e012f6e6d8dd9d4b5ec758b4dc2L343-R345),
[link](https://github.com/radius-project/radius/pull/6881/files?diff=unified&w=0#diff-a52153bfeb452a3dd497767a98db0dc1a0621e012f6e6d8dd9d4b5ec758b4dc2L535-R535),
[link](https://github.com/radius-project/radius/pull/6881/files?diff=unified&w=0#diff-a52153bfeb452a3dd497767a98db0dc1a0621e012f6e6d8dd9d4b5ec758b4dc2R555-R556))

Signed-off-by: vinayada1 <[email protected]>
Co-authored-by: Nithya Subramanian <[email protected]>

pkg/cli/cmd/app/delete/delete.go

@@ -163,7 +163,7 @@ func (r *Runner) Run(ctx context.Context) error {
 	}
 
 	if deleted {
-		r.Output.LogInfo("Application deleted")
+		r.Output.LogInfo("Application %s deleted", r.ApplicationName)
 	} else {
 		r.Output.LogInfo("Application '%s' does not exist or has already been deleted.", r.ApplicationName)
 	}

pkg/cli/cmd/app/delete/delete_test.go

@@ -149,7 +149,8 @@ func Test_Show(t *testing.T) {
 
 		expected := []any{
 			output.LogOutput{
-				Format: "Application deleted",
+				Format: "Application %s deleted",
+				Params: []any{"test-app"},
 			},
 		}
 
@@ -195,7 +196,8 @@ func Test_Show(t *testing.T) {
 
 		expected := []any{
 			output.LogOutput{
-				Format: "Application deleted",
+				Format: "Application %s deleted",
+				Params: []any{"test-app"},
 			},
 		}
 

test/functional/shared/cli/cli_test.go

@@ -534,6 +534,7 @@ func Test_CLI_Delete(t *testing.T) {
 
 	options := shared.NewRPTestOptions(t)
 	appName := "kubernetes-cli-with-resources"
+	appNameUnassociatedResources := "kubernetes-cli-with-unassociated-resources"
 	appNameEmptyResources := "kubernetes-cli-empty-resources"
 
 	cwd, err := os.Getwd()
@@ -542,6 +543,9 @@ func Test_CLI_Delete(t *testing.T) {
 	templateWithResources := "testdata/corerp-kubernetes-cli-with-resources.bicep"
 	templateFilePathWithResources := filepath.Join(cwd, templateWithResources)
 
+	templateWithResourcesUnassociated := "testdata/corerp-kubernetes-cli-with-unassociated-resources.bicep"
+	templateFilePathWithResourcesUnassociated := filepath.Join(cwd, templateWithResourcesUnassociated)
+
 	templateEmptyResources := "testdata/corerp-kubernetes-cli-app-empty-resources.bicep"
 	templateFilePathEmptyResources := filepath.Join(cwd, templateEmptyResources)
 
@@ -584,34 +588,34 @@ func Test_CLI_Delete(t *testing.T) {
 	t.Run("Validate rad app delete with resources not associated with any application", func(t *testing.T) {
 		t.Logf("deploying from file %s", templateWithResources)
 
-		err := cli.Deploy(ctx, templateFilePathWithResources, "", appName, functional.GetMagpieImage())
-		require.NoErrorf(t, err, "failed to deploy %s", appName)
+		err := cli.Deploy(ctx, templateFilePathWithResourcesUnassociated, "", appNameUnassociatedResources, functional.GetMagpieImage())
+		require.NoErrorf(t, err, "failed to deploy %s", appNameUnassociatedResources)
 
 		validation.ValidateObjectsRunning(ctx, t, options.K8sClient, options.DynamicClient, validation.K8sObjectSet{
 			Namespaces: map[string][]validation.K8sObject{
-				"default-kubernetes-cli-with-resources": {
-					validation.NewK8sPodForResource(appName, "containera-app-with-resources"),
-					validation.NewK8sPodForResource(appName, "containerb-app-with-resources"),
+				"default-kubernetes-cli-with-unassociated-resources": {
+					validation.NewK8sPodForResource(appNameUnassociatedResources, "containerX"),
+					validation.NewK8sPodForResource(appNameUnassociatedResources, "containerY"),
 				},
 			},
 		})
 
 		//ignore response for tests
-		_, err = options.ManagementClient.DeleteResource(ctx, "Applications.Core/containers", "containerb-app-with-resources")
-		require.NoErrorf(t, err, "failed to delete resource containerb-app-with-resources")
-		err = DeleteAppWithoutDeletingResources(t, ctx, options, appName)
-		require.NoErrorf(t, err, "failed to delete application %s", appName)
+		_, err = options.ManagementClient.DeleteResource(ctx, "Applications.Core/containers", "containerY")
+		require.NoErrorf(t, err, "failed to delete resource containerY")
+		err = DeleteAppWithoutDeletingResources(t, ctx, options, appNameUnassociatedResources)
+		require.NoErrorf(t, err, "failed to delete application %s", appNameUnassociatedResources)
 
 		t.Logf("deploying from file %s", templateEmptyResources)
-		err = cli.Deploy(ctx, templateFilePathEmptyResources, "", appName)
+		err = cli.Deploy(ctx, templateFilePathEmptyResources, "", appNameEmptyResources)
 		require.NoErrorf(t, err, "failed to deploy %s", appNameEmptyResources)
 
 		err = cli.ApplicationDelete(ctx, appNameEmptyResources)
 		require.NoErrorf(t, err, "failed to delete %s", appNameEmptyResources)
 
 		//ignore response for tests
-		_, err = options.ManagementClient.DeleteResource(ctx, "Applications.Core/containers", "containera-app-with-resources")
-		require.NoErrorf(t, err, "failed to delete resource containera-app-with-resources")
+		_, err = options.ManagementClient.DeleteResource(ctx, "Applications.Core/containers", "containerX")
+		require.NoErrorf(t, err, "failed to delete resource containerX")
 
 	})
 }

test/functional/shared/cli/testdata/corerp-kubernetes-cli-with-unassociated-resources.bicep

@@ -0,0 +1,40 @@
+import radius as radius
+
+@description('Specifies the location for resources.')
+param location string = 'global'
+
+@description('Specifies the environment for resources.')
+param environment string = 'test'
+
+@description('Specifies the image to be deployed.')
+param magpieimage string
+
+resource app 'Applications.Core/applications@2023-10-01-preview' = {
+  name: 'kubernetes-cli-with-unassociated-resources'
+  location: location
+  properties: {
+    environment: environment
+  }
+}
+
+resource containerx 'Applications.Core/containers@2023-10-01-preview' = {
+  name: 'containerX'
+  location: location
+  properties: {
+    application: app.id
+    container: {
+      image: magpieimage
+    }
+  }
+}
+
+resource containery 'Applications.Core/containers@2023-10-01-preview' = {
+  name: 'containerY'
+  location: location
+  properties: {
+    application: app.id
+    container: {
+      image: magpieimage
+    }
+  }
+}

test/validation/k8s.go

@@ -340,8 +340,8 @@ func ValidateObjectsRunning(ctx context.Context, t *testing.T, k8s *kubernetes.C
 						deployedResources, err = dynamic.Resource(mapping.Resource).List(ctx, metav1.ListOptions{})
 					}
 					assert.NoErrorf(t, err, "could not list deployed resources of type %s in namespace %s", resourceGVR.GroupResource(), namespace)
-
-					validated = validated && matchesActualLabels(expectedInNamespace, deployedResources.Items)
+					t.Logf("Listed %d deployed resources of type %s in namespace %s", len(deployedResources.Items), resourceGVR.GroupResource(), namespace)
+					validated = validated && matchesActualLabels(t, expectedInNamespace, deployedResources.Items)
 
 				}
 			case <-ctx.Done():
@@ -532,7 +532,7 @@ func logPods(t *testing.T, pods []corev1.Pod) {
 	}
 }
 
-func matchesActualLabels(expectedResources []K8sObject, actualResources []unstructured.Unstructured) bool {
+func matchesActualLabels(t *testing.T, expectedResources []K8sObject, actualResources []unstructured.Unstructured) bool {
 	remaining := []K8sObject{}
 
 	for _, expectedResource := range expectedResources {
@@ -552,6 +552,8 @@ func matchesActualLabels(expectedResources []K8sObject, actualResources []unstru
 					resourceExists = true
 					actualResources = append(actualResources[:idx], actualResources[idx+1:]...)
 					break
+				} else {
+					t.Logf("Resource: %s Expected labels %v, got %v", actualResource.GetName(), expectedResource.Labels, actualResource.GetLabels())
 				}
 			}
 		}