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

[MD] Replace fake data source endpoint with configurable real endpoint #1260

Merged
merged 1 commit into from
May 6, 2024

Conversation

zhongnansu
Copy link
Member

@zhongnansu zhongnansu commented May 1, 2024

Description

  • Replace fake data source endpoint with real endpoint (auth, and no auth)
  • skip sigv4 related tests since infra workflow doesn't support create , and those tests will fail

image

Test

There are 4 CI related to the function tests is OSD core, and they all failed, not caused by MDS tests and test infra.

  1. e2e windows, with security: failed to spin up opensearch, not even reach cypress test step
  2. e2e windows, without security: failed because workflow template doesn't allow passing osd arguments data_source.enabled=true
    # not useful now as the windows e2e template currently do not allow serving parameters
    #osd-serve-args: --data_source.enabled=true --vis_builder.enabled=true
  3. e2e linux, with security: cypress crashed, not even reach to MDS test
  4. e2e linux, without security: MDS test can pass, viz builder tests are failing

Issues Resolved

#1202

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.

@zhongnansu zhongnansu force-pushed the test-md-infra branch 3 times, most recently from e3efc7a to 6d23887 Compare May 2, 2024 00:44
Signed-off-by: Zhongnan Su <szhongna@amazon.com>
@zhongnansu zhongnansu changed the title test MD infra [MD] Replace fake endpoint with real endpoint May 3, 2024
@zhongnansu zhongnansu changed the title [MD] Replace fake endpoint with real endpoint [MD] Replace fake data source endpoint with real endpoint May 3, 2024
@zhongnansu zhongnansu changed the title [MD] Replace fake data source endpoint with real endpoint [MD] Replace fake data source endpoint with configurable real endpoint May 3, 2024
@zhongnansu zhongnansu marked this pull request as ready for review May 3, 2024 18:01
@@ -104,11 +104,11 @@ if (Cypress.env('DATASOURCE_MANAGEMENT_ENABLED')) {
'app/management/opensearch-dashboards/dataSources'
);
});

it('with sigV4 and all required inputs to connect to OpenSearch Service', () => {
// TODO: once create datasource with sigv4 is in plance, remove the skip
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added that on purpose, since current test infra in workflow was added to support no auth and basic auth. It doesn't support creating AOS/AOSS domain yet. We need to think of a way to either creating one from infra, or add a long running one by configuring it in github secrets for testing purpose

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can add more env variables too whether to ran specific suite of tests

@@ -17,12 +17,12 @@ jobs:
with:
test-name: Core Dashboards using Bundle Snapshot
test-command: env CYPRESS_NO_COMMAND_LOG=1 CYPRESS_ML_COMMONS_DASHBOARDS_ENABLED=true CYPRESS_VISBUILDER_ENABLED=true CYPRESS_DATASOURCE_MANAGEMENT_ENABLED=true yarn cypress:run-with-security --browser chromium --spec 'cypress/integration/core-opensearch-dashboards/opensearch-dashboards/**/*.js'
osd-serve-args: --data_source.enabled=true --vis_builder.enabled=true --ml_commons_dashboards.enabled=true
osd-serve-args: --data_source.enabled=true --data_source.ssl.verificationMode=none --vis_builder.enabled=true --ml_commons_dashboards.enabled=true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a heads up.

the cypress workflow in OSD doesn't run these tests per PR. It runs with the ciGroup scripts.

The workflow then calls them here: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/.github/workflows/cypress_workflow.yml#L122

Then OSD gets started up with:
https://github.com/opensearch-project/OpenSearch-Dashboards/blame/main/.github/workflows/cypress_workflow.yml#L33

Should creating a new start cmd in OSD? that does something like yarn start --data_source.enabled --data_source.ssl.verificationMode=none Then create a new ciGroup(s) for MDS? To start up OSD with MDS and calls the script from this repo to get the MDS tests and run them per PR.

Copy link
Member Author

@zhongnansu zhongnansu May 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kavilla Thanks for sharing how cypress workflow in OSD runs tests of functional test repo! I think we can just modify the existing START_CMD to add --data_source.enabled=true, data_source.ssl.verificationMode=none. For ciGroup, I think there's an existing one for MDS

"osd:ciGroup5": "echo \"datasource-management-plugin/*.js\"",

@SuZhou-Joe
Copy link
Member

SuZhou-Joe commented May 6, 2024

Seems this PR is tagged as 2.15, is that correct? @zhongnansu

@zhongnansu
Copy link
Member Author

Seems this PR is tagged as 2.15, is that correct? @zhongnansu

I think it should be tagged 2.14.0. Same for this PR which adds the infra for MDS, #1146 could you help add 2.14.0 label for both?

@CCongWang CCongWang merged commit 8b10f8f into opensearch-project:main May 6, 2024
39 of 43 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 6, 2024
Signed-off-by: Zhongnan Su <szhongna@amazon.com>
(cherry picked from commit 8b10f8f)
SuZhou-Joe pushed a commit that referenced this pull request May 7, 2024
Signed-off-by: Zhongnan Su <szhongna@amazon.com>
(cherry picked from commit 8b10f8f)

Co-authored-by: Zhongnan Su <szhongna@amazon.com>
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 7, 2024
Signed-off-by: Zhongnan Su <szhongna@amazon.com>
(cherry picked from commit 8b10f8f)
SuZhou-Joe pushed a commit that referenced this pull request May 7, 2024
Signed-off-by: Zhongnan Su <szhongna@amazon.com>
(cherry picked from commit 8b10f8f)

Co-authored-by: Zhongnan Su <szhongna@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants