Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow users to provide essential information (PR number, sha, etc.) when running on non-pull_request contexts #206

Closed
yih-redhat opened this issue Jul 11, 2024 · 24 comments · Fixed by #220
Assignees
Labels
type: feature New feature or request

Comments

@yih-redhat
Copy link

yih-redhat commented Jul 11, 2024

Type of issue

None

Description

  1. I have a github workflow file with two steps. The first step is to get all information of pull request, the second step is to run testing-farm github plugin. And the second step always failed with this error.
    Workflow file is at: https://github.com/osbuild/rhel-edge-ci/blob/main/.github/workflows/rhel-95.yml
    Test result is at: https://github.com/osbuild/rhel-edge-ci/actions/runs/9885421251
  2. To debug this error, I suspect that it may caused by some parameters of testing-farm plugin in second step, so I removed almost all parameters and just keep 'compose' and 'arch', but still got this error.
  3. Then I suspect maybe there is something wrong with my first step, so I created another similar workflow file, and didn't use testing-farm plugin, and it works well
    Workflow file is at: https://github.com/osbuild/rhel-edge-ci/blob/main/.github/workflows/test.yml
    Test result is at: https://github.com/osbuild/rhel-edge-ci/actions/runs/9885421253

Reproducer

No response

@yih-redhat
Copy link
Author

I think I found the reason, that tfaga may not support repository_dispatch.

To verify this bug, I created two workflow files, they are very simple and basically identical, but one is triggered by issue comment, another is triggered by repository_dispatch.

  1. triggered by issue comment : https://github.com/osbuild/rhel-edge-ci/blob/main/.github/workflows/trigger-by-comment.yml , the test result is: https://github.com/osbuild/rhel-edge-ci/actions/runs/9902506411 , the test result shows that at least tfaga didn't report this error and started to work. ( failed later because I didn't provide git url and ref)
  2. triggered by repository_dispatch: https://github.com/osbuild/rhel-edge-ci/blob/main/.github/workflows/trigger-by-gitlab.yml , the test result is: https://github.com/osbuild/rhel-edge-ci/actions/runs/9902495726 , the test result shows that tfaga failed by this error immediately.

@jamacku jamacku added the type: bug Something isn't working label Jul 15, 2024
@jamacku jamacku self-assigned this Jul 15, 2024
@jamacku
Copy link
Member

jamacku commented Jul 15, 2024

The action doesn't currently support the repository_dispatch trigger. It was designed mainly for pull_request targets, but I think we can try to make it work for other targets.

@yih-redhat
Copy link
Author

@jamacku Thanks, do you have plan to add support for repository_dispatch trigger recently? Our CI need this.

@phracek
Copy link
Member

phracek commented Jul 17, 2024

@yih-redhat Currently now, we do not have a capacity to implement repository_dispatch. I would prefer to file issue for it with an example.

@phracek
Copy link
Member

phracek commented Jul 17, 2024

  1. triggered by issue comment : https://github.com/osbuild/rhel-edge-ci/blob/main/.github/workflows/trigger-by-comment.yml , the test result is: https://github.com/osbuild/rhel-edge-ci/actions/runs/9902506411 , the test result shows that at least tfaga didn't report this error and started to work. ( failed later because I didn't provide git url and ref)

In case you want to report anything, in status, you have to allow it. It is not enabled by default. See https://github.com/sclorg/testing-farm-as-github-action?tab=readme-ov-file#miscellaneous and enable update_pull_request_status.

@yih-redhat
Copy link
Author

  1. triggered by issue comment : https://github.com/osbuild/rhel-edge-ci/blob/main/.github/workflows/trigger-by-comment.yml , the test result is: https://github.com/osbuild/rhel-edge-ci/actions/runs/9902506411 , the test result shows that at least tfaga didn't report this error and started to work. ( failed later because I didn't provide git url and ref)

In case you want to report anything, in status, you have to allow it. It is not enabled by default. See https://github.com/sclorg/testing-farm-as-github-action?tab=readme-ov-file#miscellaneous and enable update_pull_request_status.

No, it's not about update_pull_request_status. Actually I need tfaga start to work when action is triggered by repository_dispatch.

I have used tfaga in another project, and enabled update_pull_request_status, it works well (triggered by issue comment) https://github.com/virt-s1/rhel-edge/blob/main/.github/workflows/rhel-9-5.yml#L67

@yih-redhat
Copy link
Author

@yih-redhat Currently now, we do not have a capacity to implement repository_dispatch. I would prefer to file issue for it with an example.

File issue in issue.redhat.com? for which component?

@jamacku
Copy link
Member

jamacku commented Jul 17, 2024

@jamacku Thanks, do you have plan to add support for repository_dispatch trigger recently? Our CI need this.

This should be easy to fix. I'll try to have a look soonish.

@phracek
Copy link
Member

phracek commented Jul 17, 2024

redhat

Sorry, my mistake. I thought, that this is pull request.

@phracek
Copy link
Member

phracek commented Jul 17, 2024

I am still thinking whether this option is really needed in our action? What is use case? I read GitHub event https://docs.github.com/en/webhooks/webhook-events-and-payloads#repository_dispatch. What about serializing the action?

@yih-redhat
Copy link
Author

I am still thinking whether this option is really needed in our action? What is use case? I read GitHub event https://docs.github.com/en/webhooks/webhook-events-and-payloads#repository_dispatch. What about serializing the action?

Sometimes, a github project CI env can be complicated. It may need to trigger a workflow from outside github, or it may need to trigger another workflow when current workflow finished. In all these cases, repository_dispatch is a better solution.

In my case, our upstream project is https://github.com/osbuild/osbuild-composer , and as a github project, it uses gitlab CI to build and upload rpm packages for all the OS versions when a new pull request is created. I need to trigger tfaga test after gitlab CI finished build/upload.

@jamacku jamacku linked a pull request Jul 18, 2024 that will close this issue
@jamacku
Copy link
Member

jamacku commented Jul 18, 2024

@yih-redhat, I have submitted PR that should fix your issue. Could you please test it with your setup?

PR: #207

@yih-redhat
Copy link
Author

Thanks @jamacku , could you please let me know the steps to test this pr?

I usually use tfaga in my workflow file like this, I don't know how to test this pr.

  • name: Run testing-farm tests
    uses: sclorg/testing-farm-as-github-action@v2
    with:
    compose: RHEL-9.5.0-Nightly
    arch: x86_64

@jamacku
Copy link
Member

jamacku commented Jul 18, 2024

@yih-redhat, you can test it by using my fork and branch with the fix (jamacku/testing-farm-as-github-action@dispatch):

uses: jamacku/testing-farm-as-github-action@dispatch
#  ...

@yih-redhat
Copy link
Author

Trying now, will keep you posted.

@yih-redhat
Copy link
Author

@jamacku Your fix works as expected, thanks!

The pull request is osbuild/osbuild-composer#4240 , and this pr triggered the workflow in https://github.com/osbuild/rhel-edge-ci, the action log is https://github.com/osbuild/rhel-edge-ci/actions/runs/9991173802

@yih-redhat
Copy link
Author

I still need tfaga to be able to update the pull request status when it is triggered by repository_dispatch. I will file another issue for this with more details.

@jamacku
Copy link
Member

jamacku commented Jul 18, 2024

Please don't create a new issue. We can discuss it here.

When you run it as' repository_dispatch, ' we don't know which PR to update. You have to provide the PR number when calling repository_dispatch.

@yih-redhat
Copy link
Author

yih-redhat commented Jul 18, 2024

Yes, I can provide target repo name, PR number or sha number, and then tfaga can update the pull request status.

I don't know how tfaga update the pull request status, but before we use tfaga, we use this plugin to create the test status in target pull request. Here is the example for your reference.
name: Create test status
uses: octokit/request-action@v2.x
with:
route: 'POST /repos/osbuild/osbuild-composer/statuses/${{ target_pull_request.sha }}'
context: 'RHEL 9.4 test'
state: pending
description: 'RHEL 9.4 - Test has been running...'
target_url: ${{ test_job.html_url }}

I suppose tfaga needs at least the repo name, the pr number or sha number and a valid github_token to do this.

@jamacku jamacku added type: feature New feature or request and removed type: bug Something isn't working labels Jul 18, 2024
@jamacku
Copy link
Member

jamacku commented Jul 18, 2024

Yes, we need: owner/repo + pr number (when creating comments) + sha (when creating/updating statuses).

Are you able to provide this information to us when triggering repository_dispatch

@yih-redhat
Copy link
Author

Yes, I can provide this information. Actually anyone who uses repository_dispatch to trigger tfaga should be able to provide this information, as he knows all the information of original repo and pr.
Please just let me know how you will implement this, and the new input names that you will add in https://github.com/sclorg/testing-farm-as-github-action?tab=readme-ov-file#miscellaneous, I'm glad to test it.

@jamacku
Copy link
Member

jamacku commented Jul 18, 2024

Great, I'll add inputs for your usecase.

@jamacku jamacku changed the title Error: Cannot read properties of undefined (reading 'sha') Allow users to provide essential information (PR number, sha, etc.) when running on non-pull_request contexts Jul 19, 2024
@yih-redhat
Copy link
Author

@jamacku Can I get a pr to test in this week?

@jamacku
Copy link
Member

jamacku commented Jul 24, 2024

I'll try to provide testing PR by the end of the week.

@jamacku jamacku linked a pull request Aug 1, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants