Add remote cypress orchestrator integration with integtest script #988

manasvinibs · 2024-01-06T00:50:08Z

Description

Adding remote cypress workflow orchestrator script integration with existing logic of integtest.sh script. This will keep the current behavior of integtest script as it is when triggered along with silently triggering the remote cypress workflows present in the plugin/components as defined in manifest json file and continue to poll them till its completion.

Changes summary:

Orchestrator integration with integtest script.
Adding manifest json file to define plugin/components that gets onboarded to use orchestrator workflow. This manifest is used to capture plugin repo name, workflow name, branch that needs to be tested and any additional security related configuration that needs to be passed to the cypress tests running remote github workflows.
In this iteration I'm adding Dashboards component to manifest but the workflow file and branch related information will be added once they are ready within Dashboards component.

Issues Resolved

Follow-up to
#972

#991

Check List

Commits are signed per the DCO using --signoff

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.

SuZhou-Joe · 2024-01-09T02:46:34Z

Can we have a demo log or something that we can take a look that can tell us how will the feature be like in the future?

manasvinibs · 2024-01-09T19:09:24Z

Can we have a demo log or something that we can take a look that can tell us how will the feature be like in the future?

Certainly! Here is the link to my forked repository, opensearch-dashboards-functional-test, where I am currently executing the integtest.sh script through a GitHub workflow for testing purposes.
https://github.com/manasvinibs/opensearch-dashboards-functional-test/actions/runs/7427907888/job/20214377315

The Run Bash Script logs display the initiation of the remote Cypress GitHub workflow and its continuous polling until completion. The existing logic within the integtest.sh script, designed for running in-house Cypress tests, now accommodates the wait for the remote workflow to finish before executing the remaining logic at the end. Consequently, in this scenario, an expected Cypress error is anticipated due to the absence of a configured Cypress server locally, a condition that occurs towards the conclusion of the process.

SuZhou-Joe · 2024-01-10T05:10:00Z

integtest.sh

+    os_url=$(echo "$component" | jq -r '.["opensearch"]')
+    osd_url=$(echo "$component" | jq -r '.["opensearch-dashboards"]')


What about using the
https://artifacts.opensearch.org/releases/bundle/opensearch/{version_from_manifest}/opensearch-{version_from_manifest}-linux-x64.tar.gz

https://artifacts.opensearch.org/releases/bundle/opensearch-dashboards/{version_from_manifest}/opensearch-dashboards-{version_from_manifest}-linux-x64.tar.gz

as fallbacks of os_url and osd_url?

Sure! I think OS and OSD url could be other platform/distribution combination as well in addition to linux-x64.tar. But I think we can default to linux-x64.tar when we don't have one defined in manifest. Updated in the latest revision.

SuZhou-Joe · 2024-01-10T05:22:08Z

remote_cypress_manifest.json

+  "schema-version": "1.0",
+  "build": {
+    "name": "OpenSearch Dashboards Functional Test",
+    "version": "2.12.0"


On main the version should be 3.0.0 I think.

That's right. Updated.

SuZhou-Joe · 2024-01-10T05:23:18Z

remote_cypress_manifest.json

+      "workflow-name": "",
+      "ref": "",
+      "opensearch": "test",
+      "opensearch-dashboards": "test",


As this is an open PR, can we modify these dummy configs to valid ones?

Yes for main I have added linux-x64 artifact for now. Release team can choose to update component manifest json for other artifact distribution as needed.

SuZhou-Joe · 2024-01-10T05:29:43Z

integtest.sh

+done
+
+# Wait for all processes to finish
+wait "${all_process_pids[@]}"


Can we make remote cypress tasks and FTRepo test task parallel and wait for both to finish? Or I think FTRepo's task will wait for a long time to start.

We wanted to introduce this feature silently integrating in the background of existing integtest script runner for the first iteration to observe the orchestrator behavior. By keeping FTRepo test task run in the foreground after wait, we are not changing the behavior in terms of response code of the integtest script completion. We can make it run in background parallel to other remote cypress tasks, when we are ready to read the response codes from all the tasks integrated and send to integtest script caller (infra team jenkins job).
I agree there will be delay in the start of the FTRepo task but this will help in to ensure better control, immediate visibility into the results, and simplified error handling. This approach aligns with our current requirements and allows us to seamlessly integrate the remote test results into subsequent processes. Let me know if you have other thoughts.

ruanyl · 2024-01-10T06:06:05Z

~~@manasvinibs Is there design doc or RFC of this change? I'd like to understand the motivation and long term vision of introducing this feature, it seems this is just the first step :)~~

Found this issue: opensearch-project/OpenSearch-Dashboards#5392

ruanyl · 2024-01-10T08:03:07Z

remote_cypress_manifest.json

+    "version": "2.12.0"
+  },
+  "ci": {
+    "image": {


Q: how will this image be used?

This manifest value is optional for now and serves to specify a container image where tests are run , as required by the Orchestrator caller (release workflow). It becomes relevant when we aim to ensure not only the execution of tests on specific version/artifacts but also the use of a specific image configured by the release team. The Orchestrator passes this information to the remote Cypress workflows running in the components/plugins, allowing them to align their GitHub workflows with the image provided by the Orchestrator. While not mandatory, it provides added flexibility to the Orchestrator, allowing it to dictate the container image if/when necessary.

ruanyl · 2024-01-10T08:37:52Z

remoteCypress.sh

+# Check if required arguments are provided
+if [[ -n "$REPO" && -n "$WORKFLOW_NAME" && -n "$OS_URL" && -n "$OSD_URL" && -n "$BRANCH_REF" ]]; then
+  # Accessing the secret as an environment variable using Github actions while invoking this script
+  GITHUB_TOKEN=$GITHUB_SECRET_TOKEN


Is GITHUB_SECRET_TOKEN a newly added env var or existing one? Which party is maintaining it?

Yes GITHUB_SECRET_TOKEN is newly added env variable part of the github workflow - https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/.github/workflows/remote-cypress-workflow.yml#L33 but the token used is existing one within the team's repo used by other github workflows such as delete workflow

I see, is there cases that this script is run outside of github workflow where GITHUB_TOKEN is not set?

Currently, this will be run as part of integtest script. Along with that one can manually trigger the remoteCypress script using dispatch event. When integtest script is triggered as part of jenkins job from infra team, Github token is already set in Jenkins environment which is accessed within this script when run in the jenkins environment.

kavilla · 2024-01-25T00:21:35Z

integtest.sh

+    source remoteCypress.sh -r "$repo" -w "$workflow_name" -o "$os_url" -d "$osd_url" -b "$branch_ref" &
+    bg_process_pid=$!
+    echo "PID for the repo $repo is : $bg_process_pid"
+    all_process_pids+=($bg_process_pid)


i think it might be worth to consider using pid files https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/config/opensearch_dashboards.yml#L122

i didn't realize this logic existed but has long as the file is created we can reference the process id file instead of it in memory

kavilla · 2024-01-25T00:24:50Z

integtest.sh

+REMOTE_MANIFEST_FILE="remote_cypress_manifest.json"
+
+# Parse the JSON file using jq and iterate over the components array
+components=$(jq -c '.components[]' "$REMOTE_MANIFEST_FILE")


nit: for readability it might be helpful to have a helper function for this and perhaps it returns back a tuple with the required information

kavilla · 2024-01-25T00:25:30Z

integtest.sh

+    branch_ref=$(echo "$component" | jq -r '.["ref"]')
+
+    # Set default values if the opensearch and opensearch-dashboards are not set in the manifest
+    os_url=${os_url:-https://artifacts.opensearch.org/releases/bundle/opensearch/$release_version/opensearch-$release_version-linux-x64.tar.gz}


linux-x64. i think it should get the operating system of the runner and arch

I've adjusted the manifest to get operating system/arch and constructing the url while parsing the manifest with the base url being https://ci.opensearch.org/ci/dbc/distribution-build/.

kavilla · 2024-01-25T00:34:23Z

integtest.sh

+    echo "branch_ref: $branch_ref"
+
+    # Call the function for each component
+    run_remote_cypress "$repo" "$workflow_name" "$os_url" "$osd_url" "$branch_ref" 


i think we should add a flag or env variable that enables us to conditional run the run remote cypress tests are not. since this repo is now being locked as well to commits a new build will have to be made if there are any issues. so it pre-planning a feature flag could be a fallback to disabling this if it causes issues.

I referenced the developer guide doc about setting up flag for experimental features and its tests. As my feature is not actually running cypress tests but instead running orchestrator workflows, I've introduced a simple boolean flag as part of integtest.sh script params defaulting to true. Build script which triggers integtest.sh can set the feature flag to true/false as required. Is this the behavior you were expecting or is there some other way to configure this setting in FTRepo?

Signed-off-by: manasvinibs <[email protected]>

manasvinibs · 2024-01-31T19:51:04Z

@kavilla @SuZhou-Joe @ruanyl Can you please review and let me know if there anything else you would like me to address to get this checked in? I'm hoping to get this in for 2.12 and would like to have some time to bake this changes in main and address any issues related to integration.

kavilla · 2024-02-01T22:14:31Z

remote_cypress_manifest.json

+      "repository": "opensearch-project/OpenSearch-Dashboards",
+      "workflow-name": "",
+      "ref": "",
+      "operating-system": "linux",


kavilla · 2024-02-01T22:15:26Z

integtest.sh

+    done < "$pid_file"
+}
+
+if [[ $REMOTE_CYPRESS_ENABLED = "true" &&  $ORCHESTRATOR_FEATURE_FLAG = 'true' ]]; then


check if env variable for github token if not then don't execute this.

Sure! I have a follow-up PR to add manifest data to this. I'll address this one in that.

…earch-project#988 backports Signed-off-by: Peter Zhu <[email protected]>

…backports and disable windows video recording (#1070) * [2.x] Adding placeholder to keep main/2.x/1.x consistent before #988 backports Signed-off-by: Peter Zhu <[email protected]> * Add more Signed-off-by: Peter Zhu <[email protected]> --------- Signed-off-by: Peter Zhu <[email protected]>

…backports and disable windows video recording (#1070) * [2.x] Adding placeholder to keep main/2.x/1.x consistent before #988 backports Signed-off-by: Peter Zhu <[email protected]> * Add more Signed-off-by: Peter Zhu <[email protected]> --------- Signed-off-by: Peter Zhu <[email protected]> (cherry picked from commit 4add696)

…backports and disable windows video recording (#1070) (#1071) * [2.x] Adding placeholder to keep main/2.x/1.x consistent before #988 backports Signed-off-by: Peter Zhu <[email protected]> * Add more Signed-off-by: Peter Zhu <[email protected]> --------- Signed-off-by: Peter Zhu <[email protected]> (cherry picked from commit 4add696) Co-authored-by: Peter Zhu <[email protected]>

peterzhuamazon · 2024-02-06T19:46:44Z

Hi @manasvinibs if you ever want to backport please note that I have add placeholder for -r in case 1.x/2.x wont fail when we call -r false.
#1070

We have to disable the remote test settings in build repo due to it failing out production integTest on Jenkins. Thanks.

opensearch-trigger-bot · 2024-02-14T21:53:01Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-988-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 a844c23837830641a3273778c96179d3c5099bc4
# Push it to GitHub
git push --set-upstream origin backport/backport-988-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-988-to-2.x.

manasvinibs requested review from xluo-aws, Hailong-am, SuZhou-Joe, ruanyl, wanglam and raintygao as code owners January 6, 2024 00:50

SuZhou-Joe reviewed Jan 10, 2024

View reviewed changes

ruanyl reviewed Jan 11, 2024

View reviewed changes

manasvinibs force-pushed the Issue-972-integration branch 2 times, most recently from d96118f to dd3511f Compare January 17, 2024 20:26

manasvinibs requested review from ruanyl and SuZhou-Joe January 18, 2024 00:06

kavilla linked an issue Jan 23, 2024 that may be closed by this pull request

Add remote cypress tests orchestrator integration as part of integtest.sh script #991

Closed

kavilla mentioned this pull request Jan 23, 2024

[Tests] Add Github workflow for Test Orchestrator in FT Repo to run cypress tests within Dashboards repo opensearch-project/OpenSearch-Dashboards#5725

Merged

7 tasks

kavilla reviewed Jan 25, 2024

View reviewed changes

manasvinibs force-pushed the Issue-972-integration branch 2 times, most recently from 7b55235 to cb4644a Compare January 29, 2024 19:53

manasvinibs requested a review from kavilla January 30, 2024 01:38

manasvinibs force-pushed the Issue-972-integration branch from cb4644a to 51411a8 Compare January 31, 2024 00:22

Add remote cypress orchestrator integration with intestest script

fdb9bf3

Signed-off-by: manasvinibs <[email protected]>

manasvinibs force-pushed the Issue-972-integration branch from 51411a8 to fdb9bf3 Compare January 31, 2024 19:28

kavilla reviewed Feb 1, 2024

View reviewed changes

kavilla approved these changes Feb 1, 2024

View reviewed changes

ananzh approved these changes Feb 2, 2024

View reviewed changes

ananzh merged commit a844c23 into opensearch-project:main Feb 2, 2024
38 checks passed

manasvinibs mentioned this pull request Feb 3, 2024

Add token credential check before calling orchestrator logic #1050

Draft

1 task

peterzhuamazon added a commit to peterzhuamazon/opensearch-dashboards-functional-test that referenced this pull request Feb 6, 2024

[2.x] Adding placeholder to keep main/2.x/1.x consistent before opens…

b10f8d5

…earch-project#988 backports Signed-off-by: Peter Zhu <[email protected]>

peterzhuamazon mentioned this pull request Feb 6, 2024

[2.x] Adding placeholder to keep main/2.x/1.x consistent before #988 backports and disable windows video recording #1070

Merged

1 task

ashwin-pc added the backport 2.x label Feb 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add remote cypress orchestrator integration with integtest script #988

Add remote cypress orchestrator integration with integtest script #988

manasvinibs commented Jan 6, 2024 •

edited

Loading

SuZhou-Joe commented Jan 9, 2024

manasvinibs commented Jan 9, 2024

SuZhou-Joe Jan 10, 2024

manasvinibs Jan 17, 2024

SuZhou-Joe Jan 10, 2024

manasvinibs Jan 17, 2024

SuZhou-Joe Jan 10, 2024

manasvinibs Jan 17, 2024

SuZhou-Joe Jan 10, 2024

manasvinibs Jan 17, 2024

ruanyl commented Jan 10, 2024 •

edited

Loading

ruanyl Jan 10, 2024

manasvinibs Jan 17, 2024

ruanyl Jan 10, 2024

manasvinibs Jan 18, 2024

ruanyl Jan 22, 2024

manasvinibs Jan 22, 2024 •

edited

Loading

kavilla Jan 25, 2024

kavilla Jan 25, 2024

kavilla Jan 25, 2024

manasvinibs Jan 29, 2024

kavilla Jan 25, 2024

manasvinibs Jan 29, 2024

manasvinibs commented Jan 31, 2024

kavilla Feb 1, 2024

kavilla Feb 1, 2024

manasvinibs Feb 1, 2024

peterzhuamazon commented Feb 6, 2024 •

edited

Loading

opensearch-trigger-bot bot commented Feb 14, 2024

		os_url=$(echo "$component" \| jq -r '.["opensearch"]')
		osd_url=$(echo "$component" \| jq -r '.["opensearch-dashboards"]')

Add remote cypress orchestrator integration with integtest script #988

Add remote cypress orchestrator integration with integtest script #988

Conversation

manasvinibs commented Jan 6, 2024 • edited Loading

Description

Issues Resolved

Check List

SuZhou-Joe commented Jan 9, 2024

manasvinibs commented Jan 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruanyl commented Jan 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manasvinibs Jan 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manasvinibs commented Jan 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterzhuamazon commented Feb 6, 2024 • edited Loading

opensearch-trigger-bot bot commented Feb 14, 2024

manasvinibs commented Jan 6, 2024 •

edited

Loading

ruanyl commented Jan 10, 2024 •

edited

Loading

manasvinibs Jan 22, 2024 •

edited

Loading

peterzhuamazon commented Feb 6, 2024 •

edited

Loading