From 1b32aa11c719850c6ade8a87d228c1b4b464d8cc Mon Sep 17 00:00:00 2001 From: vinayada1 <28875764+vinayada1@users.noreply.github.com> Date: Mon, 4 Dec 2023 11:29:49 -0800 Subject: [PATCH] Logs and some test changes to help debug test failure in Test_CLI_Delete (#6881) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Changes to debug test failure issue: https://github.com/radius-project/radius/issues/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 - 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). Fixes: #issue_number ## Auto-generated summary ### ๐Ÿค–[[deprecated]](https://githubnext.com/copilot-for-prs-sunset) Generated by Copilot at fb725ff ### Summary ๐Ÿ—‘๏ธ๐Ÿณ๐Ÿงช 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 <28875764+vinayada1@users.noreply.github.com> Co-authored-by: Nithya Subramanian <98416062+nithyatsu@users.noreply.github.com> --- pkg/cli/cmd/app/delete/delete.go | 2 +- pkg/cli/cmd/app/delete/delete_test.go | 6 ++- test/functional/shared/cli/cli_test.go | 28 +++++++------ ...etes-cli-with-unassociated-resources.bicep | 40 +++++++++++++++++++ test/validation/k8s.go | 8 ++-- 5 files changed, 66 insertions(+), 18 deletions(-) create mode 100644 test/functional/shared/cli/testdata/corerp-kubernetes-cli-with-unassociated-resources.bicep diff --git a/pkg/cli/cmd/app/delete/delete.go b/pkg/cli/cmd/app/delete/delete.go index f836033429..fe24fd169e 100644 --- a/pkg/cli/cmd/app/delete/delete.go +++ b/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) } diff --git a/pkg/cli/cmd/app/delete/delete_test.go b/pkg/cli/cmd/app/delete/delete_test.go index 2bb491260b..8ce34e89cf 100644 --- a/pkg/cli/cmd/app/delete/delete_test.go +++ b/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"}, }, } diff --git a/test/functional/shared/cli/cli_test.go b/test/functional/shared/cli/cli_test.go index f3090d43dc..f675f9c6fa 100644 --- a/test/functional/shared/cli/cli_test.go +++ b/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") }) } diff --git a/test/functional/shared/cli/testdata/corerp-kubernetes-cli-with-unassociated-resources.bicep b/test/functional/shared/cli/testdata/corerp-kubernetes-cli-with-unassociated-resources.bicep new file mode 100644 index 0000000000..7851aa4046 --- /dev/null +++ b/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 + } + } +} diff --git a/test/validation/k8s.go b/test/validation/k8s.go index 6620202d3d..e612b9553f 100644 --- a/test/validation/k8s.go +++ b/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()) } } }