-
Notifications
You must be signed in to change notification settings - Fork 113
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
Add test orchestrator script to trigger remote workflow runner outside functional test repo for cypress tests #971
Add test orchestrator script to trigger remote workflow runner outside functional test repo for cypress tests #971
Conversation
65636fd
to
11a4668
Compare
i think we will need a way of telling that the security plugin is installed for specific logic by the test runner and a way to add cluster configs like: https://github.com/opensearch-project/opensearch-build/blob/537ccdbfc23be07e6de08236dce95906ea5b6719/manifests/2.10.0/opensearch-dashboards-2.10.0-test.yml#L14 |
PAYLOAD="{\"ref\": \"$BRANCH_REF\",\"inputs\":{\"build_id\":\"$BUILD_ID\", \"OS_URL\":\"$OS_URL\", \"OSD_URL\":\"$OSD_URL\", \"UNIQUE_ID\":\"$UNIQUE_WORKFLOW_ID\"}}" | ||
|
||
# Maximum number of retries for triggering the remote runner | ||
MAX_RETRIES=3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the automatic? or this the amount of triggered from the UI?
if so i think it should just fail fast.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for every run of the script, it triggers max 3 times the dispatch event API. Are you suggesting we can fail at first try if dispatch API errors out?
on: | ||
workflow_dispatch: | ||
inputs: | ||
repo: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we follow the format of the current manifest?: https://github.com/opensearch-project/opensearch-build/blob/537ccdbfc23be07e6de08236dce95906ea5b6719/manifests/2.10.0/opensearch-2.10.0.yml#L18 it passes by url
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, we can take input in url format but then we will have to extract the owner
and repo
name from it as the workflow dispatch API expects it in that format https://docs.github.com/en/rest/actions/workflow-runs?apiVersion=2022-11-28#get-a-workflow-run.
required: true | ||
os_url: | ||
description: 'Release artifact of OpenSearch' | ||
required: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not opposed to passing this value but i think it can be optional. in case i wanted to setup a workflow that scales up a snapshot for cypress tests but then i can write my workflow so that if i do get release artifacts then i can utilize that to scale up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that can be totally optional. Just to make it clear, this workflow yml file is different from the workflow yml file we will use in components repo. Here the input params are required because this gets triggered from release and hence it expects to pass release artifact to the test orchestrator script. But yes, within component repo workflow yml file, it can be optional as the same workflow can be used by plugins to scale up snapshot to run the tests against.
remoteCypress.sh
Outdated
} | ||
|
||
# Initialize variables to empty strings | ||
REMOTE_REPO="" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it should be caught with the -z
on line 65 so you can just delete this section .
remoteCypress.sh
Outdated
exit 1 | ||
;; | ||
r) | ||
REMOTE_REPO="$OPTARG" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: REPO since the input is just repo
remoteCypress.sh
Outdated
# name in the component repository yaml file for polling purpose. | ||
UNIQUE_WORKFLOW_ID=$(uuidgen) | ||
echo "Unique Execution ID: $UNIQUE_WORKFLOW_ID" | ||
API_URL="https://api.github.com/repos/$REMOTE_REPO/actions/workflows/$WORKFLOW_NAME/dispatches" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wondering if we want to turn this into a function or leave a TODO if we were interested in using other testing systems like jenkins
remoteCypress.sh
Outdated
local polling_run_id_retries=1 | ||
local polling_workflow_completion_retries=1 | ||
local max_polling_run_id_retries=5 # Keep polling for the first 5 minutes to fetch the workflow run id till the workflow gets generated | ||
local max_polling_workflow_completion_retries=12 # Set the polling window period to be 1 hour |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this be another variable?
remoteCypress.sh
Outdated
workflow_runs=$(curl -s -H "Authorization: Bearer $GITHUB_TOKEN" \ | ||
-H "Accept: application/vnd.github.v3+json" \ | ||
-H "X-GitHub-Api-Version: 2022-11-28" \ | ||
"https://api.github.com/repos/$REMOTE_REPO/actions/workflows/$WORKFLOW_NAME/runs") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be API url and then we append dispatch
remoteCypress.sh
Outdated
done | ||
|
||
# Function to check the status of the remote workflow by constantly polling the workflow-run | ||
check_remote_workflow_status() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can be separate files and source
them in a main script
w) | ||
WORKFLOW_NAME="$OPTARG" | ||
;; | ||
o) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we think we have a pre-verification step? don't dispatch jobs. if OpenSearch and OpenSearch Dashboards can't start
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, I'm dispatching workflow event and not really dispatching jobs which are part of the workflow. I believe Opensearch and Opensearch Dashboards start setup script is part of the workflow yaml. I think if we have it as a jobs and if they cannot start which fails the job, it will terminate the orchestrator script as it is. But let me try to see if there is way to do pre-verification as you are suggesting in the follow-up iteration.
…e functional test repo for cypress tests Issue details - opensearch-project/OpenSearch-Dashboards#5392 Signed-off-by: manasvinibs <[email protected]>
11a4668
to
6ded4d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, it's not wired into anything yet. So this can be iterative and we can consider to flip to switch and add it to something that gets called automatically but we should do so in a way that calls the remote runner and still execute the tests here.
This way we can let people optionally migrate to the new setup if they please, still execute both ways for anyone consuming the results and not cause any breaking changes.
…e functional test repo for cypress tests (#971) Issue details - opensearch-project/OpenSearch-Dashboards#5392 Signed-off-by: manasvinibs <[email protected]> (cherry picked from commit 76270ea)
…e functional test repo for cypress tests (#971) (#986) Issue details - opensearch-project/OpenSearch-Dashboards#5392 Signed-off-by: manasvinibs <[email protected]> (cherry picked from commit 76270ea) Co-authored-by: Manasvini B Suryanarayana <[email protected]>
Issue details - opensearch-project/OpenSearch-Dashboards#5392
POC - opensearch-project/OpenSearch-Dashboards#5526
Description
Adds a script which acts as a test orchestrator to trigger Github workflow runners in a plugin/component repository. With this feature, during release testing build team will be able to use this script to trigger the cypress tests hosted within plugins repositories and make sure if those workflows succeeded or not. Every plugin component can write tests in the same repository where their feature resides and do not have to paste the tests in functional test repo which is outside of the feature changes.
Issues Resolved
#972
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.