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

Onboard alerting dashboards plugin #103

Merged
merged 13 commits into from
Mar 7, 2022

Conversation

annie3431
Copy link
Contributor

@annie3431 annie3431 commented Mar 4, 2022

Description

Moving most util and constant files over. There are still a few spec test files pending refactor. Also adding the SNS alerting test (might fail with open source domain).

The remaining spec files to be ported are still working in progress on this branch:
https://github.com/leeyun-amzn/opensearch-dashboards-functional-test/tree/alerting-tests

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.

Annie Lee added 8 commits March 3, 2022 14:22
Signed-off-by: Annie Lee <leeyun@amazon.com>
Signed-off-by: Annie Lee <leeyun@amazon.com>
Signed-off-by: Annie Lee <leeyun@amazon.com>
Signed-off-by: Annie Lee <leeyun@amazon.com>
Signed-off-by: Annie Lee <leeyun@amazon.com>
Signed-off-by: Annie Lee <leeyun@amazon.com>
Signed-off-by: Annie Lee <leeyun@amazon.com>
Will add the rest once the modification is done

Signed-off-by: Annie Lee <leeyun@amazon.com>
@tianleh
Copy link
Member

tianleh commented Mar 4, 2022

I am upgrading the main branch to support 1.3.0 in parallel #104 The Github workflows will be updated to use 1.3.0 artifacts once my pr is merged.

@annie3431 annie3431 marked this pull request as ready for review March 4, 2022 01:29
@annie3431 annie3431 requested a review from a team as a code owner March 4, 2022 01:29
@annie3431
Copy link
Contributor Author

annie3431 commented Mar 4, 2022

I am upgrading the main branch to support 1.3.0 in parallel #104 The Github workflows will be updated to use 1.3.0 artifacts once my pr is merged.

Sounds good. The only test that is failing is a managed service specific test. So currently that failure is expected.
Update: added MANAGED_SERVICE_ENDPOINT flag to distinguish the managed service specific tests

@tianleh
Copy link
Member

tianleh commented Mar 4, 2022

We have upgraded the main branch to 1.3.0 #104 There are some known failures where we are working with owners to fix them. #104 (comment)

By syncing the latest main branch into your PR, you will be able to see your test results against latest 1.3.0 and address any failures related to your change in this PR.

Annie Lee added 3 commits March 4, 2022 11:10
Signed-off-by: Annie Lee <leeyun@amazon.com>
Signed-off-by: Annie Lee <leeyun@amazon.com>
@@ -14,7 +14,8 @@
"SECURITY_ENABLED": false,
"username": "admin",
"password": "admin",
"ENDPOINT_WITH_PROXY": false
"ENDPOINT_WITH_PROXY": false,
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the difference between ENDPOINT_WITH_PROXY and MANAGED_SERVICE_ENDPOINT ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think for the MANAGED_SERVICE_ENDPOINT flag we are trying to separate the tests that are only applicable to managed service side domain. In this case the Amazon SNS type destination is only on AWS Opensearch so it will fail on an open source domain. I believe there are cases when the domain is on managed service side and is not proxy enabled, so it could be 2 different flags.

I will add the description of this flag to the documentation too.


export const ALERTING_PLUGIN_NAME = 'alerting';

export const ADMIN_AUTH = {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use the common ADMIN_AUTH constant instead ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I can do that. Thanks for checking this.

@tianleh
Copy link
Member

tianleh commented Mar 4, 2022

@tianleh
Copy link
Member

tianleh commented Mar 4, 2022

The main branch has fixed all the failures. You can perform the rebase.

@annie3431
Copy link
Contributor Author

annie3431 commented Mar 4, 2022

Seeing a alerting test failure. Might be related to a fix from this PR. Checking into it.

Seems like the cypress videos are not capturing the test runs.

…ain flag

Signed-off-by: Annie Lee <leeyun@amazon.com>
@tianleh tianleh requested a review from Tengda-He March 7, 2022 18:32
@tianleh tianleh merged commit ee5716c into opensearch-project:main Mar 7, 2022
@tianleh tianleh mentioned this pull request Mar 7, 2022
@annie3431 annie3431 deleted the alerting-sns branch June 19, 2022 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants