From fb725ff75d5d103fe6c6abf26cc450cce6a3ac31 Mon Sep 17 00:00:00 2001 From: vinayada1 <28875764+vinayada1@users.noreply.github.com> Date: Wed, 29 Nov 2023 10:19:07 -0800 Subject: [PATCH] Changes to help debug test failure by using a different pod name --- .github/scripts/radius-bot.js | 69 +++++++++++++++++++ .../workflows/download-pr-data-artifact.yaml | 59 ---------------- .github/workflows/functional-test.yaml | 35 ++++------ .../workflows/functional-tests-approval.yaml | 18 ----- .github/workflows/save-pr-as-artifact.yaml | 21 ------ pkg/cli/cmd/app/delete/delete.go | 2 +- test/functional/shared/cli/cli_test.go | 28 ++++---- ...etes-cli-with-unassociated-resources.bicep | 40 +++++++++++ test/validation/k8s.go | 8 ++- 9 files changed, 146 insertions(+), 134 deletions(-) delete mode 100644 .github/workflows/download-pr-data-artifact.yaml delete mode 100644 .github/workflows/functional-tests-approval.yaml delete mode 100644 .github/workflows/save-pr-as-artifact.yaml create mode 100644 test/functional/shared/cli/testdata/corerp-kubernetes-cli-with-unassociated-resources.bicep diff --git a/.github/scripts/radius-bot.js b/.github/scripts/radius-bot.js index 72c6a55e9a9..1b8ef5b7f79 100644 --- a/.github/scripts/radius-bot.js +++ b/.github/scripts/radius-bot.js @@ -44,6 +44,9 @@ async function handleIssueCommentCreate({ github, context }) { case '/assign': await cmdAssign(github, issue, isFromPulls, username); break; + case '/ok-to-test': + await cmdOkToTest(github, issue, isFromPulls, username); + break; default: console.log(`[handleIssueCommentCreate] command ${command} not found, exiting.`); break; @@ -73,3 +76,69 @@ async function cmdAssign(github, issue, isFromPulls, username) { assignees: [username], }); } + +/** + * Trigger e2e test for the pull request. + * @param {*} github GitHub object reference + * @param {*} issue GitHub issue object + * @param {boolean} isFromPulls is the workflow triggered by a pull request? + * @param {string} username is the user who trigger the command + */ +async function cmdOkToTest(github, issue, isFromPulls, username) { + if (!isFromPulls) { + console.log('[cmdOkToTest] only pull requests supported, skipping command execution.'); + return; + } + + // Check if the user has permission to trigger e2e test with an issue comment + const org = 'radius-project'; + console.log(`Checking team membership for: ${username}`); + const isMember = await checkTeamMembership(github, org, process.env.TEAM_SLUG, username); + if (!isMember) { + console.log(`${username} is not a member of the ${teamSlug} team.`); + return; + } + + // Get pull request + const pull = await github.rest.pulls.get({ + owner: issue.owner, + repo: issue.repo, + pull_number: issue.number, + }); + + if (pull && pull.data) { + // Get commit id and repo from pull head + const testPayload = { + pull_head_ref: pull.data.head.sha, + pull_head_repo: pull.data.head.repo.full_name, + command: 'ok-to-test', + issue: issue, + }; + + console.log('Creating repository dispatch event for e2e test'); + + // Fire repository_dispatch event to trigger e2e test + await github.rest.repos.createDispatchEvent({ + owner: issue.owner, + repo: issue.repo, + event_type: 'functional-tests', + client_payload: testPayload, + }); + + console.log(`[cmdOkToTest] triggered E2E test for ${JSON.stringify(testPayload)}`); + } +} + +async function checkTeamMembership(github, org, teamSlug, username) { + try { + const response = await github.rest.teams.getMembershipForUserInOrg({ + org: org, + team_slug: teamSlug, + username: username, + }); + return response.data.state === 'active'; + } catch (error) { + console.log(`error: ${error}`) + return false; + } +} diff --git a/.github/workflows/download-pr-data-artifact.yaml b/.github/workflows/download-pr-data-artifact.yaml deleted file mode 100644 index aba8093917a..00000000000 --- a/.github/workflows/download-pr-data-artifact.yaml +++ /dev/null @@ -1,59 +0,0 @@ -# This workflow can be used together with save-pr-as-artifact custom action which uploads the PR number as an artifact. -# It downloads the artifact to retrieve the PR number. -name: Download PR number saved as an artifact -on: - workflow_run: - workflows: - - "save-pr-as-artifact" - types: - - completed - workflow_call: - outputs: - pr_number: - description: "PR number" - value: ${{ jobs.download_pr_number.outputs.pr_number }} - -jobs: - download_pr_number: - outputs: - pr_number: ${{ steps.set-pr-number.outputs.pr_number }} - if: github.event_name == 'workflow_run' - runs-on: ubuntu-latest - steps: - - name: Checkout repository - uses: actions/checkout@v2 - - - name: Download artifact - uses: actions/github-script@v6 - with: - script: | - let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({ - owner: context.repo.owner, - repo: context.repo.repo, - run_id: context.payload.workflow_run.id, - }); - let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => { - return artifact.name == "pr_number" - })[0]; - let download = await github.rest.actions.downloadArtifact({ - owner: context.repo.owner, - repo: context.repo.repo, - artifact_id: matchArtifact.id, - archive_format: 'zip', - }); - let fs = require('fs'); - fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/pr_number.zip`, Buffer.from(download.data)); - - - name: Unzip artifact - shell: bash - run: unzip pr_number.zip - - - name: Set PR number - id: set-pr-number - uses: actions/github-script@v6 - with: - script: | - let fs = require('fs'); - PR_NUMBER=fs.readFileSync('./pr_number').toString(); - console.log(`Setting output: pr_number=${PR_NUMBER}`); - core.setOutput('pr_number', PR_NUMBER) \ No newline at end of file diff --git a/.github/workflows/functional-test.yaml b/.github/workflows/functional-test.yaml index 1578ca79a6d..8ce09ac8574 100644 --- a/.github/workflows/functional-test.yaml +++ b/.github/workflows/functional-test.yaml @@ -23,11 +23,7 @@ on: - cron: "30 0,12 * * 0,6" # Dispatch on external events repository_dispatch: - types: [de-functional-test] - workflow_run: - workflows: ['Approve Functional Tests'] - types: - - completed + types: [functional-tests, de-functional-test] env: # Go version @@ -72,13 +68,9 @@ env: TF_RECIPE_MODULE_SERVER_URL: "http://tf-module-server.radius-test-tf-module-server.svc.cluster.local" jobs: - download_pr_data: - name: Download PR data - uses: test-org-admin/radius-main/.github/workflows/download-pr-data-artifact.yaml@main build: name: Build Radius for test runs-on: ubuntu-latest - needs: download_pr_data env: DE_IMAGE: 'ghcr.io/radius-project/deployment-engine' DE_TAG: 'latest' @@ -103,19 +95,22 @@ jobs: echo "CHECKOUT_REPO=${{ github.repository }}" >> $GITHUB_ENV echo "CHECKOUT_REF=${{ github.ref }}" >> $GITHUB_ENV echo "PR_NUMBER=${{ github.event.pull_request.number }}" >> $GITHUB_ENV - - name: 'Set PR context (workflow_run)' - if: github.event_name == 'workflow_run' + - name: Set up checkout target (repository_dispatch from /ok-to-test) + if: github.event_name == 'repository_dispatch' uses: actions/github-script@v6 with: - github-token: ${{ secrets.GITHUB_TOKEN }} + github-token: ${{ secrets.GH_RAD_CI_BOT_PAT }} script: | - const payload = context.payload.workflow_run; - let fs = require('fs'); - // Set environment variables - fs.appendFileSync(process.env.GITHUB_ENV, - `CHECKOUT_REPO=${payload.head_repository.full_name}\n`+ - `CHECKOUT_REF=${payload.head_sha}\n` + - `PR_NUMBER=${{ needs.download_pr_data.outputs.pr_number }}\n`); + const testPayload = context.payload.client_payload; + if (testPayload && testPayload.command === `ok-to-test`) { + var fs = require('fs'); + // Set environment variables + fs.appendFileSync(process.env.GITHUB_ENV, + `CHECKOUT_REPO=${testPayload.pull_head_repo}\n`+ + `CHECKOUT_REF=${testPayload.pull_head_ref}\n`+ + `PR_NUMBER=${testPayload.issue.number}` + ); + } - name: Set DE image and tag (repository_dispatch from de-functional-test) if: github.event_name == 'repository_dispatch' uses: actions/github-script@v6 @@ -581,4 +576,4 @@ jobs: title: `Scheduled functional test failed - Run ID: ${context.runId}`, labels: ['bug', 'test-failure'], body: `## Bug information \n\nThis bug is generated automatically if the scheduled functional test fails. The Radius functional test operates on a schedule of every 4 hours during weekdays and every 12 hours over the weekend. It's important to understand that the test may fail due to workflow infrastructure issues, like network problems, rather than the flakiness of the test itself. For the further investigation, please visit [here](${process.env.ACTION_LINK}).` - }) \ No newline at end of file + }) diff --git a/.github/workflows/functional-tests-approval.yaml b/.github/workflows/functional-tests-approval.yaml deleted file mode 100644 index 733d812b3bf..00000000000 --- a/.github/workflows/functional-tests-approval.yaml +++ /dev/null @@ -1,18 +0,0 @@ -name: 'Approve Functional Tests' -on: - pull_request: - branches: - - main - - features/* - - release/* -jobs: - wait_for_approval: - runs-on: ubuntu-latest - environment: functional-tests - steps: - - name: Wait for approval - run: | - echo 'Wait for approval' - save_pr_as_artifact: - name: 'Approve Functional Tests' - uses: test-org-admin/radius-main/.github/workflows/save-pr-as-artifact.yaml@main diff --git a/.github/workflows/save-pr-as-artifact.yaml b/.github/workflows/save-pr-as-artifact.yaml deleted file mode 100644 index 80e9028923b..00000000000 --- a/.github/workflows/save-pr-as-artifact.yaml +++ /dev/null @@ -1,21 +0,0 @@ -name: Save PR number as artifact -on: - workflow_call - -jobs: - save_pr_as_artifact: - runs-on: ubuntu-latest - steps: - - name: Checkout repository - uses: actions/checkout@v2 - - - name: Save PR number - run: | - mkdir -p ./pr - echo ${{ github.event.pull_request.number }} > ./pr/pr_number - - - name: Upload artifact - uses: actions/upload-artifact@v3 - with: - name: pr_number - path: pr/ \ No newline at end of file diff --git a/pkg/cli/cmd/app/delete/delete.go b/pkg/cli/cmd/app/delete/delete.go index f836033429e..fe24fd169ed 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/test/functional/shared/cli/cli_test.go b/test/functional/shared/cli/cli_test.go index e10af0fbe3a..ad9134c9c9e 100644 --- a/test/functional/shared/cli/cli_test.go +++ b/test/functional/shared/cli/cli_test.go @@ -526,6 +526,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() @@ -534,6 +535,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) @@ -576,34 +580,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, "containera-app-with-unassociated-resources"), + validation.NewK8sPodForResource(appNameUnassociatedResources, "containerb-app-with-unassociated-resources"), }, }, }) //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", "containerb-app-with-unassociated-resources") + require.NoErrorf(t, err, "failed to delete resource containerb-app-with-unassociated-resources") + 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", "containera-app-with-unassociated-resources") + require.NoErrorf(t, err, "failed to delete resource containera-app-with-unassociated-resources") }) } 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 00000000000..aef350689ce --- /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 containera 'Applications.Core/containers@2023-10-01-preview' = { + name: 'containera-app-with-unassociated-resources' + location: location + properties: { + application: app.id + container: { + image: magpieimage + } + } +} + +resource containerb 'Applications.Core/containers@2023-10-01-preview' = { + name: 'containerb-app-with-unassociated-resources' + location: location + properties: { + application: app.id + container: { + image: magpieimage + } + } +} diff --git a/test/validation/k8s.go b/test/validation/k8s.go index 6620202d3d6..e612b9553f6 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()) } } }